linux-kernel - Re: [PATCH] wait_on_bit: add an acquire memory barrier

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-=wh-6RJQWxdVaZSsntyXJWJhivVX8JFH4MqkXv12AHm_=Q@mail.gmail.com>
Date:   Mon, 22 Aug 2022 10:08:48 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Mikulas Patocka <mpatocka@...hat.com>
Cc:     Alan Stern <stern@...land.harvard.edu>,
        Andrea Parri <parri.andrea@...il.com>,
        Will Deacon <will@...nel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Boqun Feng <boqun.feng@...il.com>,
        Nicholas Piggin <npiggin@...il.com>,
        David Howells <dhowells@...hat.com>,
        Jade Alglave <j.alglave@....ac.uk>,
        Luc Maranget <luc.maranget@...ia.fr>,
        "Paul E. McKenney" <paulmck@...nel.org>,
        Akira Yokosawa <akiyks@...il.com>,
        Daniel Lustig <dlustig@...dia.com>,
        Joel Fernandes <joel@...lfernandes.org>,
        linux-kernel@...r.kernel.org, linux-arch@...r.kernel.org
Subject: Re: [PATCH] wait_on_bit: add an acquire memory barrier

On Mon, Aug 22, 2022 at 2:39 AM Mikulas Patocka <mpatocka@...hat.com> wrote:
>
> I'd like to ask what do you think about this patch?

I really don't like it. It adds a pointless read barrier only because
you didn't want to do it properly.

On x86, it doesn't matter, since rmb is a no-op and only a scheduling
barrier (and not noticeable in this case anyway).

On other architectures, it might.

But on all architectures it's just ugly.

I suggested in an earlier thread that you just do it right with an
explicit smp_load_acquire() and a manual bit test.

So why don't we just create a "test_bit_acquire()" and be done with
it? We literally created clear_bit_unlock() for the opposite reason,
and your comments about the new barrier hack even point to it.

Why is "clear_bit_unlock()" worthy of a real helper, but
"test_bit_acquire()" is not and people who want it have to use this
horrendous hack?

Please stop adding random barriers already. Just do it right. I've
said this before, why do you then keep doing this and asking for
comments?

My reply will remain the same: JUST DO IT RIGHT.

                Linus