Messages in this thread | | | Date | Tue, 19 Jan 2016 09:42:36 +0900 | From | Sergey Senozhatsky <> | Subject | Re: [RFC][PATCH -next 1/2] printk: move can_use_console out of console_trylock_for_printk |
| |
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
| |