Messages in this thread | | | Date | Thu, 5 Jul 2018 17:25:42 +0100 | From | Will Deacon <> | Subject | Re: [PATCHv2 06/11] atomics/treewide: rework ordering barriers |
| |
On Thu, Jul 05, 2018 at 11:12:41AM +0100, Mark Rutland wrote: > On Wed, Jul 04, 2018 at 06:50:00PM +0100, Will Deacon wrote: > > 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. > > Sure thing. > > Perhaps __atomic_{pre,post}_full_fence() might be better, assuming > you're trying to avoid people erroneously assuming that > __atomic_{pre,post}_fence() are like acquire/release fences.
Good idea, I think that's better.
Will
| |