linux-kernel - Re: [GIT PULL] printk for 5.19

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAHk-=wgC47n_7E6UtFx_agkJtLmWOXGsjdFjybBFYNA1AheQLQ@mail.gmail.com>
Date:   Wed, 25 May 2022 11:09:02 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Petr Mladek <pmladek@...e.com>
Cc:     Sergey Senozhatsky <senozhatsky@...omium.org>,
        Steven Rostedt <rostedt@...dmis.org>,
        John Ogness <john.ogness@...utronix.de>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Rasmus Villemoes <linux@...musvillemoes.dk>,
        Jan Kara <jack@...e.cz>, Peter Zijlstra <peterz@...radead.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [GIT PULL] printk for 5.19

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