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

Groups > linux.kernel > #1533750

Re: [RFC][PATCHv4 4/6] printk: report lost messages in printk safe/nmi contexts

Show all headers | View raw

On (11/25/16 12:07), Petr Mladek wrote:
[..]
> > +static void report_message_lost(atomic_t *num_lost, char *fmt)
> > +{
> > +	int lost = atomic_xchg(num_lost, 0);
> > +
> > +	if (lost) {
> > +		char msg[56];
> 
> I would really like to avoid a hard coded buffer size. Such things
> are likely to bite us in the future.

why would scnprintf() overflow.

> I thought about reshuffling a lot of logic, adding more wrappers,
> ... But the solution might be easy in the end, see below.
> 
> > +
> > +		scnprintf(msg, sizeof(msg), fmt, lost);
> > +
> > +		printk_safe_flush_line(msg, strlen(msg));
> 
> This made my brain spin a lot. I wondered if it did what we wanted
> and it was safe.
> 
> On one hand, it is supposed to work because use exactly this
> function in __printk_safe_flush() where you call this from.
> 
> One question is if it does what we want in different contexts.
> Let's look at it:
> 
> 1. It calls printk_deferred() in NMI context. There is a risk
>    of a deadlock. But it is called only from
>    printk_safe_flush_on_panic() which is the last resort. Therefore
>    it does exactly what we want.
> 
> 2. It calls printk()->printk_func()->vprintk_emit() in normal context.
>    It is what we want in normal context.
> 
> 3. It calls printk()->printk_func()->v printk_safe() in printk_safe
>    context. This does not look correct. IMHO, this might happen
>    only printk_safe_flush_on_panic() and we want to use
>    printk_deferred() here as well.
[..]
> The easiest solution would be to simply call printk_deferred()
> here. Everything will be deferred after the async printk() patchset
> anyway.
> 
> I would even use printk_deferred() in printk_safe_flush_line()
> for each context. It is not optimal but it works very well
> and it makes the code much more straightforward.

yes, good point.
we can call deferred printk for anything there; or replace that in_nmi()
check with the `printk_safe_context != 0' one, and then route the message
via printk or printk_deferred.


[..]
> >   * Flush data from the associated per-CPU buffer. The function
> >   * can be called either via IRQ work or independently.
> > @@ -147,6 +183,9 @@ static void __printk_safe_flush(struct irq_work *work)
> >  
> >  	i = 0;
> >  more:
> > +	report_nmi_message_lost();
> > +	report_safe_message_lost();
> 
> Please, move this at the end of this function.

ok.

	-ss

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

Re: [RFC][PATCHv4 4/6] printk: report lost messages in printk  safe/nmi contexts Petr Mladek <pmladek@suse.com> - 2016-11-25 12:10 +0100
  Re: [RFC][PATCHv4 4/6] printk: report lost messages in printk  safe/nmi contexts Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2016-12-01 03:20 +0100
    Re: [RFC][PATCHv4 4/6] printk: report lost messages in printk  safe/nmi contexts Petr Mladek <pmladek@suse.com> - 2016-12-01 14:00 +0100

csiph-web