2020-02-27 20:24:44

by Madhuparna Bhowmik

[permalink] [raw]
Subject: [PATCH] Enable RCU list lockdep debugging and drop CONFIG_PROVE_RCU_LIST

From: Madhuparna Bhowmik <[email protected]>

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 <[email protected]>
Signed-off-by: Amol Grover <[email protected]>
Signed-off-by: Madhuparna Bhowmik <[email protected]>
---
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


2020-02-27 21:17:11

by Paul E. McKenney

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

On Fri, Feb 28, 2020 at 01:53:55AM +0530, [email protected] wrote:
> From: Madhuparna Bhowmik <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: Amol Grover <[email protected]>
> Signed-off-by: Madhuparna Bhowmik <[email protected]>

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
>

2020-02-28 09:28:22

by Madhuparna Bhowmik

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

On Thu, Feb 27, 2020 at 01:16:11PM -0800, Paul E. McKenney wrote:
> On Fri, Feb 28, 2020 at 01:53:55AM +0530, [email protected] wrote:
> > From: Madhuparna Bhowmik <[email protected]>
> >
> > 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 <[email protected]>
> > Signed-off-by: Amol Grover <[email protected]>
> > Signed-off-by: Madhuparna Bhowmik <[email protected]>
>
> 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.
>
Sure, I have sent the patch with these changes.

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

Thank you,
Madhuparna
>
> 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
> >