lkml.org 
[lkml]   [2009]   [Jun]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] WARN(): add a \n to the message printk

* Linus Torvalds <torvalds@linux-foundation.org> 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 "<c>"
>
> 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


\
 
 \ /
  Last update: 2009-06-15 19:15    [from the cache]
©2003-2014 Jasper Spaans. hosted at Digital Ocean