Messages in this thread Patch in this message | | | Date | Thu, 9 Jun 2011 17:23:50 -0700 | From | "Paul E. McKenney" <> | Subject | Re: [PATCH 1/4] rcu: Detect uses of rcu read side in extended quiescent states |
| |
On Fri, Jun 10, 2011 at 01:47:24AM +0200, Frederic Weisbecker wrote: > Detect uses of rcu that are not supposed to happen when we > are in an extended quiescent state. > > This can happen for example if we use rcu between the time we > stop the tick and the time we restart it. Or inside an irq that > didn't use rcu_irq_enter,exit() or other possible kind of rcu API > misuse. > > v2: Rebase against latest rcu changes, handle tiny RCU as well
Good idea on checking for RCU read-side critical sections happening in dyntick-idle periods!
But wouldn't it be better to put the checks in rcu_read_lock() and friends? The problem I see with putting them in rcu_dereference_check() is that someone can legitimately do something like the following while in dyntick-idle mode:
spin_lock(&mylock); /* do a bunch of stuff */ p = rcu_dereference_check(myrcuptr, lockdep_is_held(&mylock));
The logic below would complain about this usage, despite the fact that it is perfectly safe because the update-side lock is held.
Make sense, or am I missing something?
Thanx, Paul
> Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com> > Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com> > Cc: Ingo Molnar <mingo@elte.hu> > Cc: Peter Zijlstra <a.p.zijlstra@chello.nl> > Cc: Thomas Gleixner <tglx@linutronix.de> > Cc: Milton Miller <miltonm@bga.com> > --- > include/linux/rcupdate.h | 9 +++++++++ > kernel/rcutiny.c | 13 +++++++++++++ > kernel/rcutree.c | 14 ++++++++++++++ > 3 files changed, 36 insertions(+), 0 deletions(-) > > diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h > index 9db50df..6cad1f3 100644 > --- a/include/linux/rcupdate.h > +++ b/include/linux/rcupdate.h > @@ -388,6 +388,12 @@ extern int rcu_my_thread_group_empty(void); > > #endif /* #else #ifdef CONFIG_PROVE_RCU */ > > +#if defined(CONFIG_PROVE_RCU) && defined(CONFIG_NO_HZ) > +extern bool rcu_check_extended_qs(void); > +#else > +static inline bool rcu_check_extended_qs(void) { return false; } > +#endif > + > /* > * Helper functions for rcu_dereference_check(), rcu_dereference_protected() > * and rcu_assign_pointer(). Some of these could be folded into their > @@ -415,6 +421,9 @@ extern int rcu_my_thread_group_empty(void); > typeof(*p) *_________p1 = (typeof(*p)*__force )ACCESS_ONCE(p); \ > rcu_lockdep_assert(c, "suspicious rcu_dereference_check()" \ > " usage"); \ > + rcu_lockdep_assert(!rcu_check_extended_qs(), \ > + "use of rcu_dereference_check() in an extended" \ > + " quiescent state"); \ > rcu_dereference_sparse(p, space); \ > smp_read_barrier_depends(); \ > ((typeof(*p) __force __kernel *)(_________p1)); \ > diff --git a/kernel/rcutiny.c b/kernel/rcutiny.c > index 775d69a..44a2a15 100644 > --- a/kernel/rcutiny.c > +++ b/kernel/rcutiny.c > @@ -76,6 +76,19 @@ void rcu_exit_nohz(void) > rcu_dynticks_nesting++; > } > > + > +#ifdef CONFIG_PROVE_RCU > + > +bool rcu_check_extended_qs(void) > +{ > + if (!rcu_dynticks_nesting) > + return true; > + > + return false; > +} > + > +#endif > + > #endif /* #ifdef CONFIG_NO_HZ */ > > /* > diff --git a/kernel/rcutree.c b/kernel/rcutree.c > index 305dfae..8211527 100644 > --- a/kernel/rcutree.c > +++ b/kernel/rcutree.c > @@ -432,6 +432,20 @@ void rcu_irq_exit(void) > rcu_enter_nohz(); > } > > +#ifdef CONFIG_PROVE_RCU > + > +bool rcu_check_extended_qs(void) > +{ > + struct rcu_dynticks *rdtp = &__get_cpu_var(rcu_dynticks); > + > + if (atomic_read(&rdtp->dynticks) & 0x1) > + return false; > + > + return true; > +} > + > +#endif /* CONFIG_PROVE_RCU */ > + > #ifdef CONFIG_SMP > > /* > -- > 1.7.5.4 >
| |