diff mbox

[03/14] pstore/ram_core: Do not reset restored zone's position and size

Message ID 20120518222426.GC23089@lizard
State Superseded
Headers show

Commit Message

Anton Vorontsov May 18, 2012, 10:24 p.m. UTC
Otherwise, the files will survive just one reboot, and on a subsequent
boot they will disappear.

Signed-off-by: Anton Vorontsov <anton.vorontsov@linaro.org>
---
 fs/pstore/ram_core.c |    1 +
 1 file changed, 1 insertion(+)

Comments

Colin Cross May 18, 2012, 11:42 p.m. UTC | #1
On Fri, May 18, 2012 at 3:24 PM, Anton Vorontsov
<anton.vorontsov@linaro.org> wrote:
> Otherwise, the files will survive just one reboot, and on a subsequent
> boot they will disappear.
>
> Signed-off-by: Anton Vorontsov <anton.vorontsov@linaro.org>
> ---
>  fs/pstore/ram_core.c |    1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/fs/pstore/ram_core.c b/fs/pstore/ram_core.c
> index 849a542..dff5127 100644
> --- a/fs/pstore/ram_core.c
> +++ b/fs/pstore/ram_core.c
> @@ -422,6 +422,7 @@ static int __init persistent_ram_post_init(struct persistent_ram_zone *prz, bool
>                                " size %zu, start %zu\n",
>                               buffer_size(prz), buffer_start(prz));
>                        persistent_ram_save_old(prz);
> +                       return 0;
>                }
>        } else {
>                pr_info("persistent_ram: no valid data in buffer"
> --
> 1.7.9.2
>

This causes an interesting behavior change in the console logging.
Before this change, the console log would show only the messages from
the last reboot.  After this change, the console log will have logs
from multiple boots appended to each other.  I can think of some
places where that could very handy, so I'm not against the change, but
as is it makes reading the logs much harder - the first oops you see
while skimming the log may not be from the last reboot.

One possibility would be to insert an obvious (and script parseable)
header during probe to separate the boots.  Another option would be to
expand the ringbuffer metadata to contain duplicate start and size
fields that only cover the most recent reboot, and export two files,
console-ramoops that contains the last log, and console-all-ramoops
that contains all the logs.  Or you could just zap the console buffer
at boot to keep the old behavior.
Anton Vorontsov May 22, 2012, 1:19 p.m. UTC | #2
On Fri, May 18, 2012 at 04:42:03PM -0700, Colin Cross wrote:
[...]
> This causes an interesting behavior change in the console logging.
> Before this change, the console log would show only the messages from
> the last reboot.  After this change, the console log will have logs
> from multiple boots appended to each other.

Heh. A nice feature, but frankly speaking, that wasn't my intent to
introduce it. :-)

I will document the new behaviour in the patch description.

> I can think of some
> places where that could very handy, so I'm not against the change, but
> as is it makes reading the logs much harder - the first oops you see
> while skimming the log may not be from the last reboot.
> 
> One possibility would be to insert an obvious (and script parseable)
> header during probe to separate the boots.

Yep, we have it already, i.e. linux_banner. This script should be
reliable enough:

	tac ramoops-console | sed '/^Linux version.*(.*@.*)/ q' | tac

> Another option would be to
> expand the ringbuffer metadata to contain duplicate start and size
> fields that only cover the most recent reboot, and export two files,
> console-ramoops that contains the last log, and console-all-ramoops
> that contains all the logs.

This seems like an overkill. :-)

> Or you could just zap the console buffer
> at boot to keep the old behavior.

Well, I actually like the new behaviour, so assuming that we're all
happy with the new feature, there's no need to zap it at boot. Or
we can at least make it configurable...

Thanks!
diff mbox

Patch

diff --git a/fs/pstore/ram_core.c b/fs/pstore/ram_core.c
index 849a542..dff5127 100644
--- a/fs/pstore/ram_core.c
+++ b/fs/pstore/ram_core.c
@@ -422,6 +422,7 @@  static int __init persistent_ram_post_init(struct persistent_ram_zone *prz, bool
 				" size %zu, start %zu\n",
 			       buffer_size(prz), buffer_start(prz));
 			persistent_ram_save_old(prz);
+			return 0;
 		}
 	} else {
 		pr_info("persistent_ram: no valid data in buffer"