RE: [PATCH] printk: make printk_safe_flush safe in NMI context by skipping flushing

[Hoeun Ryu]

> -----Original Message-----
> From: Sergey Senozhatsky [mailto:sergey.senozhatsky.work@xxxxxxxxx]
> Sent: Tuesday, May 29, 2018 9:13 PM
> To: Hoeun Ryu <hoeun.ryu@xxxxxxxxxxx>
> Cc: Petr Mladek <pmladek@xxxxxxxx>; Sergey Senozhatsky
> <sergey.senozhatsky@xxxxxxxxx>; Steven Rostedt <rostedt@xxxxxxxxxxx>;
> Hoeun Ryu <hoeun.ryu@xxxxxxx>; linux-kernel@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH] printk: make printk_safe_flush safe in NMI context by
> skipping flushing
> 
> On (05/29/18 11:51), Hoeun Ryu wrote:
> >  Make printk_safe_flush() safe in NMI context.
> > nmi_trigger_cpumask_backtrace() can be called in NMI context. For
> example the
> > function is called in watchdog_overflow_callback() if the flag of
> hardlockup
> > backtrace (sysctl_hardlockup_all_cpu_backtrace) is true and
> > watchdog_overflow_callback() function is called in NMI context on some
> > architectures.
> >  Calling printk_safe_flush() in nmi_trigger_cpumask_backtrace()
> eventually tries
> > to lock logbuf_lock in vprintk_emit() but the logbuf_lock can be already
> locked in
> > preempted contexts (task or irq in this case) or by other CPUs and it
> may cause
> > deadlocks.
> >  By making printk_safe_flush() safe in NMI context, the backtrace
> triggering CPU
> > just skips flushing if the lock is not avaiable in NMI context. The
> messages in
> > per-cpu nmi buffer of the backtrace triggering CPU can be lost if the
> CPU is in
> > hard lockup (because irq is disabled here) but if panic() is not called.
> The
> > flushing can be delayed by the next irq work in normal cases.
> 
> Any chance we can add more info to the commit message? E.g. backtraces
> which would describe "how" is this possible (like the one I posted in
> another email). Just to make it more clear.
> 

OK, I will.

> > @@ -254,6 +254,16 @@ void printk_safe_flush(void)
> >  {
> >  	int cpu;
> >
> > +	/*
> > +	 * Just avoid deadlocks here, we could loose the messages in per-
> cpu nmi buffer
> > +	 * in the case that hardlockup happens but panic() is not called
> (irq_work won't
> > +	 * work).
> > +	 * The flushing can be delayed by the next irq_work if flushing is
> skippped here
>
^^ skipped
> 

Typo will be fixed.

> 	-ss