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

[Petr Mladek]

On Wed 2018-09-05 17:20:53, Hans de Goede wrote:
> HI,
> 
> On 05-09-18 13:02, Petr Mladek wrote:
> > On Wed 2018-09-05 17:33:26, Sergey Senozhatsky wrote:
> > > On (09/05/18 14:36), Sergey Senozhatsky wrote:
> > > > 
> > > > Just a demonstration of the idea. It does not look very good, tho.
> > > > I'd rather have just one suppress_message_printing() in printk code.
> > > > 
> > > > // This is not a proposed patch, hence the 80-cols violation.
> > > > 
> > > > ---
> > > > 
> > > > diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> > > > index c036f128cdc3..231ac18423e1 100644
> > > > --- a/kernel/printk/printk.c
> > > > +++ b/kernel/printk/printk.c
> > > > @@ -2416,7 +2416,7 @@ void console_unlock(void)
> > > >   			break;
> > > >   		msg = log_from_idx(console_idx);
> > > > -		if (msg->flags & LOG_NOCONS) {
> > > > +		if (msg->flags & LOG_NOCONS || (exclusive_console && suppress_message_printing(msg->level))) {
> > > >   			/*
> > > >   			 * Skip record if !ignore_loglevel, and
> > > >   			 * record has level above the console loglevel.
> > > 
> > > D'oh... Sorry about that, but, believe it or not, this is completely
> > > not what I had in my mind. What I had, was something like this:
> > > 
> > > ---
> > > 
> > > diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> > > index c036f128cdc3..dadb8c11b0d6 100644
> > > --- a/kernel/printk/printk.c
> > > +++ b/kernel/printk/printk.c
> > > @@ -2416,6 +2416,11 @@ void console_unlock(void)
> > >   			break;
> > >   		msg = log_from_idx(console_idx);
> > > +
> > > +		if (exclusive_console &&
> > > +				!suppress_message_printing(msg->level))
> > > +			msg->flags &= ~LOG_NOCONS;
> > 
> > Hmm, this does not help with consoles without CON_PRINTBUFFER
> > flag. Note that the first registered console prints all messages
> > even without this flag.
> > 
> > Also there is "debug" earlyparam where we need the opposite. I mean
> > that we want to show messages that were suppressed by default.
> > 
> > I played with another solution, see the patch below. It defines
> > which messages have a valid NOCONS flag according to the msg_seq
> > number. IMHO, it is a bit more straightforward but it is still
> > a hack. I am not super happy about it.
> > 
> > 
> > Hmm, I seriously think about reverting the commit 375899cddcbb
> > ("printk: make sure to print log on console.") and solving it
> > another way.
> > 
> > For example, the commit was primary about locations that
> > wanted to make some messages always visible or always
> > suppressed. We could create LOG_FORCE_NOCONS and
> > LOG_FORCE_CONS for these two special cases.
> > 
> > 
> > 
> > Possible solution:
> 
> So do you want me to give this solution a try or was this mainly for
> discussion purposes?   If you've a fix which you think you are
> happy with and plan to merge I would be happy to try it.

It was rather for discussion purposes. I prefer the revert
for the moment.

If nobody comes with a better idea or is not against, I am going
to send the revert.

Best Regards,
Petr