On 01/26/18 at 01:17pm, Petr Mladek wrote:
> On Fri 2018-01-26 15:37:24, Dave Young wrote:
> > On 01/19/18 at 12:47pm, Dave Young wrote:
> > > On 01/18/18 at 01:57pm, Steven Rostedt wrote:
> > > > On Thu, 18 Jan 2018 10:02:17 -0800
> > > > Andi Kleen <a...@linux.intel.com> wrote:
> > > >
> > > > > Dave Young <dyo...@redhat.com> writes:
> > > > > > printk("%sHardware name: %s\n",
> > > > > > log_lvl, dump_stack_arch_desc_str);
> > > > > > + if (kexec_crash_loaded())
> > > > > > + printk("%skdump kernel loaded\n", log_lvl);
> > > > >
> > > > > Oops/warnings are getting longer and longer, often scrolling away
> > > > > from the screen, and if the kernel crashes backscroll does not work
> > > > > anymore, so precious information is lost.
> > > > >
> > > > > Can you merge it with some other line?
> > > > >
> > > > > Just a [KDUMP] or so somewhere should be good enough.
> > > >
> > > > Or perhaps we should add it as a TAINT. Not all taints are bad.
> > >
> > > Hmm, I also thought about this before but It sounds like not match the
> > > "tainted" meaning with the assumption that it is bad :(
> > >
> > > Maybe it would be better to do like Andi said, but print a better word
> > > than "KDUMP", eg. "Kdumpable" sounds better. If this is fine I can
> > > repost the patch.
> >
> > I have been not available recently, sorry for late about the update,
> > rethinking about this, it is looks good to use "[KDUMP]". Also for
> > the tainted flags, I tried but it is not what we want since kdump kernel
> > can be unloaded, this is not like "tainted" which can only be added and
> > it can not be removed.
> >
> > How about below version?
> > ---
> >
> > It is useful to print kdump kernel loaded status in dump_stack()
> > especially when panic happens so that we can differentiate
> > kdump kernel early hang and a normal panic in a bug report.
> >
> > Signed-off-by: Dave Young <dyo...@redhat.com>
> > ---
> > [v1 -> v2] merge the status in other line as Andi Kleen suggested
> > kernel/printk/printk.c | 3 +++
> > --- linux.orig/kernel/printk/printk.c
> > +++ linux/kernel/printk/printk.c
> > @@ -48,6 +48,7 @@
> > #include <linux/sched/clock.h>
> > #include <linux/sched/debug.h>
> > #include <linux/sched/task_stack.h>
> > +#include <linux/kexec.h>
> >
> > #include <linux/uaccess.h>
> > #include <asm/sections.h>
> > @@ -3118,9 +3119,11 @@ void __init dump_stack_set_arch_desc(con
> > */
> > void dump_stack_print_info(const char *log_lvl)
> > {
> > - printk("%sCPU: %d PID: %d Comm: %.20s %s %s %.*s\n",
> > + printk("%sCPU: %d PID: %d Comm: %.20s %s %s %s %.*s\n",
> > log_lvl, raw_smp_processor_id(), current->pid, current->comm,
> > - print_tainted(), init_utsname()->release,
> > + print_tainted(),
> > + kexec_crash_loaded() ? "[KDUMP]" : "",
>
> I am afraid that people might be confused what that exactly means.
> For example, if I would wonder if it was already running the crashdump
> kernel.
>
> And two small nits. It always prints the extra space. It does not fit
> the style of the line.
>
> What about the following?
>
> printk("%sCPU: %d PID: %d Comm: %.20s %s%s %s %.*s\n",
> log_lvl, raw_smp_processor_id(), current->pid, current->comm,
> kexec_crash_loaded() ? "Kdump: loaded " : "",
> print_tainted(),
> init_utsname()->release,
> (int)strcspn(init_utsname()->version, " "),
> init_utsname()->version);
>
> It would produce something like:
>
> CPU: 0 PID: 0 Comm: swapper/0 Kdump: loaded Not tainted 4.14.0-4-default+ #670
That should be a better version, thanks for catching the extra
whitespace. Let me do a test and send it out as V2 separately.
>
>
> Best Regards,
> Petr
