Re: [RFC][PATCH -next 1/2] printk: move can_use_console out of console_trylock_for_printk

From: Sergey Senozhatsky
Date: Mon Jan 18 2016 - 19:41:31 EST


Hello,
thanks for review.

On (01/18/16 16:42), Petr Mladek wrote:
> On Thu 2016-01-14 13:57:22, Sergey Senozhatsky wrote:
> > vprintk_emit() disables preemption around console_trylock_for_printk()
> > and console_unlock() calls for a strong reason -- can_use_console()
> > check. The thing is that vprintl_emit() can be called on a CPU that
> > is not fully brought up yet (!cpu_online()), which potentially can
> > cause problems if console driver accesses per-cpu data. A console
> > driver can explicitly state that it's safe to call it from !online
> > cpu by setting CON_ANYTIME bit in console ->flags. That's why for
> > !cpu_online() can_use_console() iterates all the console to find out
> > if there is a CON_ANYTIME console, otherwise console_unlock() must be
> > avoided.
> >
> > call_console_drivers(), called from console_cont_flush() and
> > console_unlock(), does the same test during for_each_console() loop.
> > However, we can have the following corner case. Assume that we have 2
> > cpus -- CPU0 is online, CPU1 is !online; and no CON_ANYTIME consoles
> > available.
> >
> > CPU0 online CPU1 !online
> > console_trylock()
> > ...
> > console_unlock()
>
> Please, where this console_unlock() comes from?

from UP* or DOWN* (_PREPARE f.e.) notifiers on this CPU, for example, we don't
know what's going on there. what prevents it from calling console_trylock(),
grabbing the console_sem and eventually doing console_unlock()? there is
a can_use_console() check, but it handles only one case -- printk().
there is also an extra '!cpu_online() && !CON_ANYTIME' test done for_each_console
in call_console_drivers(), but it's too late -- we already msg_print_text()
and advanced console_seq/console_idx/etc., the message will be lost, we
don't put it back.

> If I get this correctly, this CPU is not online and no CON_ANYTIME
> console exists
> => can_use_console() fails
> => console_trylock() fails
> => console_unlock() is not called from vprintk_emit().

the current flow is
vprintk_emit()
console_trylock_for_printk
can_use_console fails -- !cpu online and no CON_ANYTIME
console_unlock() is not called from vprintk_emit()

the missing path
console_trylock
console_unlock
for (;;) {
msg_print_text
call_console_drivers
!cpu_online && !CON_ANYTIME -- lost it and repeat again
}


the new one is

vprintk_emit()
console_trylock_for_printk -- ok
console_unlock
can_use_console fails -- !cpu online and no CON_ANYTIME

and it also covers the case
console_trylock -- detour can_use_console()
console_unlock
can_use_console fails -- !cpu online and no CON_ANYTIME, abort

-ss