linux-kernel - Re: [PATCH 5/9] completion: Use simple wait queues

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]

Date:   Sat, 14 Mar 2020 08:40:08 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Sebastian Andrzej Siewior <bigeasy@...utronix.de>
Cc:     Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...nel.org>, Will Deacon <will@...nel.org>,
        "Paul E . McKenney" <paulmck@...nel.org>,
        Joel Fernandes <joel@...lfernandes.org>,
        Steven Rostedt <rostedt@...dmis.org>,
        Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [PATCH 5/9] completion: Use simple wait queues

On Fri, Mar 13, 2020 at 10:47 AM Sebastian Andrzej Siewior
<bigeasy@...utronix.de> wrote:
>
> Replace the wait queue in the completion with a simple wait queue (swait),

This is almost certainly completely and utterly wrong.

Every time somebody uses those horrible swait queues, the end result is buggy.

Don't do it.

And most definitely, don't do it like this, which seems to be entirely
mindlessly just randomly changing things by some brute force in
multiple places.

The swait semantics are completely different from the normal
wait-queue semantics, and generally not in good ways.

There's a *REASON* why the comment at the top of <linux/swait.h> starts with

 * BROKEN wait-queues.
 *
 * These "simple" wait-queues are broken garbage, and should never be
 * used. The comments below claim that they are "similar" to regular
 * wait-queues, but the semantics are actually completely different, and
 * every single user we have ever had has been buggy (or pointless).

and before you do a conversion, you need to spend a _lot_ of time
thinking about why that is the case.

And _after_ you do the conversion, you damn well need to explain why
it's safe. Not just state that it's a good idea.

For example, this patch just randomly changes wait events to the swait
event _exclusive_ waits. With not a single explanation of why that
would be ok.

I want an explanation for EVERY SINGLE CASE. Because people have done
this kind of conversion before, and it's been buggy garbage before. I
want to see that people actually thought about what the semantic
differences were, and _documented_ that thinking process.

            Linus