Re: [PATCH 4/4] arch: Introduce smp_load_acquire(), smp_store_release()

["Paul E. McKenney" <paulmck@linux.vnet.ibm.com>]

On Tue, Dec 17, 2013 at 10:14:30AM +0100, Peter Zijlstra wrote:
> On Mon, Dec 16, 2013 at 01:37:20PM -0800, Paul E. McKenney wrote:
> > rcu: Define rcu_assign_pointer() in terms of smp_store_release()
> > 
> > The new smp_store_release() function provides better guarantees than did
> > rcu_assign_pointer(), and potentially less overhead on some architectures.
> > The guarantee that smp_store_release() provides that rcu_assign_pointer()
> > does that is obscure, but its lack could cause considerable confusion.
> > This guarantee is illustrated by the following code fragment:
> >     
> >     	struct foo {
> >     		int a;
> >     		int b;
> >     		int c;
> >     		struct foo *next;
> >     	};
> >     	struct foo foo1;
> >     	struct foo foo2;
> >     	struct foo __rcu *foop;
> >     
> >     	...
> >     
> >     	foo2.a = 1;
> >     	foo2.b = 2;
> >     	BUG_ON(foo2.c);
> >     	rcu_assign_pointer(foop, &foo);
> >     
> >     	...
> >     
> >     	fp = rcu_dereference(foop);
> >     	fp.c = 3;
> > 
> > The current rcu_assign_pointer() semantics permit the BUG_ON() to
> > trigger because rcu_assign_pointer()'s smp_wmb() is not guaranteed to
> > order prior reads against later writes.  This commit therefore upgrades
> > rcu_assign_pointer() from smp_wmb() to smp_store_release() to avoid this
> > counter-intuitive outcome.
> 
> This of course raises the obvious question; why doesn't
> rcu_dereference() need to be a smp_load_acquire?
> 
> And I think I know the answer; which would be that the data dependency
> is still sufficient. But the situation does create an unpleasant
> asymmetry in things.

Yep, rcu_dereference() does rely on data dependency rather than explicit
barriers.  That said, for TSO architectures, the code emitted for
data dependency and for the acquire memory barrier is often identical.
And no, many the C/C++11 guys didn't like this asymmetry much either.  ;-)

							Thanx, Paul