[4/7] rcu/nocb: Add an option to offload all CPUs on boot

Commit Message

Paul E. McKenney June 20, 2022, 10:45 p.m. UTC

From: Joel Fernandes <[e-mail]>

Systems built with CONFIG_RCU_NOCB_CPU=y but booted without either
the rcu_nocbs= or rcu_nohz_full= kernel-boot parameters will not have
callback offloading on any of the CPUs, nor can any of the CPUs be
switched to enable callback offloading at runtime.  Although this is
intentional, it would be nice to have a way to offload all the CPUs
without having to make random bootloaders specify either the rcu_nocbs=
or the rcu_nohz_full= kernel-boot parameters.

This commit therefore provides a new CONFIG_RCU_NOCB_CPU_DEFAULT_ALL
Kconfig option that switches the default so as to offload callback
processing on all of the CPUs.  This default can still be overridden
using the rcu_nocbs= and rcu_nohz_full= kernel-boot parameters.

Reviewed-by: Kalesh Singh <[e-mail]>
Reviewed-by: Uladzislau Rezki <[e-mail]>
(In v4.1, fixed issues with CONFIG maze reported by kernel test robot).
Reported-by: kernel test robot <[e-mail]>
Signed-off-by: Joel Fernandes <[e-mail]>
Signed-off-by: Paul E. McKenney <[e-mail]>
---
 Documentation/admin-guide/kernel-parameters.txt |  6 ++++++
 kernel/rcu/Kconfig                              | 13 +++++++++++++
 kernel/rcu/tree_nocb.h                          | 15 ++++++++++++++-
 3 files changed, 33 insertions(+), 1 deletion(-)

Comments

Neeraj Upadhyay July 19, 2022, 9:34 a.m. UTC | #1

On 6/21/2022 4:15 AM, Paul E. McKenney wrote:
> From: Joel Fernandes <[e-mail]>
> 
> Systems built with CONFIG_RCU_NOCB_CPU=y but booted without either
> the rcu_nocbs= or rcu_nohz_full= kernel-boot parameters will not have
> callback offloading on any of the CPUs, nor can any of the CPUs be
> switched to enable callback offloading at runtime.  Although this is
> intentional, it would be nice to have a way to offload all the CPUs
> without having to make random bootloaders specify either the rcu_nocbs=
> or the rcu_nohz_full= kernel-boot parameters.
> 
> This commit therefore provides a new CONFIG_RCU_NOCB_CPU_DEFAULT_ALL
> Kconfig option that switches the default so as to offload callback
> processing on all of the CPUs.  This default can still be overridden
> using the rcu_nocbs= and rcu_nohz_full= kernel-boot parameters.
> 
> Reviewed-by: Kalesh Singh <[e-mail]>
> Reviewed-by: Uladzislau Rezki <[e-mail]>
> (In v4.1, fixed issues with CONFIG maze reported by kernel test robot).
> Reported-by: kernel test robot <[e-mail]>
> Signed-off-by: Joel Fernandes <[e-mail]>
> Signed-off-by: Paul E. McKenney <[e-mail]>
> ---


Reviewed-by: Neeraj Upadhyay <[e-mail]>

One query on cpumask_setall() below

>   Documentation/admin-guide/kernel-parameters.txt |  6 ++++++
>   kernel/rcu/Kconfig                              | 13 +++++++++++++
>   kernel/rcu/tree_nocb.h                          | 15 ++++++++++++++-
>   3 files changed, 33 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 2522b11e593f2..34605c275294c 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -3659,6 +3659,9 @@
>   			just as if they had also been called out in the
>   			rcu_nocbs= boot parameter.
>   
> +			Note that this argument takes precedence over
> +			the CONFIG_RCU_NOCB_CPU_DEFAULT_ALL option.
> +
>   	noiotrap	[SH] Disables trapped I/O port accesses.
>   
>   	noirqdebug	[X86-32] Disables the code which attempts to detect and
> @@ -4557,6 +4560,9 @@
>   			no-callback mode from boot but the mode may be
>   			toggled at runtime via cpusets.
>   
> +			Note that this argument takes precedence over
> +			the CONFIG_RCU_NOCB_CPU_DEFAULT_ALL option.
> +
>   	rcu_nocb_poll	[KNL]
>   			Rather than requiring that offloaded CPUs
>   			(specified by rcu_nocbs= above) explicitly
> diff --git a/kernel/rcu/Kconfig b/kernel/rcu/Kconfig
> index 1c630e573548d..27aab870ae4cf 100644
> --- a/kernel/rcu/Kconfig
> +++ b/kernel/rcu/Kconfig
> @@ -262,6 +262,19 @@ config RCU_NOCB_CPU
>   	  Say Y here if you need reduced OS jitter, despite added overhead.
>   	  Say N here if you are unsure.
>   
> +config RCU_NOCB_CPU_DEFAULT_ALL
> +	bool "Offload RCU callback processing from all CPUs by default"
> +	depends on RCU_NOCB_CPU
> +	default n
> +	help
> +	  Use this option to offload callback processing from all CPUs
> +	  by default, in the absence of the rcu_nocbs or nohz_full boot
> +	  parameter. This also avoids the need to use any boot parameters
> +	  to achieve the effect of offloading all CPUs on boot.
> +
> +	  Say Y here if you want offload all CPUs by default on boot.
> +	  Say N here if you are unsure.
> +
>   config TASKS_TRACE_RCU_READ_MB
>   	bool "Tasks Trace RCU readers use memory barriers in user and idle"
>   	depends on RCU_EXPERT && TASKS_TRACE_RCU
> diff --git a/kernel/rcu/tree_nocb.h b/kernel/rcu/tree_nocb.h
> index 4cf9a29bba79d..60cc92cc66552 100644
> --- a/kernel/rcu/tree_nocb.h
> +++ b/kernel/rcu/tree_nocb.h
> @@ -1197,11 +1197,21 @@ void __init rcu_init_nohz(void)
>   {
>   	int cpu;
>   	bool need_rcu_nocb_mask = false;
> +	bool offload_all = false;
>   	struct rcu_data *rdp;
>   
> +#if defined(CONFIG_RCU_NOCB_CPU_DEFAULT_ALL)
> +	if (!rcu_state.nocb_is_setup) {
> +		need_rcu_nocb_mask = true;
> +		offload_all = true;
> +	}
> +#endif /* #if defined(CONFIG_RCU_NOCB_CPU_DEFAULT_ALL) */
> +
>   #if defined(CONFIG_NO_HZ_FULL)
> -	if (tick_nohz_full_running && !cpumask_empty(tick_nohz_full_mask))
> +	if (tick_nohz_full_running && !cpumask_empty(tick_nohz_full_mask)) {
>   		need_rcu_nocb_mask = true;
> +		offload_all = false; /* NO_HZ_FULL has its own mask. */
> +	}
>   #endif /* #if defined(CONFIG_NO_HZ_FULL) */
>   
>   	if (need_rcu_nocb_mask) {
> @@ -1222,6 +1232,9 @@ void __init rcu_init_nohz(void)
>   		cpumask_or(rcu_nocb_mask, rcu_nocb_mask, tick_nohz_full_mask);
>   #endif /* #if defined(CONFIG_NO_HZ_FULL) */
>   
> +	if (offload_all)
> +		cpumask_setall(rcu_nocb_mask);

Do we need to do a cpumask_and(rcu_nocb_mask, cpu_possible_mask, 
rcu_nocb_mask) after setting all cpus in rcu_nocb_mask (cpumask_subset() 
check below takes care of it though)?


Thanks
Neeraj

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

Paul E. McKenney July 19, 2022, 6:12 p.m. UTC | #2

On Tue, Jul 19, 2022 at 03:04:07PM +0530, Neeraj Upadhyay wrote:
> 
> 
> On 6/21/2022 4:15 AM, Paul E. McKenney wrote:
> > From: Joel Fernandes <[e-mail]>
> > 
> > Systems built with CONFIG_RCU_NOCB_CPU=y but booted without either
> > the rcu_nocbs= or rcu_nohz_full= kernel-boot parameters will not have
> > callback offloading on any of the CPUs, nor can any of the CPUs be
> > switched to enable callback offloading at runtime.  Although this is
> > intentional, it would be nice to have a way to offload all the CPUs
> > without having to make random bootloaders specify either the rcu_nocbs=
> > or the rcu_nohz_full= kernel-boot parameters.
> > 
> > This commit therefore provides a new CONFIG_RCU_NOCB_CPU_DEFAULT_ALL
> > Kconfig option that switches the default so as to offload callback
> > processing on all of the CPUs.  This default can still be overridden
> > using the rcu_nocbs= and rcu_nohz_full= kernel-boot parameters.
> > 
> > Reviewed-by: Kalesh Singh <[e-mail]>
> > Reviewed-by: Uladzislau Rezki <[e-mail]>
> > (In v4.1, fixed issues with CONFIG maze reported by kernel test robot).
> > Reported-by: kernel test robot <[e-mail]>
> > Signed-off-by: Joel Fernandes <[e-mail]>
> > Signed-off-by: Paul E. McKenney <[e-mail]>
> > ---
> 
> 
> Reviewed-by: Neeraj Upadhyay <[e-mail]>
> 
> One query on cpumask_setall() below
> 
> >   Documentation/admin-guide/kernel-parameters.txt |  6 ++++++
> >   kernel/rcu/Kconfig                              | 13 +++++++++++++
> >   kernel/rcu/tree_nocb.h                          | 15 ++++++++++++++-
> >   3 files changed, 33 insertions(+), 1 deletion(-)
> > 
> > diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> > index 2522b11e593f2..34605c275294c 100644
> > --- a/Documentation/admin-guide/kernel-parameters.txt
> > +++ b/Documentation/admin-guide/kernel-parameters.txt
> > @@ -3659,6 +3659,9 @@
> >   			just as if they had also been called out in the
> >   			rcu_nocbs= boot parameter.
> > +			Note that this argument takes precedence over
> > +			the CONFIG_RCU_NOCB_CPU_DEFAULT_ALL option.
> > +
> >   	noiotrap	[SH] Disables trapped I/O port accesses.
> >   	noirqdebug	[X86-32] Disables the code which attempts to detect and
> > @@ -4557,6 +4560,9 @@
> >   			no-callback mode from boot but the mode may be
> >   			toggled at runtime via cpusets.
> > +			Note that this argument takes precedence over
> > +			the CONFIG_RCU_NOCB_CPU_DEFAULT_ALL option.
> > +
> >   	rcu_nocb_poll	[KNL]
> >   			Rather than requiring that offloaded CPUs
> >   			(specified by rcu_nocbs= above) explicitly
> > diff --git a/kernel/rcu/Kconfig b/kernel/rcu/Kconfig
> > index 1c630e573548d..27aab870ae4cf 100644
> > --- a/kernel/rcu/Kconfig
> > +++ b/kernel/rcu/Kconfig
> > @@ -262,6 +262,19 @@ config RCU_NOCB_CPU
> >   	  Say Y here if you need reduced OS jitter, despite added overhead.
> >   	  Say N here if you are unsure.
> > +config RCU_NOCB_CPU_DEFAULT_ALL
> > +	bool "Offload RCU callback processing from all CPUs by default"
> > +	depends on RCU_NOCB_CPU
> > +	default n
> > +	help
> > +	  Use this option to offload callback processing from all CPUs
> > +	  by default, in the absence of the rcu_nocbs or nohz_full boot
> > +	  parameter. This also avoids the need to use any boot parameters
> > +	  to achieve the effect of offloading all CPUs on boot.
> > +
> > +	  Say Y here if you want offload all CPUs by default on boot.
> > +	  Say N here if you are unsure.
> > +
> >   config TASKS_TRACE_RCU_READ_MB
> >   	bool "Tasks Trace RCU readers use memory barriers in user and idle"
> >   	depends on RCU_EXPERT && TASKS_TRACE_RCU
> > diff --git a/kernel/rcu/tree_nocb.h b/kernel/rcu/tree_nocb.h
> > index 4cf9a29bba79d..60cc92cc66552 100644
> > --- a/kernel/rcu/tree_nocb.h
> > +++ b/kernel/rcu/tree_nocb.h
> > @@ -1197,11 +1197,21 @@ void __init rcu_init_nohz(void)
> >   {
> >   	int cpu;
> >   	bool need_rcu_nocb_mask = false;
> > +	bool offload_all = false;
> >   	struct rcu_data *rdp;
> > +#if defined(CONFIG_RCU_NOCB_CPU_DEFAULT_ALL)
> > +	if (!rcu_state.nocb_is_setup) {
> > +		need_rcu_nocb_mask = true;
> > +		offload_all = true;
> > +	}
> > +#endif /* #if defined(CONFIG_RCU_NOCB_CPU_DEFAULT_ALL) */
> > +
> >   #if defined(CONFIG_NO_HZ_FULL)
> > -	if (tick_nohz_full_running && !cpumask_empty(tick_nohz_full_mask))
> > +	if (tick_nohz_full_running && !cpumask_empty(tick_nohz_full_mask)) {
> >   		need_rcu_nocb_mask = true;
> > +		offload_all = false; /* NO_HZ_FULL has its own mask. */
> > +	}
> >   #endif /* #if defined(CONFIG_NO_HZ_FULL) */
> >   	if (need_rcu_nocb_mask) {
> > @@ -1222,6 +1232,9 @@ void __init rcu_init_nohz(void)
> >   		cpumask_or(rcu_nocb_mask, rcu_nocb_mask, tick_nohz_full_mask);
> >   #endif /* #if defined(CONFIG_NO_HZ_FULL) */
> > +	if (offload_all)
> > +		cpumask_setall(rcu_nocb_mask);
> 
> Do we need to do a cpumask_and(rcu_nocb_mask, cpu_possible_mask,
> rcu_nocb_mask) after setting all cpus in rcu_nocb_mask (cpumask_subset()
> check below takes care of it though)?

Without that cpumask_and(), systems with sparse CPU numbering schemes
(for example, 0, 4, 8, 12, ...) will get a pr_info(), and as you noted,
the needed cpumask_and().

I am inclined to see a complaint before we change this.  And perhaps if
this is to change, the change should be in cpumask_setall() rather than
in rcu_init_nohz().  But that is an argument for later, if at all.  ;-)

And thank you for reviewing this series!

							Thanx, Paul

> Thanks
> Neeraj
> 
> > +
> >   	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,

Joel Fernandes July 19, 2022, 10:42 p.m. UTC | #3

On 7/19/2022 2:12 PM, Paul E. McKenney wrote:
> On Tue, Jul 19, 2022 at 03:04:07PM +0530, Neeraj Upadhyay wrote:
>>
>>
>> On 6/21/2022 4:15 AM, Paul E. McKenney wrote:
>>> From: Joel Fernandes <[e-mail]>
>>>
>>> Systems built with CONFIG_RCU_NOCB_CPU=y but booted without either
>>> the rcu_nocbs= or rcu_nohz_full= kernel-boot parameters will not have
>>> callback offloading on any of the CPUs, nor can any of the CPUs be
>>> switched to enable callback offloading at runtime.  Although this is
>>> intentional, it would be nice to have a way to offload all the CPUs
>>> without having to make random bootloaders specify either the rcu_nocbs=
>>> or the rcu_nohz_full= kernel-boot parameters.
>>>
>>> This commit therefore provides a new CONFIG_RCU_NOCB_CPU_DEFAULT_ALL
>>> Kconfig option that switches the default so as to offload callback
>>> processing on all of the CPUs.  This default can still be overridden
>>> using the rcu_nocbs= and rcu_nohz_full= kernel-boot parameters.
>>>
>>> Reviewed-by: Kalesh Singh <[e-mail]>
>>> Reviewed-by: Uladzislau Rezki <[e-mail]>
>>> (In v4.1, fixed issues with CONFIG maze reported by kernel test robot).
>>> Reported-by: kernel test robot <[e-mail]>
>>> Signed-off-by: Joel Fernandes <[e-mail]>
>>> Signed-off-by: Paul E. McKenney <[e-mail]>
>>> ---
>>
>>
>> Reviewed-by: Neeraj Upadhyay <[e-mail]>
>>
>> One query on cpumask_setall() below
>>
>>>   Documentation/admin-guide/kernel-parameters.txt |  6 ++++++
>>>   kernel/rcu/Kconfig                              | 13 +++++++++++++
>>>   kernel/rcu/tree_nocb.h                          | 15 ++++++++++++++-
>>>   3 files changed, 33 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
>>> index 2522b11e593f2..34605c275294c 100644
>>> --- a/Documentation/admin-guide/kernel-parameters.txt
>>> +++ b/Documentation/admin-guide/kernel-parameters.txt
>>> @@ -3659,6 +3659,9 @@
>>>   			just as if they had also been called out in the
>>>   			rcu_nocbs= boot parameter.
>>> +			Note that this argument takes precedence over
>>> +			the CONFIG_RCU_NOCB_CPU_DEFAULT_ALL option.
>>> +
>>>   	noiotrap	[SH] Disables trapped I/O port accesses.
>>>   	noirqdebug	[X86-32] Disables the code which attempts to detect and
>>> @@ -4557,6 +4560,9 @@
>>>   			no-callback mode from boot but the mode may be
>>>   			toggled at runtime via cpusets.
>>> +			Note that this argument takes precedence over
>>> +			the CONFIG_RCU_NOCB_CPU_DEFAULT_ALL option.
>>> +
>>>   	rcu_nocb_poll	[KNL]
>>>   			Rather than requiring that offloaded CPUs
>>>   			(specified by rcu_nocbs= above) explicitly
>>> diff --git a/kernel/rcu/Kconfig b/kernel/rcu/Kconfig
>>> index 1c630e573548d..27aab870ae4cf 100644
>>> --- a/kernel/rcu/Kconfig
>>> +++ b/kernel/rcu/Kconfig
>>> @@ -262,6 +262,19 @@ config RCU_NOCB_CPU
>>>   	  Say Y here if you need reduced OS jitter, despite added overhead.
>>>   	  Say N here if you are unsure.
>>> +config RCU_NOCB_CPU_DEFAULT_ALL
>>> +	bool "Offload RCU callback processing from all CPUs by default"
>>> +	depends on RCU_NOCB_CPU
>>> +	default n
>>> +	help
>>> +	  Use this option to offload callback processing from all CPUs
>>> +	  by default, in the absence of the rcu_nocbs or nohz_full boot
>>> +	  parameter. This also avoids the need to use any boot parameters
>>> +	  to achieve the effect of offloading all CPUs on boot.
>>> +
>>> +	  Say Y here if you want offload all CPUs by default on boot.
>>> +	  Say N here if you are unsure.
>>> +
>>>   config TASKS_TRACE_RCU_READ_MB
>>>   	bool "Tasks Trace RCU readers use memory barriers in user and idle"
>>>   	depends on RCU_EXPERT && TASKS_TRACE_RCU
>>> diff --git a/kernel/rcu/tree_nocb.h b/kernel/rcu/tree_nocb.h
>>> index 4cf9a29bba79d..60cc92cc66552 100644
>>> --- a/kernel/rcu/tree_nocb.h
>>> +++ b/kernel/rcu/tree_nocb.h
>>> @@ -1197,11 +1197,21 @@ void __init rcu_init_nohz(void)
>>>   {
>>>   	int cpu;
>>>   	bool need_rcu_nocb_mask = false;
>>> +	bool offload_all = false;
>>>   	struct rcu_data *rdp;
>>> +#if defined(CONFIG_RCU_NOCB_CPU_DEFAULT_ALL)
>>> +	if (!rcu_state.nocb_is_setup) {
>>> +		need_rcu_nocb_mask = true;
>>> +		offload_all = true;
>>> +	}
>>> +#endif /* #if defined(CONFIG_RCU_NOCB_CPU_DEFAULT_ALL) */
>>> +
>>>   #if defined(CONFIG_NO_HZ_FULL)
>>> -	if (tick_nohz_full_running && !cpumask_empty(tick_nohz_full_mask))
>>> +	if (tick_nohz_full_running && !cpumask_empty(tick_nohz_full_mask)) {
>>>   		need_rcu_nocb_mask = true;
>>> +		offload_all = false; /* NO_HZ_FULL has its own mask. */
>>> +	}
>>>   #endif /* #if defined(CONFIG_NO_HZ_FULL) */
>>>   	if (need_rcu_nocb_mask) {
>>> @@ -1222,6 +1232,9 @@ void __init rcu_init_nohz(void)
>>>   		cpumask_or(rcu_nocb_mask, rcu_nocb_mask, tick_nohz_full_mask);
>>>   #endif /* #if defined(CONFIG_NO_HZ_FULL) */
>>> +	if (offload_all)
>>> +		cpumask_setall(rcu_nocb_mask);
>>
>> Do we need to do a cpumask_and(rcu_nocb_mask, cpu_possible_mask,
>> rcu_nocb_mask) after setting all cpus in rcu_nocb_mask (cpumask_subset()
>> check below takes care of it though)?
> 
> Without that cpumask_and(), systems with sparse CPU numbering schemes
> (for example, 0, 4, 8, 12, ...) will get a pr_info(), and as you noted,
> the needed cpumask_and().
> 
> I am inclined to see a complaint before we change this.  And perhaps if
> this is to change, the change should be in cpumask_setall() rather than
> in rcu_init_nohz().  But that is an argument for later, if at all.  ;-)
> 
>>> +
>>>   	if (!cpumask_subset(rcu_nocb_mask, cpu_possible_mask)) {

We could also suppress the pr_info() by making it conditional.

like:

if (!CONFIG_RCU_NOCB_CPU_DEFAULT_ALL) {
	pr_info(...);
}

In other words, we could make the cpumask_and() as expected/normal on
systems with sparse CPU numbering schemes. Would that work?

Thanks,

 - Joel


>>>   		pr_info("\tNote: kernel parameter 'rcu_nocbs=', 'nohz_full', or 'isolcpus=' contains nonexistent CPUs.\n");



>>>   		cpumask_and(rcu_nocb_mask, cpu_possible_mask,

Paul E. McKenney July 19, 2022, 10:53 p.m. UTC | #4

On Tue, Jul 19, 2022 at 06:42:00PM -0400, Joel Fernandes wrote:
> 
> 
> On 7/19/2022 2:12 PM, Paul E. McKenney wrote:
> > On Tue, Jul 19, 2022 at 03:04:07PM +0530, Neeraj Upadhyay wrote:
> >>
> >>
> >> On 6/21/2022 4:15 AM, Paul E. McKenney wrote:
> >>> From: Joel Fernandes <[e-mail]>
> >>>
> >>> Systems built with CONFIG_RCU_NOCB_CPU=y but booted without either
> >>> the rcu_nocbs= or rcu_nohz_full= kernel-boot parameters will not have
> >>> callback offloading on any of the CPUs, nor can any of the CPUs be
> >>> switched to enable callback offloading at runtime.  Although this is
> >>> intentional, it would be nice to have a way to offload all the CPUs
> >>> without having to make random bootloaders specify either the rcu_nocbs=
> >>> or the rcu_nohz_full= kernel-boot parameters.
> >>>
> >>> This commit therefore provides a new CONFIG_RCU_NOCB_CPU_DEFAULT_ALL
> >>> Kconfig option that switches the default so as to offload callback
> >>> processing on all of the CPUs.  This default can still be overridden
> >>> using the rcu_nocbs= and rcu_nohz_full= kernel-boot parameters.
> >>>
> >>> Reviewed-by: Kalesh Singh <[e-mail]>
> >>> Reviewed-by: Uladzislau Rezki <[e-mail]>
> >>> (In v4.1, fixed issues with CONFIG maze reported by kernel test robot).
> >>> Reported-by: kernel test robot <[e-mail]>
> >>> Signed-off-by: Joel Fernandes <[e-mail]>
> >>> Signed-off-by: Paul E. McKenney <[e-mail]>
> >>> ---
> >>
> >>
> >> Reviewed-by: Neeraj Upadhyay <[e-mail]>
> >>
> >> One query on cpumask_setall() below
> >>
> >>>   Documentation/admin-guide/kernel-parameters.txt |  6 ++++++
> >>>   kernel/rcu/Kconfig                              | 13 +++++++++++++
> >>>   kernel/rcu/tree_nocb.h                          | 15 ++++++++++++++-
> >>>   3 files changed, 33 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> >>> index 2522b11e593f2..34605c275294c 100644
> >>> --- a/Documentation/admin-guide/kernel-parameters.txt
> >>> +++ b/Documentation/admin-guide/kernel-parameters.txt
> >>> @@ -3659,6 +3659,9 @@
> >>>   			just as if they had also been called out in the
> >>>   			rcu_nocbs= boot parameter.
> >>> +			Note that this argument takes precedence over
> >>> +			the CONFIG_RCU_NOCB_CPU_DEFAULT_ALL option.
> >>> +
> >>>   	noiotrap	[SH] Disables trapped I/O port accesses.
> >>>   	noirqdebug	[X86-32] Disables the code which attempts to detect and
> >>> @@ -4557,6 +4560,9 @@
> >>>   			no-callback mode from boot but the mode may be
> >>>   			toggled at runtime via cpusets.
> >>> +			Note that this argument takes precedence over
> >>> +			the CONFIG_RCU_NOCB_CPU_DEFAULT_ALL option.
> >>> +
> >>>   	rcu_nocb_poll	[KNL]
> >>>   			Rather than requiring that offloaded CPUs
> >>>   			(specified by rcu_nocbs= above) explicitly
> >>> diff --git a/kernel/rcu/Kconfig b/kernel/rcu/Kconfig
> >>> index 1c630e573548d..27aab870ae4cf 100644
> >>> --- a/kernel/rcu/Kconfig
> >>> +++ b/kernel/rcu/Kconfig
> >>> @@ -262,6 +262,19 @@ config RCU_NOCB_CPU
> >>>   	  Say Y here if you need reduced OS jitter, despite added overhead.
> >>>   	  Say N here if you are unsure.
> >>> +config RCU_NOCB_CPU_DEFAULT_ALL
> >>> +	bool "Offload RCU callback processing from all CPUs by default"
> >>> +	depends on RCU_NOCB_CPU
> >>> +	default n
> >>> +	help
> >>> +	  Use this option to offload callback processing from all CPUs
> >>> +	  by default, in the absence of the rcu_nocbs or nohz_full boot
> >>> +	  parameter. This also avoids the need to use any boot parameters
> >>> +	  to achieve the effect of offloading all CPUs on boot.
> >>> +
> >>> +	  Say Y here if you want offload all CPUs by default on boot.
> >>> +	  Say N here if you are unsure.
> >>> +
> >>>   config TASKS_TRACE_RCU_READ_MB
> >>>   	bool "Tasks Trace RCU readers use memory barriers in user and idle"
> >>>   	depends on RCU_EXPERT && TASKS_TRACE_RCU
> >>> diff --git a/kernel/rcu/tree_nocb.h b/kernel/rcu/tree_nocb.h
> >>> index 4cf9a29bba79d..60cc92cc66552 100644
> >>> --- a/kernel/rcu/tree_nocb.h
> >>> +++ b/kernel/rcu/tree_nocb.h
> >>> @@ -1197,11 +1197,21 @@ void __init rcu_init_nohz(void)
> >>>   {
> >>>   	int cpu;
> >>>   	bool need_rcu_nocb_mask = false;
> >>> +	bool offload_all = false;
> >>>   	struct rcu_data *rdp;
> >>> +#if defined(CONFIG_RCU_NOCB_CPU_DEFAULT_ALL)
> >>> +	if (!rcu_state.nocb_is_setup) {
> >>> +		need_rcu_nocb_mask = true;
> >>> +		offload_all = true;
> >>> +	}
> >>> +#endif /* #if defined(CONFIG_RCU_NOCB_CPU_DEFAULT_ALL) */
> >>> +
> >>>   #if defined(CONFIG_NO_HZ_FULL)
> >>> -	if (tick_nohz_full_running && !cpumask_empty(tick_nohz_full_mask))
> >>> +	if (tick_nohz_full_running && !cpumask_empty(tick_nohz_full_mask)) {
> >>>   		need_rcu_nocb_mask = true;
> >>> +		offload_all = false; /* NO_HZ_FULL has its own mask. */
> >>> +	}
> >>>   #endif /* #if defined(CONFIG_NO_HZ_FULL) */
> >>>   	if (need_rcu_nocb_mask) {
> >>> @@ -1222,6 +1232,9 @@ void __init rcu_init_nohz(void)
> >>>   		cpumask_or(rcu_nocb_mask, rcu_nocb_mask, tick_nohz_full_mask);
> >>>   #endif /* #if defined(CONFIG_NO_HZ_FULL) */
> >>> +	if (offload_all)
> >>> +		cpumask_setall(rcu_nocb_mask);
> >>
> >> Do we need to do a cpumask_and(rcu_nocb_mask, cpu_possible_mask,
> >> rcu_nocb_mask) after setting all cpus in rcu_nocb_mask (cpumask_subset()
> >> check below takes care of it though)?
> > 
> > Without that cpumask_and(), systems with sparse CPU numbering schemes
> > (for example, 0, 4, 8, 12, ...) will get a pr_info(), and as you noted,
> > the needed cpumask_and().
> > 
> > I am inclined to see a complaint before we change this.  And perhaps if
> > this is to change, the change should be in cpumask_setall() rather than
> > in rcu_init_nohz().  But that is an argument for later, if at all.  ;-)
> > 
> >>> +
> >>>   	if (!cpumask_subset(rcu_nocb_mask, cpu_possible_mask)) {
> 
> We could also suppress the pr_info() by making it conditional.
> 
> like:
> 
> if (!CONFIG_RCU_NOCB_CPU_DEFAULT_ALL) {
> 	pr_info(...);
> }
> 
> In other words, we could make the cpumask_and() as expected/normal on
> systems with sparse CPU numbering schemes. Would that work?

That would be a good within-RCU workaround if we get an urgent complaint,
but if this requires a change, shouldn't cpumask_setall() refrain from
setting bits for non-existent CPUs?  It does refrain from setting any
bits beyond the largest-numbered CPU.

But perhaps there is an early boot reason why cpumask_setall() cannot
do this?

Either way, we are just doing a pr_info(), not a WARN_ON() or similar,
so the current state is probably fine.

							Thanx, Paul

> Thanks,
> 
>  - Joel
> 
> 
> >>>   		pr_info("\tNote: kernel parameter 'rcu_nocbs=', 'nohz_full', or 'isolcpus=' contains nonexistent CPUs.\n");
> 
> 
> 
> >>>   		cpumask_and(rcu_nocb_mask, cpu_possible_mask,

Joel Fernandes July 19, 2022, 10:57 p.m. UTC | #5

On 7/19/2022 6:53 PM, Paul E. McKenney wrote:
> On Tue, Jul 19, 2022 at 06:42:00PM -0400, Joel Fernandes wrote:
>>
>>
>> On 7/19/2022 2:12 PM, Paul E. McKenney wrote:
>>> On Tue, Jul 19, 2022 at 03:04:07PM +0530, Neeraj Upadhyay wrote:
>>>>
>>>>
>>>> On 6/21/2022 4:15 AM, Paul E. McKenney wrote:
>>>>> From: Joel Fernandes <[e-mail]>
>>>>>
>>>>> Systems built with CONFIG_RCU_NOCB_CPU=y but booted without either
>>>>> the rcu_nocbs= or rcu_nohz_full= kernel-boot parameters will not have
>>>>> callback offloading on any of the CPUs, nor can any of the CPUs be
>>>>> switched to enable callback offloading at runtime.  Although this is
>>>>> intentional, it would be nice to have a way to offload all the CPUs
>>>>> without having to make random bootloaders specify either the rcu_nocbs=
>>>>> or the rcu_nohz_full= kernel-boot parameters.
>>>>>
>>>>> This commit therefore provides a new CONFIG_RCU_NOCB_CPU_DEFAULT_ALL
>>>>> Kconfig option that switches the default so as to offload callback
>>>>> processing on all of the CPUs.  This default can still be overridden
>>>>> using the rcu_nocbs= and rcu_nohz_full= kernel-boot parameters.
>>>>>
>>>>> Reviewed-by: Kalesh Singh <[e-mail]>
>>>>> Reviewed-by: Uladzislau Rezki <[e-mail]>
>>>>> (In v4.1, fixed issues with CONFIG maze reported by kernel test robot).
>>>>> Reported-by: kernel test robot <[e-mail]>
>>>>> Signed-off-by: Joel Fernandes <[e-mail]>
>>>>> Signed-off-by: Paul E. McKenney <[e-mail]>
>>>>> ---
>>>>
>>>>
>>>> Reviewed-by: Neeraj Upadhyay <[e-mail]>
>>>>
>>>> One query on cpumask_setall() below
>>>>
>>>>>   Documentation/admin-guide/kernel-parameters.txt |  6 ++++++
>>>>>   kernel/rcu/Kconfig                              | 13 +++++++++++++
>>>>>   kernel/rcu/tree_nocb.h                          | 15 ++++++++++++++-
>>>>>   3 files changed, 33 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
>>>>> index 2522b11e593f2..34605c275294c 100644
>>>>> --- a/Documentation/admin-guide/kernel-parameters.txt
>>>>> +++ b/Documentation/admin-guide/kernel-parameters.txt
>>>>> @@ -3659,6 +3659,9 @@
>>>>>   			just as if they had also been called out in the
>>>>>   			rcu_nocbs= boot parameter.
>>>>> +			Note that this argument takes precedence over
>>>>> +			the CONFIG_RCU_NOCB_CPU_DEFAULT_ALL option.
>>>>> +
>>>>>   	noiotrap	[SH] Disables trapped I/O port accesses.
>>>>>   	noirqdebug	[X86-32] Disables the code which attempts to detect and
>>>>> @@ -4557,6 +4560,9 @@
>>>>>   			no-callback mode from boot but the mode may be
>>>>>   			toggled at runtime via cpusets.
>>>>> +			Note that this argument takes precedence over
>>>>> +			the CONFIG_RCU_NOCB_CPU_DEFAULT_ALL option.
>>>>> +
>>>>>   	rcu_nocb_poll	[KNL]
>>>>>   			Rather than requiring that offloaded CPUs
>>>>>   			(specified by rcu_nocbs= above) explicitly
>>>>> diff --git a/kernel/rcu/Kconfig b/kernel/rcu/Kconfig
>>>>> index 1c630e573548d..27aab870ae4cf 100644
>>>>> --- a/kernel/rcu/Kconfig
>>>>> +++ b/kernel/rcu/Kconfig
>>>>> @@ -262,6 +262,19 @@ config RCU_NOCB_CPU
>>>>>   	  Say Y here if you need reduced OS jitter, despite added overhead.
>>>>>   	  Say N here if you are unsure.
>>>>> +config RCU_NOCB_CPU_DEFAULT_ALL
>>>>> +	bool "Offload RCU callback processing from all CPUs by default"
>>>>> +	depends on RCU_NOCB_CPU
>>>>> +	default n
>>>>> +	help
>>>>> +	  Use this option to offload callback processing from all CPUs
>>>>> +	  by default, in the absence of the rcu_nocbs or nohz_full boot
>>>>> +	  parameter. This also avoids the need to use any boot parameters
>>>>> +	  to achieve the effect of offloading all CPUs on boot.
>>>>> +
>>>>> +	  Say Y here if you want offload all CPUs by default on boot.
>>>>> +	  Say N here if you are unsure.
>>>>> +
>>>>>   config TASKS_TRACE_RCU_READ_MB
>>>>>   	bool "Tasks Trace RCU readers use memory barriers in user and idle"
>>>>>   	depends on RCU_EXPERT && TASKS_TRACE_RCU
>>>>> diff --git a/kernel/rcu/tree_nocb.h b/kernel/rcu/tree_nocb.h
>>>>> index 4cf9a29bba79d..60cc92cc66552 100644
>>>>> --- a/kernel/rcu/tree_nocb.h
>>>>> +++ b/kernel/rcu/tree_nocb.h
>>>>> @@ -1197,11 +1197,21 @@ void __init rcu_init_nohz(void)
>>>>>   {
>>>>>   	int cpu;
>>>>>   	bool need_rcu_nocb_mask = false;
>>>>> +	bool offload_all = false;
>>>>>   	struct rcu_data *rdp;
>>>>> +#if defined(CONFIG_RCU_NOCB_CPU_DEFAULT_ALL)
>>>>> +	if (!rcu_state.nocb_is_setup) {
>>>>> +		need_rcu_nocb_mask = true;
>>>>> +		offload_all = true;
>>>>> +	}
>>>>> +#endif /* #if defined(CONFIG_RCU_NOCB_CPU_DEFAULT_ALL) */
>>>>> +
>>>>>   #if defined(CONFIG_NO_HZ_FULL)
>>>>> -	if (tick_nohz_full_running && !cpumask_empty(tick_nohz_full_mask))
>>>>> +	if (tick_nohz_full_running && !cpumask_empty(tick_nohz_full_mask)) {
>>>>>   		need_rcu_nocb_mask = true;
>>>>> +		offload_all = false; /* NO_HZ_FULL has its own mask. */
>>>>> +	}
>>>>>   #endif /* #if defined(CONFIG_NO_HZ_FULL) */
>>>>>   	if (need_rcu_nocb_mask) {
>>>>> @@ -1222,6 +1232,9 @@ void __init rcu_init_nohz(void)
>>>>>   		cpumask_or(rcu_nocb_mask, rcu_nocb_mask, tick_nohz_full_mask);
>>>>>   #endif /* #if defined(CONFIG_NO_HZ_FULL) */
>>>>> +	if (offload_all)
>>>>> +		cpumask_setall(rcu_nocb_mask);
>>>>
>>>> Do we need to do a cpumask_and(rcu_nocb_mask, cpu_possible_mask,
>>>> rcu_nocb_mask) after setting all cpus in rcu_nocb_mask (cpumask_subset()
>>>> check below takes care of it though)?
>>>
>>> Without that cpumask_and(), systems with sparse CPU numbering schemes
>>> (for example, 0, 4, 8, 12, ...) will get a pr_info(), and as you noted,
>>> the needed cpumask_and().
>>>
>>> I am inclined to see a complaint before we change this.  And perhaps if
>>> this is to change, the change should be in cpumask_setall() rather than
>>> in rcu_init_nohz().  But that is an argument for later, if at all.  ;-)
>>>
>>>>> +
>>>>>   	if (!cpumask_subset(rcu_nocb_mask, cpu_possible_mask)) {
>>
>> We could also suppress the pr_info() by making it conditional.
>>
>> like:
>>
>> if (!CONFIG_RCU_NOCB_CPU_DEFAULT_ALL) {
>> 	pr_info(...);
>> }
>>
>> In other words, we could make the cpumask_and() as expected/normal on
>> systems with sparse CPU numbering schemes. Would that work?
> 
> That would be a good within-RCU workaround if we get an urgent complaint,
> but if this requires a change, shouldn't cpumask_setall() refrain from
> setting bits for non-existent CPUs?  It does refrain from setting any
> bits beyond the largest-numbered CPU.
> 
> But perhaps there is an early boot reason why cpumask_setall() cannot
> do this?

Agreed, it would be great if it did not set those bits. I checked other
places in the kernel like kernel/sched/core.c and cannot find that it is
masking the bits after the setall(), so maybe its Ok?

> Either way, we are just doing a pr_info(), not a WARN_ON() or similar,
> so the current state is probably fine.

Agreed, thanks.

 - Joel

> 
> 							Thanx, Paul
> 
>> Thanks,
>>
>>  - Joel
>>
>>
>>>>>   		pr_info("\tNote: kernel parameter 'rcu_nocbs=', 'nohz_full', or 'isolcpus=' contains nonexistent CPUs.\n");
>>
>>
>>
>>>>>   		cpumask_and(rcu_nocb_mask, cpu_possible_mask,

Patch

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 2522b11e593f2..34605c275294c 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -3659,6 +3659,9 @@ 
 			just as if they had also been called out in the
 			rcu_nocbs= boot parameter.
 
+			Note that this argument takes precedence over
+			the CONFIG_RCU_NOCB_CPU_DEFAULT_ALL option.
+
 	noiotrap	[SH] Disables trapped I/O port accesses.
 
 	noirqdebug	[X86-32] Disables the code which attempts to detect and
@@ -4557,6 +4560,9 @@ 
 			no-callback mode from boot but the mode may be
 			toggled at runtime via cpusets.
 
+			Note that this argument takes precedence over
+			the CONFIG_RCU_NOCB_CPU_DEFAULT_ALL option.
+
 	rcu_nocb_poll	[KNL]
 			Rather than requiring that offloaded CPUs
 			(specified by rcu_nocbs= above) explicitly
diff --git a/kernel/rcu/Kconfig b/kernel/rcu/Kconfig
index 1c630e573548d..27aab870ae4cf 100644
--- a/kernel/rcu/Kconfig
+++ b/kernel/rcu/Kconfig
@@ -262,6 +262,19 @@  config RCU_NOCB_CPU
 	  Say Y here if you need reduced OS jitter, despite added overhead.
 	  Say N here if you are unsure.
 
+config RCU_NOCB_CPU_DEFAULT_ALL
+	bool "Offload RCU callback processing from all CPUs by default"
+	depends on RCU_NOCB_CPU
+	default n
+	help
+	  Use this option to offload callback processing from all CPUs
+	  by default, in the absence of the rcu_nocbs or nohz_full boot
+	  parameter. This also avoids the need to use any boot parameters
+	  to achieve the effect of offloading all CPUs on boot.
+
+	  Say Y here if you want offload all CPUs by default on boot.
+	  Say N here if you are unsure.
+
 config TASKS_TRACE_RCU_READ_MB
 	bool "Tasks Trace RCU readers use memory barriers in user and idle"
 	depends on RCU_EXPERT && TASKS_TRACE_RCU
diff --git a/kernel/rcu/tree_nocb.h b/kernel/rcu/tree_nocb.h
index 4cf9a29bba79d..60cc92cc66552 100644
--- a/kernel/rcu/tree_nocb.h
+++ b/kernel/rcu/tree_nocb.h
@@ -1197,11 +1197,21 @@  void __init rcu_init_nohz(void)
 {
 	int cpu;
 	bool need_rcu_nocb_mask = false;
+	bool offload_all = false;
 	struct rcu_data *rdp;
 
+#if defined(CONFIG_RCU_NOCB_CPU_DEFAULT_ALL)
+	if (!rcu_state.nocb_is_setup) {
+		need_rcu_nocb_mask = true;
+		offload_all = true;
+	}
+#endif /* #if defined(CONFIG_RCU_NOCB_CPU_DEFAULT_ALL) */
+
 #if defined(CONFIG_NO_HZ_FULL)
-	if (tick_nohz_full_running && !cpumask_empty(tick_nohz_full_mask))
+	if (tick_nohz_full_running && !cpumask_empty(tick_nohz_full_mask)) {
 		need_rcu_nocb_mask = true;
+		offload_all = false; /* NO_HZ_FULL has its own mask. */
+	}
 #endif /* #if defined(CONFIG_NO_HZ_FULL) */
 
 	if (need_rcu_nocb_mask) {
@@ -1222,6 +1232,9 @@  void __init rcu_init_nohz(void)
 		cpumask_or(rcu_nocb_mask, rcu_nocb_mask, tick_nohz_full_mask);
 #endif /* #if defined(CONFIG_NO_HZ_FULL) */
 
+	if (offload_all)
+		cpumask_setall(rcu_nocb_mask);
+
 	if (!cpumask_subset(rcu_nocb_mask, cpu_possible_mask)) {
 		pr_info("\tNote: kernel parameter 'rcu_nocbs=', 'nohz_full', or 'isolcpus=' contains nonexistent CPUs.\n");
 		cpumask_and(rcu_nocb_mask, cpu_possible_mask,