[PATCH RFC] rcu/nocb: Provide default all-CPUs mask for RCU_NOCB_CPU=y

[PATCH RFC] rcu/nocb: Provide default all-CPUs mask for RCU_NOCB_CPU=y

[Joel Fernandes]

On systems with CONFIG_RCU_NOCB_CPU=y, there is no default mask provided
which ends up not offloading any CPU. This patch removes yet another
dependency from the bootloader having to know about RCU, about how many
CPUs the system has, and about how to provide the mask. Basically, I
think we should stop pretending that the user knows what they are doing :).
In other words, if NO_CB_CPU is enabled, lets make use of it.

My goal is to make RCU as zero-config as possible with sane defaults. If
user wants to provide rcu_nocbs= or nohz_full= options, then those will
take precedence and this patch will have no effect.

I tested providing rcu_nocbs= option, ensuring that is preferred over this.

Signed-off-by: Joel Fernandes <joel@joelfernandes.org>
---
 kernel/rcu/tree_nocb.h | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/kernel/rcu/tree_nocb.h b/kernel/rcu/tree_nocb.h
index eeafb546a7a0..607fbf843467 100644
--- a/kernel/rcu/tree_nocb.h
+++ b/kernel/rcu/tree_nocb.h
@@ -1165,12 +1165,25 @@ 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, set_nocb_mask_all = false;
 	struct rcu_data *rdp;
 
+	/*
+	 * In case rcu_nocbs= was not passed on the kernel command line,
+	 * provide a sane default by offloading all CPUs. This provides a
+	 * sane default for rcu_nocbs and prevents users overlooking these
+	 * details.
+	 */
+	if (!rcu_nocb_is_setup) {
+		need_rcu_nocb_mask = true;
+		set_nocb_mask_all = true;
+	}
+
 #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;
+		set_nocb_mask_all = false; /* NO_HZ_FULL provides its own mask. */
+	}
 #endif /* #if defined(CONFIG_NO_HZ_FULL) */
 
 	if (need_rcu_nocb_mask) {
@@ -1191,6 +1204,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 (set_nocb_mask_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.35.1.1178.g4f1659d476-goog

Re: [PATCH RFC] rcu/nocb: Provide default all-CPUs mask for RCU_NOCB_CPU=y

[Paul E. McKenney]

On Thu, Apr 07, 2022 at 09:07:33PM +0000, Joel Fernandes wrote:
> On systems with CONFIG_RCU_NOCB_CPU=y, there is no default mask provided
> which ends up not offloading any CPU. This patch removes yet another
> dependency from the bootloader having to know about RCU, about how many
> CPUs the system has, and about how to provide the mask. Basically, I
> think we should stop pretending that the user knows what they are doing :).
> In other words, if NO_CB_CPU is enabled, lets make use of it.
> 
> My goal is to make RCU as zero-config as possible with sane defaults. If
> user wants to provide rcu_nocbs= or nohz_full= options, then those will
> take precedence and this patch will have no effect.
> 
> I tested providing rcu_nocbs= option, ensuring that is preferred over this.

Unless something has changed, this would change behavior relied upon
the enterprise distros.  Last I checked, they want to supply a single
binary, as evidenced by the recent CONFIG_PREEMPT_DYNAMIC Kconfig option,
and they also want the default to be non-offloaded.  That is, given a
kernel built with CONFIG_RCU_NOCB_CPU=y and without either a nohz_full
or a nocbs_cpu boot parameter, all of the CPUs must be non-offloaded.

So for me to push this to mainline, I need an ack from someone from each
of the enterprise distros, and each of those someones needs to understand
the single-binary strategy used by the corresponding distro.

And is it really all -that- hard to specify an additional boot parameter
across ChromeOS devices?  Android seems to manage it.  ;-)

							Thanx, Paul

> Signed-off-by: Joel Fernandes <joel@joelfernandes.org>
> ---
>  kernel/rcu/tree_nocb.h | 20 ++++++++++++++++++--
>  1 file changed, 18 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/rcu/tree_nocb.h b/kernel/rcu/tree_nocb.h
> index eeafb546a7a0..607fbf843467 100644
> --- a/kernel/rcu/tree_nocb.h
> +++ b/kernel/rcu/tree_nocb.h
> @@ -1165,12 +1165,25 @@ 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, set_nocb_mask_all = false;
>  	struct rcu_data *rdp;
>  
> +	/*
> +	 * In case rcu_nocbs= was not passed on the kernel command line,
> +	 * provide a sane default by offloading all CPUs. This provides a
> +	 * sane default for rcu_nocbs and prevents users overlooking these
> +	 * details.
> +	 */
> +	if (!rcu_nocb_is_setup) {
> +		need_rcu_nocb_mask = true;
> +		set_nocb_mask_all = true;
> +	}
> +
>  #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;
> +		set_nocb_mask_all = false; /* NO_HZ_FULL provides its own mask. */
> +	}
>  #endif /* #if defined(CONFIG_NO_HZ_FULL) */
>  
>  	if (need_rcu_nocb_mask) {
> @@ -1191,6 +1204,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 (set_nocb_mask_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.35.1.1178.g4f1659d476-goog
> 

Re: [PATCH RFC] rcu/nocb: Provide default all-CPUs mask for RCU_NOCB_CPU=y

[Joel Fernandes]

On Fri, Apr 8, 2022 at 10:22 AM Paul E. McKenney <paulmck@kernel.org> wrote:
>
> On Thu, Apr 07, 2022 at 09:07:33PM +0000, Joel Fernandes wrote:
> > On systems with CONFIG_RCU_NOCB_CPU=y, there is no default mask provided
> > which ends up not offloading any CPU. This patch removes yet another
> > dependency from the bootloader having to know about RCU, about how many
> > CPUs the system has, and about how to provide the mask. Basically, I
> > think we should stop pretending that the user knows what they are doing :).
> > In other words, if NO_CB_CPU is enabled, lets make use of it.
> >
> > My goal is to make RCU as zero-config as possible with sane defaults. If
> > user wants to provide rcu_nocbs= or nohz_full= options, then those will
> > take precedence and this patch will have no effect.
> >
> > I tested providing rcu_nocbs= option, ensuring that is preferred over this.
>
> Unless something has changed, this would change behavior relied upon
> the enterprise distros.  Last I checked, they want to supply a single
> binary, as evidenced by the recent CONFIG_PREEMPT_DYNAMIC Kconfig option,
> and they also want the default to be non-offloaded.  That is, given a
> kernel built with CONFIG_RCU_NOCB_CPU=y and without either a nohz_full
> or a nocbs_cpu boot parameter, all of the CPUs must be non-offloaded.

Just curious, do you have information (like data, experiment results)
on why they want default non-offloaded? Or maybe they haven't tried
the recent work done in NOCB code?

Another option I think is to make it enforce NOCB if NR_CPUS <= 32 if
that makes sense.

> So for me to push this to mainline, I need an ack from someone from each
> of the enterprise distros, and each of those someones needs to understand
> the single-binary strategy used by the corresponding distro.

Ok.

> And is it really all -that- hard to specify an additional boot parameter
> across ChromeOS devices?  Android seems to manage it.  ;-)

That's not the hard part I think. The hard part is to make sure a
future Linux user who is not an RCU expert does not forget to turn it
on. ChromeOS is not the only OS that I've seen someone forget to do it
;-D. AFAIR, there were Android devices too in the past where I saw
this forgotten. I don't think we should rely on the users doing the
right thing (as much as possible).

The single kernel binary point makes sense but in this case, I think
the bigger question that I'd have is what is the default behavior and
what do *most* users of RCU want. So we can keep sane defaults for the
majority and reduce human errors related to configuration.

thanks,

-Joel

Re: [PATCH RFC] rcu/nocb: Provide default all-CPUs mask for RCU_NOCB_CPU=y

[Paul E. McKenney]

On Fri, Apr 08, 2022 at 10:52:21AM -0400, Joel Fernandes wrote:
> On Fri, Apr 8, 2022 at 10:22 AM Paul E. McKenney <paulmck@kernel.org> wrote:
> >
> > On Thu, Apr 07, 2022 at 09:07:33PM +0000, Joel Fernandes wrote:
> > > On systems with CONFIG_RCU_NOCB_CPU=y, there is no default mask provided
> > > which ends up not offloading any CPU. This patch removes yet another
> > > dependency from the bootloader having to know about RCU, about how many
> > > CPUs the system has, and about how to provide the mask. Basically, I
> > > think we should stop pretending that the user knows what they are doing :).
> > > In other words, if NO_CB_CPU is enabled, lets make use of it.
> > >
> > > My goal is to make RCU as zero-config as possible with sane defaults. If
> > > user wants to provide rcu_nocbs= or nohz_full= options, then those will
> > > take precedence and this patch will have no effect.
> > >
> > > I tested providing rcu_nocbs= option, ensuring that is preferred over this.
> >
> > Unless something has changed, this would change behavior relied upon
> > the enterprise distros.  Last I checked, they want to supply a single
> > binary, as evidenced by the recent CONFIG_PREEMPT_DYNAMIC Kconfig option,
> > and they also want the default to be non-offloaded.  That is, given a
> > kernel built with CONFIG_RCU_NOCB_CPU=y and without either a nohz_full
> > or a nocbs_cpu boot parameter, all of the CPUs must be non-offloaded.
> 
> Just curious, do you have information (like data, experiment results)
> on why they want default non-offloaded? Or maybe they haven't tried
> the recent work done in NOCB code?

I most definitely do.  When I first introduced callback offloading, I
made it completely replace softirq callback invocation.  There were some
important throughput-oriented workloads that got hit with significant
performance degradation due to this change.  Enterprise Java workloads
were the worst hit.

Android does not run these workloads, and I am not aware of ChromeOS
running them, either.

> Another option I think is to make it enforce NOCB if NR_CPUS <= 32 if
> that makes sense.

That would avoid hurting RHEL and SLES users, so this would be better
than making the change unconditionally.  But there are a lot of distros
out there.

I have to ask...  Isn't there already a way of specifying a set of kernel
boot parameters that are required for ChromeOS?  If so, add rcu_nocbs=0-N
to that list and be happy.

> > So for me to push this to mainline, I need an ack from someone from each
> > of the enterprise distros, and each of those someones needs to understand
> > the single-binary strategy used by the corresponding distro.
> 
> Ok.
> 
> > And is it really all -that- hard to specify an additional boot parameter
> > across ChromeOS devices?  Android seems to manage it.  ;-)
> 
> That's not the hard part I think. The hard part is to make sure a
> future Linux user who is not an RCU expert does not forget to turn it
> on. ChromeOS is not the only OS that I've seen someone forget to do it
> ;-D. AFAIR, there were Android devices too in the past where I saw
> this forgotten. I don't think we should rely on the users doing the
> right thing (as much as possible).
> 
> The single kernel binary point makes sense but in this case, I think
> the bigger question that I'd have is what is the default behavior and
> what do *most* users of RCU want. So we can keep sane defaults for the
> majority and reduce human errors related to configuration.

If both the ChromeOS and Android guys need it, I could reinstate the
old RCU_NOCB_CPU_ALL Kconfig option.  This was removed due to complaints
about RCU Kconfig complexity, but I believe that Reviewed-by from ChromeOS
and Android movers and shakers would overcome lingering objections.

Would that help?

							Thanx, Paul

Re: [PATCH RFC] rcu/nocb: Provide default all-CPUs mask for RCU_NOCB_CPU=y

[Joel Fernandes]

On Fri, Apr 8, 2022 at 11:50 AM Paul E. McKenney <paulmck@kernel.org> wrote:
>
> On Fri, Apr 08, 2022 at 10:52:21AM -0400, Joel Fernandes wrote:
> > On Fri, Apr 8, 2022 at 10:22 AM Paul E. McKenney <paulmck@kernel.org> wrote:
> > >
> > > On Thu, Apr 07, 2022 at 09:07:33PM +0000, Joel Fernandes wrote:
> > > > On systems with CONFIG_RCU_NOCB_CPU=y, there is no default mask provided
> > > > which ends up not offloading any CPU. This patch removes yet another
> > > > dependency from the bootloader having to know about RCU, about how many
> > > > CPUs the system has, and about how to provide the mask. Basically, I
> > > > think we should stop pretending that the user knows what they are doing :).
> > > > In other words, if NO_CB_CPU is enabled, lets make use of it.
> > > >
> > > > My goal is to make RCU as zero-config as possible with sane defaults. If
> > > > user wants to provide rcu_nocbs= or nohz_full= options, then those will
> > > > take precedence and this patch will have no effect.
> > > >
> > > > I tested providing rcu_nocbs= option, ensuring that is preferred over this.
> > >
> > > Unless something has changed, this would change behavior relied upon
> > > the enterprise distros.  Last I checked, they want to supply a single
> > > binary, as evidenced by the recent CONFIG_PREEMPT_DYNAMIC Kconfig option,
> > > and they also want the default to be non-offloaded.  That is, given a
> > > kernel built with CONFIG_RCU_NOCB_CPU=y and without either a nohz_full
> > > or a nocbs_cpu boot parameter, all of the CPUs must be non-offloaded.
> >
> > Just curious, do you have information (like data, experiment results)
> > on why they want default non-offloaded? Or maybe they haven't tried
> > the recent work done in NOCB code?
>
> I most definitely do.  When I first introduced callback offloading, I
> made it completely replace softirq callback invocation.  There were some
> important throughput-oriented workloads that got hit with significant
> performance degradation due to this change.  Enterprise Java workloads
> were the worst hit.
>
> Android does not run these workloads, and I am not aware of ChromeOS
> running them, either.

Thanks a lot for mentioning this, I was not aware and will make note
of it :-). I wonder if the scheduler had something to do with the
degradation.

> > Another option I think is to make it enforce NOCB if NR_CPUS <= 32 if
> > that makes sense.
>
> That would avoid hurting RHEL and SLES users, so this would be better
> than making the change unconditionally.  But there are a lot of distros
> out there.
>
> I have to ask...  Isn't there already a way of specifying a set of kernel
> boot parameters that are required for ChromeOS?  If so, add rcu_nocbs=0-N
> to that list and be happy.

Yes, that's doable.

> > > And is it really all -that- hard to specify an additional boot parameter
> > > across ChromeOS devices?  Android seems to manage it.  ;-)
> >
> > That's not the hard part I think. The hard part is to make sure a
> > future Linux user who is not an RCU expert does not forget to turn it
> > on. ChromeOS is not the only OS that I've seen someone forget to do it
> > ;-D. AFAIR, there were Android devices too in the past where I saw
> > this forgotten. I don't think we should rely on the users doing the
> > right thing (as much as possible).
> >
> > The single kernel binary point makes sense but in this case, I think
> > the bigger question that I'd have is what is the default behavior and
> > what do *most* users of RCU want. So we can keep sane defaults for the
> > majority and reduce human errors related to configuration.
>
> If both the ChromeOS and Android guys need it, I could reinstate the
> old RCU_NOCB_CPU_ALL Kconfig option.  This was removed due to complaints
> about RCU Kconfig complexity, but I believe that Reviewed-by from ChromeOS
> and Android movers and shakers would overcome lingering objections.
>
> Would that help?

Yes, I think I would love for such a change. I am planning to add a
test to ChromeOS to check whether config options were correctly set
up. So I can test for both the RCU_NOCB_CPU options.

Thanks!

- Joel

Re: [PATCH RFC] rcu/nocb: Provide default all-CPUs mask for RCU_NOCB_CPU=y

[Paul E. McKenney]

On Fri, Apr 08, 2022 at 01:20:02PM -0400, Joel Fernandes wrote:
> On Fri, Apr 8, 2022 at 11:50 AM Paul E. McKenney <paulmck@kernel.org> wrote:
> >
> > On Fri, Apr 08, 2022 at 10:52:21AM -0400, Joel Fernandes wrote:
> > > On Fri, Apr 8, 2022 at 10:22 AM Paul E. McKenney <paulmck@kernel.org> wrote:
> > > >
> > > > On Thu, Apr 07, 2022 at 09:07:33PM +0000, Joel Fernandes wrote:
> > > > > On systems with CONFIG_RCU_NOCB_CPU=y, there is no default mask provided
> > > > > which ends up not offloading any CPU. This patch removes yet another
> > > > > dependency from the bootloader having to know about RCU, about how many
> > > > > CPUs the system has, and about how to provide the mask. Basically, I
> > > > > think we should stop pretending that the user knows what they are doing :).
> > > > > In other words, if NO_CB_CPU is enabled, lets make use of it.
> > > > >
> > > > > My goal is to make RCU as zero-config as possible with sane defaults. If
> > > > > user wants to provide rcu_nocbs= or nohz_full= options, then those will
> > > > > take precedence and this patch will have no effect.
> > > > >
> > > > > I tested providing rcu_nocbs= option, ensuring that is preferred over this.
> > > >
> > > > Unless something has changed, this would change behavior relied upon
> > > > the enterprise distros.  Last I checked, they want to supply a single
> > > > binary, as evidenced by the recent CONFIG_PREEMPT_DYNAMIC Kconfig option,
> > > > and they also want the default to be non-offloaded.  That is, given a
> > > > kernel built with CONFIG_RCU_NOCB_CPU=y and without either a nohz_full
> > > > or a nocbs_cpu boot parameter, all of the CPUs must be non-offloaded.
> > >
> > > Just curious, do you have information (like data, experiment results)
> > > on why they want default non-offloaded? Or maybe they haven't tried
> > > the recent work done in NOCB code?
> >
> > I most definitely do.  When I first introduced callback offloading, I
> > made it completely replace softirq callback invocation.  There were some
> > important throughput-oriented workloads that got hit with significant
> > performance degradation due to this change.  Enterprise Java workloads
> > were the worst hit.
> >
> > Android does not run these workloads, and I am not aware of ChromeOS
> > running them, either.
> 
> Thanks a lot for mentioning this, I was not aware and will make note
> of it :-). I wonder if the scheduler had something to do with the
> degradation.

It is all too easy to blame the scheduler and all too easy to forget
that the scheduler has a hard job.  ;-)

And in this case, the scheduler was just doing what it was told.

With softirq, a common RCU code path sets a bit in a per-CPU variable.
With offloaded kthreads, that same code path does a much more expensive
wakeup operation.  In addition, in the common case, the softirq handler
runs on the back of an interrupt, and that interrupt handler has already
disrupted the caches, so additional disruption by the softirq handler
is not a big deal.  In contrast, the scheduler has to do the wakeup at
some point in time, and that point in time is very likely quite a bit
more disruptive.

> > > Another option I think is to make it enforce NOCB if NR_CPUS <= 32 if
> > > that makes sense.
> >
> > That would avoid hurting RHEL and SLES users, so this would be better
> > than making the change unconditionally.  But there are a lot of distros
> > out there.
> >
> > I have to ask...  Isn't there already a way of specifying a set of kernel
> > boot parameters that are required for ChromeOS?  If so, add rcu_nocbs=0-N
> > to that list and be happy.
> 
> Yes, that's doable.

Very good, thank you!

> > > > And is it really all -that- hard to specify an additional boot parameter
> > > > across ChromeOS devices?  Android seems to manage it.  ;-)
> > >
> > > That's not the hard part I think. The hard part is to make sure a
> > > future Linux user who is not an RCU expert does not forget to turn it
> > > on. ChromeOS is not the only OS that I've seen someone forget to do it
> > > ;-D. AFAIR, there were Android devices too in the past where I saw
> > > this forgotten. I don't think we should rely on the users doing the
> > > right thing (as much as possible).
> > >
> > > The single kernel binary point makes sense but in this case, I think
> > > the bigger question that I'd have is what is the default behavior and
> > > what do *most* users of RCU want. So we can keep sane defaults for the
> > > majority and reduce human errors related to configuration.
> >
> > If both the ChromeOS and Android guys need it, I could reinstate the
> > old RCU_NOCB_CPU_ALL Kconfig option.  This was removed due to complaints
> > about RCU Kconfig complexity, but I believe that Reviewed-by from ChromeOS
> > and Android movers and shakers would overcome lingering objections.
> >
> > Would that help?
> 
> Yes, I think I would love for such a change. I am planning to add a
> test to ChromeOS to check whether config options were correctly set
> up. So I can test for both the RCU_NOCB_CPU options.

Very good!

Do you love such a change enough to create the patch and to collect
convincing Reviewed-by tags?

							Thanx, Paul

Re: [PATCH RFC] rcu/nocb: Provide default all-CPUs mask for RCU_NOCB_CPU=y

[Joel Fernandes]

On Fri, Apr 8, 2022 at 1:49 PM Paul E. McKenney <paulmck@kernel.org> wrote:
>
> On Fri, Apr 08, 2022 at 01:20:02PM -0400, Joel Fernandes wrote:
> > On Fri, Apr 8, 2022 at 11:50 AM Paul E. McKenney <paulmck@kernel.org> wrote:
> > >
> > > On Fri, Apr 08, 2022 at 10:52:21AM -0400, Joel Fernandes wrote:
> > > > On Fri, Apr 8, 2022 at 10:22 AM Paul E. McKenney <paulmck@kernel.org> wrote:
> > > > >
> > > > > On Thu, Apr 07, 2022 at 09:07:33PM +0000, Joel Fernandes wrote:
> > > > > > On systems with CONFIG_RCU_NOCB_CPU=y, there is no default mask provided
> > > > > > which ends up not offloading any CPU. This patch removes yet another
> > > > > > dependency from the bootloader having to know about RCU, about how many
> > > > > > CPUs the system has, and about how to provide the mask. Basically, I
> > > > > > think we should stop pretending that the user knows what they are doing :).
> > > > > > In other words, if NO_CB_CPU is enabled, lets make use of it.
> > > > > >
> > > > > > My goal is to make RCU as zero-config as possible with sane defaults. If
> > > > > > user wants to provide rcu_nocbs= or nohz_full= options, then those will
> > > > > > take precedence and this patch will have no effect.
> > > > > >
> > > > > > I tested providing rcu_nocbs= option, ensuring that is preferred over this.
> > > > >
> > > > > Unless something has changed, this would change behavior relied upon
> > > > > the enterprise distros.  Last I checked, they want to supply a single
> > > > > binary, as evidenced by the recent CONFIG_PREEMPT_DYNAMIC Kconfig option,
> > > > > and they also want the default to be non-offloaded.  That is, given a
> > > > > kernel built with CONFIG_RCU_NOCB_CPU=y and without either a nohz_full
> > > > > or a nocbs_cpu boot parameter, all of the CPUs must be non-offloaded.
> > > >
> > > > Just curious, do you have information (like data, experiment results)
> > > > on why they want default non-offloaded? Or maybe they haven't tried
> > > > the recent work done in NOCB code?
> > >
> > > I most definitely do.  When I first introduced callback offloading, I
> > > made it completely replace softirq callback invocation.  There were some
> > > important throughput-oriented workloads that got hit with significant
> > > performance degradation due to this change.  Enterprise Java workloads
> > > were the worst hit.
> > >
> > > Android does not run these workloads, and I am not aware of ChromeOS
> > > running them, either.
> >
> > Thanks a lot for mentioning this, I was not aware and will make note
> > of it :-). I wonder if the scheduler had something to do with the
> > degradation.
>
> It is all too easy to blame the scheduler and all too easy to forget
> that the scheduler has a hard job.  ;-)
>
> And in this case, the scheduler was just doing what it was told.

No was just saying the scheduler has to do more work with NOCB because
of the extra threads, so that likely degrades the workloads (context
switch, wake ups, etc).

> > > > > And is it really all -that- hard to specify an additional boot parameter
> > > > > across ChromeOS devices?  Android seems to manage it.  ;-)
> > > >
> > > > That's not the hard part I think. The hard part is to make sure a
> > > > future Linux user who is not an RCU expert does not forget to turn it
> > > > on. ChromeOS is not the only OS that I've seen someone forget to do it
> > > > ;-D. AFAIR, there were Android devices too in the past where I saw
> > > > this forgotten. I don't think we should rely on the users doing the
> > > > right thing (as much as possible).
> > > >
> > > > The single kernel binary point makes sense but in this case, I think
> > > > the bigger question that I'd have is what is the default behavior and
> > > > what do *most* users of RCU want. So we can keep sane defaults for the
> > > > majority and reduce human errors related to configuration.
> > >
> > > If both the ChromeOS and Android guys need it, I could reinstate the
> > > old RCU_NOCB_CPU_ALL Kconfig option.  This was removed due to complaints
> > > about RCU Kconfig complexity, but I believe that Reviewed-by from ChromeOS
> > > and Android movers and shakers would overcome lingering objections.
> > >
> > > Would that help?
> >
> > Yes, I think I would love for such a change. I am planning to add a
> > test to ChromeOS to check whether config options were correctly set
> > up. So I can test for both the RCU_NOCB_CPU options.
>
> Very good!
>
> Do you love such a change enough to create the patch and to collect
> convincing Reviewed-by tags?

Yes sure, just so I understand - basically I have to make the code in
my patch run when RCU_NOCB_CPU_ALL option is passed (and keep the
option default disabled), but otherwise default to the current
behavior, right?

Thanks,

- Joel

Re: [PATCH RFC] rcu/nocb: Provide default all-CPUs mask for RCU_NOCB_CPU=y

[Joel Fernandes]

On Fri, Apr 8, 2022 at 2:22 PM Joel Fernandes <joel@joelfernandes.org> wrote:
>
> On Fri, Apr 8, 2022 at 1:49 PM Paul E. McKenney <paulmck@kernel.org> wrote:
> >
> > On Fri, Apr 08, 2022 at 01:20:02PM -0400, Joel Fernandes wrote:
> > > On Fri, Apr 8, 2022 at 11:50 AM Paul E. McKenney <paulmck@kernel.org> wrote:
> > > >
> > > > On Fri, Apr 08, 2022 at 10:52:21AM -0400, Joel Fernandes wrote:
> > > > > On Fri, Apr 8, 2022 at 10:22 AM Paul E. McKenney <paulmck@kernel.org> wrote:
> > > > > >
> > > > > > On Thu, Apr 07, 2022 at 09:07:33PM +0000, Joel Fernandes wrote:
> > > > > > > On systems with CONFIG_RCU_NOCB_CPU=y, there is no default mask provided
> > > > > > > which ends up not offloading any CPU. This patch removes yet another
> > > > > > > dependency from the bootloader having to know about RCU, about how many
> > > > > > > CPUs the system has, and about how to provide the mask. Basically, I
> > > > > > > think we should stop pretending that the user knows what they are doing :).
> > > > > > > In other words, if NO_CB_CPU is enabled, lets make use of it.
> > > > > > >
> > > > > > > My goal is to make RCU as zero-config as possible with sane defaults. If
> > > > > > > user wants to provide rcu_nocbs= or nohz_full= options, then those will
> > > > > > > take precedence and this patch will have no effect.
> > > > > > >
> > > > > > > I tested providing rcu_nocbs= option, ensuring that is preferred over this.
> > > > > >
> > > > > > Unless something has changed, this would change behavior relied upon
> > > > > > the enterprise distros.  Last I checked, they want to supply a single
> > > > > > binary, as evidenced by the recent CONFIG_PREEMPT_DYNAMIC Kconfig option,
> > > > > > and they also want the default to be non-offloaded.  That is, given a
> > > > > > kernel built with CONFIG_RCU_NOCB_CPU=y and without either a nohz_full
> > > > > > or a nocbs_cpu boot parameter, all of the CPUs must be non-offloaded.
> > > > >
> > > > > Just curious, do you have information (like data, experiment results)
> > > > > on why they want default non-offloaded? Or maybe they haven't tried
> > > > > the recent work done in NOCB code?
> > > >
> > > > I most definitely do.  When I first introduced callback offloading, I
> > > > made it completely replace softirq callback invocation.  There were some
> > > > important throughput-oriented workloads that got hit with significant
> > > > performance degradation due to this change.  Enterprise Java workloads
> > > > were the worst hit.
> > > >
> > > > Android does not run these workloads, and I am not aware of ChromeOS
> > > > running them, either.
> > >
> > > Thanks a lot for mentioning this, I was not aware and will make note
> > > of it :-). I wonder if the scheduler had something to do with the
> > > degradation.
> >
> > It is all too easy to blame the scheduler and all too easy to forget
> > that the scheduler has a hard job.  ;-)
> >
> > And in this case, the scheduler was just doing what it was told.
>
> No was just saying the scheduler has to do more work with NOCB because
> of the extra threads, so that likely degrades the workloads (context
> switch, wake ups, etc).
>
> > > > > > And is it really all -that- hard to specify an additional boot parameter
> > > > > > across ChromeOS devices?  Android seems to manage it.  ;-)
> > > > >
> > > > > That's not the hard part I think. The hard part is to make sure a
> > > > > future Linux user who is not an RCU expert does not forget to turn it
> > > > > on. ChromeOS is not the only OS that I've seen someone forget to do it
> > > > > ;-D. AFAIR, there were Android devices too in the past where I saw
> > > > > this forgotten. I don't think we should rely on the users doing the
> > > > > right thing (as much as possible).
> > > > >
> > > > > The single kernel binary point makes sense but in this case, I think
> > > > > the bigger question that I'd have is what is the default behavior and
> > > > > what do *most* users of RCU want. So we can keep sane defaults for the
> > > > > majority and reduce human errors related to configuration.
> > > >
> > > > If both the ChromeOS and Android guys need it, I could reinstate the
> > > > old RCU_NOCB_CPU_ALL Kconfig option.  This was removed due to complaints
> > > > about RCU Kconfig complexity, but I believe that Reviewed-by from ChromeOS
> > > > and Android movers and shakers would overcome lingering objections.
> > > >
> > > > Would that help?
> > >
> > > Yes, I think I would love for such a change. I am planning to add a
> > > test to ChromeOS to check whether config options were correctly set
> > > up. So I can test for both the RCU_NOCB_CPU options.
> >
> > Very good!
> >
> > Do you love such a change enough to create the patch and to collect
> > convincing Reviewed-by tags?
>
> Yes sure, just so I understand - basically I have to make the code in
> my patch run when RCU_NOCB_CPU_ALL option is passed (and keep the
> option default disabled), but otherwise default to the current
> behavior, right?

Sorry rephrasing, "make the code in my patch run when the new
CONFIG_RCU_NOCB_CPU_ALL is enabled".

thanks,

- Joel

Re: [PATCH RFC] rcu/nocb: Provide default all-CPUs mask for RCU_NOCB_CPU=y

[Paul E. McKenney]

On Fri, Apr 08, 2022 at 02:23:34PM -0400, Joel Fernandes wrote:
> On Fri, Apr 8, 2022 at 2:22 PM Joel Fernandes <joel@joelfernandes.org> wrote:
> >
> > On Fri, Apr 8, 2022 at 1:49 PM Paul E. McKenney <paulmck@kernel.org> wrote:
> > >
> > > On Fri, Apr 08, 2022 at 01:20:02PM -0400, Joel Fernandes wrote:
> > > > On Fri, Apr 8, 2022 at 11:50 AM Paul E. McKenney <paulmck@kernel.org> wrote:
> > > > >
> > > > > On Fri, Apr 08, 2022 at 10:52:21AM -0400, Joel Fernandes wrote:
> > > > > > On Fri, Apr 8, 2022 at 10:22 AM Paul E. McKenney <paulmck@kernel.org> wrote:
> > > > > > >
> > > > > > > On Thu, Apr 07, 2022 at 09:07:33PM +0000, Joel Fernandes wrote:
> > > > > > > > On systems with CONFIG_RCU_NOCB_CPU=y, there is no default mask provided
> > > > > > > > which ends up not offloading any CPU. This patch removes yet another
> > > > > > > > dependency from the bootloader having to know about RCU, about how many
> > > > > > > > CPUs the system has, and about how to provide the mask. Basically, I
> > > > > > > > think we should stop pretending that the user knows what they are doing :).
> > > > > > > > In other words, if NO_CB_CPU is enabled, lets make use of it.
> > > > > > > >
> > > > > > > > My goal is to make RCU as zero-config as possible with sane defaults. If
> > > > > > > > user wants to provide rcu_nocbs= or nohz_full= options, then those will
> > > > > > > > take precedence and this patch will have no effect.
> > > > > > > >
> > > > > > > > I tested providing rcu_nocbs= option, ensuring that is preferred over this.
> > > > > > >
> > > > > > > Unless something has changed, this would change behavior relied upon
> > > > > > > the enterprise distros.  Last I checked, they want to supply a single
> > > > > > > binary, as evidenced by the recent CONFIG_PREEMPT_DYNAMIC Kconfig option,
> > > > > > > and they also want the default to be non-offloaded.  That is, given a
> > > > > > > kernel built with CONFIG_RCU_NOCB_CPU=y and without either a nohz_full
> > > > > > > or a nocbs_cpu boot parameter, all of the CPUs must be non-offloaded.
> > > > > >
> > > > > > Just curious, do you have information (like data, experiment results)
> > > > > > on why they want default non-offloaded? Or maybe they haven't tried
> > > > > > the recent work done in NOCB code?
> > > > >
> > > > > I most definitely do.  When I first introduced callback offloading, I
> > > > > made it completely replace softirq callback invocation.  There were some
> > > > > important throughput-oriented workloads that got hit with significant
> > > > > performance degradation due to this change.  Enterprise Java workloads
> > > > > were the worst hit.
> > > > >
> > > > > Android does not run these workloads, and I am not aware of ChromeOS
> > > > > running them, either.
> > > >
> > > > Thanks a lot for mentioning this, I was not aware and will make note
> > > > of it :-). I wonder if the scheduler had something to do with the
> > > > degradation.
> > >
> > > It is all too easy to blame the scheduler and all too easy to forget
> > > that the scheduler has a hard job.  ;-)
> > >
> > > And in this case, the scheduler was just doing what it was told.
> >
> > No was just saying the scheduler has to do more work with NOCB because
> > of the extra threads, so that likely degrades the workloads (context
> > switch, wake ups, etc).
> >
> > > > > > > And is it really all -that- hard to specify an additional boot parameter
> > > > > > > across ChromeOS devices?  Android seems to manage it.  ;-)
> > > > > >
> > > > > > That's not the hard part I think. The hard part is to make sure a
> > > > > > future Linux user who is not an RCU expert does not forget to turn it
> > > > > > on. ChromeOS is not the only OS that I've seen someone forget to do it
> > > > > > ;-D. AFAIR, there were Android devices too in the past where I saw
> > > > > > this forgotten. I don't think we should rely on the users doing the
> > > > > > right thing (as much as possible).
> > > > > >
> > > > > > The single kernel binary point makes sense but in this case, I think
> > > > > > the bigger question that I'd have is what is the default behavior and
> > > > > > what do *most* users of RCU want. So we can keep sane defaults for the
> > > > > > majority and reduce human errors related to configuration.
> > > > >
> > > > > If both the ChromeOS and Android guys need it, I could reinstate the
> > > > > old RCU_NOCB_CPU_ALL Kconfig option.  This was removed due to complaints
> > > > > about RCU Kconfig complexity, but I believe that Reviewed-by from ChromeOS
> > > > > and Android movers and shakers would overcome lingering objections.
> > > > >
> > > > > Would that help?
> > > >
> > > > Yes, I think I would love for such a change. I am planning to add a
> > > > test to ChromeOS to check whether config options were correctly set
> > > > up. So I can test for both the RCU_NOCB_CPU options.
> > >
> > > Very good!
> > >
> > > Do you love such a change enough to create the patch and to collect
> > > convincing Reviewed-by tags?
> >
> > Yes sure, just so I understand - basically I have to make the code in
> > my patch run when RCU_NOCB_CPU_ALL option is passed (and keep the
> > option default disabled), but otherwise default to the current
> > behavior, right?
> 
> Sorry rephrasing, "make the code in my patch run when the new
> CONFIG_RCU_NOCB_CPU_ALL is enabled".

Here is what I believe you are proposing:


				---	rcu_nocbs	rcu_nocbs=???

CONFIG_RCU_NOCB_CPU_ALL=n	[1]	[2]		[3]

CONFIG_RCU_NOCB_CPU_ALL=y	[4]	[4]		[3]


[1]	No CPUs are offloaded at boot.	CPUs cannot be offloaded at
	runtime.

[2]	No CPUs are offloaded at boot, but any CPU can be offloaded
	(and later de-offloaded) at runtime.

[3]	The set of CPUs that are offloaded at boot are specified by the
	mask, represented above with "???".  The CPUs that are offloaded
	at boot can be de-offloaded and offloaded at runtime.  The CPUs
	not offloaded at boot cannot be offloaded at runtime.

[4]	All CPUs are offloaded at boot, and any CPU can be de-offloaded
	and offloaded at runtime.  This is the same behavior that
	you would currently get with CONFIG_RCU_NOCB_CPU_ALL=n and
	rcu_nocbs=0-N.


I am adding Frederic on CC, who will not be shy about correcting any
confusion I be suffering from have with respect to the current code.

Either way, if this is not what you had in mind, what are you suggesting
instead?

I believe that Steve Rostedt's review would carry weight for ChromeOS,
however, I am suffering a senior moment on the right person for Android.

							Thanx, Paul

Re: [PATCH RFC] rcu/nocb: Provide default all-CPUs mask for RCU_NOCB_CPU=y

[Uladzislau Rezki]

> 
> Here is what I believe you are proposing:
> 
> 
> 				---	rcu_nocbs	rcu_nocbs=???
> 
> CONFIG_RCU_NOCB_CPU_ALL=n	[1]	[2]		[3]
> 
> CONFIG_RCU_NOCB_CPU_ALL=y	[4]	[4]		[3]
> 
> 
> [1]	No CPUs are offloaded at boot.	CPUs cannot be offloaded at
> 	runtime.
> 
> [2]	No CPUs are offloaded at boot, but any CPU can be offloaded
> 	(and later de-offloaded) at runtime.
> 
> [3]	The set of CPUs that are offloaded at boot are specified by the
> 	mask, represented above with "???".  The CPUs that are offloaded
> 	at boot can be de-offloaded and offloaded at runtime.  The CPUs
> 	not offloaded at boot cannot be offloaded at runtime.
> 
> [4]	All CPUs are offloaded at boot, and any CPU can be de-offloaded
> 	and offloaded at runtime.  This is the same behavior that
> 	you would currently get with CONFIG_RCU_NOCB_CPU_ALL=n and
> 	rcu_nocbs=0-N.
> 
> 
> I am adding Frederic on CC, who will not be shy about correcting any
> confusion I be suffering from have with respect to the current code.
> 
> Either way, if this is not what you had in mind, what are you suggesting
> instead?
> 
> I believe that Steve Rostedt's review would carry weight for ChromeOS,
> however, I am suffering a senior moment on the right person for Android.
> 
We(in Sony) mark all CPUs as offloaded ones because of power reasons. The
energy aware scheduler has a better knowledge where to place an rcuop/x
task to invoke callbacks. The decision is taken based on many reason and
the most important is to drain less power as a result of task placement.
For example, power table, if OPP becomes higher or not, CPU is idle, etc.

What Joel does in this patch sounds natural to me at least from the first
glance. I mean converting the RCU_NOCB_CPU=y to make all CPUs to do offloading.

--
Uladzislau Rezki

Re: [PATCH RFC] rcu/nocb: Provide default all-CPUs mask for RCU_NOCB_CPU=y

[Uladzislau Rezki]

Hello, Joel.

> On systems with CONFIG_RCU_NOCB_CPU=y, there is no default mask provided
> which ends up not offloading any CPU. This patch removes yet another
> dependency from the bootloader having to know about RCU, about how many
> CPUs the system has, and about how to provide the mask. Basically, I
> think we should stop pretending that the user knows what they are doing :).
> In other words, if NO_CB_CPU is enabled, lets make use of it.
> 
Could you also please modify the documentation accordingly and send v2?

<snip>
urezki@pc638:~/data/raid0/coding/linux-rcu.git$ grep -rn RCU_NOCB_CPU ./Documentation/
./Documentation/timers/no_hz.rst:188:using the CONFIG_RCU_NOCB_CPU=y Kconfig option.  The specific CPUs to
./Documentation/admin-guide/kernel-parameters.txt:4380:                 In kernels built with CONFIG_RCU_NOCB_CPU=y,
./Documentation/admin-guide/kernel-parameters.txt:4507:                 When RCU_NOCB_CPU is set, also adjust the
./Documentation/admin-guide/kernel-per-CPU-kthreads.rst:311:3.  Build with CONFIG_RCU_NOCB_CPU=y and boot with the rcu_nocbs=
./Documentation/admin-guide/kernel-per-CPU-kthreads.rst:331:2.  Build with CONFIG_RCU_NOCB_CPU=n, which will prevent these
./Documentation/RCU/Design/Requirements/Requirements.rst:1424:``CONFIG_RCU_NOCB_CPU=y`` and booted with ``rcu_nocbs=1-63``, where
urezki@pc638:~/data/raid0/coding/linux-rcu.git$ grep -rn RCU_NOCB_CPU ./kernel/rcu/
...
./kernel/rcu/Kconfig:198:config RCU_NOCB_CPU
..
<snip>

--
Uladzislau Rezki

Re: [PATCH RFC] rcu/nocb: Provide default all-CPUs mask for RCU_NOCB_CPU=y

[Paul E. McKenney]

On Fri, Apr 08, 2022 at 11:46:15PM +0200, Uladzislau Rezki wrote:
> > 
> > Here is what I believe you are proposing:
> > 
> > 
> > 				---	rcu_nocbs	rcu_nocbs=???
> > 
> > CONFIG_RCU_NOCB_CPU_ALL=n	[1]	[2]		[3]
> > 
> > CONFIG_RCU_NOCB_CPU_ALL=y	[4]	[4]		[3]
> > 
> > 
> > [1]	No CPUs are offloaded at boot.	CPUs cannot be offloaded at
> > 	runtime.
> > 
> > [2]	No CPUs are offloaded at boot, but any CPU can be offloaded
> > 	(and later de-offloaded) at runtime.
> > 
> > [3]	The set of CPUs that are offloaded at boot are specified by the
> > 	mask, represented above with "???".  The CPUs that are offloaded
> > 	at boot can be de-offloaded and offloaded at runtime.  The CPUs
> > 	not offloaded at boot cannot be offloaded at runtime.
> > 
> > [4]	All CPUs are offloaded at boot, and any CPU can be de-offloaded
> > 	and offloaded at runtime.  This is the same behavior that
> > 	you would currently get with CONFIG_RCU_NOCB_CPU_ALL=n and
> > 	rcu_nocbs=0-N.
> > 
> > 
> > I am adding Frederic on CC, who will not be shy about correcting any
> > confusion I be suffering from have with respect to the current code.
> > 
> > Either way, if this is not what you had in mind, what are you suggesting
> > instead?
> > 
> > I believe that Steve Rostedt's review would carry weight for ChromeOS,
> > however, I am suffering a senior moment on the right person for Android.
> > 
> We(in Sony) mark all CPUs as offloaded ones because of power reasons. The
> energy aware scheduler has a better knowledge where to place an rcuop/x
> task to invoke callbacks. The decision is taken based on many reason and
> the most important is to drain less power as a result of task placement.
> For example, power table, if OPP becomes higher or not, CPU is idle, etc.
> 
> What Joel does in this patch sounds natural to me at least from the first
> glance. I mean converting the RCU_NOCB_CPU=y to make all CPUs to do offloading.

Just to be very clear, given appropriate acks/reviews, adding something
like CONFIG_RCU_NOCB_CPU_ALL to get default rcu_nocbs=0-N is fine.
However, Joel's original patch would not be good for the enterprise
distros, which rely on the current default.

							Thanx, Paul

Re: [PATCH RFC] rcu/nocb: Provide default all-CPUs mask for RCU_NOCB_CPU=y

[Joel Fernandes]

Hi Paul,

On Fri, Apr 8, 2022 at 4:54 PM Paul E. McKenney <paulmck@kernel.org> wrote:
[...]
> > > > > > > > And is it really all -that- hard to specify an additional boot parameter
> > > > > > > > across ChromeOS devices?  Android seems to manage it.  ;-)
> > > > > > >
> > > > > > > That's not the hard part I think. The hard part is to make sure a
> > > > > > > future Linux user who is not an RCU expert does not forget to turn it
> > > > > > > on. ChromeOS is not the only OS that I've seen someone forget to do it
> > > > > > > ;-D. AFAIR, there were Android devices too in the past where I saw
> > > > > > > this forgotten. I don't think we should rely on the users doing the
> > > > > > > right thing (as much as possible).
> > > > > > >
> > > > > > > The single kernel binary point makes sense but in this case, I think
> > > > > > > the bigger question that I'd have is what is the default behavior and
> > > > > > > what do *most* users of RCU want. So we can keep sane defaults for the
> > > > > > > majority and reduce human errors related to configuration.
> > > > > >
> > > > > > If both the ChromeOS and Android guys need it, I could reinstate the
> > > > > > old RCU_NOCB_CPU_ALL Kconfig option.  This was removed due to complaints
> > > > > > about RCU Kconfig complexity, but I believe that Reviewed-by from ChromeOS
> > > > > > and Android movers and shakers would overcome lingering objections.
> > > > > >
> > > > > > Would that help?
> > > > >
> > > > > Yes, I think I would love for such a change. I am planning to add a
> > > > > test to ChromeOS to check whether config options were correctly set
> > > > > up. So I can test for both the RCU_NOCB_CPU options.
> > > >
> > > > Very good!
> > > >
> > > > Do you love such a change enough to create the patch and to collect
> > > > convincing Reviewed-by tags?
> > >
> > > Yes sure, just so I understand - basically I have to make the code in
> > > my patch run when RCU_NOCB_CPU_ALL option is passed (and keep the
> > > option default disabled), but otherwise default to the current
> > > behavior, right?
> >
> > Sorry rephrasing, "make the code in my patch run when the new
> > CONFIG_RCU_NOCB_CPU_ALL is enabled".
>
> Here is what I believe you are proposing:
>
>
>                                 ---     rcu_nocbs       rcu_nocbs=???
>
> CONFIG_RCU_NOCB_CPU_ALL=n       [1]     [2]             [3]
>
> CONFIG_RCU_NOCB_CPU_ALL=y       [4]     [4]             [3]

It is always a pleasure to read your well thought out emails ;-)

>
> [1]     No CPUs are offloaded at boot.  CPUs cannot be offloaded at
>         runtime.
>
> [2]     No CPUs are offloaded at boot, but any CPU can be offloaded
>         (and later de-offloaded) at runtime.
>
> [3]     The set of CPUs that are offloaded at boot are specified by the
>         mask, represented above with "???".  The CPUs that are offloaded
>         at boot can be de-offloaded and offloaded at runtime.  The CPUs
>         not offloaded at boot cannot be offloaded at runtime.

Hmm, in other words you are saying that in current code, if only
select CPUs are offloaded at boot - then only those can be toggled,
but the others are deemed not offload-able? I am happy to leave that
quirk/behavior alone as I don't care much right now (for our use
cases) for runtime toggling.

> [4]     All CPUs are offloaded at boot, and any CPU can be de-offloaded
>         and offloaded at runtime.  This is the same behavior that
>         you would currently get with CONFIG_RCU_NOCB_CPU_ALL=n and
>         rcu_nocbs=0-N.

Yes, this is the behavior I intend. So then there would not be a need
to pass a mask (and I suspect for a large number of users, it
simplifies boot params).

> I believe that Steve Rostedt's review would carry weight for ChromeOS,
> however, I am suffering a senior moment on the right person for Android.

I think for Android, Kalesh Singh is in the kernel team and Tim Murray
is the performance lead. They could appropriately represent their RCU
needs.

Thanks,

 - Joel

Re: [PATCH RFC] rcu/nocb: Provide default all-CPUs mask for RCU_NOCB_CPU=y

[Joel Fernandes]

On Mon, Apr 11, 2022 at 9:49 AM Uladzislau Rezki <urezki@gmail.com> wrote:
>
> Hello, Joel.
>
> > On systems with CONFIG_RCU_NOCB_CPU=y, there is no default mask provided
> > which ends up not offloading any CPU. This patch removes yet another
> > dependency from the bootloader having to know about RCU, about how many
> > CPUs the system has, and about how to provide the mask. Basically, I
> > think we should stop pretending that the user knows what they are doing :).
> > In other words, if NO_CB_CPU is enabled, lets make use of it.
> >
> Could you also please modify the documentation accordingly and send v2?

Yes good point, I can go clean that up in v2.

 - Joel

> <snip>
> urezki@pc638:~/data/raid0/coding/linux-rcu.git$ grep -rn RCU_NOCB_CPU ./Documentation/
> ./Documentation/timers/no_hz.rst:188:using the CONFIG_RCU_NOCB_CPU=y Kconfig option.  The specific CPUs to
> ./Documentation/admin-guide/kernel-parameters.txt:4380:                 In kernels built with CONFIG_RCU_NOCB_CPU=y,
> ./Documentation/admin-guide/kernel-parameters.txt:4507:                 When RCU_NOCB_CPU is set, also adjust the
> ./Documentation/admin-guide/kernel-per-CPU-kthreads.rst:311:3.  Build with CONFIG_RCU_NOCB_CPU=y and boot with the rcu_nocbs=
> ./Documentation/admin-guide/kernel-per-CPU-kthreads.rst:331:2.  Build with CONFIG_RCU_NOCB_CPU=n, which will prevent these
> ./Documentation/RCU/Design/Requirements/Requirements.rst:1424:``CONFIG_RCU_NOCB_CPU=y`` and booted with ``rcu_nocbs=1-63``, where
> urezki@pc638:~/data/raid0/coding/linux-rcu.git$ grep -rn RCU_NOCB_CPU ./kernel/rcu/
> ...
> ./kernel/rcu/Kconfig:198:config RCU_NOCB_CPU
> ..
> <snip>
>
> --
> Uladzislau Rezki

Re: [PATCH RFC] rcu/nocb: Provide default all-CPUs mask for RCU_NOCB_CPU=y

[Uladzislau Rezki]

> On Fri, Apr 08, 2022 at 11:46:15PM +0200, Uladzislau Rezki wrote:
> > > 
> > > Here is what I believe you are proposing:
> > > 
> > > 
> > > 				---	rcu_nocbs	rcu_nocbs=???
> > > 
> > > CONFIG_RCU_NOCB_CPU_ALL=n	[1]	[2]		[3]
> > > 
> > > CONFIG_RCU_NOCB_CPU_ALL=y	[4]	[4]		[3]
> > > 
> > > 
> > > [1]	No CPUs are offloaded at boot.	CPUs cannot be offloaded at
> > > 	runtime.
> > > 
> > > [2]	No CPUs are offloaded at boot, but any CPU can be offloaded
> > > 	(and later de-offloaded) at runtime.
> > > 
> > > [3]	The set of CPUs that are offloaded at boot are specified by the
> > > 	mask, represented above with "???".  The CPUs that are offloaded
> > > 	at boot can be de-offloaded and offloaded at runtime.  The CPUs
> > > 	not offloaded at boot cannot be offloaded at runtime.
> > > 
> > > [4]	All CPUs are offloaded at boot, and any CPU can be de-offloaded
> > > 	and offloaded at runtime.  This is the same behavior that
> > > 	you would currently get with CONFIG_RCU_NOCB_CPU_ALL=n and
> > > 	rcu_nocbs=0-N.
> > > 
> > > 
> > > I am adding Frederic on CC, who will not be shy about correcting any
> > > confusion I be suffering from have with respect to the current code.
> > > 
> > > Either way, if this is not what you had in mind, what are you suggesting
> > > instead?
> > > 
> > > I believe that Steve Rostedt's review would carry weight for ChromeOS,
> > > however, I am suffering a senior moment on the right person for Android.
> > > 
> > We(in Sony) mark all CPUs as offloaded ones because of power reasons. The
> > energy aware scheduler has a better knowledge where to place an rcuop/x
> > task to invoke callbacks. The decision is taken based on many reason and
> > the most important is to drain less power as a result of task placement.
> > For example, power table, if OPP becomes higher or not, CPU is idle, etc.
> > 
> > What Joel does in this patch sounds natural to me at least from the first
> > glance. I mean converting the RCU_NOCB_CPU=y to make all CPUs to do offloading.
> 
> Just to be very clear, given appropriate acks/reviews, adding something
> like CONFIG_RCU_NOCB_CPU_ALL to get default rcu_nocbs=0-N is fine.
> However, Joel's original patch would not be good for the enterprise
> distros, which rely on the current default.
> 
Absolutely. It would be even easier in terms of changing the current concept
of RCU_NOCB_CPU config. Having an extra CONFIG_RCU_NOCB_CPU_ALL would simplify 
and get rid of a need of modifying the "rcu_nocbs=" boot parameter.

--
Uladzislau Rezki

Re: [PATCH RFC] rcu/nocb: Provide default all-CPUs mask for RCU_NOCB_CPU=y

[Paul E. McKenney]

On Mon, Apr 11, 2022 at 11:17:02AM -0400, Joel Fernandes wrote:
> Hi Paul,
> 
> On Fri, Apr 8, 2022 at 4:54 PM Paul E. McKenney <paulmck@kernel.org> wrote:
> [...]
> > > > > > > > > And is it really all -that- hard to specify an additional boot parameter
> > > > > > > > > across ChromeOS devices?  Android seems to manage it.  ;-)
> > > > > > > >
> > > > > > > > That's not the hard part I think. The hard part is to make sure a
> > > > > > > > future Linux user who is not an RCU expert does not forget to turn it
> > > > > > > > on. ChromeOS is not the only OS that I've seen someone forget to do it
> > > > > > > > ;-D. AFAIR, there were Android devices too in the past where I saw
> > > > > > > > this forgotten. I don't think we should rely on the users doing the
> > > > > > > > right thing (as much as possible).
> > > > > > > >
> > > > > > > > The single kernel binary point makes sense but in this case, I think
> > > > > > > > the bigger question that I'd have is what is the default behavior and
> > > > > > > > what do *most* users of RCU want. So we can keep sane defaults for the
> > > > > > > > majority and reduce human errors related to configuration.
> > > > > > >
> > > > > > > If both the ChromeOS and Android guys need it, I could reinstate the
> > > > > > > old RCU_NOCB_CPU_ALL Kconfig option.  This was removed due to complaints
> > > > > > > about RCU Kconfig complexity, but I believe that Reviewed-by from ChromeOS
> > > > > > > and Android movers and shakers would overcome lingering objections.
> > > > > > >
> > > > > > > Would that help?
> > > > > >
> > > > > > Yes, I think I would love for such a change. I am planning to add a
> > > > > > test to ChromeOS to check whether config options were correctly set
> > > > > > up. So I can test for both the RCU_NOCB_CPU options.
> > > > >
> > > > > Very good!
> > > > >
> > > > > Do you love such a change enough to create the patch and to collect
> > > > > convincing Reviewed-by tags?
> > > >
> > > > Yes sure, just so I understand - basically I have to make the code in
> > > > my patch run when RCU_NOCB_CPU_ALL option is passed (and keep the
> > > > option default disabled), but otherwise default to the current
> > > > behavior, right?
> > >
> > > Sorry rephrasing, "make the code in my patch run when the new
> > > CONFIG_RCU_NOCB_CPU_ALL is enabled".
> >
> > Here is what I believe you are proposing:
> >
> >
> >                                 ---     rcu_nocbs       rcu_nocbs=???
> >
> > CONFIG_RCU_NOCB_CPU_ALL=n       [1]     [2]             [3]
> >
> > CONFIG_RCU_NOCB_CPU_ALL=y       [4]     [4]             [3]
> 
> It is always a pleasure to read your well thought out emails ;-)
> 
> >
> > [1]     No CPUs are offloaded at boot.  CPUs cannot be offloaded at
> >         runtime.
> >
> > [2]     No CPUs are offloaded at boot, but any CPU can be offloaded
> >         (and later de-offloaded) at runtime.
> >
> > [3]     The set of CPUs that are offloaded at boot are specified by the
> >         mask, represented above with "???".  The CPUs that are offloaded
> >         at boot can be de-offloaded and offloaded at runtime.  The CPUs
> >         not offloaded at boot cannot be offloaded at runtime.
> 
> Hmm, in other words you are saying that in current code, if only
> select CPUs are offloaded at boot - then only those can be toggled,
> but the others are deemed not offload-able? I am happy to leave that
> quirk/behavior alone as I don't care much right now (for our use
> cases) for runtime toggling.

That is intentional behavior.  To see why, suppose that only CPU 0 was
offloaded at boot.  Now try offloading some other CPU.  A large quantity
of previously read-only data becomes read-write.  Synchronization is
not pretty.

On the other hand, default-enabling (de-)offloading on all CPUs creates
lots of unneeded rcuo kthreads.

So we didn't get here by accident.  ;-)

If this becomes a problem, I would be thinking in terms of an additional
kernel-boot parameter that made all CPUs offloadable by default.  But if
you have a better idea, please do not keep it a secret!

> > [4]     All CPUs are offloaded at boot, and any CPU can be de-offloaded
> >         and offloaded at runtime.  This is the same behavior that
> >         you would currently get with CONFIG_RCU_NOCB_CPU_ALL=n and
> >         rcu_nocbs=0-N.
> 
> Yes, this is the behavior I intend. So then there would not be a need
> to pass a mask (and I suspect for a large number of users, it
> simplifies boot params).

Very good, and from what I can see, this should work for everyone.

> > I believe that Steve Rostedt's review would carry weight for ChromeOS,
> > however, I am suffering a senior moment on the right person for Android.
> 
> I think for Android, Kalesh Singh is in the kernel team and Tim Murray
> is the performance lead. They could appropriately represent their RCU
> needs.

Sounds good!  Please collect a Reviewed-by from one or both of them.

							Thanx, Paul

Re: [PATCH RFC] rcu/nocb: Provide default all-CPUs mask for RCU_NOCB_CPU=y

[Joel Fernandes]

On Mon, Apr 11, 2022 at 08:41:09AM -0700, Paul E. McKenney wrote:
[..]
> > > [4]     All CPUs are offloaded at boot, and any CPU can be de-offloaded
> > >         and offloaded at runtime.  This is the same behavior that
> > >         you would currently get with CONFIG_RCU_NOCB_CPU_ALL=n and
> > >         rcu_nocbs=0-N.
> > 
> > Yes, this is the behavior I intend. So then there would not be a need
> > to pass a mask (and I suspect for a large number of users, it
> > simplifies boot params).
> 
> Very good, and from what I can see, this should work for everyone.

Just to clarify, what I am going to do is, if this new option =y, then
rcu_nocbs effectively wont do anything. i.e. All CPUs are offloaded at boot.
Let me know if we are not on the same page about it though. I do feel that is
a sensible choice given =y. If we are on same page, please ignore my comment.

> > > I believe that Steve Rostedt's review would carry weight for ChromeOS,
> > > however, I am suffering a senior moment on the right person for Android.
> > 
> > I think for Android, Kalesh Singh is in the kernel team and Tim Murray
> > is the performance lead. They could appropriately represent their RCU
> > needs.
> 
> Sounds good!  Please collect a Reviewed-by from one or both of them.

Ok.

thanks,

 - Joel

Re: [PATCH RFC] rcu/nocb: Provide default all-CPUs mask for RCU_NOCB_CPU=y

[Paul E. McKenney]

On Thu, Apr 14, 2022 at 07:19:48PM +0000, Joel Fernandes wrote:
> On Mon, Apr 11, 2022 at 08:41:09AM -0700, Paul E. McKenney wrote:
> [..]
> > > > [4]     All CPUs are offloaded at boot, and any CPU can be de-offloaded
> > > >         and offloaded at runtime.  This is the same behavior that
> > > >         you would currently get with CONFIG_RCU_NOCB_CPU_ALL=n and
> > > >         rcu_nocbs=0-N.
> > > 
> > > Yes, this is the behavior I intend. So then there would not be a need
> > > to pass a mask (and I suspect for a large number of users, it
> > > simplifies boot params).
> > 
> > Very good, and from what I can see, this should work for everyone.
> 
> Just to clarify, what I am going to do is, if this new option =y, then
> rcu_nocbs effectively wont do anything. i.e. All CPUs are offloaded at boot.
> Let me know if we are not on the same page about it though. I do feel that is
> a sensible choice given =y. If we are on same page, please ignore my comment.

I was assuming that the rcu_nocbs=??? for non-empty "???" would override
the CONFIG_RCU_NOCB_CPU_ALL=y.  If you choose not to do that, shouldn't
you at least issue some sort of diagnostic?  After all, the sysadmin
gave a kernel-boot parameter asking the code to do something and the
code is choosing not to do that something.

Of course, such a sysadmin might want the CONFIG_RCU_NOCB_CPU_ALL=y
Kconfig option to affect only the default, that is, when no rcu_nocbs
kernel boot parameter is specified.  This would change the second "[4]"
in my original table to "[2]".

Thoughts?

> > > > I believe that Steve Rostedt's review would carry weight for ChromeOS,
> > > > however, I am suffering a senior moment on the right person for Android.
> > > 
> > > I think for Android, Kalesh Singh is in the kernel team and Tim Murray
> > > is the performance lead. They could appropriately represent their RCU
> > > needs.
> > 
> > Sounds good!  Please collect a Reviewed-by from one or both of them.
> 
> Ok.

							Thanx, Paul

Re: [PATCH RFC] rcu/nocb: Provide default all-CPUs mask for RCU_NOCB_CPU=y

[Joel Fernandes]

On Thu, Apr 14, 2022 at 3:42 PM Paul E. McKenney <paulmck@kernel.org> wrote:
>
> On Thu, Apr 14, 2022 at 07:19:48PM +0000, Joel Fernandes wrote:
> > On Mon, Apr 11, 2022 at 08:41:09AM -0700, Paul E. McKenney wrote:
> > [..]
> > > > > [4]     All CPUs are offloaded at boot, and any CPU can be de-offloaded
> > > > >         and offloaded at runtime.  This is the same behavior that
> > > > >         you would currently get with CONFIG_RCU_NOCB_CPU_ALL=n and
> > > > >         rcu_nocbs=0-N.
> > > >
> > > > Yes, this is the behavior I intend. So then there would not be a need
> > > > to pass a mask (and I suspect for a large number of users, it
> > > > simplifies boot params).
> > >
> > > Very good, and from what I can see, this should work for everyone.
> >
> > Just to clarify, what I am going to do is, if this new option =y, then
> > rcu_nocbs effectively wont do anything. i.e. All CPUs are offloaded at boot.
> > Let me know if we are not on the same page about it though. I do feel that is
> > a sensible choice given =y. If we are on same page, please ignore my comment.
>
> I was assuming that the rcu_nocbs=??? for non-empty "???" would override
> the CONFIG_RCU_NOCB_CPU_ALL=y.  If you choose not to do that, shouldn't
> you at least issue some sort of diagnostic?  After all, the sysadmin
> gave a kernel-boot parameter asking the code to do something and the
> code is choosing not to do that something.
>
> Of course, such a sysadmin might want the CONFIG_RCU_NOCB_CPU_ALL=y
> Kconfig option to affect only the default, that is, when no rcu_nocbs
> kernel boot parameter is specified.  This would change the second "[4]"
> in my original table to "[2]".
>
> Thoughts?

I thought about that. I feel that since we are defaulting the new
config option to =n , it is a conscious choice by the distro to set it
to =y.  In such a case, they should be Ok with offloading all CPUs. If
they decide to selectively offload some CPUs in the future, then they
could revisit the config option at that time.

I feel the kernel config should override the boot parameter behavior.
It is the same effect as a sysadmin passing kernel parameter X
assuming the kernel does something but the CONFIG option might not
even build code corresponding to X.

I feel to address your concern, we can document in kernel command line
documentation that rcu_nocbs= does not have an effect if
CONFIG_RCU_NOCB_CPU_ALL=y, would that work for you?

Thanks,

- Joel

>
> > > > > I believe that Steve Rostedt's review would carry weight for ChromeOS,
> > > > > however, I am suffering a senior moment on the right person for Android.
> > > >
> > > > I think for Android, Kalesh Singh is in the kernel team and Tim Murray
> > > > is the performance lead. They could appropriately represent their RCU
> > > > needs.
> > >
> > > Sounds good!  Please collect a Reviewed-by from one or both of them.
> >
> > Ok.
>
>                                                         Thanx, Paul

Re: [PATCH RFC] rcu/nocb: Provide default all-CPUs mask for RCU_NOCB_CPU=y

[Joel Fernandes]

On Thu, Apr 14, 2022 at 3:49 PM Joel Fernandes <joel@joelfernandes.org> wrote:
>
> On Thu, Apr 14, 2022 at 3:42 PM Paul E. McKenney <paulmck@kernel.org> wrote:
> >
> > On Thu, Apr 14, 2022 at 07:19:48PM +0000, Joel Fernandes wrote:
> > > On Mon, Apr 11, 2022 at 08:41:09AM -0700, Paul E. McKenney wrote:
> > > [..]
> > > > > > [4]     All CPUs are offloaded at boot, and any CPU can be de-offloaded
> > > > > >         and offloaded at runtime.  This is the same behavior that
> > > > > >         you would currently get with CONFIG_RCU_NOCB_CPU_ALL=n and
> > > > > >         rcu_nocbs=0-N.
> > > > >
> > > > > Yes, this is the behavior I intend. So then there would not be a need
> > > > > to pass a mask (and I suspect for a large number of users, it
> > > > > simplifies boot params).
> > > >
> > > > Very good, and from what I can see, this should work for everyone.
> > >
> > > Just to clarify, what I am going to do is, if this new option =y, then
> > > rcu_nocbs effectively wont do anything. i.e. All CPUs are offloaded at boot.
> > > Let me know if we are not on the same page about it though. I do feel that is
> > > a sensible choice given =y. If we are on same page, please ignore my comment.
> >
> > I was assuming that the rcu_nocbs=??? for non-empty "???" would override
> > the CONFIG_RCU_NOCB_CPU_ALL=y.  If you choose not to do that, shouldn't
> > you at least issue some sort of diagnostic?  After all, the sysadmin
> > gave a kernel-boot parameter asking the code to do something and the
> > code is choosing not to do that something.
> >
> > Of course, such a sysadmin might want the CONFIG_RCU_NOCB_CPU_ALL=y
> > Kconfig option to affect only the default, that is, when no rcu_nocbs
> > kernel boot parameter is specified.  This would change the second "[4]"
> > in my original table to "[2]".
> >
> > Thoughts?
>
> I thought about that. I feel that since we are defaulting the new
> config option to =n , it is a conscious choice by the distro to set it
> to =y.  In such a case, they should be Ok with offloading all CPUs. If
> they decide to selectively offload some CPUs in the future, then they
> could revisit the config option at that time.
>
> I feel the kernel config should override the boot parameter behavior.
> It is the same effect as a sysadmin passing kernel parameter X
> assuming the kernel does something but the CONFIG option might not
> even build code corresponding to X.
>
> I feel to address your concern, we can document in kernel command line
> documentation that rcu_nocbs= does not have an effect if
> CONFIG_RCU_NOCB_CPU_ALL=y, would that work for you?

Along with documentation, I like your idea of printing a diagnostic in
such a situation. I will certainly do that.

Thanks,

 - Joel

>
> Thanks,
>
> - Joel
>
> >
> > > > > > I believe that Steve Rostedt's review would carry weight for ChromeOS,
> > > > > > however, I am suffering a senior moment on the right person for Android.
> > > > >
> > > > > I think for Android, Kalesh Singh is in the kernel team and Tim Murray
> > > > > is the performance lead. They could appropriately represent their RCU
> > > > > needs.
> > > >
> > > > Sounds good!  Please collect a Reviewed-by from one or both of them.
> > >
> > > Ok.
> >
> >                                                         Thanx, Paul

Re: [PATCH RFC] rcu/nocb: Provide default all-CPUs mask for RCU_NOCB_CPU=y

[Paul E. McKenney]

On Thu, Apr 14, 2022 at 03:49:16PM -0400, Joel Fernandes wrote:
> On Thu, Apr 14, 2022 at 3:42 PM Paul E. McKenney <paulmck@kernel.org> wrote:
> >
> > On Thu, Apr 14, 2022 at 07:19:48PM +0000, Joel Fernandes wrote:
> > > On Mon, Apr 11, 2022 at 08:41:09AM -0700, Paul E. McKenney wrote:
> > > [..]
> > > > > > [4]     All CPUs are offloaded at boot, and any CPU can be de-offloaded
> > > > > >         and offloaded at runtime.  This is the same behavior that
> > > > > >         you would currently get with CONFIG_RCU_NOCB_CPU_ALL=n and
> > > > > >         rcu_nocbs=0-N.
> > > > >
> > > > > Yes, this is the behavior I intend. So then there would not be a need
> > > > > to pass a mask (and I suspect for a large number of users, it
> > > > > simplifies boot params).
> > > >
> > > > Very good, and from what I can see, this should work for everyone.
> > >
> > > Just to clarify, what I am going to do is, if this new option =y, then
> > > rcu_nocbs effectively wont do anything. i.e. All CPUs are offloaded at boot.
> > > Let me know if we are not on the same page about it though. I do feel that is
> > > a sensible choice given =y. If we are on same page, please ignore my comment.
> >
> > I was assuming that the rcu_nocbs=??? for non-empty "???" would override
> > the CONFIG_RCU_NOCB_CPU_ALL=y.  If you choose not to do that, shouldn't
> > you at least issue some sort of diagnostic?  After all, the sysadmin
> > gave a kernel-boot parameter asking the code to do something and the
> > code is choosing not to do that something.
> >
> > Of course, such a sysadmin might want the CONFIG_RCU_NOCB_CPU_ALL=y
> > Kconfig option to affect only the default, that is, when no rcu_nocbs
> > kernel boot parameter is specified.  This would change the second "[4]"
> > in my original table to "[2]".
> >
> > Thoughts?
> 
> I thought about that. I feel that since we are defaulting the new
> config option to =n , it is a conscious choice by the distro to set it
> to =y.  In such a case, they should be Ok with offloading all CPUs. If
> they decide to selectively offload some CPUs in the future, then they
> could revisit the config option at that time.
> 
> I feel the kernel config should override the boot parameter behavior.
> It is the same effect as a sysadmin passing kernel parameter X
> assuming the kernel does something but the CONFIG option might not
> even build code corresponding to X.
> 
> I feel to address your concern, we can document in kernel command line
> documentation that rcu_nocbs= does not have an effect if
> CONFIG_RCU_NOCB_CPU_ALL=y, would that work for you?

Not me so much, because I would just set CONFIG_RCU_NOCB_CPU_ALL=n so
as to not worry about it.

But I am not at all looking forward to complaints about rcu_nocbs not
working the way people expect.  So let's take some time to think more
carefully about this.

							Thanx, Paul

> Thanks,
> 
> - Joel
> 
> >
> > > > > > I believe that Steve Rostedt's review would carry weight for ChromeOS,
> > > > > > however, I am suffering a senior moment on the right person for Android.
> > > > >
> > > > > I think for Android, Kalesh Singh is in the kernel team and Tim Murray
> > > > > is the performance lead. They could appropriately represent their RCU
> > > > > needs.
> > > >
> > > > Sounds good!  Please collect a Reviewed-by from one or both of them.
> > >
> > > Ok.
> >
> >                                                         Thanx, Paul

Re: [PATCH RFC] rcu/nocb: Provide default all-CPUs mask for RCU_NOCB_CPU=y

[Paul E. McKenney]

On Thu, Apr 14, 2022 at 03:51:29PM -0400, Joel Fernandes wrote:
> On Thu, Apr 14, 2022 at 3:49 PM Joel Fernandes <joel@joelfernandes.org> wrote:
> >
> > On Thu, Apr 14, 2022 at 3:42 PM Paul E. McKenney <paulmck@kernel.org> wrote:
> > >
> > > On Thu, Apr 14, 2022 at 07:19:48PM +0000, Joel Fernandes wrote:
> > > > On Mon, Apr 11, 2022 at 08:41:09AM -0700, Paul E. McKenney wrote:
> > > > [..]
> > > > > > > [4]     All CPUs are offloaded at boot, and any CPU can be de-offloaded
> > > > > > >         and offloaded at runtime.  This is the same behavior that
> > > > > > >         you would currently get with CONFIG_RCU_NOCB_CPU_ALL=n and
> > > > > > >         rcu_nocbs=0-N.
> > > > > >
> > > > > > Yes, this is the behavior I intend. So then there would not be a need
> > > > > > to pass a mask (and I suspect for a large number of users, it
> > > > > > simplifies boot params).
> > > > >
> > > > > Very good, and from what I can see, this should work for everyone.
> > > >
> > > > Just to clarify, what I am going to do is, if this new option =y, then
> > > > rcu_nocbs effectively wont do anything. i.e. All CPUs are offloaded at boot.
> > > > Let me know if we are not on the same page about it though. I do feel that is
> > > > a sensible choice given =y. If we are on same page, please ignore my comment.
> > >
> > > I was assuming that the rcu_nocbs=??? for non-empty "???" would override
> > > the CONFIG_RCU_NOCB_CPU_ALL=y.  If you choose not to do that, shouldn't
> > > you at least issue some sort of diagnostic?  After all, the sysadmin
> > > gave a kernel-boot parameter asking the code to do something and the
> > > code is choosing not to do that something.
> > >
> > > Of course, such a sysadmin might want the CONFIG_RCU_NOCB_CPU_ALL=y
> > > Kconfig option to affect only the default, that is, when no rcu_nocbs
> > > kernel boot parameter is specified.  This would change the second "[4]"
> > > in my original table to "[2]".
> > >
> > > Thoughts?
> >
> > I thought about that. I feel that since we are defaulting the new
> > config option to =n , it is a conscious choice by the distro to set it
> > to =y.  In such a case, they should be Ok with offloading all CPUs. If
> > they decide to selectively offload some CPUs in the future, then they
> > could revisit the config option at that time.
> >
> > I feel the kernel config should override the boot parameter behavior.
> > It is the same effect as a sysadmin passing kernel parameter X
> > assuming the kernel does something but the CONFIG option might not
> > even build code corresponding to X.
> >
> > I feel to address your concern, we can document in kernel command line
> > documentation that rcu_nocbs= does not have an effect if
> > CONFIG_RCU_NOCB_CPU_ALL=y, would that work for you?
> 
> Along with documentation, I like your idea of printing a diagnostic in
> such a situation. I will certainly do that.

Seriously.  Take some time.  Think carefully about it.

							Thanx, Paul

> Thanks,
> 
>  - Joel
> 
> >
> > Thanks,
> >
> > - Joel
> >
> > >
> > > > > > > I believe that Steve Rostedt's review would carry weight for ChromeOS,
> > > > > > > however, I am suffering a senior moment on the right person for Android.
> > > > > >
> > > > > > I think for Android, Kalesh Singh is in the kernel team and Tim Murray
> > > > > > is the performance lead. They could appropriately represent their RCU
> > > > > > needs.
> > > > >
> > > > > Sounds good!  Please collect a Reviewed-by from one or both of them.
> > > >
> > > > Ok.
> > >
> > >                                                         Thanx, Paul

Re: [PATCH RFC] rcu/nocb: Provide default all-CPUs mask for RCU_NOCB_CPU=y

[Joel Fernandes]

On Thu, Apr 14, 2022 at 02:09:33PM -0700, Paul E. McKenney wrote:
> On Thu, Apr 14, 2022 at 03:49:16PM -0400, Joel Fernandes wrote:
> > On Thu, Apr 14, 2022 at 3:42 PM Paul E. McKenney <paulmck@kernel.org> wrote:
> > >
> > > On Thu, Apr 14, 2022 at 07:19:48PM +0000, Joel Fernandes wrote:
> > > > On Mon, Apr 11, 2022 at 08:41:09AM -0700, Paul E. McKenney wrote:
> > > > [..]
> > > > > > > [4]     All CPUs are offloaded at boot, and any CPU can be de-offloaded
> > > > > > >         and offloaded at runtime.  This is the same behavior that
> > > > > > >         you would currently get with CONFIG_RCU_NOCB_CPU_ALL=n and
> > > > > > >         rcu_nocbs=0-N.
> > > > > >
> > > > > > Yes, this is the behavior I intend. So then there would not be a need
> > > > > > to pass a mask (and I suspect for a large number of users, it
> > > > > > simplifies boot params).
> > > > >
> > > > > Very good, and from what I can see, this should work for everyone.
> > > >
> > > > Just to clarify, what I am going to do is, if this new option =y, then
> > > > rcu_nocbs effectively wont do anything. i.e. All CPUs are offloaded at boot.
> > > > Let me know if we are not on the same page about it though. I do feel that is
> > > > a sensible choice given =y. If we are on same page, please ignore my comment.
> > >
> > > I was assuming that the rcu_nocbs=??? for non-empty "???" would override
> > > the CONFIG_RCU_NOCB_CPU_ALL=y.  If you choose not to do that, shouldn't
> > > you at least issue some sort of diagnostic?  After all, the sysadmin
> > > gave a kernel-boot parameter asking the code to do something and the
> > > code is choosing not to do that something.
> > >
> > > Of course, such a sysadmin might want the CONFIG_RCU_NOCB_CPU_ALL=y
> > > Kconfig option to affect only the default, that is, when no rcu_nocbs
> > > kernel boot parameter is specified.  This would change the second "[4]"
> > > in my original table to "[2]".
> > >
> > > Thoughts?
> > 
> > I thought about that. I feel that since we are defaulting the new
> > config option to =n , it is a conscious choice by the distro to set it
> > to =y.  In such a case, they should be Ok with offloading all CPUs. If
> > they decide to selectively offload some CPUs in the future, then they
> > could revisit the config option at that time.
> > 
> > I feel the kernel config should override the boot parameter behavior.
> > It is the same effect as a sysadmin passing kernel parameter X
> > assuming the kernel does something but the CONFIG option might not
> > even build code corresponding to X.
> > 
> > I feel to address your concern, we can document in kernel command line
> > documentation that rcu_nocbs= does not have an effect if
> > CONFIG_RCU_NOCB_CPU_ALL=y, would that work for you?
> 
> Not me so much, because I would just set CONFIG_RCU_NOCB_CPU_ALL=n so
> as to not worry about it.
> 
> But I am not at all looking forward to complaints about rcu_nocbs not
> working the way people expect.  So let's take some time to think more
> carefully about this.

That's a fair concern. But we are defaulting it to 'n' so I think if it is
unconsciously enabled without someone reading documentation, then that's a
slightly different issue.

On the other hand, I can also make it such that if rcu_nocbs= is passed, then
the CONFIG does not take effect. That's quite a bit weird/quirky IMHO.

thanks,

 - Joel



> 							Thanx, Paul
> 
> > Thanks,
> > 
> > - Joel
> > 
> > >
> > > > > > > I believe that Steve Rostedt's review would carry weight for ChromeOS,
> > > > > > > however, I am suffering a senior moment on the right person for Android.
> > > > > >
> > > > > > I think for Android, Kalesh Singh is in the kernel team and Tim Murray
> > > > > > is the performance lead. They could appropriately represent their RCU
> > > > > > needs.
> > > > >
> > > > > Sounds good!  Please collect a Reviewed-by from one or both of them.
> > > >
> > > > Ok.
> > >
> > >                                                         Thanx, Paul

Re: [PATCH RFC] rcu/nocb: Provide default all-CPUs mask for RCU_NOCB_CPU=y

[Paul E. McKenney]

On Thu, Apr 14, 2022 at 09:14:05PM +0000, Joel Fernandes wrote:
> On Thu, Apr 14, 2022 at 02:09:33PM -0700, Paul E. McKenney wrote:
> > On Thu, Apr 14, 2022 at 03:49:16PM -0400, Joel Fernandes wrote:
> > > On Thu, Apr 14, 2022 at 3:42 PM Paul E. McKenney <paulmck@kernel.org> wrote:
> > > >
> > > > On Thu, Apr 14, 2022 at 07:19:48PM +0000, Joel Fernandes wrote:
> > > > > On Mon, Apr 11, 2022 at 08:41:09AM -0700, Paul E. McKenney wrote:
> > > > > [..]
> > > > > > > > [4]     All CPUs are offloaded at boot, and any CPU can be de-offloaded
> > > > > > > >         and offloaded at runtime.  This is the same behavior that
> > > > > > > >         you would currently get with CONFIG_RCU_NOCB_CPU_ALL=n and
> > > > > > > >         rcu_nocbs=0-N.
> > > > > > >
> > > > > > > Yes, this is the behavior I intend. So then there would not be a need
> > > > > > > to pass a mask (and I suspect for a large number of users, it
> > > > > > > simplifies boot params).
> > > > > >
> > > > > > Very good, and from what I can see, this should work for everyone.
> > > > >
> > > > > Just to clarify, what I am going to do is, if this new option =y, then
> > > > > rcu_nocbs effectively wont do anything. i.e. All CPUs are offloaded at boot.
> > > > > Let me know if we are not on the same page about it though. I do feel that is
> > > > > a sensible choice given =y. If we are on same page, please ignore my comment.
> > > >
> > > > I was assuming that the rcu_nocbs=??? for non-empty "???" would override
> > > > the CONFIG_RCU_NOCB_CPU_ALL=y.  If you choose not to do that, shouldn't
> > > > you at least issue some sort of diagnostic?  After all, the sysadmin
> > > > gave a kernel-boot parameter asking the code to do something and the
> > > > code is choosing not to do that something.
> > > >
> > > > Of course, such a sysadmin might want the CONFIG_RCU_NOCB_CPU_ALL=y
> > > > Kconfig option to affect only the default, that is, when no rcu_nocbs
> > > > kernel boot parameter is specified.  This would change the second "[4]"
> > > > in my original table to "[2]".
> > > >
> > > > Thoughts?
> > > 
> > > I thought about that. I feel that since we are defaulting the new
> > > config option to =n , it is a conscious choice by the distro to set it
> > > to =y.  In such a case, they should be Ok with offloading all CPUs. If
> > > they decide to selectively offload some CPUs in the future, then they
> > > could revisit the config option at that time.
> > > 
> > > I feel the kernel config should override the boot parameter behavior.
> > > It is the same effect as a sysadmin passing kernel parameter X
> > > assuming the kernel does something but the CONFIG option might not
> > > even build code corresponding to X.
> > > 
> > > I feel to address your concern, we can document in kernel command line
> > > documentation that rcu_nocbs= does not have an effect if
> > > CONFIG_RCU_NOCB_CPU_ALL=y, would that work for you?
> > 
> > Not me so much, because I would just set CONFIG_RCU_NOCB_CPU_ALL=n so
> > as to not worry about it.
> > 
> > But I am not at all looking forward to complaints about rcu_nocbs not
> > working the way people expect.  So let's take some time to think more
> > carefully about this.
> 
> That's a fair concern. But we are defaulting it to 'n' so I think if it is
> unconsciously enabled without someone reading documentation, then that's a
> slightly different issue.

Suppose that one group decides to change to CONFIG_RCU_NOCB_CPU_ALL=y,
and some other group on some other continent happens to be using the
rcu_nocbs boot parameter (having read the documentation).  And suppose
that the level of communication between the two groups is typical,
that is to say, nonexistent.

Sure, we can argue that groups should communicate, but our making that
argument won't necessarily prevent the group using rcu_nocbs from taking
us to task in the course of their debugging effort.

> On the other hand, I can also make it such that if rcu_nocbs= is passed, then
> the CONFIG does not take effect. That's quite a bit weird/quirky IMHO.

Not at all.  We can simply say that CONFIG_RCU_NOCB_CPU_ALL controls
only the default situation, that is, when rcu_nocbs is not specified.

							Thanx, Paul

> thanks,
> 
>  - Joel
> 
> 
> 
> > 							Thanx, Paul
> > 
> > > Thanks,
> > > 
> > > - Joel
> > > 
> > > >
> > > > > > > > I believe that Steve Rostedt's review would carry weight for ChromeOS,
> > > > > > > > however, I am suffering a senior moment on the right person for Android.
> > > > > > >
> > > > > > > I think for Android, Kalesh Singh is in the kernel team and Tim Murray
> > > > > > > is the performance lead. They could appropriately represent their RCU
> > > > > > > needs.
> > > > > >
> > > > > > Sounds good!  Please collect a Reviewed-by from one or both of them.
> > > > >
> > > > > Ok.
> > > >
> > > >                                                         Thanx, Paul

Re: [PATCH RFC] rcu/nocb: Provide default all-CPUs mask for RCU_NOCB_CPU=y

[Joel Fernandes]

On Thu, Apr 14, 2022 at 5:31 PM Paul E. McKenney <paulmck@kernel.org> wrote:
>
> On Thu, Apr 14, 2022 at 09:14:05PM +0000, Joel Fernandes wrote:
> > On Thu, Apr 14, 2022 at 02:09:33PM -0700, Paul E. McKenney wrote:
> > > On Thu, Apr 14, 2022 at 03:49:16PM -0400, Joel Fernandes wrote:
> > > > On Thu, Apr 14, 2022 at 3:42 PM Paul E. McKenney <paulmck@kernel.org> wrote:
> > > > >
> > > > > On Thu, Apr 14, 2022 at 07:19:48PM +0000, Joel Fernandes wrote:
> > > > > > On Mon, Apr 11, 2022 at 08:41:09AM -0700, Paul E. McKenney wrote:
> > > > > > [..]
> > > > > > > > > [4]     All CPUs are offloaded at boot, and any CPU can be de-offloaded
> > > > > > > > >         and offloaded at runtime.  This is the same behavior that
> > > > > > > > >         you would currently get with CONFIG_RCU_NOCB_CPU_ALL=n and
> > > > > > > > >         rcu_nocbs=0-N.
> > > > > > > >
> > > > > > > > Yes, this is the behavior I intend. So then there would not be a need
> > > > > > > > to pass a mask (and I suspect for a large number of users, it
> > > > > > > > simplifies boot params).
> > > > > > >
> > > > > > > Very good, and from what I can see, this should work for everyone.
> > > > > >
> > > > > > Just to clarify, what I am going to do is, if this new option =y, then
> > > > > > rcu_nocbs effectively wont do anything. i.e. All CPUs are offloaded at boot.
> > > > > > Let me know if we are not on the same page about it though. I do feel that is
> > > > > > a sensible choice given =y. If we are on same page, please ignore my comment.
> > > > >
> > > > > I was assuming that the rcu_nocbs=??? for non-empty "???" would override
> > > > > the CONFIG_RCU_NOCB_CPU_ALL=y.  If you choose not to do that, shouldn't
> > > > > you at least issue some sort of diagnostic?  After all, the sysadmin
> > > > > gave a kernel-boot parameter asking the code to do something and the
> > > > > code is choosing not to do that something.
> > > > >
> > > > > Of course, such a sysadmin might want the CONFIG_RCU_NOCB_CPU_ALL=y
> > > > > Kconfig option to affect only the default, that is, when no rcu_nocbs
> > > > > kernel boot parameter is specified.  This would change the second "[4]"
> > > > > in my original table to "[2]".
> > > > >
> > > > > Thoughts?
> > > >
> > > > I thought about that. I feel that since we are defaulting the new
> > > > config option to =n , it is a conscious choice by the distro to set it
> > > > to =y.  In such a case, they should be Ok with offloading all CPUs. If
> > > > they decide to selectively offload some CPUs in the future, then they
> > > > could revisit the config option at that time.
> > > >
> > > > I feel the kernel config should override the boot parameter behavior.
> > > > It is the same effect as a sysadmin passing kernel parameter X
> > > > assuming the kernel does something but the CONFIG option might not
> > > > even build code corresponding to X.
> > > >
> > > > I feel to address your concern, we can document in kernel command line
> > > > documentation that rcu_nocbs= does not have an effect if
> > > > CONFIG_RCU_NOCB_CPU_ALL=y, would that work for you?
> > >
> > > Not me so much, because I would just set CONFIG_RCU_NOCB_CPU_ALL=n so
> > > as to not worry about it.
> > >
> > > But I am not at all looking forward to complaints about rcu_nocbs not
> > > working the way people expect.  So let's take some time to think more
> > > carefully about this.
> >
> > That's a fair concern. But we are defaulting it to 'n' so I think if it is
> > unconsciously enabled without someone reading documentation, then that's a
> > slightly different issue.
>
> Suppose that one group decides to change to CONFIG_RCU_NOCB_CPU_ALL=y,
> and some other group on some other continent happens to be using the
> rcu_nocbs boot parameter (having read the documentation).  And suppose
> that the level of communication between the two groups is typical,
> that is to say, nonexistent.
>
> Sure, we can argue that groups should communicate, but our making that
> argument won't necessarily prevent the group using rcu_nocbs from taking
> us to task in the course of their debugging effort.
>
> > On the other hand, I can also make it such that if rcu_nocbs= is passed, then
> > the CONFIG does not take effect. That's quite a bit weird/quirky IMHO.
>
> Not at all.  We can simply say that CONFIG_RCU_NOCB_CPU_ALL controls
> only the default situation, that is, when rcu_nocbs is not specified.

Then it should be called: CONFIG_RCU_NOCB_CPU_DEFAULT_ALL , or
something. Otherwise I can tell you that I will be the first one to be
confused by menuconfig unless I also read the code :-D

thanks,

- Joel

Re: [PATCH RFC] rcu/nocb: Provide default all-CPUs mask for RCU_NOCB_CPU=y

[Paul E. McKenney]

On Thu, Apr 14, 2022 at 05:38:08PM -0400, Joel Fernandes wrote:
> On Thu, Apr 14, 2022 at 5:31 PM Paul E. McKenney <paulmck@kernel.org> wrote:
> >
> > On Thu, Apr 14, 2022 at 09:14:05PM +0000, Joel Fernandes wrote:
> > > On Thu, Apr 14, 2022 at 02:09:33PM -0700, Paul E. McKenney wrote:
> > > > On Thu, Apr 14, 2022 at 03:49:16PM -0400, Joel Fernandes wrote:
> > > > > On Thu, Apr 14, 2022 at 3:42 PM Paul E. McKenney <paulmck@kernel.org> wrote:
> > > > > >
> > > > > > On Thu, Apr 14, 2022 at 07:19:48PM +0000, Joel Fernandes wrote:
> > > > > > > On Mon, Apr 11, 2022 at 08:41:09AM -0700, Paul E. McKenney wrote:
> > > > > > > [..]
> > > > > > > > > > [4]     All CPUs are offloaded at boot, and any CPU can be de-offloaded
> > > > > > > > > >         and offloaded at runtime.  This is the same behavior that
> > > > > > > > > >         you would currently get with CONFIG_RCU_NOCB_CPU_ALL=n and
> > > > > > > > > >         rcu_nocbs=0-N.
> > > > > > > > >
> > > > > > > > > Yes, this is the behavior I intend. So then there would not be a need
> > > > > > > > > to pass a mask (and I suspect for a large number of users, it
> > > > > > > > > simplifies boot params).
> > > > > > > >
> > > > > > > > Very good, and from what I can see, this should work for everyone.
> > > > > > >
> > > > > > > Just to clarify, what I am going to do is, if this new option =y, then
> > > > > > > rcu_nocbs effectively wont do anything. i.e. All CPUs are offloaded at boot.
> > > > > > > Let me know if we are not on the same page about it though. I do feel that is
> > > > > > > a sensible choice given =y. If we are on same page, please ignore my comment.
> > > > > >
> > > > > > I was assuming that the rcu_nocbs=??? for non-empty "???" would override
> > > > > > the CONFIG_RCU_NOCB_CPU_ALL=y.  If you choose not to do that, shouldn't
> > > > > > you at least issue some sort of diagnostic?  After all, the sysadmin
> > > > > > gave a kernel-boot parameter asking the code to do something and the
> > > > > > code is choosing not to do that something.
> > > > > >
> > > > > > Of course, such a sysadmin might want the CONFIG_RCU_NOCB_CPU_ALL=y
> > > > > > Kconfig option to affect only the default, that is, when no rcu_nocbs
> > > > > > kernel boot parameter is specified.  This would change the second "[4]"
> > > > > > in my original table to "[2]".
> > > > > >
> > > > > > Thoughts?
> > > > >
> > > > > I thought about that. I feel that since we are defaulting the new
> > > > > config option to =n , it is a conscious choice by the distro to set it
> > > > > to =y.  In such a case, they should be Ok with offloading all CPUs. If
> > > > > they decide to selectively offload some CPUs in the future, then they
> > > > > could revisit the config option at that time.
> > > > >
> > > > > I feel the kernel config should override the boot parameter behavior.
> > > > > It is the same effect as a sysadmin passing kernel parameter X
> > > > > assuming the kernel does something but the CONFIG option might not
> > > > > even build code corresponding to X.
> > > > >
> > > > > I feel to address your concern, we can document in kernel command line
> > > > > documentation that rcu_nocbs= does not have an effect if
> > > > > CONFIG_RCU_NOCB_CPU_ALL=y, would that work for you?
> > > >
> > > > Not me so much, because I would just set CONFIG_RCU_NOCB_CPU_ALL=n so
> > > > as to not worry about it.
> > > >
> > > > But I am not at all looking forward to complaints about rcu_nocbs not
> > > > working the way people expect.  So let's take some time to think more
> > > > carefully about this.
> > >
> > > That's a fair concern. But we are defaulting it to 'n' so I think if it is
> > > unconsciously enabled without someone reading documentation, then that's a
> > > slightly different issue.
> >
> > Suppose that one group decides to change to CONFIG_RCU_NOCB_CPU_ALL=y,
> > and some other group on some other continent happens to be using the
> > rcu_nocbs boot parameter (having read the documentation).  And suppose
> > that the level of communication between the two groups is typical,
> > that is to say, nonexistent.
> >
> > Sure, we can argue that groups should communicate, but our making that
> > argument won't necessarily prevent the group using rcu_nocbs from taking
> > us to task in the course of their debugging effort.
> >
> > > On the other hand, I can also make it such that if rcu_nocbs= is passed, then
> > > the CONFIG does not take effect. That's quite a bit weird/quirky IMHO.
> >
> > Not at all.  We can simply say that CONFIG_RCU_NOCB_CPU_ALL controls
> > only the default situation, that is, when rcu_nocbs is not specified.
> 
> Then it should be called: CONFIG_RCU_NOCB_CPU_DEFAULT_ALL , or
> something. Otherwise I can tell you that I will be the first one to be
> confused by menuconfig unless I also read the code :-D

I am OK with CONFIG_RCU_NOCB_CPU_DEFAULT_ALL.

							Thanx, Paul

Re: [PATCH RFC] rcu/nocb: Provide default all-CPUs mask for RCU_NOCB_CPU=y

[Steven Rostedt]

On Thu, 14 Apr 2022 17:38:08 -0400
Joel Fernandes <joel@joelfernandes.org> wrote:

> Then it should be called: CONFIG_RCU_NOCB_CPU_DEFAULT_ALL , or
> something. Otherwise I can tell you that I will be the first one to be
> confused by menuconfig unless I also read the code :-D

Naw, I think calling it CONFIG_RCU_NOCB_CPU_ALL is fine, it's what you
write in the help message of the Kconfig file that really matters.

I would always say that what is on the command line should override the
configs (when feasible). To add something to the command line takes effort,
and is always visible (cat /proc/cmdline), whereas configs are hidden and
not as visible to those using the kernel. Not having the command line work
would be annoying, and even if it came up with a message of "The config
says *all* so I'm ignoring what you want to do" would be exceptionally more
annoying as now the user has to recompile the kernel (and only if they
could).

-- Steve