lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Mon, 15 Feb 2016 21:55:05 -0800 From: Josh Triplett <josh@...htriplett.org> To: Boqun Feng <boqun.feng@...il.com> Cc: "Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>, linux-kernel@...r.kernel.org, Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...nel.org>, Steven Rostedt <rostedt@...dmis.org>, Mathieu Desnoyers <mathieu.desnoyers@...icios.com>, Lai Jiangshan <jiangshanlai@...il.com>, sasha.levin@...cle.com Subject: Re: [RFC 6/6] RCU: Track rcu_dereference() in RCU read-side critical section On Tue, Feb 16, 2016 at 11:01:43AM +0800, Boqun Feng wrote: > On Mon, Feb 15, 2016 at 05:05:53PM -0800, Paul E. McKenney wrote: > > On Thu, Feb 04, 2016 at 12:45:12AM +0800, Boqun Feng wrote: > > > The variables protected by an RCU read-side critical section are > > > sometimes hard to figure out, especially when the critical section is > > > long or has some function calls in it. However, figuring out which > > > variable a RCU read-side critical section protects could save > > > us a lot of time for code reviewing, bug fixing or performance tuning. > > > > > > This patch therefore uses the LOCKED_ACCESS to collect the information > > > of relationship between rcu_dereference*() and rcu_read_lock*() by > > > doing: > > > > > > Step 0: define a locked_access_class for RCU. > > > > > > Step 1: set the content of rcu_*_lock_key and __srcu_key to the > > > address of the locked_access_class for RCU. > > > > > > Step 2: add locked_access_point() in __rcu_dereference_check() > > > > > > After that we can figure out not only in which RCU read-side critical > > > section but also after which rcu_read_lock*() called an > > > rcu_dereference*() is called. > > > > > > This feature is controlled by a config option RCU_LOCKED_ACCESS. > > > > > > Also clean up the initialization code of lockdep_maps for different > > > flavors of RCU a little bit. > > > > > > Signed-off-by: Boqun Feng <boqun.feng@...il.com> > > > --- > > > include/linux/rcupdate.h | 8 ++++++++ > > > include/linux/srcu.h | 8 +++++++- > > > kernel/locking/lockdep.c | 3 +++ > > > kernel/rcu/update.c | 31 +++++++++++++++++-------------- > > > lib/Kconfig.debug | 12 ++++++++++++ > > > 5 files changed, 47 insertions(+), 15 deletions(-) > > > > > > diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h > > > index 14e6f47..4bab658 100644 > > > --- a/include/linux/rcupdate.h > > > +++ b/include/linux/rcupdate.h > > > @@ -610,6 +610,13 @@ static inline void rcu_preempt_sleep_check(void) > > > #define rcu_dereference_sparse(p, space) > > > #endif /* #else #ifdef __CHECKER__ */ > > > > > > +#ifdef CONFIG_RCU_LOCKED_ACCESS > > > +extern struct locked_access_class rcu_laclass; > > > +#define rcu_dereference_access() \ > > > + locked_access_point(&rcu_laclass, LOCKED_ACCESS_TYPE_READ) > > > +#else /* #ifdef CONFIG_LOCKED_ACCESS */ > > > +#define rcu_dereference_access() > > > +#endif /* #else #ifdef CONFIG_LOCKED_ACCESS */ > > > #define __rcu_access_pointer(p, space) \ > > > ({ \ > > > typeof(*p) *_________p1 = (typeof(*p) *__force)READ_ONCE(p); \ > > > @@ -622,6 +629,7 @@ static inline void rcu_preempt_sleep_check(void) > > > typeof(*p) *________p1 = (typeof(*p) *__force)lockless_dereference(p); \ > > > RCU_LOCKDEP_WARN(!(c), "suspicious rcu_dereference_check() usage"); \ > > > rcu_dereference_sparse(p, space); \ > > > + rcu_dereference_access(); \ > > > ((typeof(*p) __force __kernel *)(________p1)); \ > > > }) > > > #define __rcu_dereference_protected(p, c, space) \ > > > diff --git a/include/linux/srcu.h b/include/linux/srcu.h > > > index f5f80c5..ff048a2 100644 > > > --- a/include/linux/srcu.h > > > +++ b/include/linux/srcu.h > > > @@ -64,12 +64,18 @@ struct srcu_struct { > > > > > > #ifdef CONFIG_DEBUG_LOCK_ALLOC > > > > > > +# ifdef CONFIG_RCU_LOCKED_ACCESS > > > +# define RCU_KEY { .laclass = &rcu_laclass } > > > +# else > > > +# define RCU_KEY { 0 } > > > +# endif > > > + > > > int __init_srcu_struct(struct srcu_struct *sp, const char *name, > > > struct lock_class_key *key); > > > > > > #define init_srcu_struct(sp) \ > > > ({ \ > > > - static struct lock_class_key __srcu_key; \ > > > + static struct lock_class_key __srcu_key = RCU_KEY; \ > > > > This gets me the following for the TASKS01, TINY02, TREE02, TREE05, > > TREE06, and TREE08 configurations: > > > > CC mm/mmap.o > > In file included from /home/paulmck/public_git/linux-rcu/include/linux/notifier.h:15:0, > > from /home/paulmck/public_git/linux-rcu/arch/x86/include/asm/kdebug.h:4, > > from /home/paulmck/public_git/linux-rcu/include/linux/kdebug.h:4, > > from /home/paulmck/public_git/linux-rcu/kernel/notifier.c:1: > > /home/paulmck/public_git/linux-rcu/kernel/notifier.c: In function ‘srcu_init_notifier_head’: > > /home/paulmck/public_git/linux-rcu/include/linux/srcu.h:78:16: warning: missing braces around initializer [-Wmissing-braces] > > static struct lock_class_key __srcu_key = RCU_KEY; \ > > ^ > > /home/paulmck/public_git/linux-rcu/kernel/notifier.c:526:6: note: in expansion of macro ‘init_srcu_struct’ > > if (init_srcu_struct(&nh->srcu) < 0) > > ^ > > /home/paulmck/public_git/linux-rcu/include/linux/srcu.h:78:16: warning: (near initialization for ‘__srcu_key.subkeys’) [-Wmissing-braces] > > static struct lock_class_key __srcu_key = RCU_KEY; \ > > ^ > > /home/paulmck/public_git/linux-rcu/kernel/notifier.c:526:6: note: in expansion of macro ‘init_srcu_struct’ > > if (init_srcu_struct(&nh->srcu) < 0) > > ^ > > > > These have the following in their .config files: > > > > CONFIG_DEBUG_LOCK_ALLOC=y > > > > However, none of your new Kconfig options are set, which needs to be > > tested because this will be the common case. Several of them have > > CONFIG_PROVE_LOCKING=y, but others do not. > > > > Oh.. this is embarrassing ;-( > > Hmm.. when CONFIG_RCU_LOCKED_ACCESS=n and CONFIG_DEBUG_LOCK_ALLOC=y, > this line becomes: > > static struct lock_class_key __srcu_key = { 0 }; > > IIUC, "={ 0 }" is the unverisal zero initializer in C, could be used for > zero initialising any structure or array, so it's OK here, right? Compilers seem touchy about that. Sometimes { 0 } works, and sometimes {} works (that really should *always* work, but it doesn't). However, for a static struct, you can always just leave off the initializer. - Josh Triplett
Powered by blists - more mailing lists