Messages in this thread | | | Date | Mon, 11 Apr 2005 15:15:06 -0700 | From | "Paul E. McKenney" <> | Subject | Re: [patch 4/5] sched: RCU sched domains |
| |
On Thu, Apr 07, 2005 at 05:58:40PM +1000, Nick Piggin wrote: > Ingo Molnar wrote: > >* Nick Piggin <nickpiggin@yahoo.com.au> wrote: > > > > > >>>At a minimum i think we need the fix+comment below. > >> > >>Well if we say "this is actually RCU", then yes. And we should > >>probably change the preempt_{dis|en}ables in other places to > >>rcu_read_lock. > >> > >>OTOH, if we say we just want all running threads to process through a > >>preemption stage, then this would just be a preempt_disable/enable > >>pair. > >> > >>In practice that makes no difference yet, but it looks like you and > >>Paul are working to distinguish these two cases in the RCU code, to > >>accomodate your low latency RCU stuff? > > > > > >it doesnt impact PREEMPT_RCU/PREEMPT_RT directly, because the scheduler > >itself always needs to be non-preemptible. > > > >those few places where we currently do preempt_disable(), which should > >thus be rcu_read_lock(), are never in codepaths that can take alot of > >time. > > > >but yes, in principle you are right, but in this particular (and > >special) case it's not a big issue. We should document the RCU read-lock > >dependencies cleanly and make all rcu-read-lock cases truly > >rcu_read_lock(), but it's not a pressing issue even considering possible > >future features like PREEMPT_RT. > > > >the only danger in this area is to PREEMPT_RT: it is a bug on PREEMPT_RT > >if kernel code has an implicit 'spinlock means preempt-off and thus > >RCU-read-lock' assumption. Most of the time these get discovered via > >PREEMPT_DEBUG. (preempt_disable() disables preemption on PREEMPT_RT too, > >so that is not a problem either.) > > > > 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().
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:
#define for_each_domain(cpu, domain) \ for (domain = cpu_rq(cpu)->sd; domain; domain = rcu_dereference(domain->parent))
Thanx, Paul - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |