lkml.org 
[lkml]   [2016]   [Jan]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [RFC][PATCH -next 2/2] printk: set may_schedule for some of console_trylock callers
Thanks for the review.

On (01/18/16 17:17), Petr Mladek wrote:
> On Sun 2016-01-17 23:23:32, Sergey Senozhatsky wrote:
> > console_unlock() allows to cond_resched() if its caller has
> > set `console_may_schedule' to 1 (this functionality present
> > since commit '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()) are
> > not necessarily executing in atomic contexts, and some of them
> > can cond_resched() in console_unlock(). So console_trylock()
> > can set `console_may_schedule' to 0 only if cond_resched() is
> > invalid in the current context, and set it to 1 otherwise.
> >
> > The patch also drops explicit preempt_disable()/preempt_enable()
> > calls in vprintk_emit().
>
> I do not see any explanation why it is safe to remove these
> calls in this patch. If they were required only because of the
> can_use_console() call

The comment in the code states

/*
* Disable preemption to avoid being preempted while holding
* console_sem which would prevent anyone from printing to
* console
*/

which is not really a problem -- we schedule from console_unlock() with
the console_sem being held, it's fine.

https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/kernel/printk/printk.c?id=8d91f8b15361dfb438ab6eb3b319e2ded43458ff

> it would make sense to move this change to the previous patch.
> The previous patch moved the can_use_console() to locations
> protected by lockbuf_lock that have disabled preemption because
> of the lock.

old:
vprintk_emit
preempt_disable
can_use_console
console_unlock
spin_lock, save IRQ
spin_unlock
call_console_drivers
restore IRQ
preempt_enable

new:
vprintk_emit
console_unlock
spin_lock, save IRQ
can_use_console
spin_unlock
call_console_drivers
restore IRQ

so preemption is disabled during the transition from can_use_console()
to call_console_drivers().


[..]
> > console_locked = 1;
> > - console_may_schedule = 0;
> > + console_may_schedule = !(oops_in_progress ||
> > + irqs_disabled() ||
> > + in_atomic() ||
> > + rcu_preempt_depth());
>
> Is it safe to call cond_resched() when the CPU is not online
> and preemption is enabled, please? Your previous patch
> suggests that this situation might happen. I guess that
> it might break the CPU initialization code.

CPU notifies are preemptible. CPU_UP_PREPARE/etc. can schedule,
there are GFP_KERNEL kmalloc-s from CPU_UP_PREPARE (where cpu
is not yet online), mutex_lock() calls in cpu_notify handlers,
and so on.

-ss

\
 
 \ /
  Last update: 2016-01-19 02:21    [W:0.038 / U:0.060 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site