Re: [PATCH 2/2] printk: always report lost messages on serial console

[Petr Mladek]

On Wed 2017-01-04 00:47:45, Sergey Senozhatsky wrote:
> On (01/03/17 15:55), Petr Mladek wrote:
> [..]
> > This causes the opposite problem. We might print a message that was supposed
> > to be suppressed.
> 
> so what? yes, we print a message that otherwise would have been suppressed.
> not a big deal. at all. we are under high printk load and the best thing
> we can do is to report "we are losing the messages" straight ahead. the
> next 'visible' message may be seconds/minutes/forever away. think of a
> printk() flood of messages with suppressed loglevel coming from CPUA-CPUX,
> big enough to drain all 'visible' loglevel messages from CPUZ. we are
> back to problem "a".
> 
> thus I want a simple bool flag and a simple rule: we see something - we say it.

So, you prefer to print some random debug message instead of an
emergency one? The console_level is there for a reason.

If there is a flood of messages, console_level = 1 and use your
solution, you might see:

** 1324 printk messages dropped ** <notice: random message>
** 4234 printk messages dropped ** <debug: random message>
** 3243 printk messages dropped ** <info: random message>
** 2343 printk messages dropped ** <debug: random message>

It will always drop a message because you always process only one
and many new appear in the meantime. While with my solution,
you should see:

** 1324 printk messages dropped ** <alert: random message>
** 523 printk messages dropped ** <emerg: random message>
** 324 printk messages dropped ** <emerg: random message>
** 345 printk messages dropped ** <alert: random message>

You will see messages filtered by the console_level. Also
less number of messages should get dropped because you quickly
skip the less important ones.

The filtering might be the only way to see the important
messages. On the other hand, the fact that messages are
dropped might be quessed from the context.

Your patch fixes a bug by introducing another bug that is
probably even more serious.


I understand that my patch is much more complex. But is
the final code realy more complex?

console_unlock() is too long. The helper for the msg_print()
calls makes sense on its own. The rest is just reshufling
of the conditions.

It actually would make sense to hide also the increment
of console_idx/console_seq into the helper function.
Especially console_idx is related to the struct printk_log
that is no longer accessed in console_unlock() directly.

Here is v2: