All of lore.kernel.org
 help / color / mirror / Atom feed
From: John Ogness <john.ogness@linutronix.de>
To: Petr Mladek <pmladek@suse.com>
Cc: Peter Zijlstra <peterz@infradead.org>,
	Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>,
	Sergey Senozhatsky <sergey.senozhatsky@gmail.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Andrea Parri <parri.andrea@gmail.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Paul McKenney <paulmck@kernel.org>,
	kexec@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v4 0/4] printk: replace ringbuffer
Date: Fri, 10 Jul 2020 11:58:34 +0206	[thread overview]
Message-ID: <87o8onhf31.fsf@jogness.linutronix.de> (raw)
In-Reply-To: <20200710091137.GN4751@alley>

On 2020-07-10, Petr Mladek <pmladek@suse.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.

> 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.

> 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.

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).

John Ogness

WARNING: multiple messages have this Message-ID (diff)
From: John Ogness <john.ogness@linutronix.de>
To: Petr Mladek <pmladek@suse.com>
Cc: Andrea Parri <parri.andrea@gmail.com>,
	Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>,
	Paul McKenney <paulmck@kernel.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	kexec@lists.infradead.org, linux-kernel@vger.kernel.org,
	Steven Rostedt <rostedt@goodmis.org>,
	Sergey Senozhatsky <sergey.senozhatsky@gmail.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Linus Torvalds <torvalds@linux-foundation.org>
Subject: Re: [PATCH v4 0/4] printk: replace ringbuffer
Date: Fri, 10 Jul 2020 11:58:34 +0206	[thread overview]
Message-ID: <87o8onhf31.fsf@jogness.linutronix.de> (raw)
In-Reply-To: <20200710091137.GN4751@alley>

On 2020-07-10, Petr Mladek <pmladek@suse.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.

> 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.

> 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.

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).

John Ogness

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

  reply	other threads:[~2020-07-10  9:52 UTC|newest]

Thread overview: 55+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-07 14:59 [PATCH v4 0/4] printk: replace ringbuffer John Ogness
2020-07-07 14:59 ` John Ogness
2020-07-07 14:59 ` [PATCH v4 1/4] crash: add VMCOREINFO macro to define offset in a struct declared by typedef John Ogness
2020-07-07 14:59   ` John Ogness
2020-07-07 14:59 ` [PATCH v4 2/4] printk: add lockless ringbuffer John Ogness
2020-07-07 14:59   ` John Ogness
2020-07-07 14:59 ` [PATCH v4 3/4] Revert "printk: lock/unlock console only for new logbuf entries" John Ogness
2020-07-07 14:59   ` John Ogness
2020-07-08 14:34   ` Petr Mladek
2020-07-08 14:34     ` Petr Mladek
2020-07-09  1:20   ` Sergey Senozhatsky
2020-07-09  1:20     ` Sergey Senozhatsky
2020-07-07 14:59 ` [PATCH v4 4/4] printk: use the lockless ringbuffer John Ogness
2020-07-07 14:59   ` John Ogness
2020-07-07 19:25   ` kernel test robot
2020-07-07 19:25     ` kernel test robot
2020-07-07 19:25     ` kernel test robot
2020-07-08 13:18     ` John Ogness
2020-07-08 13:18       ` John Ogness
2020-07-08 13:18       ` John Ogness
2020-07-08 14:35   ` Petr Mladek
2020-07-08 14:35     ` Petr Mladek
2020-07-08 19:24   ` kernel test robot
2020-07-08 19:24     ` kernel test robot
2020-07-08 19:24     ` kernel test robot
2020-07-09  7:14   ` [printk] 18a2dc6982: ltp.kmsg01.fail kernel test robot
2020-07-09  7:14     ` kernel test robot
2020-07-09  8:33     ` Sergey Senozhatsky
2020-07-09  8:33       ` Sergey Senozhatsky
2020-07-09 10:14       ` John Ogness
2020-07-09 10:14         ` John Ogness
2020-07-09 10:59         ` Petr Mladek
2020-07-09 10:59           ` Petr Mladek
2020-07-09 11:13           ` Petr Mladek
2020-07-09 11:13             ` Petr Mladek
2020-07-09 11:17             ` John Ogness
2020-07-09 11:17               ` John Ogness
2020-07-09 12:25               ` Petr Mladek
2020-07-09 12:25                 ` Petr Mladek
2020-07-09 13:07                 ` Sergey Senozhatsky
2020-07-09 13:07                   ` Sergey Senozhatsky
2020-07-09 14:41                   ` Petr Mladek
2020-07-09 14:41                     ` Petr Mladek
2020-07-08 15:20 ` [PATCH v4 0/4] printk: replace ringbuffer Petr Mladek
2020-07-08 15:20   ` Petr Mladek
2020-07-09  7:03   ` John Ogness
2020-07-09  7:03     ` John Ogness
2020-07-10  9:11     ` Petr Mladek
2020-07-10  9:11       ` Petr Mladek
2020-07-10  9:52       ` John Ogness [this message]
2020-07-10  9:52         ` John Ogness
2020-07-10 14:15         ` Petr Mladek
2020-07-10 14:15           ` Petr Mladek
2020-07-14  2:56         ` Sergey Senozhatsky
2020-07-14  2:56           ` Sergey Senozhatsky

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87o8onhf31.fsf@jogness.linutronix.de \
    --to=john.ogness@linutronix.de \
    --cc=gregkh@linuxfoundation.org \
    --cc=kexec@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=parri.andrea@gmail.com \
    --cc=paulmck@kernel.org \
    --cc=peterz@infradead.org \
    --cc=pmladek@suse.com \
    --cc=rostedt@goodmis.org \
    --cc=sergey.senozhatsky.work@gmail.com \
    --cc=sergey.senozhatsky@gmail.com \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.