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

From: Petr Mladek
Date: Tue Jan 03 2017 - 09:55:52 EST


On Sat 2016-12-24 23:09:02, Sergey Senozhatsky wrote:
> The "printk messages dropped" report is 'attached' to a kernel
> message located at console_idx offset. This does not work well
> if we skip that message due to loglevel filtering, because in
> this case we also skip/lose dropped message report.

Good catch!

> Disable suppress_message_printing() loglevel filtering if we
> must report "printk messages dropped" condition.
>
> Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@xxxxxxxxx>
> ---
> kernel/printk/printk.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index 11a9842a2f47..6a7ebcb0bb6e 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -2218,6 +2218,7 @@ void console_unlock(void)
> struct printk_log *msg;
> size_t ext_len = 0;
> size_t len;
> + bool report_dropped_msg = false;
>
> raw_spin_lock_irqsave(&logbuf_lock, flags);
> if (seen_seq != log_next_seq) {
> @@ -2232,6 +2233,7 @@ void console_unlock(void)
> /* messages are gone, move to first one */
> console_seq = log_first_seq;
> console_idx = log_first_idx;
> + report_dropped_msg = true;
> } else {
> len = 0;
> }
> @@ -2240,7 +2242,8 @@ void console_unlock(void)
> break;
>
> msg = log_from_idx(console_idx);
> - if (suppress_message_printing(msg->level)) {
> + if (!report_dropped_msg &&
> + suppress_message_printing(msg->level)) {
> /*
> * Skip record we have buffered and already printed
> * directly to the console when we received it, and

This causes the opposite problem. We might print a message that was supposed
to be suppressed. I think that it is not a good idea in this
situation. I played with it and cooked a patch, see below.

Please, do not feel offended. I do not want to take you credits
for finding the problem.

But the printk code is very twisted and console_unlock() is one
of the worst pieces. Solution based on my concerns could make
it even worse. I do not want to push you into clean ups and
tried it myself.

The result is not as good as I hoped. But it it is not worse, IMHO.
Which is good given the circumstances.

Feel free to provide even better one and use pieces from my patch.
I also think about spling it into two patches and make msg_print()
helper in a separate patch.