[PATCH v2 01/14] printk: drop ambiguous LOG_CONT flag

From: Jan H. SchÃnherr
Date: Thu Dec 06 2012 - 12:08:06 EST


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@xxxxxxxx>
Signed-off-by: Jan H. SchÃnherr <schnhrr@xxxxxxxxxxxxxxx>
---
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

--
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/