On Tue, 16 Jan 2018 13:47:16 +0900 Sergey Senozhatsky <[email protected]> wrote:
> From: Sergey Senozhatsky <[email protected]> > Subject: [PATCH] printk: never set console_may_schedule in console_trylock() > > This patch, basically, reverts commit 6b97a20d3a79 ("printk: > set may_schedule for some of console_trylock() callers"). > That commit was a mistake, it introduced a big dependency > on the scheduler, by enabling preemption under console_sem > in printk()->console_unlock() path, which is rather too > critical. The patch did not significantly reduce the > possibilities of printk() lockups, but made it possible to > stall printk(), as has been reported by Tetsuo Handa [1]. > > Another issues is that preemption under console_sem also > messes up with Steven Rostedt's hand off scheme, by making > it possible to sleep with console_sem both in console_unlock() > and in vprintk_emit(), after acquiring the console_sem > ownership (anywhere between printk_safe_exit_irqrestore() in > console_trylock_spinning() and printk_safe_enter_irqsave() > in console_unlock()). This makes hand off less likely and, > at the same time, may result in a significant amount of > pending logbuf messages. Preempted console_sem owner makes > it impossible for other CPUs to emit logbuf messages, but > does not make it impossible for other CPUs to append new > messages to the logbuf. > > Reinstate the old behavior and make printk() non-preemptible. > Should any printk() lockup reports arrive they must be handled > in a different way. > > [1] https://marc.info/?l=linux-mm&m=145692016122716 Especially since Konstantin is working on pulling in all LKML archives, the above should be denoted as: Link: http://lkml.kernel.org/r/201603022101.CAH73907.OVOOMFHFFtQJSL%20()%20I-love%20!%20SAKURA%20!%20ne%20!%20jp Although the above is for linux-mm and not LKML (it still works), I should ask Konstantin if he will be pulling in any of the other archives. Perhaps have both? (in case marc.info goes away). > Fixes: 6b97a20d3a79 ("printk: set may_schedule for some of console_trylock() > callers") Should we Cc [email protected]? > Signed-off-by: Sergey Senozhatsky <[email protected]> > Reported-by: Tetsuo Handa <[email protected]> > --- > kernel/printk/printk.c | 22 ++++++++-------------- > 1 file changed, 8 insertions(+), 14 deletions(-) > > diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c > index ffe05024c622..9cb943c90d98 100644 > --- a/kernel/printk/printk.c > +++ b/kernel/printk/printk.c > @@ -1895,6 +1895,12 @@ asmlinkage int vprintk_emit(int facility, int level, > > /* If called from the scheduler, we can not call up(). */ > if (!in_sched) { > + /* > + * Disable preemption to avoid being preempted while holding > + * console_sem which would prevent anyone from printing to > + * console > + */ > + preempt_disable(); > /* > * Try to acquire and then immediately release the console > * semaphore. The release will print out buffers and wake up > @@ -1902,6 +1908,7 @@ asmlinkage int vprintk_emit(int facility, int level, > */ > if (console_trylock_spinning()) > console_unlock(); > + preempt_enable(); > } > > return printed_len; > @@ -2229,20 +2236,7 @@ int console_trylock(void) > return 0; > } > console_locked = 1; > - /* > - * When PREEMPT_COUNT disabled we can't reliably detect if it's > - * safe to schedule (e.g. calling printk while holding a spin_lock), > - * because preempt_disable()/preempt_enable() are just barriers there > - * and preempt_count() is always 0. > - * > - * RCU read sections have a separate preemption counter when > - * PREEMPT_RCU enabled thus we must take extra care and check > - * rcu_preempt_depth(), otherwise RCU read sections modify > - * preempt_count(). > - */ > - console_may_schedule = !oops_in_progress && > - preemptible() && > - !rcu_preempt_depth(); > + console_may_schedule = 0; > return 1; > } > EXPORT_SYMBOL(console_trylock); Reviewed-by: Steven Rostedt (VMware) <[email protected]> Thanks Sergey! -- Steve
