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.f...@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
>
