All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
To: Jean Delvare <jdelvare@suse.de>
Cc: LKML <linux-kernel@vger.kernel.org>
Subject: Re: config RCU_EQS_DEBUG
Date: Sat, 4 Jul 2015 21:20:22 -0700	[thread overview]
Message-ID: <20150705042022.GH3717@linux.vnet.ibm.com> (raw)
In-Reply-To: <20150704185310.616b087b@endymion.delvare>

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
> 


      reply	other threads:[~2015-07-05  9:34 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-07-03  8:07 config RCU_EQS_DEBUG Jean Delvare
2015-07-03 16:00 ` Paul E. McKenney
2015-07-04 16:53   ` Jean Delvare
2015-07-05  4:20     ` Paul E. McKenney [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150705042022.GH3717@linux.vnet.ibm.com \
    --to=paulmck@linux.vnet.ibm.com \
    --cc=jdelvare@suse.de \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.