Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1764208AbZFORLT (ORCPT ); Mon, 15 Jun 2009 13:11:19 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1763653AbZFORK7 (ORCPT ); Mon, 15 Jun 2009 13:10:59 -0400 Received: from mx2.mail.elte.hu ([157.181.151.9]:41772 "EHLO mx2.mail.elte.hu" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1764136AbZFORK6 (ORCPT ); Mon, 15 Jun 2009 13:10:58 -0400 Date: Mon, 15 Jun 2009 19:10:44 +0200 From: Ingo Molnar To: Linus Torvalds , =?iso-8859-1?Q?Fr=E9d=E9ric?= Weisbecker , Li Zefan Cc: Arjan van de Ven , Linux Kernel Mailing List , Alan Cox Subject: Re: [PATCH] WARN(): add a \n to the message printk Message-ID: <20090615171044.GC25760@elte.hu> References: <20090615000822.6e6bf282@infradead.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.18 (2008-05-17) X-ELTE-SpamScore: -1.5 X-ELTE-SpamLevel: X-ELTE-SpamCheck: no X-ELTE-SpamVersion: ELTE 2.0 X-ELTE-SpamCheck-Details: score=-1.5 required=5.9 tests=BAYES_00 autolearn=no SpamAssassin version=3.2.5 -1.5 BAYES_00 BODY: Bayesian spam probability is 0 to 1% [score: 0.0000] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3562 Lines: 102 * Linus Torvalds wrote: > On Mon, 15 Jun 2009, Linus Torvalds wrote: > > > > On Mon, 15 Jun 2009, Arjan van de Ven wrote: > > > > > > - if (args) > > > + if (args) { > > > vprintk(args->fmt, args->args); > > > + printk("\n"); > > > + } > > > > I really don't like this. What if the format already does have a '\n'? And > > what if some other CPU is printing at the same time? > > > > I'd almost be open to adding a "flags" field to vprintk, and allow setting > > things like "finish line with \n" there. Or perhaps even better, have a > > "vprintk_line()" function that does it with no dynamic flags. Maybe make > > it static, and move all these panic helper funtions into kernel/printk.c > > (since this really is a special case). > > > > I dunno. I'm just throwing out suggestions. I just don't think the above > > patch is very nice. > > Oh, I actually think I have a preference. > > I think we should _always_ cause a line break at the beginning of a new > line, unless the new printk() starts with a KERN_CONT thing. > > Right now KERN_CONT is "", but we could easily make it an explicit > "loglevel" thing. Like this. > > NOTE! This is, of course, totally untested. And we're bound to have > continuation printk's that don't have the KERN_CONT at front, and need > them added, but I think this is generally a saner model than what we have > now, or your suggested explicit addition of '\n'. > > Basically, it tries to guarantee that new messages always get a newline, > unless they _explicitly_ say that they don't want one. Doesn't that make > sense? > > Linus > > --- > include/linux/kernel.h | 2 +- > kernel/printk.c | 16 +++++++++++++++- > 2 files changed, 16 insertions(+), 2 deletions(-) > > diff --git a/include/linux/kernel.h b/include/linux/kernel.h > index 883cd44..066bb1e 100644 > --- a/include/linux/kernel.h > +++ b/include/linux/kernel.h > @@ -102,7 +102,7 @@ extern const char linux_proc_banner[]; > * line that had no enclosing \n). Only to be used by core/arch code > * during early bootup (a continued line is not SMP-safe otherwise). > */ > -#define KERN_CONT "" > +#define KERN_CONT "" > > extern int console_printk[]; > > diff --git a/kernel/printk.c b/kernel/printk.c > index 5052b54..6f416fd 100644 > --- a/kernel/printk.c > +++ b/kernel/printk.c > @@ -691,7 +691,21 @@ asmlinkage int vprintk(const char *fmt, va_list args) > * Copy the output into log_buf. If the caller didn't provide > * appropriate log level tags, we insert them here > */ > - for (p = printk_buf; *p; p++) { > + p = printk_buf; > + > + /* Are we continuing a previous printk? */ > + if (!new_text_line) { > + if (!memcmp(p, KERN_CONT, 3)) { > + /* We intended to do that continued printk! */ > + p += 3; > + } else { > + /* Force a line break */ > + emit_log_char('\n'); > + new_text_line = 1; > + } > + } > + Nice idea ... Puts some pressure on current intentionally 'naked' printks (there's still a few of them) - but that's OK, it's not like KERN_CONT (or pr_cont()) is that hard to add. ( Plus many of our boot printks (where most of the 'naked' printks are currently occuring) are development leftovers and should really be removed, so it's good to shake them up a bit. ) Ingo -- 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/