Re: [PATCH v3 0/6] allow ramoops to collect all kmesg_dump events

[Pavel Tatashin]

Hi Petr,

> Alternative solution is to dump all messages using ramoops. The
> problem is that it currently works only during Oops and panic
> situation. This is solved by this patchset.
>
>
> OK, I personally see this as two separate problems:
>
>    1. Missing support to set loglevel per console.
>    2. Missing support to dump messages for other reasons.
>
> I would remove the paragraph about console log levels completely.

OK, I see your point, this paragraph can be removed, however, I think
it makes it clear to understand the rationale for this change. As I
understand, the per console loglevel has been proposed but were never
accepted.

> It is your reason to use ramoops. But it is not reason to modify
> the logic about max_reason.
>
>
> Now, the max_reason logic makes sense only when all the values
> have some ordering. Is this the case?
>
> I see it as two distinct sets:
>
>    + panic, oops, emerg: describe how critical is an error situation
>    + restart, halt, poweroff: describe behavior when the system goes down
>
> Let's say that panic is more critical than oops. Is restart more
> critical than halt?
>
> If you want the dump during restart. Does it mean that you want it
> also during emergency situation?
>
> My fear is that this patchset is going to introduce user interface
> (max_reason) with a weird logic. IMHO, max_reason is confusing even
> in the code and we should not spread this to users.
>
> Is there any reason why the existing printk.always_kmsg_dump option
> is not enough for you?

printk.always_kmsg_dump is not working for me because ramoops has its
own filtering based on dump_oops boolean, and ignores everything but
panics and conditionally oops.
max_reason makes the ramoops internal logic cleaner compared to using dump_oops.

I agree, the reasons in kmsg_dump_reason do not order well  (I
actually want to add another reason for kexec type reboots, and where
do I put it?), so how about if we change the ordering list to
bitfield/flags, and instead of max_reason provide: "reasons" bitset?

Thank you,
Pasha