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

[Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>]

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