archive mirror
 help / color / mirror / Atom feed
From: John Ogness <>
To: Linus Torvalds <>,
	Petr Mladek <>
Cc: Sergey Senozhatsky <>,
	Steven Rostedt <>,
	Andy Shevchenko <>,
	Rasmus Villemoes <>,
	Jan Kara <>, Peter Zijlstra <>,
	Linux Kernel Mailing List <>
Subject: Re: [GIT PULL] printk for 5.19
Date: Thu, 26 May 2022 00:08:46 +0206	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

On 2022-05-25, Linus Torvalds <> wrote:
> On Mon, May 23, 2022 at 6:21 AM Petr Mladek <> wrote:
>> There are situations when the kthreads are either not available or
>> not reliable, for example, early boot, suspend, or panic. In these
>> situations, printk() uses the legacy mode and tries to handle
>> consoles immediately.
> Let's see how this works out, but I do have one complaint/query about
> the series.
> Looking through the commits, I don't see how that "printk: wake up all
> waiters" makes any sense at all.
> It *ALREADY* woke up all waiters as far as I can see.
> Doing a wake_up_interruptible() will stop waking things up only when
> it hits a *exclusive* waiter, and as far as I can tell, there are no
> exclusive waiters there.

You are correct. @log_wait never has exclusive waiters. I will post a
patch to revert the change in question.

Until now, I never took the time to investigate if there were any
exclusive waiters. I just wanted to be sure that all waiters wake up,
regardless of their type. But obviously the patch is unnecessary with
the currently implemented waiters.

> That "all()" form is only for when there are exclusive waiters (that
> are expected to handle the situation entirely, or wake up the next
> waiter if they don't), *and* you have some exceptional thing that then
> causes *ALL* waiters to need to be woken up.
> For example, a "read()" might be an exclusive wait, so that multiple
> potential concurrent readers don't cause a scheduling herd of
> processes all to wake up when somebody writes to the socket or pipe or
> whatever.
> So in that situation a write() uses a regular wakeup, so that we only
> wake up the one waiter that will take care of things.
> But then a *shutdown* event obviously does affect everybody, so that
> would cause a "wake_up_interruptible_all()".

Thank you for taking the time to clarify the use case for all(). Such a
use case currently does not exist for printk.

> I'm sure we have lots of drivers that are confused about core things
> like this, and I don't really care.
> But when I see something really core like printk() get confused and
> mis-understand basic wait queue behavior, that makes me go "This is

You are correct to question a series when something like this in core
code is found. I cannot guarantee that every line is perfect, but this
series does have a great deal of dedicated review, discussion, revision,
and heavy testing behind it.

Petr has done a great job of requiring these kinds of changes as
separate commits, which certainly helps to identify any false steps.

Thank you for taking extra time to scrutinize this series.

John Ogness

  reply	other threads:[~2022-05-25 22:03 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-23 13:21 [GIT PULL] printk for 5.19 Petr Mladek
2022-05-25 18:09 ` Linus Torvalds
2022-05-25 22:02   ` John Ogness [this message]
2022-05-26 12:09     ` Petr Mladek
2022-05-26 17:13       ` Linus Torvalds
2022-05-25 18:37 ` pr-tracker-bot

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:

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

  git send-email \ \ \ \ \ \ \ \ \ \ \ \

* 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).