Re: [PATCH 4.19 regression fix] printk: For early boot messages check loglevel when flushing the buffer

[Petr Mladek]

On Fri 2018-09-07 13:21:32, Sergey Senozhatsky wrote:
> On (09/06/18 16:28), Petr Mladek wrote:
> > On Thu 2018-09-06 16:29:40, Sergey Senozhatsky wrote:
> > > On (09/05/18 13:02), Petr Mladek wrote:
> > > > Note that the first registered console prints all messages
> > > > even without this flag.
> > > 
> > > Hmm, OK, interesting point.
> > > 
> > > I assumed that the first console usually has CON_PRINTBUFFER bit set.
> > > Or even a CON_PRINTBUFFER | CON_ANYTIME combo. E.g. 8250. It sort of
> > > makes sense to have CON_PRINTBUFFER for the first console. Any later
> > > consoles [e.g. fbcon, netcon] don't necessarily have CON_PRINTBUFFER.
> > > 
> > > And the first console has CON_PRINTBUFFER bit set. Well, just because
> > > it sounds reasonable. Those were the main assumptions behind my code
> > > snippet. Was any of those assumptions wrong?
> > 
> > This assumption makes sense. In fact, I was wrong. I thought that
> > console_seq/console_idx were not updated until the first console
> > was registered. But it is not the case.
> > 
> > It means that the hack with exclusive_console might be usable.
> 
> Yeah, it is a hack. But not as dirty as it might appear, I think. In some
> sense it's aligned with what we do for exlusive_consoles - we treat exclusive
> consoles specially. So specially that even if the system panics while we
> re-flush logbuf messages to a new exclusive console, we flush_on_panic() only
> to that exclusive console, ignoring the rest of them.

I consider this a bug. Another bug is that other consoles might miss
messages that appear while the older messages are being
replayed on exclusive_console.

AFAIK, nobody complains about it. But I not comfortable with reusing
this racy pattern even more ;-)


> Not sure if it's totally right. There can be a netcon, for instance,
> available, which will not see panic flush() because of a exclusive
> console:

Good catch.

> ---
> 
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index c036f128cdc3..ede29a7ba6db 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -2545,6 +2545,7 @@ void console_flush_on_panic(void)
>  	 * ensure may_schedule is cleared.
>  	 */
>  	console_trylock();
> +	exclusive_console = NULL;

This is not be enough. It would cause replying old messages
on all consoles.

Most problems should probably be solved when we store console_seq
before setting exclusive_console. Then we could clear
exclusive_console when reaching the stored sequence number.

Can this be that simple? ;-)


> > I prefer the revert for now.
> 
> OK, agreed.
> IIRC I didn't see any upstream code which would have been fixed
> by the commit in question.

My proposal for 4.19: