lkml.org 
[lkml]   [2020]   [May]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v1 2/3] pstore/ram: allow to dump kmesg during regular reboot
On Mon, May 04, 2020 at 04:30:52PM -0400, Pavel Tatashin wrote:
> > > -static void pstore_register_kmsg(void)
> > > +static void pstore_register_kmsg(int dmesg_all)
> > > {
> > > + if (dmesg_all)
> > > + pstore_dumper.max_reason = KMSG_DUMP_MAX;
> >
> > So, I'd like to avoid any new arguments in the API and instead add a new
> > field to struct pstore_info, which will be valid when PSTORE_FLAGS_DMESG
> > is set, and the max kdump reason can be set there by the pstore backends.
>
> Hi Kees,
>
> I am trying to verify that I understand the request correctly:
>
> 1. pstore_register_kmsg() -> remove argument.

Yes (or, from the perspective of what v2 will look like, "do not add
an argument to pstore_register_kmsg()").

> 2. pstore_info -> add a new field max_kmsg_reason: contains the
> actual reason value

Let's just call it max_reason, but yes. And perhaps instead of adding
KMSG_DUMP_MAX, maybe just use INT_MAX or something for "all reasons".

> 3. Modify: pstore_register() to set this field in pstore_dumper prior
> to calling pstore_register_kmsg().

Correct.

> 4. remove ramoops.dump_all boolean parameter

Yes, or more specifically, "don't add ramoops.dump_all".

> 5. add a new parameter ramoops.max_reason integer variable, which will
> be set in pstore_register_kmsg

Right, though this will likely require some refactoring of the existing
handling of the dump_oops parameter, likely as a separate patch, since
we should not remove the parameter, as some systems may be expecting to
use it still. But it should be reworked in terms of the new max_reason.

> 6. Modify other users of pstore_register() to provide the correct
> max_kmsg_reason.

Yes, which should be a trivial adjustment. You can look for all the
initializers using PSTORE_FLAGS_DMESG:

arch/powerpc/kernel/nvram_64.c: .flags = PSTORE_FLAGS_DMESG,
drivers/acpi/apei/erst.c: .flags = PSTORE_FLAGS_DMESG,
drivers/firmware/efi/efi-pstore.c: .flags = PSTORE_FLAGS_DMESG,

It looks like all the other backends actually already dump all reasons,
so they should likely all be set to the INT_MAX, or whatever is chosen
for "all reasons".

>
> Is this correct?

But, yes, your list essentially matches what I've got in my head too. :)

--
Kees Cook

\
 
 \ /
  Last update: 2020-05-04 22:55    [W:0.050 / U:0.588 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site