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 <[email protected]>
> ---
> 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
