linux-kernel - Re: printk: what is going on with additional newlines?

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1504061881.2786.11.camel@perches.com>
Date:   Tue, 29 Aug 2017 19:58:01 -0700
From:   Joe Perches <joe@...ches.com>
To:     Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>
Cc:     Steven Rostedt <rostedt@...dmis.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Pavel Machek <pavel@....cz>,
        Sergey Senozhatsky <sergey.senozhatsky@...il.com>,
        Petr Mladek <pmladek@...e.com>, Jan Kara <jack@...e.cz>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Jiri Slaby <jslaby@...e.com>, Andreas Mohr <andi@...as.de>,
        Tetsuo Handa <penguin-kernel@...ove.sakura.ne.jp>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: printk: what is going on with additional newlines?

On Wed, 2017-08-30 at 11:47 +0900, Sergey Senozhatsky wrote:
> On (08/29/17 19:31), Joe Perches wrote:
> [..]
> > > the idea is not to do printk() on that seq buffer at all, but to
> > > log_store(), atomically, seq buffer messages
> > > 
> > > 	spin_lock(&logbuf_lock)
> > > 	while (offset < seq_buffer->len) {
> > > 		...
> > > 		log_store(seq->buffer + offset);
> > > 		...
> > > 	}
> > > 	spin_unlock(&logbuf_unlock)
> > 
> > Why?
> > 
> > What's wrong with a simple printk?
> > It'd still do a log_store.
> 
> sure, it will. but in separate logbuf entries, and between two
> consequent printk calls on the same CPU a lot of stuff can happen:

I think you don't quite understand how this would work.
The idea is that the entire concatenated bit would be emitted
in one go.

One use case already in place with seq_buf_init is in
drivers/clk/tegra/clk-bpmp.c

Basically, it's

    static void tegra_bpmp_clk_info_dump(struct tegra_bpmp *bpmp,
    	    	    	    	         const char *level,
    	    	    	    	         const struct tegra_bpmp_clk_info *info)
    {
    	    const char *prefix = "";
    	    struct seq_buf buf;
    	    unsigned int i;
    	    char flags[64];

    	    seq_buf_init(&buf, flags, sizeof(flags));

    	    if (info->flags)
    	    	    seq_buf_printf(&buf, "(");

    	    if (info->flags & TEGRA_BPMP_CLK_HAS_MUX) {
    	    	    seq_buf_printf(&buf, "%smux", prefix);
    	    	    prefix = ", ";
    	    }

    	    if ((info->flags & TEGRA_BPMP_CLK_HAS_SET_RATE) == 0) {
    	    	    seq_buf_printf(&buf, "%sfixed", prefix);
    	    	    prefix = ", ";
    	    }

    	    if (info->flags & TEGRA_BPMP_CLK_IS_ROOT) {
    	    	    seq_buf_printf(&buf, "%sroot", prefix);
    	    	    prefix = ", ";
    	    }

    	    if (info->flags)
    	    	    seq_buf_printf(&buf, ")");

    	    [...]
    	    dev_printk(level, bpmp->dev, "  flags: %lx %s\n", info->flags, flags);

so that the dev_printk is simply emitting a buffer from
concatenated strings via the seq_buf_printf uses.

The other use case would be an entire printk buffer
all at once.

ala:

	seq_buf_init(seq, buf, sizeof(buf));
	seq_buf_printf(&buf, "KERN_<LEVEL> fmt...", args...)
	for (i = 0; i < bar; i++)
		seq_buf_printf(&buf, fmt, ...)
	seq_buf_printk(&buf);

or:

	seq_buf_init(seq, buf, sizeof(buf));
	seq_buf_printf(&buf,
fmt, args...)
	for (i = 0; i < bar; i++)
		seq_buf_printf(&bu
f, fmt, args...)
	seq_buf_printk(&buf, KERN_<LEVEL>);

I can't think of another use case.

Can you?