On Tue, May 12, 2020 at 12:49:10PM -0400, Pavel Tatashin wrote:

On Tue, May 12, 2020 at 11:52 AM Petr Mladek [off-list ref] wrote:

quoted

I wonder if anyone is actually using the ramoops.dump_oops parameter
in reality. I would personally make it deprecated and change the
default behavior to work according to printk.always_kmsg_dump parameter.

This sounds alright to me with one slight problem. I am doing this
work for an embedded arm64 SoC, so controlling everything via device
tree is preferable compared to having some settings via device tree
and others via kernel parameters, especially because the kernel
parameters are hardcoded by firmware that we try not to update too
often for uptime reasons.

I'm entirely convinced that this area of pstore needs to be cleaned up
and I want to have the pstore backends be able to declare their kmsg
dump reason filters in a configurable fashion. So at least on the pstore
end, I intend to have some way to do this.

quoted

IMHO, ramoops.dump_oops just increases complexity and should not have
been introduced at all. I would try hard to avoid introducing even bigger
complecity and mess.

I agree, amoops.dump_oops should be depricated with or without
max_reason change.

Yup. dump_oops will be deprecated in favor of whatever we settle on here.

quoted

I know that there is the "do not break existing userspace" rule. The
question is if there is any user and if it is worth it.

quoted

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?

It looks too complicated. I would really try hard to avoid the
parameter at all.

OK. Should we remove max_reason from struct kmsg_dumper and also
remove the misleading comment about kmsg_dump_reason ordering?

I'm also fine with this. I can have pstore infrastructure doing the
filtering if kmsg dump doesn't want to. Given the existence of
printk.always_kmsg_dump, though, it seemed like it was better to have
kmsg dump do this filtering instead.

At this point my preference is to switch to a bit field -- I don't see a
reason for ordering. The only cases that remain "special" appear to be
PANIC and EMERG (which, again, aren't ordered adjacent).

-Kees

-- 
Kees Cook