Message-ID: <1388710666.2259.71.camel@joe-AO722>
Subject: Re: [PATCH] printk: flush conflicting continuation line
From: Joe Perches <joe@perches.com>
To: Andrew Morton <akpm@linux-foundation.org>, Kay Sievers <kay@vrfy.org>
Cc: Arun KS <arunks.linux@gmail.com>, linux-kernel@vger.kernel.org,
        tj@kernel.org, fweisbec@gmail.com, paul.gortmaker@windriver.com
Date: Thu, 02 Jan 2014 16:57:46 -0800
In-Reply-To: <20140102145548.f3fe2c3fd3ea706ed388685c@linux-foundation.org>
References: <CAKZGPAMmtDNYQfMnujG9pozkn=xd8=9hoB++8avqxgbYomvhTQ@mail.gmail.com>
	 <20140102145548.f3fe2c3fd3ea706ed388685c@linux-foundation.org>
Content-Type: text/plain; charset="ISO-8859-1"
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 3717
Lines: 102

(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@gmail.com> wrote:
> 
> > >From d751f9a0cb6329ae3171f6e1cb85e4a3aa792d73 Mon Sep 17 00:00:00 2001
> > From: Arun KS <arun.ks@broadcom.com>
> > 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@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/