Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1540404 > unrolled thread

Re: [RFC][PATCHv5 5/7] printk: report lost messages in printk safe/nmi contexts

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [RFC][PATCHv5 5/7] printk: report lost messages in printk  safe/nmi contexts Petr Mladek <pmladek@suse.com> - 2016-12-12 17:00 +0100
    Re: [RFC][PATCHv5 5/7] printk: report lost messages in printk  safe/nmi contexts Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2016-12-13 03:00 +0100
      Re: [RFC][PATCHv5 5/7] printk: report lost messages in printk  safe/nmi contexts Petr Mladek <pmladek@suse.com> - 2016-12-14 12:00 +0100

#1540404 — Re: [RFC][PATCHv5 5/7] printk: report lost messages in printk safe/nmi contexts

On Thu 2016-12-01 22:55:44, Sergey Senozhatsky wrote:
> Account lost messages in pritk-safe and printk-safe-nmi
> contexts and report those numbers during printk_safe_flush().
> 
> Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> ---
> --- a/kernel/printk/printk_safe.c
> +++ b/kernel/printk/printk_safe.c
> @@ -211,6 +242,9 @@ static void __printk_safe_flush(struct irq_work *work)
>  	if (atomic_cmpxchg(&s->len, len, 0) != len)
>  		goto more;
>  
> +	report_nmi_message_lost();
> +	report_safe_message_lost();

The great thing about this solution is that we print this message
in the right order (after the messages that fit into the buffer).

But not really because we report lost messages from both buffers
and from all CPUs here.

The perfect solution would be to remember the number of lost messages
in struct printk_safe_seq_buf. Then we might bump the value directly
in printk_safe_log_store() instead of returning the ugly -ENOSPC.
Also we could use an universal message (no "NMI" or "printk-safe")
because it could be printed right after flushing the messages
that fit the buffer.

This solution is good enough and still better than the previous one, so

Reviewed-by: Petr Mladek <pmladek@suse.com>


But if you would want to change it to the "perfect" one, it would be
appreciated ;-) It will be even more straightforward and less code.
In this, case I would do the change in an earlier patch to make
it more straightforward. But we could also do this later
(in another patchset).

Best Regards,
Petr

[toc] | [next] | [standalone]

#1540801

On (12/12/16 16:58), Petr Mladek wrote:
> On Thu 2016-12-01 22:55:44, Sergey Senozhatsky wrote:
[..]
> But not really because we report lost messages from both buffers
> and from all CPUs here.
[..]
> The perfect solution would be to remember the number of lost messages
> in struct printk_safe_seq_buf. Then we might bump the value directly
> in printk_safe_log_store() instead of returning the ugly -ENOSPC.

ok, I can take a look. this won't grow the per-CPU buffers bigger,
but will shrink the actual message buffer size by sizeof(atomic),
not that dramatic.

* unrelated, can be done later (if ever) *

speaking of tha actual message buffer size, we, may be, can move
`struct irq_work' out of printk_safe_seq_buf. there is already
a printk-related per-CPU irq_work in place - wake_up_klogd_work.
so we may be can use it, instead of defining a bunch of new irq_works.
this will increase the printk-safe/nmi per-CPU message buffer size
by sizeof(irq_work).

> Also we could use an universal message (no "NMI" or "printk-safe")
> because it could be printed right after flushing the messages
> that fit the buffer.

this "context" part probably can be dropped. both printk-safe and
printk-nmi per-CPU buffer sizes are controlled by a single .config
option anyway; user can't increase the printk-safe buffer size
without increasing the printk-nmi buffer size (in case if printk-safe
buffer is too small).

> This solution is good enough and still better than the previous one, so
> 
> Reviewed-by: Petr Mladek <pmladek@suse.com>

thanks.

	-ss

[toc] | [prev] | [next] | [standalone]

#1541846

On Tue 2016-12-13 10:52:37, Sergey Senozhatsky wrote:
> On (12/12/16 16:58), Petr Mladek wrote:
> > On Thu 2016-12-01 22:55:44, Sergey Senozhatsky wrote:
> [..]
> > But not really because we report lost messages from both buffers
> > and from all CPUs here.
> [..]
> > The perfect solution would be to remember the number of lost messages
> > in struct printk_safe_seq_buf. Then we might bump the value directly
> > in printk_safe_log_store() instead of returning the ugly -ENOSPC.
> 
> ok, I can take a look. this won't grow the per-CPU buffers bigger,
> but will shrink the actual message buffer size by sizeof(atomic),
> not that dramatic.

IMHO, the size is a negligible difference. If we are often getting
on the edge of the buffers, we have problems anyway.


> * unrelated, can be done later (if ever) *
> 
> speaking of tha actual message buffer size, we, may be, can move
> `struct irq_work' out of printk_safe_seq_buf. there is already
> a printk-related per-CPU irq_work in place - wake_up_klogd_work.
> so we may be can use it, instead of defining a bunch of new irq_works.
> this will increase the printk-safe/nmi per-CPU message buffer size
> by sizeof(irq_work).

Interesting idea! I think that there is a space for more optimization.
For example, we will not need to schedule the irq work if we are
flushing  the per-CPU buffers from irq work and we know that
we will flush consoles or wake up the kthread right after that.
Also I though about using a global "printk_pending" variable
and queue the irqwork only when the given event was not already set.

I would leave all this optimization for a later patchset.


> > Also we could use an universal message (no "NMI" or "printk-safe")
> > because it could be printed right after flushing the messages
> > that fit the buffer.
> 
> this "context" part probably can be dropped. both printk-safe and
> printk-nmi per-CPU buffer sizes are controlled by a single .config
> option anyway; user can't increase the printk-safe buffer size
> without increasing the printk-nmi buffer size (in case if printk-safe
> buffer is too small).

I agree.

Best Regards,
Petr

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web