[ltt-dev] [PATCH 7/7] urcu, call_rcu: Array initialized before pointer is planted

[mathieu.desnoyers@efficios.com (Mathieu Desnoyers)]

* Paul E. McKenney (paulmck at linux.vnet.ibm.com) wrote:
> On Wed, Sep 28, 2011 at 04:34:33PM +0800, Lai Jiangshan wrote:
> > Signed-off-by: Lai Jiangshan <laijs at cn.fujitsu.com>
> 
> This one I don't understand.  It looks to me that all dereferences
> of the per_cpu_call_rcu_data pointer are done while holding the
> call_rcu_mutex, so there should be no need for the memory barrier.
> 
> Yes, there is an access to the pointer without lock protection
> in create_all_cpu_call_rcu_data(), but that access does not
> do a dereference, so still no need for the memory barrier.
> 
> So, what am I missing?

A concurrent get_cpu_call_rcu_data(), called by get_call_rcu_data(),
could dereference this pointer without holding any mutex. So this
situation would happen if we have a concurrent call_rcu() executing
while we do the create_all_cpu_call_rcu_data().

I think we would need to put a rcu_dereference() around
per_cpu_call_rcu_data read within get_cpu_call_rcu_data() too.
per_cpu_call_rcu_data should be done with rcu_set_pointer.

Also, a rcu read-side critical section would be required around any
usage of per_cpu_call_rcu_data, and the action of tearing down the
per-cpu data would require to wait for a quiescent state. So we would
basically require that the call_rcu users need to be registered as
RCU reader threads.

Or we clearly state that no call_rcu should happen while
create_all_cpu_call_rcu_data is executed. But given we allow call_rcu to
be called within call_rcu callbacks, this seems to be hard to guarantee.

Thoughts ?

Mathieu

> 
> 							Thanx, Paul
> 
> > ---
> >  urcu-call-rcu-impl.h |    4 ++++
> >  1 files changed, 4 insertions(+), 0 deletions(-)
> > 
> > diff --git a/urcu-call-rcu-impl.h b/urcu-call-rcu-impl.h
> > index 3c68ae7..462139a 100644
> > --- a/urcu-call-rcu-impl.h
> > +++ b/urcu-call-rcu-impl.h
> > @@ -104,6 +104,10 @@ static void alloc_cpu_call_rcu_data(void)
> >  	p = malloc(maxcpus * sizeof(*per_cpu_call_rcu_data));
> >  	if (p != NULL) {
> >  		memset(p, '\0', maxcpus * sizeof(*per_cpu_call_rcu_data));
> > +
> > +		/* Array initialized before pointer is planted. */
> > +		cmm_smp_mb();
> > +
> >  		per_cpu_call_rcu_data = p;
> >  	} else {
> >  		if (!warned) {
> > -- 
> > 1.7.4.4
> > 
> 

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com