Messages in this thread | | | Date | Fri, 3 May 2013 10:34:44 -0700 | From | "Paul E. McKenney" <> | Subject | Re: [PATCH v2 1/2] sched: Add cond_resched_rcu_lock() helper |
| |
On Fri, May 03, 2013 at 07:04:47PM +0200, Peter Zijlstra wrote: > > The key point is that I don't understand why we cannot get the effect > > we are looking for with the following in sched.h (or wherever): > > > > static inline int cond_resched_rcu(void) > > { > > #if defined(CONFIG_DEBUG_ATOMIC_SLEEP) || !defined(CONFIG_PREEMPT_RCU) > > rcu_read_unlock(); > > cond_resched(); > > rcu_read_lock(); > > #endif > > } > > > > This adds absolutely no overhead in non-debug builds of CONFIG_PREEMPT_RCU, > > does the checking in debug builds, and allows voluntary preemption in > > !CONFIG_PREEMPT_RCU builds. CONFIG_PROVE_RCU builds will check for an > > (illegal) outer rcu_read_lock() in CONFIG_PREEMPT_RCU builds, and you > > will get "scheduling while atomic" in response to an outer rcu_read_lock() > > in !CONFIG_PREEMPT_RCU builds. > > > > It also seems to me a lot simpler. > > > > Does this work, or am I still missing something? > > It can do quite a number of superfluous rcu_read_unlock()/lock() pairs for > voluntary preemption kernels?
This happens in only two cases:
1. CONFIG_PREEMPT_RCU=n kernels. But in this case, rcu_read_unlock() and rcu_read_lock() are free, at least for CONFIG_PROVE_LOCKING=n kernels. And if you have CONFIG_PROVE_LOCKING=y, any contribution from rcu_read_unlock() and rcu_read_lock() will be in the noise.
2. CONFIG_DEBUG_ATOMIC_SLEEP=y kernels -- but in this case, you -want- the debugging.
So either the overhead is non-existent, or you explicitly asked for the resulting debugging.
In particular, CONFIG_PREEMPT_RCU=y kernels have an empty static inline function, which is free -- unless CONFIG_DEBUG_ATOMIC_SLEEP=y, in which case you again explicitly asked for the debugging.
So I do not believe that the extra rcu_read_unlock()/lock() pairs are a problem in any of the possible combinations of configurations.
Thanx, Paul
| |