Re: [PATCH] Enable RCU list lockdep debugging and drop CONFIG_PROVE_RCU_LIST

From: Paul E. McKenney
Date: Thu Feb 27 2020 - 16:16:14 EST


On Fri, Feb 28, 2020 at 01:53:55AM +0530, madhuparnabhowmik10@xxxxxxxxx wrote:
> From: Madhuparna Bhowmik <madhuparnabhowmik10@xxxxxxxxx>
>
> This patch drops the CONFIG_PROVE_RCU_LIST option and instead
> uses CONFIG_PROVE_RCU for RCU list lockdep debugging.
>
> With this change, RCU list lockdep debugging will be default
> enabled in CONFIG_PROVE_RCU=y kernels.
>
> Most of the RCU users (in core kernel/, drivers/, and net/
> subsystem) have already been modified to include lockdep
> expressions hence RCU list debugging can be enabled by
> default.
>
> However, there are still chances of enountering
> false-positive lockdep splats because not everything is converted,
> in case RCU list primitives are used in non-RCU read-side critical
> section but under the protection of a lock. It would be okay to
> have a few false-positives, as long as bugs are identified, since this
> patch only affects debugging kernels.
>
> Co-developed-by: Amol Grover <frextrite@xxxxxxxxx>
> Signed-off-by: Amol Grover <frextrite@xxxxxxxxx>
> Signed-off-by: Madhuparna Bhowmik <madhuparnabhowmik10@xxxxxxxxx>

Good idea, but could you please left PROVE_RCU_LIST and "select" it from
CONFIG_PROVE_RCU instead? This gets the same effect, but also makes it
easier to change our minds later if we wish to. It also makes it easier
to find the various different types of debugging.

For a template, please see how CONFIG_PROVE_LOCKING controls the
value of CONFIG_PROVE_RCU in kernel/rcu/Kconfig.debug.

Thanx, Paul

> ---
> include/linux/rculist.h | 2 +-
> kernel/rcu/Kconfig.debug | 11 -----------
> 2 files changed, 1 insertion(+), 12 deletions(-)
>
> diff --git a/include/linux/rculist.h b/include/linux/rculist.h
> index 63726577c6b8..f517eb421b5e 100644
> --- a/include/linux/rculist.h
> +++ b/include/linux/rculist.h
> @@ -56,7 +56,7 @@ static inline void INIT_LIST_HEAD_RCU(struct list_head *list)
>
> #define check_arg_count_one(dummy)
>
> -#ifdef CONFIG_PROVE_RCU_LIST
> +#ifdef CONFIG_PROVE_RCU
> #define __list_check_rcu(dummy, cond, extra...) \
> ({ \
> check_arg_count_one(extra); \
> diff --git a/kernel/rcu/Kconfig.debug b/kernel/rcu/Kconfig.debug
> index 4aa02eee8f6c..5ec3ea4028e2 100644
> --- a/kernel/rcu/Kconfig.debug
> +++ b/kernel/rcu/Kconfig.debug
> @@ -8,17 +8,6 @@ menu "RCU Debugging"
> config PROVE_RCU
> def_bool PROVE_LOCKING
>
> -config PROVE_RCU_LIST
> - bool "RCU list lockdep debugging"
> - depends on PROVE_RCU && RCU_EXPERT
> - default n
> - help
> - Enable RCU lockdep checking for list usages. By default it is
> - turned off since there are several list RCU users that still
> - need to be converted to pass a lockdep expression. To prevent
> - false-positive splats, we keep it default disabled but once all
> - users are converted, we can remove this config option.
> -
> config TORTURE_TEST
> tristate
> default n
> --
> 2.17.1
>