Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1334897 > unrolled thread

Re: [RFC 6/6] RCU: Track rcu_dereference() in RCU read-side critical section

Started by"Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
First post2016-02-16 02:10 +0100
Last post2016-02-16 07:00 +0100
Articles 5 — 3 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [RFC 6/6] RCU: Track rcu_dereference() in RCU read-side critical  section "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2016-02-16 02:10 +0100
    Re: [RFC 6/6] RCU: Track rcu_dereference() in RCU read-side critical  section Boqun Feng <boqun.feng@gmail.com> - 2016-02-16 04:10 +0100
      Re: [RFC 6/6] RCU: Track rcu_dereference() in RCU read-side critical  section "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2016-02-16 05:10 +0100
        Re: [RFC 6/6] RCU: Track rcu_dereference() in RCU read-side critical  section Boqun Feng <boqun.feng@gmail.com> - 2016-02-16 06:10 +0100
      Re: [RFC 6/6] RCU: Track rcu_dereference() in RCU read-side critical  section Josh Triplett <josh@joshtriplett.org> - 2016-02-16 07:00 +0100

#1334897 — Re: [RFC 6/6] RCU: Track rcu_dereference() in RCU read-side critical section

From"Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
Date2016-02-16 02:10 +0100
SubjectRe: [RFC 6/6] RCU: Track rcu_dereference() in RCU read-side critical section
Message-ID<r2Cjo-wW-5@gated-at.bofh.it>
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@gmail.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.

I have dequeued these for the moment.  Please send an updated patch
series when you have this fixed.

							Thanx, Paul

>  	\
>  	__init_srcu_struct((sp), #sp, &__srcu_key); \
>  })
> diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
> index 996c2d5..36bd0cc 100644
> --- a/kernel/locking/lockdep.c
> +++ b/kernel/locking/lockdep.c
> @@ -4623,5 +4623,8 @@ void locked_access(struct locked_access_class *laclass,
>  		correlate_locked_access(laclass, acqchain, loc, type);
>  }
>  EXPORT_SYMBOL(locked_access);
> +#ifdef CONFIG_RCU_LOCKED_ACCESS
> +DEFINE_LACLASS(rcu);
> +#endif
> 
>  #endif /* CONFIG_LOCKED_ACCESS */
> diff --git a/kernel/rcu/update.c b/kernel/rcu/update.c
> index 76b94e1..ba2ded6 100644
> --- a/kernel/rcu/update.c
> +++ b/kernel/rcu/update.c
> @@ -235,20 +235,23 @@ EXPORT_SYMBOL_GPL(__rcu_read_unlock);
>  #endif /* #ifdef CONFIG_PREEMPT_RCU */
> 
>  #ifdef CONFIG_DEBUG_LOCK_ALLOC
> -static struct lock_class_key rcu_lock_key;
> -struct lockdep_map rcu_lock_map =
> -	STATIC_LOCKDEP_MAP_INIT("rcu_read_lock", &rcu_lock_key);
> -EXPORT_SYMBOL_GPL(rcu_lock_map);
> -
> -static struct lock_class_key rcu_bh_lock_key;
> -struct lockdep_map rcu_bh_lock_map =
> -	STATIC_LOCKDEP_MAP_INIT("rcu_read_lock_bh", &rcu_bh_lock_key);
> -EXPORT_SYMBOL_GPL(rcu_bh_lock_map);
> -
> -static struct lock_class_key rcu_sched_lock_key;
> -struct lockdep_map rcu_sched_lock_map =
> -	STATIC_LOCKDEP_MAP_INIT("rcu_read_lock_sched", &rcu_sched_lock_key);
> -EXPORT_SYMBOL_GPL(rcu_sched_lock_map);
> +
> +# ifdef CONFIG_RCU_LOCKED_ACCESS
> +# define RCU_KEY { .laclass = &rcu_laclass }
> +# else
> +# define RCU_KEY { 0 }
> +# endif
> +
> +#define DEFINE_RCU_LOCKDEP_MAP(flavor)					\
> +	static struct lock_class_key rcu##flavor##_lock_key = RCU_KEY;	\
> +	struct lockdep_map rcu##flavor##_lock_map =			\
> +		STATIC_LOCKDEP_MAP_INIT("rcu" #flavor "_read_lock",	\
> +					&rcu##flavor##_lock_key);	\
> +	EXPORT_SYMBOL_GPL(rcu##flavor##_lock_map)
> +
> +DEFINE_RCU_LOCKDEP_MAP();
> +DEFINE_RCU_LOCKDEP_MAP(_bh);
> +DEFINE_RCU_LOCKDEP_MAP(_sched);
> 
>  static struct lock_class_key rcu_callback_key;
>  struct lockdep_map rcu_callback_map =
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index 178f288..b6003d4 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -1410,6 +1410,18 @@ config RCU_EQS_DEBUG
>  	  Say N here if you need ultimate kernel/user switch latencies
>  	  Say Y if you are unsure
> 
> +config RCU_LOCKED_ACCESS
> +	bool "Track data access in RCU read-side critical sections"
> +	depends on DEBUG_KERNEL && TRACE_IRQFLAGS_SUPPORT && STACKTRACE_SUPPORT && LOCKDEP_SUPPORT
> +	select LOCKED_ACCESS
> +	default n
> +	help
> +	  Track data acces in RCU read-side critical sections,
> +	  by doing so, one can examine which rcu_dereference() and its
> +	  friends are called in which RCU read-side critical sections,
> +	  and even more detailed, after which rcu_read_lock() and
> +	  its friends are called.
> +
>  endmenu # "RCU Debugging"
> 
>  config DEBUG_BLOCK_EXT_DEVT
> -- 
> 2.7.0
>

[toc] | [next] | [standalone]

#1334973

FromBoqun Feng <boqun.feng@gmail.com>
Date2016-02-16 04:10 +0100
Message-ID<r2Ebv-1RL-5@gated-at.bofh.it>
In reply to#1334897

[Multipart message — attachments visible in raw view] — view raw

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@gmail.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?

And may I ask your compiler's version? Because looks to me this may
be a compiler bug according to:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53119
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64709

Also I can't reproduce this with CONFIG_RCU_LOCKED_ACCESS=n and
CONFIG_DEBUG_LOCK_ALLOC=y on my machine, whose gcc version is 5.3.0.

Of course I can use a different way here and do not need a "={ 0 }"
here, because __srcu_key is static, and I just want to make sure we know
what's going on here before we use a workaround, or I just want to know
if I'm a bad C programmer ;-)

Regards,
Boqun

> I have dequeued these for the moment.  Please send an updated patch
> series when you have this fixed.
> 
> 							Thanx, Paul
> 
> >  	\
> >  	__init_srcu_struct((sp), #sp, &__srcu_key); \
> >  })
> > diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
> > index 996c2d5..36bd0cc 100644
> > --- a/kernel/locking/lockdep.c
> > +++ b/kernel/locking/lockdep.c
> > @@ -4623,5 +4623,8 @@ void locked_access(struct locked_access_class *laclass,
> >  		correlate_locked_access(laclass, acqchain, loc, type);
> >  }
> >  EXPORT_SYMBOL(locked_access);
> > +#ifdef CONFIG_RCU_LOCKED_ACCESS
> > +DEFINE_LACLASS(rcu);
> > +#endif
> > 
> >  #endif /* CONFIG_LOCKED_ACCESS */
> > diff --git a/kernel/rcu/update.c b/kernel/rcu/update.c
> > index 76b94e1..ba2ded6 100644
> > --- a/kernel/rcu/update.c
> > +++ b/kernel/rcu/update.c
> > @@ -235,20 +235,23 @@ EXPORT_SYMBOL_GPL(__rcu_read_unlock);
> >  #endif /* #ifdef CONFIG_PREEMPT_RCU */
> > 
> >  #ifdef CONFIG_DEBUG_LOCK_ALLOC
> > -static struct lock_class_key rcu_lock_key;
> > -struct lockdep_map rcu_lock_map =
> > -	STATIC_LOCKDEP_MAP_INIT("rcu_read_lock", &rcu_lock_key);
> > -EXPORT_SYMBOL_GPL(rcu_lock_map);
> > -
> > -static struct lock_class_key rcu_bh_lock_key;
> > -struct lockdep_map rcu_bh_lock_map =
> > -	STATIC_LOCKDEP_MAP_INIT("rcu_read_lock_bh", &rcu_bh_lock_key);
> > -EXPORT_SYMBOL_GPL(rcu_bh_lock_map);
> > -
> > -static struct lock_class_key rcu_sched_lock_key;
> > -struct lockdep_map rcu_sched_lock_map =
> > -	STATIC_LOCKDEP_MAP_INIT("rcu_read_lock_sched", &rcu_sched_lock_key);
> > -EXPORT_SYMBOL_GPL(rcu_sched_lock_map);
> > +
> > +# ifdef CONFIG_RCU_LOCKED_ACCESS
> > +# define RCU_KEY { .laclass = &rcu_laclass }
> > +# else
> > +# define RCU_KEY { 0 }
> > +# endif
> > +
> > +#define DEFINE_RCU_LOCKDEP_MAP(flavor)					\
> > +	static struct lock_class_key rcu##flavor##_lock_key = RCU_KEY;	\
> > +	struct lockdep_map rcu##flavor##_lock_map =			\
> > +		STATIC_LOCKDEP_MAP_INIT("rcu" #flavor "_read_lock",	\
> > +					&rcu##flavor##_lock_key);	\
> > +	EXPORT_SYMBOL_GPL(rcu##flavor##_lock_map)
> > +
> > +DEFINE_RCU_LOCKDEP_MAP();
> > +DEFINE_RCU_LOCKDEP_MAP(_bh);
> > +DEFINE_RCU_LOCKDEP_MAP(_sched);
> > 
> >  static struct lock_class_key rcu_callback_key;
> >  struct lockdep_map rcu_callback_map =
> > diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> > index 178f288..b6003d4 100644
> > --- a/lib/Kconfig.debug
> > +++ b/lib/Kconfig.debug
> > @@ -1410,6 +1410,18 @@ config RCU_EQS_DEBUG
> >  	  Say N here if you need ultimate kernel/user switch latencies
> >  	  Say Y if you are unsure
> > 
> > +config RCU_LOCKED_ACCESS
> > +	bool "Track data access in RCU read-side critical sections"
> > +	depends on DEBUG_KERNEL && TRACE_IRQFLAGS_SUPPORT && STACKTRACE_SUPPORT && LOCKDEP_SUPPORT
> > +	select LOCKED_ACCESS
> > +	default n
> > +	help
> > +	  Track data acces in RCU read-side critical sections,
> > +	  by doing so, one can examine which rcu_dereference() and its
> > +	  friends are called in which RCU read-side critical sections,
> > +	  and even more detailed, after which rcu_read_lock() and
> > +	  its friends are called.
> > +
> >  endmenu # "RCU Debugging"
> > 
> >  config DEBUG_BLOCK_EXT_DEVT
> > -- 
> > 2.7.0
> > 
>

[toc] | [prev] | [next] | [standalone]

#1334989

From"Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
Date2016-02-16 05:10 +0100
Message-ID<r2F7z-2yD-1@gated-at.bofh.it>
In reply to#1334973
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@gmail.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?
> 
> And may I ask your compiler's version? Because looks to me this may
> be a compiler bug according to:
> 
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53119
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64709

gcc version 4.8.4 (Ubuntu 4.8.4-2ubuntu1~14.04)  (KSIC)

> Also I can't reproduce this with CONFIG_RCU_LOCKED_ACCESS=n and
> CONFIG_DEBUG_LOCK_ALLOC=y on my machine, whose gcc version is 5.3.0.
> 
> Of course I can use a different way here and do not need a "={ 0 }"
> here, because __srcu_key is static, and I just want to make sure we know
> what's going on here before we use a workaround, or I just want to know
> if I'm a bad C programmer ;-)

Well, it is quite possible that your code is correct in theory, but
we do need to make it so that compilers can deal with it.  It would
not be the first compiler-bug workaround in RCU...

							Thanx, Paul

> Regards,
> Boqun
> 
> > I have dequeued these for the moment.  Please send an updated patch
> > series when you have this fixed.
> > 
> > 							Thanx, Paul
> > 
> > >  	\
> > >  	__init_srcu_struct((sp), #sp, &__srcu_key); \
> > >  })
> > > diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
> > > index 996c2d5..36bd0cc 100644
> > > --- a/kernel/locking/lockdep.c
> > > +++ b/kernel/locking/lockdep.c
> > > @@ -4623,5 +4623,8 @@ void locked_access(struct locked_access_class *laclass,
> > >  		correlate_locked_access(laclass, acqchain, loc, type);
> > >  }
> > >  EXPORT_SYMBOL(locked_access);
> > > +#ifdef CONFIG_RCU_LOCKED_ACCESS
> > > +DEFINE_LACLASS(rcu);
> > > +#endif
> > > 
> > >  #endif /* CONFIG_LOCKED_ACCESS */
> > > diff --git a/kernel/rcu/update.c b/kernel/rcu/update.c
> > > index 76b94e1..ba2ded6 100644
> > > --- a/kernel/rcu/update.c
> > > +++ b/kernel/rcu/update.c
> > > @@ -235,20 +235,23 @@ EXPORT_SYMBOL_GPL(__rcu_read_unlock);
> > >  #endif /* #ifdef CONFIG_PREEMPT_RCU */
> > > 
> > >  #ifdef CONFIG_DEBUG_LOCK_ALLOC
> > > -static struct lock_class_key rcu_lock_key;
> > > -struct lockdep_map rcu_lock_map =
> > > -	STATIC_LOCKDEP_MAP_INIT("rcu_read_lock", &rcu_lock_key);
> > > -EXPORT_SYMBOL_GPL(rcu_lock_map);
> > > -
> > > -static struct lock_class_key rcu_bh_lock_key;
> > > -struct lockdep_map rcu_bh_lock_map =
> > > -	STATIC_LOCKDEP_MAP_INIT("rcu_read_lock_bh", &rcu_bh_lock_key);
> > > -EXPORT_SYMBOL_GPL(rcu_bh_lock_map);
> > > -
> > > -static struct lock_class_key rcu_sched_lock_key;
> > > -struct lockdep_map rcu_sched_lock_map =
> > > -	STATIC_LOCKDEP_MAP_INIT("rcu_read_lock_sched", &rcu_sched_lock_key);
> > > -EXPORT_SYMBOL_GPL(rcu_sched_lock_map);
> > > +
> > > +# ifdef CONFIG_RCU_LOCKED_ACCESS
> > > +# define RCU_KEY { .laclass = &rcu_laclass }
> > > +# else
> > > +# define RCU_KEY { 0 }
> > > +# endif
> > > +
> > > +#define DEFINE_RCU_LOCKDEP_MAP(flavor)					\
> > > +	static struct lock_class_key rcu##flavor##_lock_key = RCU_KEY;	\
> > > +	struct lockdep_map rcu##flavor##_lock_map =			\
> > > +		STATIC_LOCKDEP_MAP_INIT("rcu" #flavor "_read_lock",	\
> > > +					&rcu##flavor##_lock_key);	\
> > > +	EXPORT_SYMBOL_GPL(rcu##flavor##_lock_map)
> > > +
> > > +DEFINE_RCU_LOCKDEP_MAP();
> > > +DEFINE_RCU_LOCKDEP_MAP(_bh);
> > > +DEFINE_RCU_LOCKDEP_MAP(_sched);
> > > 
> > >  static struct lock_class_key rcu_callback_key;
> > >  struct lockdep_map rcu_callback_map =
> > > diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> > > index 178f288..b6003d4 100644
> > > --- a/lib/Kconfig.debug
> > > +++ b/lib/Kconfig.debug
> > > @@ -1410,6 +1410,18 @@ config RCU_EQS_DEBUG
> > >  	  Say N here if you need ultimate kernel/user switch latencies
> > >  	  Say Y if you are unsure
> > > 
> > > +config RCU_LOCKED_ACCESS
> > > +	bool "Track data access in RCU read-side critical sections"
> > > +	depends on DEBUG_KERNEL && TRACE_IRQFLAGS_SUPPORT && STACKTRACE_SUPPORT && LOCKDEP_SUPPORT
> > > +	select LOCKED_ACCESS
> > > +	default n
> > > +	help
> > > +	  Track data acces in RCU read-side critical sections,
> > > +	  by doing so, one can examine which rcu_dereference() and its
> > > +	  friends are called in which RCU read-side critical sections,
> > > +	  and even more detailed, after which rcu_read_lock() and
> > > +	  its friends are called.
> > > +
> > >  endmenu # "RCU Debugging"
> > > 
> > >  config DEBUG_BLOCK_EXT_DEVT
> > > -- 
> > > 2.7.0
> > > 
> >

[toc] | [prev] | [next] | [standalone]

#1335000

FromBoqun Feng <boqun.feng@gmail.com>
Date2016-02-16 06:10 +0100
Message-ID<r2G3D-399-7@gated-at.bofh.it>
In reply to#1334989

[Multipart message — attachments visible in raw view] — view raw

On Mon, Feb 15, 2016 at 08:08:40PM -0800, Paul E. McKenney wrote:
> 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@gmail.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?
> > 
> > And may I ask your compiler's version? Because looks to me this may
> > be a compiler bug according to:
> > 
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53119
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64709
> 
> gcc version 4.8.4 (Ubuntu 4.8.4-2ubuntu1~14.04)  (KSIC)
> 
> > Also I can't reproduce this with CONFIG_RCU_LOCKED_ACCESS=n and
> > CONFIG_DEBUG_LOCK_ALLOC=y on my machine, whose gcc version is 5.3.0.
> > 
> > Of course I can use a different way here and do not need a "={ 0 }"
> > here, because __srcu_key is static, and I just want to make sure we know
> > what's going on here before we use a workaround, or I just want to know
> > if I'm a bad C programmer ;-)
> 
> Well, it is quite possible that your code is correct in theory, but
> we do need to make it so that compilers can deal with it.  It would
> not be the first compiler-bug workaround in RCU...
> 

Fair enough!

> 							Thanx, Paul
> 
> > Regards,
> > Boqun
> > 
> > > I have dequeued these for the moment.  Please send an updated patch
> > > series when you have this fixed.

Moreover, I hit some "defined but not used" warnings with
CONFIG_DEBUG_LOCK_ALLOC=y and CONFIG_RCU_LOCKED_ACCESS=n, will fix that
too and send out a new series.

Regards,
Boqun

> > > 
> > > 							Thanx, Paul
> > >

[toc] | [prev] | [next] | [standalone]

#1335025

FromJosh Triplett <josh@joshtriplett.org>
Date2016-02-16 07:00 +0100
Message-ID<r2GQ3-3tt-11@gated-at.bofh.it>
In reply to#1334973
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@gmail.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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web