Re: [RFC][PATCH -next 2/2] printk: set may_schedule for some of console_trylock callers
From: Sergey Senozhatsky
Date: Mon Jan 18 2016 - 20:14:08 EST
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
which is not really a problem -- we schedule from console_unlock() with
the console_sem being held, it's fine.
> 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.
spin_lock, save IRQ
spin_lock, save IRQ
so preemption is disabled during the transition from can_use_console()
> > 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.