Re: [ltt-dev] [RFC git tree] Userspace RCU (urcu) for Linux (repost)

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

On Thu, Feb 12, 2009 at 02:29:41PM -0500, Mathieu Desnoyers wrote:
> 
> * Paul E. McKenney (paulmck@linux.vnet.ibm.com) wrote:
> [...]
> > diff --git a/urcu.c b/urcu.c
> > index f2aae34..a696439 100644
> > --- a/urcu.c
> > +++ b/urcu.c
> > @@ -99,7 +99,8 @@ static void force_mb_single_thread(pthread_t tid)
> >  	 * BUSY-LOOP.
> >  	 */
> >  	while (sig_done < 1)
> > -		smp_rmb();	/* ensure we re-read sig-done */
> > +		barrier();	/* ensure compiler re-reads sig-done */
> > +				/* cache coherence guarantees CPU re-read. */
> 
> OK, this is where I think our points of view differ. Please refer to
> http://lkml.org/lkml/2007/6/18/299.
> 
> Basically, cpu_relax() used in the Linux kernel has an
> architecture-specific implementation which *could* include a smp_rmb()
> if the architecture doesn't notice writes done by other CPUs. I think
> Blackfin is the only architecture currently supported by the Linux
> kernel which defines cpu_relax() as a smp_mb(), because it does not have
> cache coherency.
> 
> Therefore, I propose that we create a memory barrier macro which is
> defined as a 
>   barrier()   when the cpu has cache coherency
>   cache flush when the cpu does not have cache coherency and is
>               compiled with smp support.
> 
> We could call that
> 
>   smp_wmc() (for memory-coherency or memory commit)
>   smp_rmc()
>   smp_mc()
> 
> It would be a good way to identify the location where data exchange
> between memory and the local cache are is required in the algorithm.
> What do you think ?

Actually the best way to do this would be:

	while (ACCESS_ONCE(sig_done) < 1)
		continue;

If ACCESS_ONCE() needs to be made architecture-specific to make this
really work on Blackfin, we should make that change.  And, now that
you mention it, I have heard rumors that other CPU families can violate
cache coherence in some circumstances.

So perhaps ACCESS_ONCE() becomes:

#ifdef CONFIG_ARCH_CACHE_COHERENT
#define ACCESS_ONCE(x) (*(volatile typeof(x) *)&(x))
#else /* #ifdef CONFIG_ARCH_CACHE_COHERENT */
#define ACCESS_ONCE(x)     ({ \
				typeof(x) _________x1; \
				_________x1 = (*(volatile typeof(x) *)&(x)); \
				cpu_relax(); \
				(_________x1); \
				})
#endif /* #else #ifdef CONFIG_ARCH_CACHE_COHERENT */

Seem reasonable?

							Thanx, Paul