linux-kernel - Re: [PATCHv2 06/11] atomics/treewide: rework ordering barriers

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: <20180704155618.higk5x3ngilbpxjo@lakrids.cambridge.arm.com>
Date:   Wed, 4 Jul 2018 16:56:19 +0100
From:   Mark Rutland <mark.rutland@....com>
To:     Will Deacon <will.deacon@....com>
Cc:     linux-kernel@...r.kernel.org, peterz@...radead.org,
        boqun.feng@...il.com, Andrea Parri <parri.andrea@...il.com>
Subject: Re: [PATCHv2 06/11] atomics/treewide: rework ordering barriers

On Wed, Jul 04, 2018 at 04:06:46PM +0100, Will Deacon wrote:
> On Mon, Jun 25, 2018 at 11:59:47AM +0100, Mark Rutland wrote:
> > Currently architectures can override __atomic_op_*() to define the barriers
> > used before/after a relaxed atomic when used to build acquire/release/fence
> > variants.
> > 
> > This has the unfortunate property of requiring the architecture to define the
> > full wrapper for the atomics, rather than just the barriers they care about,
> > and gets in the way of generating atomics which can be easily read.
> > 
> > Instead, this patch has architectures define an optional set of barriers,
> > __atomic_mb_{before,after}_{acquire,release,fence}(), which <linux/atomic.h>
> > uses to build the wrappers.
> 
> Looks like you've renamed these in the patch but not updated the commit
> message.

Yup; Peter also pointed that out. In my branch this now looks like:

----
Instead, this patch has architectures define an optional set of barriers:

* __atomic_acquire_fence()
* __atomic_release_fence()
* __atomic_pre_fence()
* __atomic_post_fence()

... which <linux/atomic.h> uses to build the wrappers.
----

... which is hopefully more legible, too!

> Also, to add to the bikeshedding, would it worth adding "rmw" in there
> somewhere, e.g. __atomic_post_rmw_fence, since I assume these only
> apply to value-returning stuff?

I don't have any opinion there, but I'm also not sure I've parsed your
rationale correctly. I guess a !RMW full-fence op doesn't make sense? Or
that's something we want to avoid in the API?

AFAICT, we only use __atomic_{pre,post}_fence() for RMW ops today.

Thanks,
Mark.