Messages in this thread | | | Date | Sat, 20 Apr 2019 10:26:15 +1000 | From | Nicholas Piggin <> | Subject | Re: [PATCH] Documentation: atomic_t.txt: Explain ordering provided by smp_mb__{before,after}_atomic() |
| |
Paul E. McKenney's on April 20, 2019 4:26 am: > On Fri, Apr 19, 2019 at 08:00:17PM +0200, Peter Zijlstra wrote: >> On Fri, Apr 19, 2019 at 01:21:45PM -0400, Alan Stern wrote: >> > Index: usb-devel/Documentation/atomic_t.txt >> > =================================================================== >> > --- usb-devel.orig/Documentation/atomic_t.txt >> > +++ usb-devel/Documentation/atomic_t.txt >> > @@ -171,7 +171,10 @@ The barriers: >> > smp_mb__{before,after}_atomic() >> > >> > only apply to the RMW ops and can be used to augment/upgrade the ordering >> > -inherent to the used atomic op. These barriers provide a full smp_mb(). >> > +inherent to the used atomic op. Unlike normal smp_mb() barriers, they order >> > +only the RMW op itself against the instructions preceding the >> > +smp_mb__before_atomic() or following the smp_mb__after_atomic(); they do >> > +not order instructions on the other side of the RMW op at all. >> >> Now it is I who is confused; what? >> >> x = 1; >> smp_mb__before_atomic(); >> atomic_add(1, &a); >> y = 1; >> >> the stores to both x and y will be ordered as if an smp_mb() where >> there. There is no order between a and y otoh. > > Let's look at x86. And a slightly different example: > > x = 1; > smp_mb__before_atomic(); > atomic_add(1, &a); > r1 = y; > > The atomic_add() asm does not have the "memory" constraint, which is > completely legitimate because atomic_add() does not return a value, > and thus guarantees no ordering. The compiler is therefore within > its rights to transform the code into the following: > > x = 1; > smp_mb__before_atomic(); > r1 = y; > atomic_add(1, &a); > > But x86's smp_mb__before_atomic() is just a compiler barrier, and > x86 is further allowed to reorder prior stores with later loads. > The CPU can therefore execute this code as follows: > > r1 = y; > x = 1; > smp_mb__before_atomic(); > atomic_add(1, &a); > > So in general, the ordering is guaranteed only to the atomic itself, > not to accesses on the other side of the atomic.
That's interesting. I don't think that's what all our code expects. I had the same idea as Peter.
IIRC the primitive was originally introduced exactly so x86 would not need to have the unnecessary hardware barrier with sequences like
smp_mb(); ... atomic_inc(&v);
The "new" semantics are a bit subtle. One option might be just to replace it entirely with atomic_xxx_mb() ?
Thanks, Nick
| |