[PATCH v3] rcu/nocb: Add an option to offload all CPUs on boot

[PATCH v3] rcu/nocb: Add an option to offload all CPUs on boot

[Joel Fernandes (Google)]

From: Joel Fernandes <joel@joelfernandes.org>

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.

With the new option enabled, all CPUs will be offloaded on boot.

Signed-off-by: Joel Fernandes <joel@joelfernandes.org>
---
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.
+
 	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
+	default n
+	help
+	  Use this option to offload callback processing from all CPUs
+	  by default, in the absence of the rcu_nocbs boot parameter.
+	  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;
 	struct rcu_data *rdp;
 
+#if defined(CONFIG_RCU_NOCB_CPU_DEFAULT_ALL)
+	if (!rcu_nocb_is_setup) {
+		need_rcu_nocb_mask = true;
+		offload_all = true;
+	}
+#endif
+
 #if defined(CONFIG_NO_HZ_FULL)
-	if (tick_nohz_full_running && cpumask_weight(tick_nohz_full_mask))
+	if (tick_nohz_full_running && cpumask_weight(tick_nohz_full_mask)) {
 		need_rcu_nocb_mask = true;
+		offload_all = false; /* NO_HZ_FULL has its own mask. */
+	}
 #endif /* #if defined(CONFIG_NO_HZ_FULL) */
 
 	if (need_rcu_nocb_mask) {
@@ -1191,6 +1200,9 @@ void __init rcu_init_nohz(void)
 		cpumask_or(rcu_nocb_mask, rcu_nocb_mask, tick_nohz_full_mask);
 #endif /* #if defined(CONFIG_NO_HZ_FULL) */
 
+	if (offload_all)
+		cpumask_setall(rcu_nocb_mask);
+
 	if (!cpumask_subset(rcu_nocb_mask, cpu_possible_mask)) {
 		pr_info("\tNote: kernel parameter 'rcu_nocbs=', 'nohz_full', or 'isolcpus=' contains nonexistent CPUs.\n");
 		cpumask_and(rcu_nocb_mask, cpu_possible_mask,
-- 
2.36.0.rc0.470.gd361397f0d-goog

Re: [PATCH v3] rcu/nocb: Add an option to offload all CPUs on boot

[Paul E. McKenney]

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.  ;-)

> With the new option enabled, all CPUs will be offloaded on boot.
> 
> Signed-off-by: Joel Fernandes <joel@joelfernandes.org>

I second Joel's call for testing and add to that a call for additional
review.

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?

> +
>  	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.)

> +	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?

> +	  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.)

>  	struct rcu_data *rdp;
>  
> +#if defined(CONFIG_RCU_NOCB_CPU_DEFAULT_ALL)
> +	if (!rcu_nocb_is_setup) {
> +		need_rcu_nocb_mask = true;
> +		offload_all = true;
> +	}
> +#endif

It would be nice to be able to use IS_ENABLED() here and below, but that
is a separate issue.  For one thing, need_rcu_nocb_mask would need to
be defined unconditionally.  So this is fine as it is for this patch.

> +
>  #if defined(CONFIG_NO_HZ_FULL)
> -	if (tick_nohz_full_running && cpumask_weight(tick_nohz_full_mask))
> +	if (tick_nohz_full_running && cpumask_weight(tick_nohz_full_mask)) {
>  		need_rcu_nocb_mask = true;
> +		offload_all = false; /* NO_HZ_FULL has its own mask. */

This is good, and is also why I am asking for the addition of nohz_full
above.

> +	}
>  #endif /* #if defined(CONFIG_NO_HZ_FULL) */
>  
>  	if (need_rcu_nocb_mask) {
> @@ -1191,6 +1200,9 @@ void __init rcu_init_nohz(void)
>  		cpumask_or(rcu_nocb_mask, rcu_nocb_mask, tick_nohz_full_mask);
>  #endif /* #if defined(CONFIG_NO_HZ_FULL) */
>  
> +	if (offload_all)
> +		cpumask_setall(rcu_nocb_mask);
> +
>  	if (!cpumask_subset(rcu_nocb_mask, cpu_possible_mask)) {
>  		pr_info("\tNote: kernel parameter 'rcu_nocbs=', 'nohz_full', or 'isolcpus=' contains nonexistent CPUs.\n");
>  		cpumask_and(rcu_nocb_mask, cpu_possible_mask,
> -- 
> 2.36.0.rc0.470.gd361397f0d-goog
> 

Re: [PATCH v3] rcu/nocb: Add an option to offload all CPUs on boot

[Joel Fernandes]

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