Re: printk: what is going on with additional newlines?

From: Joe Perches
Date: Fri Sep 01 2017 - 13:32:26 EST


On Fri, 2017-09-01 at 22:19 +0900, Sergey Senozhatsky wrote:
> On (08/29/17 21:10), Steven Rostedt wrote:
> [..]
> > > could do. for a single continuation line printk("%.*s", s.len, s.buffer)
> > > this will work perfectly fine. for a more general case - backtraces, dumps,
> > > etc. - this requires some tweaks.
> >
> > We could simply add a seq_buf_printk() that is implemented in the printk
> > proper, to parse the seq_buf buffer properly, and add the timestamps and
> > such.
>
> so I quickly cooked the first version. like really quickly. just to
> check if this is what people might like/use.
>
> RFC.
>
> so wondering if this will suffice. the name is somewhat hideous -- prbuf(),
> wanted to keep it short and somehow aligned with pr_foo().

Yes, it's a poor name. At least keep using a pr_ prefix.

> the patch also defines a number of prbuf_err()/prbuf_cont() macros that
> call __prbuf_write() -- I don't want people to invoke __prbuf_write()
> directly, because we need KERN_FOO prefix for stored messages and people
> tend to forget to provide one.

> prbuf_init() function inits the seq_buf buffer. it takes size and GFP
> mask, just to permit prbuf usage from different contexts. if we fail
> to kmalloc() the buffer, then __prbuf_write() does direct printk().

I think there's relatively little value in multiple line output.
It seems like buffering for buffering's sake.
Just keep it to a single line and simple.

> a usage example:
>
>
> struct seq_buf s;
>
> prbuf_init(&s, 256, GFP_KERNEL);
>
> prbuf_err(&s, "Opps at %lu\n", _RET_IP_);
> prbuf_info(&s, "Start of cont line");
> prbuf_cont(&s, " foo ");
> prbuf_cont(&s, " bar ");
> prbuf_cont(&s, " status: %s\n", "done");
>
> ret = 0;
> while (ret++ < 10)
> prbuf_err(&s, "%x\n", ret);
>
> prbuf_flush(&s);
> prbuf_free(&s);
>
>
> this will store everything in conseq logbuf entries. if the buffer
> was too small, we print overflow message.
>
> any comments?
[]
> diff --git a/include/linux/printk.h b/include/linux/printk.h
[]
> @@ -277,6 +288,29 @@ static inline void printk_safe_flush(void)
> static inline void printk_safe_flush_on_panic(void)
> {
> }
> +
> +struct seq_buf;
> +
> +static inline
> +int prbuf_init(struct seq_buf *s, size_t size, gfp_t flags)
> +{
> + return 0;
> +}
> +
> +static inlin __printf(2, 3) __cold

uncompiled

> +static int __prbuf_write(struct seq_buf *s, const char *fmt, ...)

inline

> +int prbuf_init(struct seq_buf *s, size_t size, gfp_t flags)
> +{
> + char *b;
> +
> + b = kmalloc(size, flags);
> + seq_buf_init(s, b, size);
> + return !!b;
> +}

Most of the time, this buffer should be on the stack
and not be malloc'd.