On Mon, Jan 27, 2014 at 11:48 AM, Luck, Tony <tony.luck@xxxxxxxxx> wrote:
>> The thing is, unless the next printk is a continuation (PR_CONT), the
>> newline should be added at that point. And if it isn't, then that's a
>> bug. So I'm wondering how this issue got noticed - does it actually
>> cause user-visible issues, or was it just code reading?
>
> The next thing that happened was a debug printk() that didn't have
> any log level specified.
>
> printk("CMCI on cpu%d\n", smp_processor_id());
>
> and no newline was added.
Ok, I think that's a bug. Only an explicit KERN_CONT prefix should
cause a line continuation.
The new msg code seems to be terminally confused about this, and for
example, seems to completely ignore the KERN_CONT flag, and instead
tests for just LOG_PREFIX (which is set for any prefix, not the
KERN_CONT 'c' prefix).
And then it turns LOG_CONT to be about the *current* printk missing a
newline. So the whole new msg code seems to be really confused about
what the whole KERN_CONT thing was all about, and mixed it up with
whether the message ended in a newline or not, which is entirely
bogus.
Adding more people. Looking at the code in vprintk_emit(), somebody
is really confused.
Anyway, attached is a *totally* untested patch. Comments?
Linus
kernel/printk/printk.c | 43 ++++++++++++-------------------------------
1 file changed, 12 insertions(+), 31 deletions(-)
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index b1d255f04135..fae5c8ab607d 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -1561,7 +1561,9 @@ asmlinkage int vprintk_emit(int facility, int level,
level = kern_level - '0';
case 'd': /* KERN_DEFAULT */
lflags |= LOG_PREFIX;
+ break;
case 'c': /* KERN_CONT */
+ lflags |= LOG_CONT;
break;
}
text_len -= end_of_header - text;
@@ -1575,40 +1577,19 @@ asmlinkage int vprintk_emit(int facility, int level,
if (dict)
lflags |= LOG_PREFIX|LOG_NEWLINE;
- if (!(lflags & LOG_NEWLINE)) {
- /*
- * 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);
-
- /* 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,
- dict, dictlen, text, text_len);
- } 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);
+ if (cont.len) {
+ if (cont.owner != current || !(lflags & LOG_CONT)) {
cont_flush(LOG_NEWLINE);
+ lflags &= ~LOG_CONT;
}
-
- if (!stored)
- log_store(facility, level, lflags, 0,
- dict, dictlen, text, text_len);
}
+
+ /*
+ * If LOG_CONT is set, try to add the new message to any old one.
+ * If that fails - or LOG_CONT wasmn't set - create a new record.
+ */
+ if (!(lflags & LOG_CONT) || !cont_add(facility, level, text, text_len))
+ log_store(facility, level, lflags, 0, dict, dictlen, text, text_len);
printed_len += text_len;
/*
