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

[Sergey Senozhatsky]

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.

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:

---

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;
 	console_may_schedule = 0;
 	console_unlock();
 }

---

Opinions?

> But I would prefer to do it a cleaner way.

OK.

> But it is rather complicated, still hacky, ...

Right.

> > 
> > I can agree, definitely. That's one of the options.
> 
> 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.

	-ss