All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
To: Leonard Crestez <leonard.crestez@nxp.com>
Cc: Shawn Guo <shawnguo@kernel.org>,
	Fabio Estevam <fabio.estevam@nxp.com>,
	Dong Aisheng <aisheng.dong@nxp.com>,
	linux-arm-kernel@lists.infradead.org,
	Matt Fleming <matt@codeblueprint.co.uk>,
	Octavian Purdila <octavian.purdila@nxp.com>,
	Stephen Rothwell <sfr@canb.auug.org.au>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] ARM: imx_v6_v7_defconfig: Explicitly restore CONFIG_DEBUG_FS
Date: Mon, 29 May 2017 08:11:00 -0700	[thread overview]
Message-ID: <20170529151100.GJ3956@linux.vnet.ibm.com> (raw)
In-Reply-To: <1496059116.31994.6.camel@nxp.com>

On Mon, May 29, 2017 at 02:58:36PM +0300, Leonard Crestez wrote:
> On Fri, 2017-05-26 at 08:42 -0700, Paul E. McKenney wrote:
> > On Fri, May 26, 2017 at 02:26:06PM +0300, Leonard Crestez wrote:
> > > 
> > > This option was removed by "make savedefconfig" in
> > > commit c5054a98bce4 ("ARM: imx_v6_v7_defconfig: Select SMSC_PHY")
> > > 
> > > This happened because CONFIG_DEBUG_FS was implicitly selected by
> > > CONFIG_TREE_RCU_TRACE which defaulted to true because CONFIG_RCU_TRACE
> > > was enabled by default by commit 961518259b3b ("rcu: Enable RCU
> > > tracepoints by default to aid in debugging")
> > > 
> > > Recently however CONFIG_RCU_TRACE was completely removed by
> > > commit 6e74c237c410 ("rcu: Remove debugfs tracing")
> 
> > CONFIG_RCU_TRACE is still very much alive in its new home at
> > kernel/rcu/Kconfig.debug:
> 
> Sorry, what was removed is the dependency on DEBUG_FS. In particular
> this snippet:
> 
> diff --git a/init/Kconfig b/init/Kconfig
> index 2aa14ff..3025383 100644
> --
> - a/init/Kconfig
> +++ b/init/Kconfig
> @@ -659,14 +659,6 @@ config
> RCU_FAST_NO_HZ
>  
>           Say N if you are unsure.
>  
> -config
> TREE_RCU_TRACE
> -       def_bool RCU_TRACE && ( TREE_RCU || PREEMPT_RCU )
> -       select DEBUG_FS

Ah, you are right.  RCU no longer needs DEBUG_FS, so it no longer
selects it.

> -       help
> -         This option provides
> tracing for the TREE_RCU and
> -         PREEMPT_RCU implementations,
> permitting Makefile to
> -         trivially select
> kernel/rcutree_trace.c.
> -
>  config RCU_BOOST
>         bool "Enable RCU
> priority boosting"
>         depends on RT_MUTEXES && PREEMPT_RCU &&
> RCU_EXPERT
> 
> > config RCU_TRACE
> > 	bool "Enable tracing for RCU"
> > 	depends on DEBUG_KERNEL
> > 	default y if TREE_RCU
> > 	select TRACE_CLOCK
> > 	help
> > 	  This option enables additional tracepoints for ftrace-style
> > 	  event tracing.
> > 
> > 	  Say Y here if you want to enable RCU tracing
> > 	  Say N if you are unsure.
> > 
> > That said, I need to make it default to "y" if PREEMPT_RCU as well as
> > the current TREE_RCU.  Would that help?
> 
> I don't think you can help unless you want to make RCU_TRACE depend on
> DEBUG_FS for no reason. The proper fix is to have DEBUG_FS inside the
> imx_v6_v7_defconfig.
> 
> My problem is just an unfortunate accident with default dependencies
> and overzealous savedefconfig.

Right you are!

> As far as I understand after recent changes RCU_TRACE is now only
> useful with tracepoints. Shouldn't it depend on TRACEPOINTS in some
> way? It's still possible to compile with RCU_TRACE on and TRACEPOINTS
> off, is this intentional? It seems that when this happens you're just
> relying on tracepoint macros to expand to nothing.
> 
> Fr example maybe RCU_TRACE should be "default y if (TREE_RCU &&
> TRACEPOINTS); depends on TRACEPOINTS"?
> 
> In theory you could also "select TRACEPOINTS" but then you would end up
> enabling TRACEPOINTS by default in all configurations that use TREE_RCU
> but don't explicitly disable RCU_TRACE.
> 
> I'd say that if $SUBSYSTEM_DEBUG depends on $MAJOR_DEBUG_FEATURE it
> should try to avoid selecting $MAJOR_DEBUG_FEATURE by default if it's
> not otherwise enabled. As far as I can tell this is how RCU ended up
> indirectly pulling in DEBUG_FS and later unexpectedly removed it.
> 
> Or I could be completely wrong, kconfig can be very confusing.

Interesting questions!  I will give them some thought.

Perhaps TRACEPOINTS is accidentally being enabled for me just as
DEBUG_FS was being accidentally enabled for you.  ;-)

							Thanx, Paul

WARNING: multiple messages have this Message-ID (diff)
From: paulmck@linux.vnet.ibm.com (Paul E. McKenney)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] ARM: imx_v6_v7_defconfig: Explicitly restore CONFIG_DEBUG_FS
Date: Mon, 29 May 2017 08:11:00 -0700	[thread overview]
Message-ID: <20170529151100.GJ3956@linux.vnet.ibm.com> (raw)
In-Reply-To: <1496059116.31994.6.camel@nxp.com>

On Mon, May 29, 2017 at 02:58:36PM +0300, Leonard Crestez wrote:
> On Fri, 2017-05-26 at 08:42 -0700, Paul E. McKenney wrote:
> > On Fri, May 26, 2017 at 02:26:06PM +0300, Leonard Crestez wrote:
> > > 
> > > This option was removed by "make savedefconfig" in
> > > commit c5054a98bce4 ("ARM: imx_v6_v7_defconfig: Select SMSC_PHY")
> > > 
> > > This happened because CONFIG_DEBUG_FS was implicitly selected by
> > > CONFIG_TREE_RCU_TRACE which defaulted to true because CONFIG_RCU_TRACE
> > > was enabled by default by commit 961518259b3b ("rcu: Enable RCU
> > > tracepoints by default to aid in debugging")
> > > 
> > > Recently however CONFIG_RCU_TRACE was completely removed by
> > > commit 6e74c237c410 ("rcu: Remove debugfs tracing")
> 
> > CONFIG_RCU_TRACE is still very much alive in its new home at
> > kernel/rcu/Kconfig.debug:
> 
> Sorry, what was removed is the dependency on DEBUG_FS. In particular
> this snippet:
> 
> diff --git a/init/Kconfig b/init/Kconfig
> index 2aa14ff..3025383 100644
> --
> - a/init/Kconfig
> +++ b/init/Kconfig
> @@ -659,14 +659,6 @@ config
> RCU_FAST_NO_HZ
> ?
> ??????????Say N if you are unsure.
> ?
> -config
> TREE_RCU_TRACE
> -???????def_bool RCU_TRACE && ( TREE_RCU || PREEMPT_RCU )
> -???????select DEBUG_FS

Ah, you are right.  RCU no longer needs DEBUG_FS, so it no longer
selects it.

> -???????help
> -?????????This option provides
> tracing for the TREE_RCU and
> -?????????PREEMPT_RCU implementations,
> permitting Makefile to
> -?????????trivially select
> kernel/rcutree_trace.c.
> -
> ?config RCU_BOOST
> ????????bool "Enable RCU
> priority boosting"
> ????????depends on RT_MUTEXES && PREEMPT_RCU &&
> RCU_EXPERT
> 
> > config RCU_TRACE
> > 	bool "Enable tracing for RCU"
> > 	depends on DEBUG_KERNEL
> > 	default y if TREE_RCU
> > 	select TRACE_CLOCK
> > 	help
> > 	??This option enables additional tracepoints for ftrace-style
> > 	??event tracing.
> > 
> > 	??Say Y here if you want to enable RCU tracing
> > 	??Say N if you are unsure.
> > 
> > That said, I need to make it default to "y" if PREEMPT_RCU as well as
> > the current TREE_RCU.??Would that help?
> 
> I don't think you can help unless you want to make RCU_TRACE depend on
> DEBUG_FS for no reason. The proper fix is to have DEBUG_FS inside the
> imx_v6_v7_defconfig.
> 
> My problem is just an unfortunate accident with default dependencies
> and overzealous savedefconfig.

Right you are!

> As far as I understand after recent changes RCU_TRACE is now only
> useful with tracepoints. Shouldn't it depend on TRACEPOINTS in some
> way? It's still possible to compile with RCU_TRACE on and TRACEPOINTS
> off, is this intentional? It seems that when this happens you're just
> relying on tracepoint macros to expand to nothing.
> 
> Fr example maybe RCU_TRACE should be "default y if (TREE_RCU &&
> TRACEPOINTS); depends on TRACEPOINTS"?
> 
> In theory you could also "select TRACEPOINTS" but then you would end up
> enabling TRACEPOINTS by default in all configurations that use TREE_RCU
> but don't explicitly disable RCU_TRACE.
> 
> I'd say that if $SUBSYSTEM_DEBUG depends on $MAJOR_DEBUG_FEATURE it
> should try to avoid selecting $MAJOR_DEBUG_FEATURE by default if it's
> not otherwise enabled. As far as I can tell this is how RCU ended up
> indirectly pulling in DEBUG_FS and later unexpectedly removed it.
> 
> Or I could be completely wrong, kconfig can be very confusing.

Interesting questions!  I will give them some thought.

Perhaps TRACEPOINTS is accidentally being enabled for me just as
DEBUG_FS was being accidentally enabled for you.  ;-)

							Thanx, Paul

  reply	other threads:[~2017-05-29 15:11 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-05-26 11:26 [PATCH] ARM: imx_v6_v7_defconfig: Explicitly restore CONFIG_DEBUG_FS Leonard Crestez
2017-05-26 11:26 ` Leonard Crestez
2017-05-26 15:42 ` Paul E. McKenney
2017-05-26 15:42   ` Paul E. McKenney
2017-05-29 11:58   ` Leonard Crestez
2017-05-29 11:58     ` Leonard Crestez
2017-05-29 15:11     ` Paul E. McKenney [this message]
2017-05-29 15:11       ` Paul E. McKenney
2017-06-04  3:42 ` Shawn Guo
2017-06-04  3:42   ` Shawn Guo
     [not found] <41ed3514ffb9a858445150a91933b96e12086d73.1575998907.git.leonard.crestez@nxp.com>
     [not found] ` <20191210204544.GA4078779@kroah.com>
2019-12-11 19:02   ` Leonard Crestez

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=20170529151100.GJ3956@linux.vnet.ibm.com \
    --to=paulmck@linux.vnet.ibm.com \
    --cc=aisheng.dong@nxp.com \
    --cc=fabio.estevam@nxp.com \
    --cc=leonard.crestez@nxp.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=matt@codeblueprint.co.uk \
    --cc=octavian.purdila@nxp.com \
    --cc=sfr@canb.auug.org.au \
    --cc=shawnguo@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.