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

From: Will Deacon
Date: Wed Jul 04 2018 - 13:49:25 EST


On Wed, Jul 04, 2018 at 04:56:19PM +0100, Mark Rutland wrote:
> 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.

No, I think you're right and my terminology is confused. Leave it as-is
for the moment.

Cheers,

Will