Message-ID: <20150705042022.GH3717@linux.vnet.ibm.com>
Date:	Sat, 4 Jul 2015 21:20:22 -0700
From:	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>
To:	Jean Delvare <jdelvare@...e.de>
Cc:	LKML <linux-kernel@...r.kernel.org>
Subject: Re: config RCU_EQS_DEBUG

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@...ux-m68k.org>
> > Signed-off-by: Paul E. McKenney <paulmck@...ux.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
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists