Re: [PATCH] printk: flush conflicting continuation line

From: Joe Perches
Date: Thu Jan 02 2014 - 19:57:54 EST


(Adding Kay to cc's)

Kay? any opinion on correctness?

On Thu, 2014-01-02 at 14:55 -0800, Andrew Morton wrote:
> On Wed, 1 Jan 2014 17:44:06 +0530 Arun KS <arunks.linux@xxxxxxxxx> wrote:
>
> > >From d751f9a0cb6329ae3171f6e1cb85e4a3aa792d73 Mon Sep 17 00:00:00 2001
> > From: Arun KS <arun.ks@xxxxxxxxxxxx>
> > Date: Wed, 1 Jan 2014 17:24:46 +0530
> > Subject: printk: flush conflicting continuation line
> >
> > An earlier newline was missing and current print is from different task.
> > In this scenario flush the continuation line and store this line seperatly.
> >
> > This patch fix the below scenario of timestamp interleaving,
> > <6>[ 28.154370 ] read_word_reg : reg[0x 3], reg[0x 4] data [0x 642]
> > <6>[ 28.155428 ] uart disconnect
> > <6>[ 31.947341 ] dvfs[cpufreq.c<275>]:plug-in cpu<1> done
> > <4>[ 28.155445 ] UART detached : send switch state 201
> > <6>[ 32.014112 ] read_reg : reg[0x 3] data[0x21]
> >
> > ...
> >
> > --- a/kernel/printk/printk.c
> > +++ b/kernel/printk/printk.c
> > @@ -1604,7 +1604,10 @@ asmlinkage int vprintk_emit(int facility, int level,
> > if (!(lflags & LOG_PREFIX))
> > stored = cont_add(facility, level, text, text_len);
> > cont_flush(LOG_NEWLINE);
> > - }
> > + /* Flush conflicting buffer. An earlier newline was missing
> > + * and current print is from different task */
> > + } else if (cont.len && cont.owner != current)
> > + cont_flush(LOG_NEWLINE);
> >
> > if (!stored)
> > log_store(facility, level, lflags, 0,
>
> Your email client makes a horrid mess of the patches :(
>
> I *think* it's right. But the code can be significantly simplified and
> optimised. Please review:
>
> } else {
> bool stored = false;
>
> /*
> * If an earlier newline was missing and it was the same task,
> * either merge it with the current buffer and flush, or if
> * there was a race with interrupts (prefix == true) then just
> * flush it out and store this line separately.
> * If the preceding printk was from a different task and missed
> * a newline, flush and append the newline.
> */
> if (cont.len) {
> if (cont.owner == current && !(lflags & LOG_PREFIX))
> stored = cont_add(facility, level, text,
> text_len);
> cont_flush(LOG_NEWLINE);
> }
>
> if (!stored)
> log_store(facility, level, lflags, 0,
> dict, dictlen, text, text_len);
> }
>
>
>
> --- a/kernel/printk/printk.c~printk-flush-conflicting-continuation-line-fix
> +++ a/kernel/printk/printk.c
> @@ -1595,15 +1595,15 @@ asmlinkage int vprintk_emit(int facility
> * either merge it with the current buffer and flush, or if
> * there was a race with interrupts (prefix == true) then just
> * flush it out and store this line separately.
> + * If the preceding printk was from a different task and missed
> + * a newline, flush and append the newline.
> */
> - if (cont.len && cont.owner == current) {
> - if (!(lflags & LOG_PREFIX))
> - stored = cont_add(facility, level, text, text_len);
> - cont_flush(LOG_NEWLINE);
> - /* Flush conflicting buffer. An earlier newline was missing
> - * and current print is from different task */
> - } else if (cont.len && cont.owner != current)
> + if (cont.len) {
> + if (cont.owner == current && !(lflags & LOG_PREFIX))
> + stored = cont_add(facility, level, text,
> + text_len);
> cont_flush(LOG_NEWLINE);
> + }
>
> if (!stored)
> log_store(facility, level, lflags, 0,
> _


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/