Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Mon, 18 Apr 2022 17:51:57 +0000
From:   Joel Fernandes <joel@joelfernandes.org>
To:     "Paul E. McKenney" <paulmck@kernel.org>
Cc:     linux-kernel@vger.kernel.org,
        Josh Triplett <josh@joshtriplett.org>,
        Lai Jiangshan <jiangshanlai@gmail.com>,
        Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
        rcu@vger.kernel.org, Steven Rostedt <rostedt@goodmis.org>,
        rushikesh.s.kadam@intel.com, vineethrp@gmail.com, urezki@gmail.com
Subject: Re: [PATCH v3] rcu/nocb: Add an option to offload all CPUs on boot
Message-ID: <Yl2lPSKZ8fLOkfyZ@google.com>
References: <20220415160224.1904505-1-joel@joelfernandes.org>
 <20220415173515.GL4285@paulmck-ThinkPad-P17-Gen-1>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220415173515.GL4285@paulmck-ThinkPad-P17-Gen-1>
Precedence: bulk

On Fri, Apr 15, 2022 at 10:35:15AM -0700, Paul E. McKenney wrote:
> On Fri, Apr 15, 2022 at 04:02:24PM +0000, Joel Fernandes (Google) wrote:
> > From: Joel Fernandes <joel@joelfernandes.org>
> 
> Much better, thank you!
> 
> > On systems with CONFIG_RCU_NOCB_CPU=y, there is no default mask provided
> > which ends up not offloading any CPU. This patch removes a dependency
> > from the bootloader having to know about RCU, about how many CPUs the
> > system has, and about how to provide the mask.
> 
> The "about how many CPUs the system has" does not apply to current
> mainline, in which "rcu_nocbs=0-N" says to offload all CPUs.  It can be
> added back in for a backport to v5.10.  ;-)

True, will fix.

> My thought is to queue this after some independent testing.
> 
> > ---
> > v2 was forcing the option to override no_cbs=
> > v3 is back to v1 but with a config option defaulting to 'n'.
> > 
> >  Documentation/admin-guide/kernel-parameters.txt |  3 +++
> >  kernel/rcu/Kconfig                              | 13 +++++++++++++
> >  kernel/rcu/tree_nocb.h                          | 16 ++++++++++++++--
> >  3 files changed, 30 insertions(+), 2 deletions(-)
> > 
> > diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> > index f5a27f067db9..7648a7dd335e 100644
> > --- a/Documentation/admin-guide/kernel-parameters.txt
> > +++ b/Documentation/admin-guide/kernel-parameters.txt
> > @@ -4398,6 +4398,9 @@
> >  			no-callback mode from boot but the mode may be
> >  			toggled at runtime via cpusets.
> >  
> > +			Note that this argument takes precedence over
> > +			the CONFIG_RCU_NOCB_CPU_DEFAULT_ALL option.
> 
> Very good, thank you!
> 
> Do we want to say the same about nohz_full?  Or am I misreading the
> code below?

You're right, will fix.

> > +
> >  	rcu_nocb_poll	[KNL]
> >  			Rather than requiring that offloaded CPUs
> >  			(specified by rcu_nocbs= above) explicitly
> > diff --git a/kernel/rcu/Kconfig b/kernel/rcu/Kconfig
> > index bf8e341e75b4..2f8bd694ed85 100644
> > --- a/kernel/rcu/Kconfig
> > +++ b/kernel/rcu/Kconfig
> > @@ -223,6 +223,19 @@ config RCU_NOCB_CPU
> >  	  Say Y here if you need reduced OS jitter, despite added overhead.
> >  	  Say N here if you are unsure.
> >  
> > +config RCU_NOCB_CPU_DEFAULT_ALL
> > +	bool "Offload RCU callback processing from all CPUs by default"
> > +	depends on RCU_NOCB_CPU
> 
> The needed dependency on RCU_EXPERT is provided transitively via
> RCU_NOCB_CPU, so this should be OK.  (To check, build a .config file,
> queue this patch, and do "make oldconfig".  If any questions are asked,
> a change is needed.)

Ok. I did that and no questions were asked.

> > +	default n
> > +	help
> > +	  Use this option to offload callback processing from all CPUs
> > +	  by default, in the absence of the rcu_nocbs boot parameter.
> 
> And also in the absence of the nohz_full boot parameter, correct?

Yes, fixed.

> > +	  This also avoids the need to use any boot parameters to achieve
> > +	  the effect of offloading all CPUs on boot.
> > +
> > +	  Say Y here if you want offload all CPUs by default on boot.
> > +	  Say N here if you are unsure.
> > +
> >  config TASKS_TRACE_RCU_READ_MB
> >  	bool "Tasks Trace RCU readers use memory barriers in user and idle"
> >  	depends on RCU_EXPERT
> > diff --git a/kernel/rcu/tree_nocb.h b/kernel/rcu/tree_nocb.h
> > index eeafb546a7a0..673fa0d1f801 100644
> > --- a/kernel/rcu/tree_nocb.h
> > +++ b/kernel/rcu/tree_nocb.h
> > @@ -1165,12 +1165,21 @@ EXPORT_SYMBOL_GPL(rcu_nocb_cpu_offload);
> >  void __init rcu_init_nohz(void)
> >  {
> >  	int cpu;
> > -	bool need_rcu_nocb_mask = false;
> > +	bool need_rcu_nocb_mask = false, offload_all = false;
> 
> Please use the extra line to perserve alphabetical order.  (I do know
> about inverse tree, though I have no idea why it is considered to be a
> good thing.)

Done.

thanks,

 - Joel