Re: [patch 4/5] sched: RCU sched domains

[Nick Piggin <nickpiggin@yahoo.com.au>]

Paul E. McKenney wrote:
> On Thu, Apr 07, 2005 at 05:58:40PM +1000, Nick Piggin wrote:
> 

>>
>>OK thanks for the good explanation. So I'll keep it as is for now,
>>and whatever needs cleaning up later can be worked out as it comes
>>up.
> 
> 
> Looking forward to the split of synchronize_kernel() into synchronize_rcu()
> and synchronize_sched(), the two choices are:
> 
> o	Use synchronize_rcu(), but insert rcu_read_lock()/rcu_read_unlock()
> 	pairs on the read side.
> 
> o	Use synchronize_sched(), and make sure all read-side code is
> 	under preempt_disable().
> 

Yep, I think we'll go for the second option initially (because that
pretty closely matches the homebrew locking scheme that it used to
use).

> Either way, there may also need to be some rcu_dereference()s when picking
> up pointer and rcu_assign_pointer()s when updating the pointers.
> For example, if traversing the domain parent list is to be RCU protected,
> the for_each_domain() macro should change to something like:
> 

Yes, I think you're right, because there's no barriers or synchronisation
when attaching a new domain. Just a small point though:

> #define for_each_domain(cpu, domain) \
> 	for (domain = cpu_rq(cpu)->sd; domain; domain = rcu_dereference(domain->parent))
> 

This should probably be done like so?

#define for_each_domain(cpu, domain) \
	for (domain = rcu_dereference(cpu_rq(cpu)->sd); domain; domain = domain->parent)

And I think it would be wise to use rcu_assign_pointer in the update too.
Thanks Paul.

-- 
SUSE Labs, Novell Inc.