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:   Mon, 22 Aug 2022 10:39:17 -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 10:08 AM Linus Torvalds
<torvalds@...ux-foundation.org> wrote:
>
> 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.

Here's a patch that is

 (a) almost entirely untested (I checked that one single case builds
and seems to generate the expected code)

 (b) needs some more loving

but seems to superficially work.

At a minimum this needs to be split into two (so the bitop and the
wait_on_bit parts split up), and that whole placement of
<asm/barrier.h> and generic_bit_test_acquire() need at least some
thinking about, but on the whole it seems reasonable.

For example, it would make more sense to have this in
<asm-generic/bitops/lock.h>, but not all architectures include that,
and some do their own version. I didn't want to mess with
architecture-specific headers, so this illogically just uses
generic-non-atomic.h.

Maybe just put it in <linux/bitops.h> directly?

So I'm not at all claiming that this is a great patch. It definitely
needs more work, and a lot more testing.

But I think this is at least the right _direction_ to take here.

And yes, I think it also would have been better if
"clear_bit_unlock()" would have been called "clear_bit_release()", and
we'd have more consistent naming with our ordered atomics. But it's
probably not worth changing.

               Linus

View attachment "patch.diff" of type "text/x-patch" (4197 bytes)

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ