All of lore.kernel.org
 help / color / mirror / Atom feed
From: John Ogness <john.ogness@linutronix.de>
To: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
Cc: Petr Mladek <pmladek@suse.com>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>,
	Sergey Senozhatsky <sergey.senozhatsky@gmail.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	kexec@lists.infradead.org,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: POC: Alternative solution: Re: [PATCH 0/4] printk: reimplement LOG_CONT handling
Date: Thu, 13 Aug 2020 09:50:25 +0206	[thread overview]
Message-ID: <875z9nvvl2.fsf@jogness.linutronix.de> (raw)
In-Reply-To: <20200813051853.GA510@jagdpanzerIV.localdomain>

On 2020-08-13, Sergey Senozhatsky <sergey.senozhatsky@gmail.com> wrote:
> This is not an unseen pattern, I'm afraid. And the problem here can
> be more general:
>
> 	pr_info("text");
> 	pr_cont("1");
> 	exception/IRQ/NMI
> 		pr_alert("text\n");
> 	pr_cont("2");
> 	pr_cont("\n");
>
> I guess the solution would be to store "last log_level" in task_struct
> and get current (new) timestamp for broken cont line?

(Warning: new ideas ahead)

The fundamental problem is that there is no real association between
the cont parts. So any interruption results in a broken record. If we
really want to do this correctly, we need real association.

With the new finalize flag for records, I thought about perhaps adding
support for chaining data blocks.

A data block currently stores an unsigned long for the ID of the
associated descriptor. But it could optionally include a second unsigned
long, which is the lpos of the next text part. All the data blocks of a
chain would point back to the same descriptor. The descriptor would only
point to the first data block of the chain and include a flag that it is
using chained data blocks.

Then we would only need to track the sequence number of the open record
and new data blocks could be added to the data block chain of the
correct record. Readers cannot see the record until it is finalized.

Also, since only finalized records can be invalidated, there are no
races of chains becoming invalidated while being appended.

My concerns about this idea:

- What if the printk user does not correctly terminate the cont message?
  There is no mechanism to allow that open record to be force-finalized
  so that readers can read newer records.

- For tasks, the sequence number of the open record could be stored on
  the task_struct. For non-tasks, we could use a global per-cpu variable
  where each CPU stores 2 sequence numbers: the sequence number of the
  open record for the non-task and the sequence number of the open
  record for an interrupting NMI. Is that sufficient?

John Ogness

WARNING: multiple messages have this Message-ID (diff)
From: John Ogness <john.ogness@linutronix.de>
To: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
Cc: Petr Mladek <pmladek@suse.com>,
	Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	kexec@lists.infradead.org,
	Linux Kernel Mailing List <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: POC: Alternative solution: Re: [PATCH 0/4] printk: reimplement LOG_CONT handling
Date: Thu, 13 Aug 2020 09:50:25 +0206	[thread overview]
Message-ID: <875z9nvvl2.fsf@jogness.linutronix.de> (raw)
In-Reply-To: <20200813051853.GA510@jagdpanzerIV.localdomain>

On 2020-08-13, Sergey Senozhatsky <sergey.senozhatsky@gmail.com> wrote:
> This is not an unseen pattern, I'm afraid. And the problem here can
> be more general:
>
> 	pr_info("text");
> 	pr_cont("1");
> 	exception/IRQ/NMI
> 		pr_alert("text\n");
> 	pr_cont("2");
> 	pr_cont("\n");
>
> I guess the solution would be to store "last log_level" in task_struct
> and get current (new) timestamp for broken cont line?

(Warning: new ideas ahead)

The fundamental problem is that there is no real association between
the cont parts. So any interruption results in a broken record. If we
really want to do this correctly, we need real association.

With the new finalize flag for records, I thought about perhaps adding
support for chaining data blocks.

A data block currently stores an unsigned long for the ID of the
associated descriptor. But it could optionally include a second unsigned
long, which is the lpos of the next text part. All the data blocks of a
chain would point back to the same descriptor. The descriptor would only
point to the first data block of the chain and include a flag that it is
using chained data blocks.

Then we would only need to track the sequence number of the open record
and new data blocks could be added to the data block chain of the
correct record. Readers cannot see the record until it is finalized.

Also, since only finalized records can be invalidated, there are no
races of chains becoming invalidated while being appended.

My concerns about this idea:

- What if the printk user does not correctly terminate the cont message?
  There is no mechanism to allow that open record to be force-finalized
  so that readers can read newer records.

- For tasks, the sequence number of the open record could be stored on
  the task_struct. For non-tasks, we could use a global per-cpu variable
  where each CPU stores 2 sequence numbers: the sequence number of the
  open record for the non-task and the sequence number of the open
  record for an interrupting NMI. Is that sufficient?

John Ogness

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

  reply	other threads:[~2020-08-13  7:44 UTC|newest]

Thread overview: 80+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-17 23:48 [PATCH 0/4] printk: reimplement LOG_CONT handling John Ogness
2020-07-17 23:48 ` John Ogness
2020-07-17 23:48 ` [PATCH 1/4] printk: ringbuffer: support dataless records John Ogness
2020-07-17 23:48   ` John Ogness
2020-07-20 14:49   ` John Ogness
2020-07-20 14:49     ` John Ogness
2020-07-17 23:48 ` [PATCH 2/4] printk: store instead of processing cont parts John Ogness
2020-07-17 23:48   ` John Ogness
2020-07-19 14:35   ` Sergey Senozhatsky
2020-07-19 14:35     ` Sergey Senozhatsky
2020-07-19 18:27     ` Linus Torvalds
2020-07-19 18:27       ` Linus Torvalds
2020-07-20  1:50       ` Sergey Senozhatsky
2020-07-20  1:50         ` Sergey Senozhatsky
2020-07-20 18:30         ` Linus Torvalds
2020-07-20 18:30           ` Linus Torvalds
2020-07-21  3:53           ` Joe Perches
2020-07-21  3:53             ` Joe Perches
2020-07-21 14:42           ` Sergey Senozhatsky
2020-07-21 14:42             ` Sergey Senozhatsky
2020-07-21 14:57             ` John Ogness
2020-07-21 14:57               ` John Ogness
2020-07-29  2:03               ` Sergey Senozhatsky
2020-07-29  2:03                 ` Sergey Senozhatsky
2020-07-21 15:40             ` Linus Torvalds
2020-07-21 15:40               ` Linus Torvalds
2020-07-28  2:22               ` Sergey Senozhatsky
2020-07-28  2:22                 ` Sergey Senozhatsky
2020-07-17 23:48 ` [PATCH 3/4] printk: process cont records during reading John Ogness
2020-07-17 23:48   ` John Ogness
2020-07-17 23:48 ` [PATCH 4/4] ipconfig: cleanup printk usage John Ogness
2020-07-17 23:48   ` John Ogness
2020-07-18  0:00 ` [PATCH 0/4] printk: reimplement LOG_CONT handling Linus Torvalds
2020-07-18  0:00   ` Linus Torvalds
2020-07-18 14:42   ` John Ogness
2020-07-18 14:42     ` John Ogness
2020-07-18 17:42     ` Linus Torvalds
2020-07-18 17:42       ` Linus Torvalds
2020-08-11 16:05     ` Petr Mladek
2020-08-11 16:05       ` Petr Mladek
2020-08-12 16:39       ` POC: Alternative solution: " Petr Mladek
2020-08-12 16:39         ` Petr Mladek
2020-08-13  0:24         ` John Ogness
2020-08-13  0:24           ` John Ogness
2020-08-13  5:18           ` Sergey Senozhatsky
2020-08-13  5:18             ` Sergey Senozhatsky
2020-08-13  7:44             ` John Ogness [this message]
2020-08-13  7:44               ` John Ogness
2020-08-13  8:41               ` Petr Mladek
2020-08-13  8:41                 ` Petr Mladek
2020-08-13 10:29                 ` John Ogness
2020-08-13 10:29                   ` John Ogness
2020-08-13 11:30                   ` Petr Mladek
2020-08-13 11:30                     ` Petr Mladek
2020-08-14  3:34                   ` Sergey Senozhatsky
2020-08-14  3:34                     ` Sergey Senozhatsky
2020-08-14  8:16                     ` John Ogness
2020-08-14  8:16                       ` John Ogness
2020-08-14  9:12                       ` Petr Mladek
2020-08-14  9:12                         ` Petr Mladek
2020-08-13 11:54                 ` Sergey Senozhatsky
2020-08-13 11:54                   ` Sergey Senozhatsky
2020-08-14  9:04                   ` Petr Mladek
2020-08-14  9:04                     ` Petr Mladek
2020-08-14 22:46                   ` Linus Torvalds
2020-08-14 22:46                     ` Linus Torvalds
2020-08-14 23:52                     ` Joe Perches
2020-08-14 23:52                       ` Joe Perches
2020-08-15  2:33                       ` Linus Torvalds
2020-08-15  2:33                         ` Linus Torvalds
2020-08-15  3:08                         ` Joe Perches
2020-08-15  3:08                           ` Joe Perches
2020-08-15  9:25                       ` David Laight
2020-08-15  9:25                         ` David Laight
2020-08-15  5:41                     ` Sergey Senozhatsky
2020-08-15  5:41                       ` Sergey Senozhatsky
2020-08-13  7:51             ` Petr Mladek
2020-08-13  7:51               ` Petr Mladek
2020-08-13  7:31           ` Petr Mladek
2020-08-13  7:31             ` Petr Mladek

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=875z9nvvl2.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=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.