linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Jan H. Schönherr" <schnhrr@cs.tu-berlin.de>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Kay Sievers <kay@vrfy.org>
Cc: linux-kernel@vger.kernel.org, "Joe Perches" <joe@perches.com>,
	"Andrew Morton" <akpm@linux-foundation.org>,
	"Stephen Rothwell" <sfr@canb.auug.org.au>,
	"Jan H. Schönherr" <schnhrr@cs.tu-berlin.de>
Subject: [PATCH v2 01/14] printk: drop ambiguous LOG_CONT flag
Date: Thu,  6 Dec 2012 18:05:58 +0100	[thread overview]
Message-ID: <1354813571-11253-2-git-send-email-schnhrr@cs.tu-berlin.de> (raw)
In-Reply-To: <1354813571-11253-1-git-send-email-schnhrr@cs.tu-berlin.de>

The meaning of LOG_CONT is unclear, i. e., whether a message is a starting,
ending, or middle fragment. Unfortunately, this cannot be inferred from
the LOG_PREFIX and LOG_NEWLINE flags, as they are not always kept.
Furthermore, in some cases LOG_CONT is set, although it is unknown if
there will be a continuation. This leads to wrongly concatenated output.

Fix this by dropping LOG_CONT and rely on LOG_PREFIX and LOG_NEWLINE to
distinguish the type of fragment. That is, if LOG_PREFIX is set, this
fragment does not continue the previous fragment. And if LOG_NEWLINE is
set, this fragment is not continued by the next fragment.

(Unfortunately, we still have to look at the previous fragment to catch the
case of an unset LOG_PREFIX on this fragment, but a set LOG_NEWLINE on
the previous fragment -- and vice versa)

Tested-by: Kay Sievers <kay@vrfy.org>
Signed-off-by: Jan H. Schönherr <schnhrr@cs.tu-berlin.de>
---
v2:
- minor commit message clarification
v1b:
- fixed check for flushed buffer
- minor code adjustment
- coding style fixes
---
 kernel/printk.c | 58 ++++++++++++++++++++++++++++++++-------------------------
 1 file changed, 33 insertions(+), 25 deletions(-)

diff --git a/kernel/printk.c b/kernel/printk.c
index 2d607f4..857ff7c 100644
--- a/kernel/printk.c
+++ b/kernel/printk.c
@@ -197,7 +197,6 @@ enum log_flags {
 	LOG_NOCONS	= 1,	/* already flushed, do not print to console */
 	LOG_NEWLINE	= 2,	/* text ended with a newline */
 	LOG_PREFIX	= 4,	/* text started with a prefix */
-	LOG_CONT	= 8,	/* text is a fragment of a continuation line */
 };
 
 struct log {
@@ -481,10 +480,9 @@ static ssize_t devkmsg_read(struct file *file, char __user *buf,
 	 * better readable output. 'c' in the record flags mark the first
 	 * fragment of a line, '+' the following.
 	 */
-	if (msg->flags & LOG_CONT && !(user->prev & LOG_CONT))
+	if (!(msg->flags & LOG_NEWLINE) && msg->flags & LOG_PREFIX)
 		cont = 'c';
-	else if ((msg->flags & LOG_CONT) ||
-		 ((user->prev & LOG_CONT) && !(msg->flags & LOG_PREFIX)))
+	else if (!(user->prev & LOG_NEWLINE) && !(msg->flags & LOG_PREFIX))
 		cont = '+';
 
 	len = sprintf(user->buf, "%u,%llu,%llu,%c;",
@@ -887,15 +885,16 @@ static size_t msg_print_text(const struct log *msg, enum log_flags prev,
 	bool newline = true;
 	size_t len = 0;
 
-	if ((prev & LOG_CONT) && !(msg->flags & LOG_PREFIX))
+	if (!(prev & LOG_NEWLINE) && !(msg->flags & LOG_PREFIX))
 		prefix = false;
 
-	if (msg->flags & LOG_CONT) {
-		if ((prev & LOG_CONT) && !(prev & LOG_NEWLINE))
-			prefix = false;
+	if (!(msg->flags & LOG_NEWLINE))
+		newline = false;
 
-		if (!(msg->flags & LOG_NEWLINE))
-			newline = false;
+	if (!(prev & LOG_NEWLINE) && prefix) {
+		if (buf)
+			buf[len] = '\n';
+		len++;
 	}
 
 	do {
@@ -1400,35 +1399,36 @@ static void cont_flush(enum log_flags flags)
 	if (cont.len == 0)
 		return;
 
+	cont.flags |= flags;
 	if (cont.cons) {
 		/*
 		 * If a fragment of this line was directly flushed to the
 		 * console; wait for the console to pick up the rest of the
 		 * line. LOG_NOCONS suppresses a duplicated output.
 		 */
-		log_store(cont.facility, cont.level, flags | LOG_NOCONS,
+		log_store(cont.facility, cont.level, cont.flags | LOG_NOCONS,
 			  cont.ts_nsec, NULL, 0, cont.buf, cont.len);
-		cont.flags = flags;
 		cont.flushed = true;
 	} else {
 		/*
 		 * If no fragment of this line ever reached the console,
 		 * just submit it to the store and free the buffer.
 		 */
-		log_store(cont.facility, cont.level, flags, 0,
+		log_store(cont.facility, cont.level, cont.flags, 0,
 			  NULL, 0, cont.buf, cont.len);
 		cont.len = 0;
 	}
 }
 
-static bool cont_add(int facility, int level, const char *text, size_t len)
+static bool cont_add(int facility, int level, enum log_flags flags,
+		     const char *text, size_t len)
 {
 	if (cont.len && cont.flushed)
 		return false;
 
 	if (cont.len + len > sizeof(cont.buf)) {
 		/* the line gets too long, split it up in separate records */
-		cont_flush(LOG_CONT);
+		cont_flush(0);
 		return false;
 	}
 
@@ -1437,7 +1437,7 @@ static bool cont_add(int facility, int level, const char *text, size_t len)
 		cont.level = level;
 		cont.owner = current;
 		cont.ts_nsec = local_clock();
-		cont.flags = 0;
+		cont.flags = flags;
 		cont.cons = 0;
 		cont.flushed = false;
 	}
@@ -1446,7 +1446,7 @@ static bool cont_add(int facility, int level, const char *text, size_t len)
 	cont.len += len;
 
 	if (cont.len > (sizeof(cont.buf) * 80) / 100)
-		cont_flush(LOG_CONT);
+		cont_flush(0);
 
 	return true;
 }
@@ -1456,8 +1456,11 @@ static size_t cont_print_text(char *text, size_t size)
 	size_t textlen = 0;
 	size_t len;
 
-	if (cont.cons == 0 && (console_prev & LOG_NEWLINE)) {
-		textlen += print_time(cont.ts_nsec, text);
+	if (cont.cons == 0 && (console_prev & LOG_NEWLINE ||
+			       cont.flags & LOG_PREFIX)) {
+		if (!(console_prev & LOG_NEWLINE))
+			text[textlen++] = '\n';
+		textlen += print_time(cont.ts_nsec, text + textlen);
 		size -= textlen;
 	}
 
@@ -1575,12 +1578,16 @@ asmlinkage int vprintk_emit(int facility, int level,
 		 * Flush the conflicting buffer. An earlier newline was missing,
 		 * or another task also prints continuation lines.
 		 */
-		if (cont.len && (lflags & LOG_PREFIX || cont.owner != current))
-			cont_flush(LOG_NEWLINE);
+		if (cont.len && !cont.flushed) {
+			if (cont.owner != current)
+				lflags |= LOG_PREFIX;
+			if (lflags & LOG_PREFIX)
+				cont_flush(LOG_NEWLINE);
+		}
 
 		/* buffer line if possible, otherwise store it right away */
-		if (!cont_add(facility, level, text, text_len))
-			log_store(facility, level, lflags | LOG_CONT, 0,
+		if (!cont_add(facility, level, lflags, text, text_len))
+			log_store(facility, level, lflags, 0,
 				  dict, dictlen, text, text_len);
 	} else {
 		bool stored = false;
@@ -1591,9 +1598,10 @@ asmlinkage int vprintk_emit(int facility, int level,
 		 * there was a race with interrupts (prefix == true) then just
 		 * flush it out and store this line separately.
 		 */
-		if (cont.len && cont.owner == current) {
+		if (cont.len && !cont.flushed && cont.owner == current) {
 			if (!(lflags & LOG_PREFIX))
-				stored = cont_add(facility, level, text, text_len);
+				stored = cont_add(facility, level, lflags,
+							text, text_len);
 			cont_flush(LOG_NEWLINE);
 		}
 
-- 
1.8.0.1.20.g7c65b2e.dirty


  reply	other threads:[~2012-12-06 17:08 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-12-06 17:05 [PATCH v2 00/14] printk() fixes, optimizations, and clean ups Jan H. Schönherr
2012-12-06 17:05 ` Jan H. Schönherr [this message]
2012-12-06 17:05 ` [PATCH v2 02/14] printk: use saved timestamp for temporarily buffered message Jan H. Schönherr
2012-12-06 17:06 ` [PATCH v2 03/14] printk: reclaim cont buffer immediately for fully printed messages Jan H. Schönherr
2012-12-06 17:06 ` [PATCH v2 04/14] printk: do not add unnecessary newlines to the continuation buffer Jan H. Schönherr
2012-12-06 17:06 ` [PATCH v2 05/14] printk: reuse reclaimed continuation buffer immediately Jan H. Schönherr
2012-12-06 17:06 ` [PATCH v2 06/14] printk: move wake_klogd-check out of the loop Jan H. Schönherr
2012-12-06 17:06 ` [PATCH v2 07/14] printk: let cont_print_text() reuse existing code Jan H. Schönherr
2012-12-06 17:06 ` [PATCH v2 08/14] printk: refactor locking in console_unlock() Jan H. Schönherr
2012-12-06 17:06 ` [PATCH v2 09/14] printk: retain unknown log-level until cont_add()/log_store() Jan H. Schönherr
2012-12-06 17:06 ` [PATCH v2 10/14] printk: track previously logged message in log_store() Jan H. Schönherr
2012-12-06 17:06 ` [PATCH v2 11/14] printk: avoid glitches in console output Jan H. Schönherr
2012-12-06 17:06 ` [PATCH v2 12/14] printk: encode formatting in message flags Jan H. Schönherr
2012-12-06 17:06 ` [PATCH v2 13/14] printk: drop now useless tracking of previous " Jan H. Schönherr
2012-12-06 17:06 ` [PATCH v2 14/14] printk: refactor continuation buffer handling Jan H. Schönherr
     [not found] ` <20121206133907.37c255e9.akpm@linux-foundation.org>
2012-12-06 23:37   ` [PATCH v2 00/14] printk() fixes, optimizations, and clean ups Joe Perches
     [not found]     ` <20121206161943.78633125.akpm@linux-foundation.org>
2012-12-07  2:51       ` Joe Perches
2012-12-07 11:47         ` "Jan H. Schönherr"
2012-12-07 15:04           ` Greg Kroah-Hartman
2012-12-07 15:42             ` Joe Perches
2012-12-07 15:58               ` Frederic Weisbecker

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1354813571-11253-2-git-send-email-schnhrr@cs.tu-berlin.de \
    --to=schnhrr@cs.tu-berlin.de \
    --cc=akpm@linux-foundation.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=joe@perches.com \
    --cc=kay@vrfy.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=sfr@canb.auug.org.au \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).