Messages in this thread | | | Date | Mon, 15 Jun 2009 19:10:44 +0200 | From | Ingo Molnar <> | Subject | Re: [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
| |