Message-ID: <20200710141527.GP4751@alley>
Date:   Fri, 10 Jul 2020 16:15:27 +0200
From:   Petr Mladek <pmladek@...e.com>
To:     John Ogness <john.ogness@...utronix.de>
Cc:     Peter Zijlstra <peterz@...radead.org>,
        Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>,
        Sergey Senozhatsky <sergey.senozhatsky@...il.com>,
        Steven Rostedt <rostedt@...dmis.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Andrea Parri <parri.andrea@...il.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Paul McKenney <paulmck@...nel.org>, kexec@...ts.infradead.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 0/4] printk: replace ringbuffer

On Fri 2020-07-10 11:58:34, John Ogness wrote:
> On 2020-07-10, Petr Mladek <pmladek@...e.com> wrote:
> >> The next series in the printk-rework (move LOG_CONT handling from
> >> writers to readers) makes some further changes that, while not
> >> incompatible, could affect the output of existing tools. It may be a
> >> good idea to let the new ringbuffer sit in linux-next until the next
> >> series has been discussed/reviewed/merged. After the next series,
> >> everything will be in place (with regard to userspace tools) to
> >> finish the rework.
> >
> > I know that it might be premature question. But I wonder what kind
> > of changes are expected because of the continuous lines.
> 
> I will be posting the next series quite soon, so I think it will be
> better to discuss it when we have a working example in front of us.

sure

> > Do you expect some changes in the ring buffer structures so that
> > the debugging tools would need yet another update to actually
> > access the data?
>
> The next series will be modifying the ringbuffer to allow data-less
> records. This is necessary to support the thousands of
> 
>     pr_cont("\n");
> 
> calls in the kernel code. Failed dataring allocations will still be
> detected because the message flags for those records will be 0. For the
> above pr_cont() line, they will be LOG_NEWLINE|LOG_CONT.
> 
> Since the dump tools need to make changes for the new ringbuffer anyway,
> I think it would be good to hammer out the accepted LOG_CONT
> implementation first, just in case we do need to make any subtle
> internal changes.

Makes sense.


> > Or do you expect backward compatible changes that would allow
> > to pass related parts of the continuous lines via syslog/dev_kmsg
> > interface and join them later in userspace?
> 
> For users of console, non-extended netconsole, syslog, and kmsg_dump,
> there will be no external changes whatsoever. These interfaces have no
> awareness of sequence numbers, which will allow the kernel to
> re-assemble the LOG_CONT messages for them.

yup

> Users of /dev/kmsg and extended netconsole see sequence numbers. Offlist
> we discussed various hacks how to get around this without causing errors
> for existing software, but it was all ugly.
> 
> IMHO users of these sequence number interfaces need to see all the
> records individually and reassemble the LOG_CONT messages themselves if
> they want to. I believe that is the only sane path forward. To do this,
> the caller id will no longer be optional to the sequence number output
> since that is vital information to re-assemble the LOG_CONT messages.
> 
> Keep in mind that current software already needs to be able to handle
> the caller id being shown. Also, currently in mainline there is no
> guarantee that LOG_CONT messages are contiguous. So current software
> must also be ready to accept broken up LOG_CONT messages. This is why I
> think it would be acceptable to make this change for /dev/kmsg and
> extended netconsole. But I understand it is controversial since tools
> like systemd and dmesg use /dev/kmsg. Until they are modified to
> re-assemble LOG_CONT messages, they will present the user with the
> ugliness of LOG_CONT pieces (always, rather than as is now rarely).

I am not completely sure how often newline is used in common messages.
I hope that it is more frequently used in a debug code that has to
dump many details.

If my assumption is correct then your proposed approach should be well
acceptable. Normal people will not notice. Developers should be able
to deal with it, especially when a reasonable output might be seen
on consoles, via dmesg or crashdumnp.

Anyway, the proposal looks fine to me. I hope that it will work :-)
Let's continue the discussion when you send the patchset.

Best Regards,
Petr

Powered by blists - more mailing lists