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    [W:0.033 / U:0.992 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site