Messages in this thread | | | Date | Thu, 11 Feb 2016 15:41:05 +0100 | From | Petr Mladek <> | Subject | Re: [RFC][PATCH v3 4/4] printk: set may_schedule for some of console_trylock callers |
| |
On Sat 2016-01-23 17:15:13, Sergey Senozhatsky wrote: > console_unlock() allows to cond_resched() if its caller has > set `console_may_schedule' to 1, since > 'commit 8d91f8b15361 ("printk: do cond_resched() between lines while > outputting to consoles")'. > > The rules are: > -- console_lock() always sets `console_may_schedule' to 1 > -- console_trylock() always sets `console_may_schedule' to 0 > > However, console_trylock() callers (among them is printk()) do > not always call printk() from atomic contexts, and some of them > can cond_resched() in console_unlock(), so console_trylock() > can set `console_may_schedule' to 1 for such processes. > > For !CONFIG_PREEMPT_COUNT kernels, however, console_trylock() > always sets `console_may_schedule' to 0. > > It's possible to drop explicit preempt_disable()/preempt_enable() > in vprintk_emit(), because console_unlock() and console_trylock() > are now smart enough: > a) console_unlock() does not cond_resched() when it's unsafe > (console_trylock() takes care of that) > b) console_unlock() does can_use_console() check. > > Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com> > --- > kernel/printk/printk.c | 19 ++++++++++--------- > 1 file changed, 10 insertions(+), 9 deletions(-) > > diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c > index 99925ce..097ca8b 100644 > --- a/kernel/printk/printk.c > +++ b/kernel/printk/printk.c > @@ -1769,20 +1769,12 @@ asmlinkage int vprintk_emit(int facility, int level, > if (!in_sched) { > lockdep_off(); > /* > - * 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 > * /dev/kmsg and syslog() users. > */ > if (console_trylock()) > console_unlock(); > - preempt_enable(); > lockdep_on(); > } > > @@ -2115,7 +2107,16 @@ int console_trylock(void) > return 0; > } > console_locked = 1; > - console_may_schedule = 0; > + /* > + * On !PREEMPT_COUNT kernels 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 and > + * don't modify preempt_count() there. console_may_schedule is > + * always 0 on !PREEMPT_COUNT kernels. > + */ > + console_may_schedule = !oops_in_progress && > + preemptible() && > + !rcu_preempt_depth(); > return 1;
We discussed this a lot but I am still a bit nervous ;-)
Avoid scheduling when oops_in_progress makes sense.
preemptible() takes care of preemption and IRQ contexts. The comment above explains that it is safe to use here.
The check for rcu_preempt_depth() makes sense. But is it safe, please?
rcu_preempt_depth() returns 0 if CONFIG_PREEMPT_RCU is not enabled. It means that you are not able to detect RCU read section and it might cause problems.
I rather add Paul into CC.
Best Regards, Petr
| |