linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Linus Torvalds <torvalds@linux-foundation.org>
To: Petr Mladek <pmladek@suse.com>
Cc: Sergey Senozhatsky <senozhatsky@chromium.org>,
	Steven Rostedt <rostedt@goodmis.org>,
	John Ogness <john.ogness@linutronix.de>,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	Rasmus Villemoes <linux@rasmusvillemoes.dk>,
	Jan Kara <jack@suse.cz>, Peter Zijlstra <peterz@infradead.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [GIT PULL] printk for 5.19
Date: Wed, 25 May 2022 11:09:02 -0700	[thread overview]
Message-ID: <CAHk-=wgC47n_7E6UtFx_agkJtLmWOXGsjdFjybBFYNA1AheQLQ@mail.gmail.com> (raw)
In-Reply-To: <YouKQw72H7y9EJQK@alley>

On Mon, May 23, 2022 at 6:21 AM Petr Mladek <pmladek@suse.com> 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.

And if they were there, the "wake_up_all()" wouldn't be the right thing anyway.

So that commit seems to be fundamentally confused about things.

You should NEVER use wake_up_interruptible_all() in any normal code.

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

I really don't see why the printk() code decided to use that wakeup
function, and the commit message doesn't explain why it was done
either.

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

Please explain.

                    Linus

  reply	other threads:[~2022-05-25 18:09 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 [this message]
2022-05-25 22:02   ` John Ogness
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:
  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='CAHk-=wgC47n_7E6UtFx_agkJtLmWOXGsjdFjybBFYNA1AheQLQ@mail.gmail.com' \
    --to=torvalds@linux-foundation.org \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=jack@suse.cz \
    --cc=john.ogness@linutronix.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@rasmusvillemoes.dk \
    --cc=peterz@infradead.org \
    --cc=pmladek@suse.com \
    --cc=rostedt@goodmis.org \
    --cc=senozhatsky@chromium.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 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).