config RCU_EQS_DEBUG

config RCU_EQS_DEBUG

[Jean Delvare]

Hi Paul,

You just introduced a Linux kernel configuration option named
RCU_EQS_DEBUG. Its short description is "Use this when adding any sort
of NO_HZ support to your arch".

I'm afraid this is a bad way to briefly explain what the option
actually does (which is what the short description is for.) A sentence
like "use this when adding any sort of NO_HZ support to your arch"
should go in the help text, not the short description. The short
description should be something like along the lines of "Enable
consistency checks for RCU", for example.

Additionally I see some inconsistency in the fact that this option
defaults to n but the help text says "Say Y if you are unsure". BTW,
option RCU_CPU_STALL_INFO is equally inconsistent with a default y and
"say N if you are unsure" in the help text.

-- 
Jean Delvare
SUSE L3 Support

Re: config RCU_EQS_DEBUG

[Paul E. McKenney]

On Fri, Jul 03, 2015 at 10:07:45AM +0200, Jean Delvare wrote:
> Hi Paul,
> 
> You just introduced a Linux kernel configuration option named
> RCU_EQS_DEBUG. Its short description is "Use this when adding any sort
> of NO_HZ support to your arch".
> 
> I'm afraid this is a bad way to briefly explain what the option
> actually does (which is what the short description is for.) A sentence
> like "use this when adding any sort of NO_HZ support to your arch"
> should go in the help text, not the short description. The short
> description should be something like along the lines of "Enable
> consistency checks for RCU", for example.
> 
> Additionally I see some inconsistency in the fact that this option
> defaults to n but the help text says "Say Y if you are unsure". BTW,
> option RCU_CPU_STALL_INFO is equally inconsistent with a default y and
> "say N if you are unsure" in the help text.

Hello, Jean,

I have the following queued, which should address your first point.

Would adding "default y" address your other point?

On RCU_CPU_STALL_INFO, I have a patch queued for v4.3 that eliminates
this Kconfig option completely.

							Thanx, Paul

------------------------------------------------------------------------

rcu: Clarify CONFIG_RCU_EQS_DEBUG help text

Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>

diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 6be521990d61..80efaade5e59 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -1360,7 +1360,7 @@ config RCU_TRACE
 	  Say N if you are unsure.
 
 config RCU_EQS_DEBUG
-	bool "Use this when adding any sort of NO_HZ support to your arch"
+	bool "Provide debugging asserts for adding NO_HZ support to an arch"
 	depends on DEBUG_KERNEL
 	help
 	  This option provides consistency checks in RCU's handling of

Re: config RCU_EQS_DEBUG

[Jean Delvare]

Hi Paul,

On Fri, 3 Jul 2015 09:00:37 -0700, Paul E. McKenney wrote:
> On Fri, Jul 03, 2015 at 10:07:45AM +0200, Jean Delvare wrote:
> > You just introduced a Linux kernel configuration option named
> > RCU_EQS_DEBUG. Its short description is "Use this when adding any sort
> > of NO_HZ support to your arch".
> > 
> > I'm afraid this is a bad way to briefly explain what the option
> > actually does (which is what the short description is for.) A sentence
> > like "use this when adding any sort of NO_HZ support to your arch"
> > should go in the help text, not the short description. The short
> > description should be something like along the lines of "Enable
> > consistency checks for RCU", for example.
> > 
> > Additionally I see some inconsistency in the fact that this option
> > defaults to n but the help text says "Say Y if you are unsure". BTW,
> > option RCU_CPU_STALL_INFO is equally inconsistent with a default y and
> > "say N if you are unsure" in the help text.
> 
> I have the following queued, which should address your first point.

Yes it does, thank you. If I were to nitpick I'd say that the option is
adding (or including) the asserts rather than providing them.

> Would adding "default y" address your other point?

It would make things consistent, but I have a serious doubt that this
is the right direction. An option which is described as being useful
"when adding any sort of NO_HZ support to a new architecture" really
does not seem to be one that should be enabled by default. You may
argue that it still depends on DEBUG_KERNEL but well, that option is
enabled in pretty much every distribution kernel these days. So I
believe that default n (no default actually) was correct, and what
needs to be removed is the statement "Say Y if you are unsure". If the
user is unsure, then certainly he/she is not working on NO_HZ support
for a new architecture and thus doesn't need to enable RCU_EQS_DEBUG.

> On RCU_CPU_STALL_INFO, I have a patch queued for v4.3 that eliminates
> this Kconfig option completely.

That solves the problem nicely :-)

> rcu: Clarify CONFIG_RCU_EQS_DEBUG help text
> 
> Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
> Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> 
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index 6be521990d61..80efaade5e59 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -1360,7 +1360,7 @@ config RCU_TRACE
>  	  Say N if you are unsure.
>  
>  config RCU_EQS_DEBUG
> -	bool "Use this when adding any sort of NO_HZ support to your arch"
> +	bool "Provide debugging asserts for adding NO_HZ support to an arch"
>  	depends on DEBUG_KERNEL
>  	help
>  	  This option provides consistency checks in RCU's handling of
> 

-- 
Jean Delvare
SUSE L3 Support

Re: config RCU_EQS_DEBUG

[Paul E. McKenney]

On Sat, Jul 04, 2015 at 06:53:10PM +0200, Jean Delvare wrote:
> Hi Paul,
> 
> On Fri, 3 Jul 2015 09:00:37 -0700, Paul E. McKenney wrote:
> > On Fri, Jul 03, 2015 at 10:07:45AM +0200, Jean Delvare wrote:
> > > You just introduced a Linux kernel configuration option named
> > > RCU_EQS_DEBUG. Its short description is "Use this when adding any sort
> > > of NO_HZ support to your arch".
> > > 
> > > I'm afraid this is a bad way to briefly explain what the option
> > > actually does (which is what the short description is for.) A sentence
> > > like "use this when adding any sort of NO_HZ support to your arch"
> > > should go in the help text, not the short description. The short
> > > description should be something like along the lines of "Enable
> > > consistency checks for RCU", for example.
> > > 
> > > Additionally I see some inconsistency in the fact that this option
> > > defaults to n but the help text says "Say Y if you are unsure". BTW,
> > > option RCU_CPU_STALL_INFO is equally inconsistent with a default y and
> > > "say N if you are unsure" in the help text.
> > 
> > I have the following queued, which should address your first point.
> 
> Yes it does, thank you. If I were to nitpick I'd say that the option is
> adding (or including) the asserts rather than providing them.
> 
> > Would adding "default y" address your other point?
> 
> It would make things consistent, but I have a serious doubt that this
> is the right direction. An option which is described as being useful
> "when adding any sort of NO_HZ support to a new architecture" really
> does not seem to be one that should be enabled by default. You may
> argue that it still depends on DEBUG_KERNEL but well, that option is
> enabled in pretty much every distribution kernel these days. So I
> believe that default n (no default actually) was correct, and what
> needs to be removed is the statement "Say Y if you are unsure". If the
> user is unsure, then certainly he/she is not working on NO_HZ support
> for a new architecture and thus doesn't need to enable RCU_EQS_DEBUG.

What would be ideal is if everyone enabled it except for those building
systems requiring the fastest possible user-to-kernel switch times.
Because it is not that hard to break this even if you don't believe
that your are working on NO_HZ support.

> > On RCU_CPU_STALL_INFO, I have a patch queued for v4.3 that eliminates
> > this Kconfig option completely.
> 
> That solves the problem nicely :-)

Glad it works for you!  ;-)

							Thanx, Paul

> > rcu: Clarify CONFIG_RCU_EQS_DEBUG help text
> > 
> > Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
> > Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > 
> > diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> > index 6be521990d61..80efaade5e59 100644
> > --- a/lib/Kconfig.debug
> > +++ b/lib/Kconfig.debug
> > @@ -1360,7 +1360,7 @@ config RCU_TRACE
> >  	  Say N if you are unsure.
> >  
> >  config RCU_EQS_DEBUG
> > -	bool "Use this when adding any sort of NO_HZ support to your arch"
> > +	bool "Provide debugging asserts for adding NO_HZ support to an arch"
> >  	depends on DEBUG_KERNEL
> >  	help
> >  	  This option provides consistency checks in RCU's handling of
> > 
> 
> -- 
> Jean Delvare
> SUSE L3 Support
>