diff mbox series

[v6,1/2] rcu: Simplify rcu_init_nohz() cpumask handling

Message ID 20220901131436.986-2-thunder.leizhen@huawei.com (mailing list archive)
State Accepted
Commit 98ec9849c71cdca6caa293a633b262a3d91babc5
Headers show
Series rcu/nocb: Delete local variable 'need_rcu_nocb_mask' in rcu_init_nohz() | expand

Commit Message

Zhen Lei Sept. 1, 2022, 1:14 p.m. UTC
In kernels built with either CONFIG_RCU_NOCB_CPU_DEFAULT_ALL=y or
CONFIG_NO_HZ_FULL=y, additional CPUs must be added to rcu_nocb_mask.
Except that kernels booted without the rcu_nocbs= will not have
allocated rcu_nocb_mask.  And the current rcu_init_nohz() function uses
its need_rcu_nocb_mask and offload_all local variables to track the
rcu_nocb and nohz_full state.

But there is a much simpler approach, namely creating a cpumask pointer
to track the default and then using cpumask_available() to check the
rcu_nocb_mask state.  This commit takes this approach, thereby simplifying
and shortening the rcu_init_nohz() function.

Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
Reviewed-by: Joel Fernandes (Google) <joel@joelfernandes.org>
---
 kernel/rcu/tree_nocb.h | 35 ++++++++++++-----------------------
 1 file changed, 12 insertions(+), 23 deletions(-)

Comments

Zhen Lei Sept. 1, 2022, 1:25 p.m. UTC | #1
On 2022/9/1 21:14, Zhen Lei wrote:
> In kernels built with either CONFIG_RCU_NOCB_CPU_DEFAULT_ALL=y or
> CONFIG_NO_HZ_FULL=y, additional CPUs must be added to rcu_nocb_mask.
> Except that kernels booted without the rcu_nocbs= will not have
> allocated rcu_nocb_mask.  And the current rcu_init_nohz() function uses
> its need_rcu_nocb_mask and offload_all local variables to track the
> rcu_nocb and nohz_full state.
> 
> But there is a much simpler approach, namely creating a cpumask pointer
> to track the default and then using cpumask_available() to check the
> rcu_nocb_mask state.  This commit takes this approach, thereby simplifying
> and shortening the rcu_init_nohz() function.
> 
> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
> Signed-off-by: Paul E. McKenney <paulmck@kernel.org>

Sorry, I forgot remove this "Signed-off-by".

> Reviewed-by: Joel Fernandes (Google) <joel@joelfernandes.org>
> ---
>  kernel/rcu/tree_nocb.h | 35 ++++++++++++-----------------------
>  1 file changed, 12 insertions(+), 23 deletions(-)
> 
> diff --git a/kernel/rcu/tree_nocb.h b/kernel/rcu/tree_nocb.h
> index 0a5f0ef41484518..8b6dceeabde0b4d 100644
> --- a/kernel/rcu/tree_nocb.h
> +++ b/kernel/rcu/tree_nocb.h
> @@ -1210,45 +1210,34 @@ EXPORT_SYMBOL_GPL(rcu_nocb_cpu_offload);
>  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) */
> +	const struct cpumask *cpumask = NULL;
>  
>  #if defined(CONFIG_NO_HZ_FULL)
> -	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 (tick_nohz_full_running && !cpumask_empty(tick_nohz_full_mask))
> +		cpumask = tick_nohz_full_mask;
> +#endif
> +
> +#if defined(CONFIG_RCU_NOCB_CPU_DEFAULT_ALL)
> +	if (!rcu_state.nocb_is_setup && !cpumask)
> +		cpumask = cpu_possible_mask;
> +#endif
>  
> -	if (need_rcu_nocb_mask) {
> +	if (cpumask) {
>  		if (!cpumask_available(rcu_nocb_mask)) {
>  			if (!zalloc_cpumask_var(&rcu_nocb_mask, GFP_KERNEL)) {
>  				pr_info("rcu_nocb_mask allocation failed, callback offloading disabled.\n");
>  				return;
>  			}
>  		}
> +
> +		cpumask_or(rcu_nocb_mask, rcu_nocb_mask, cpumask);
>  		rcu_state.nocb_is_setup = true;
>  	}
>  
>  	if (!rcu_state.nocb_is_setup)
>  		return;
>  
> -#if defined(CONFIG_NO_HZ_FULL)
> -	if (tick_nohz_full_running)
> -		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,
>
Paul E. McKenney Sept. 1, 2022, 4:54 p.m. UTC | #2
On Thu, Sep 01, 2022 at 09:25:06PM +0800, Leizhen (ThunderTown) wrote:
> 
> 
> On 2022/9/1 21:14, Zhen Lei wrote:
> > In kernels built with either CONFIG_RCU_NOCB_CPU_DEFAULT_ALL=y or
> > CONFIG_NO_HZ_FULL=y, additional CPUs must be added to rcu_nocb_mask.
> > Except that kernels booted without the rcu_nocbs= will not have
> > allocated rcu_nocb_mask.  And the current rcu_init_nohz() function uses
> > its need_rcu_nocb_mask and offload_all local variables to track the
> > rcu_nocb and nohz_full state.
> > 
> > But there is a much simpler approach, namely creating a cpumask pointer
> > to track the default and then using cpumask_available() to check the
> > rcu_nocb_mask state.  This commit takes this approach, thereby simplifying
> > and shortening the rcu_init_nohz() function.
> > 
> > Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
> > Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
> 
> Sorry, I forgot remove this "Signed-off-by".

Not a problem, especially given that in view of my oversight on the
previous version, I am going to let Frederic take first crack at this
version.  ;-)

							Thanx, Paul

> > Reviewed-by: Joel Fernandes (Google) <joel@joelfernandes.org>
> > ---
> >  kernel/rcu/tree_nocb.h | 35 ++++++++++++-----------------------
> >  1 file changed, 12 insertions(+), 23 deletions(-)
> > 
> > diff --git a/kernel/rcu/tree_nocb.h b/kernel/rcu/tree_nocb.h
> > index 0a5f0ef41484518..8b6dceeabde0b4d 100644
> > --- a/kernel/rcu/tree_nocb.h
> > +++ b/kernel/rcu/tree_nocb.h
> > @@ -1210,45 +1210,34 @@ EXPORT_SYMBOL_GPL(rcu_nocb_cpu_offload);
> >  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) */
> > +	const struct cpumask *cpumask = NULL;
> >  
> >  #if defined(CONFIG_NO_HZ_FULL)
> > -	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 (tick_nohz_full_running && !cpumask_empty(tick_nohz_full_mask))
> > +		cpumask = tick_nohz_full_mask;
> > +#endif
> > +
> > +#if defined(CONFIG_RCU_NOCB_CPU_DEFAULT_ALL)
> > +	if (!rcu_state.nocb_is_setup && !cpumask)
> > +		cpumask = cpu_possible_mask;
> > +#endif
> >  
> > -	if (need_rcu_nocb_mask) {
> > +	if (cpumask) {
> >  		if (!cpumask_available(rcu_nocb_mask)) {
> >  			if (!zalloc_cpumask_var(&rcu_nocb_mask, GFP_KERNEL)) {
> >  				pr_info("rcu_nocb_mask allocation failed, callback offloading disabled.\n");
> >  				return;
> >  			}
> >  		}
> > +
> > +		cpumask_or(rcu_nocb_mask, rcu_nocb_mask, cpumask);
> >  		rcu_state.nocb_is_setup = true;
> >  	}
> >  
> >  	if (!rcu_state.nocb_is_setup)
> >  		return;
> >  
> > -#if defined(CONFIG_NO_HZ_FULL)
> > -	if (tick_nohz_full_running)
> > -		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,
> > 
> 
> -- 
> Regards,
>   Zhen Lei
Frederic Weisbecker Sept. 7, 2022, 11:04 a.m. UTC | #3
On Thu, Sep 01, 2022 at 09:14:35PM +0800, Zhen Lei wrote:
> In kernels built with either CONFIG_RCU_NOCB_CPU_DEFAULT_ALL=y or
> CONFIG_NO_HZ_FULL=y, additional CPUs must be added to rcu_nocb_mask.
> Except that kernels booted without the rcu_nocbs= will not have
> allocated rcu_nocb_mask.  And the current rcu_init_nohz() function uses
> its need_rcu_nocb_mask and offload_all local variables to track the
> rcu_nocb and nohz_full state.
> 
> But there is a much simpler approach, namely creating a cpumask pointer
> to track the default and then using cpumask_available() to check the
> rcu_nocb_mask state.  This commit takes this approach, thereby simplifying
> and shortening the rcu_init_nohz() function.
> 
> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
> Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
> Reviewed-by: Joel Fernandes (Google) <joel@joelfernandes.org>
> ---
>  kernel/rcu/tree_nocb.h | 35 ++++++++++++-----------------------
>  1 file changed, 12 insertions(+), 23 deletions(-)
> 
> diff --git a/kernel/rcu/tree_nocb.h b/kernel/rcu/tree_nocb.h
> index 0a5f0ef41484518..8b6dceeabde0b4d 100644
> --- a/kernel/rcu/tree_nocb.h
> +++ b/kernel/rcu/tree_nocb.h
> @@ -1210,45 +1210,34 @@ EXPORT_SYMBOL_GPL(rcu_nocb_cpu_offload);
>  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) */
> +	const struct cpumask *cpumask = NULL;
>  
>  #if defined(CONFIG_NO_HZ_FULL)
> -	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 (tick_nohz_full_running && !cpumask_empty(tick_nohz_full_mask))
> +		cpumask = tick_nohz_full_mask;
> +#endif
> +
> +#if defined(CONFIG_RCU_NOCB_CPU_DEFAULT_ALL)
> +	if (!rcu_state.nocb_is_setup && !cpumask)
> +		cpumask = cpu_possible_mask;
> +#endif

Good, and it can even use a simple condition:

if (IS_ENABLED(CONFIG_RCU_NOCB_CPU_DEFAULT_ALL) &&
    !rcu_state.nocb_is_setup && !cpumask)
    cpumask = cpu_possible_mask;


Acked-by: Frederic Weisbecker <frederic@kernel.org>


>  
> -	if (need_rcu_nocb_mask) {
> +	if (cpumask) {
>  		if (!cpumask_available(rcu_nocb_mask)) {
>  			if (!zalloc_cpumask_var(&rcu_nocb_mask, GFP_KERNEL)) {
>  				pr_info("rcu_nocb_mask allocation failed, callback offloading disabled.\n");
>  				return;
>  			}
>  		}
> +
> +		cpumask_or(rcu_nocb_mask, rcu_nocb_mask, cpumask);
>  		rcu_state.nocb_is_setup = true;
>  	}
>  
>  	if (!rcu_state.nocb_is_setup)
>  		return;
>  
> -#if defined(CONFIG_NO_HZ_FULL)
> -	if (tick_nohz_full_running)
> -		cpumask_or(rcu_nocb_mask, rcu_nocb_mask, tick_nohz_full_mask);
> -#endif /* #if defined(CONFIG_NO_HZ_FULL) */
> -
> -	if (offload_all)
> -		cpumask_setall(rcu_nocb_mask);
> -
>  	if (!cpumask_subset(rcu_nocb_mask, cpu_possible_mask)) {
>  		pr_info("\tNote: kernel parameter 'rcu_nocbs=', 'nohz_full', or 'isolcpus=' contains nonexistent CPUs.\n");
>  		cpumask_and(rcu_nocb_mask, cpu_possible_mask,
> -- 
> 2.25.1
>
Zhen Lei Sept. 13, 2022, 1:46 a.m. UTC | #4
On 2022/9/7 19:04, Frederic Weisbecker wrote:
> On Thu, Sep 01, 2022 at 09:14:35PM +0800, Zhen Lei wrote:
>> In kernels built with either CONFIG_RCU_NOCB_CPU_DEFAULT_ALL=y or
>> CONFIG_NO_HZ_FULL=y, additional CPUs must be added to rcu_nocb_mask.
>> Except that kernels booted without the rcu_nocbs= will not have
>> allocated rcu_nocb_mask.  And the current rcu_init_nohz() function uses
>> its need_rcu_nocb_mask and offload_all local variables to track the
>> rcu_nocb and nohz_full state.
>>
>> But there is a much simpler approach, namely creating a cpumask pointer
>> to track the default and then using cpumask_available() to check the
>> rcu_nocb_mask state.  This commit takes this approach, thereby simplifying
>> and shortening the rcu_init_nohz() function.
>>
>> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
>> Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
>> Reviewed-by: Joel Fernandes (Google) <joel@joelfernandes.org>
>> ---
>>  kernel/rcu/tree_nocb.h | 35 ++++++++++++-----------------------
>>  1 file changed, 12 insertions(+), 23 deletions(-)
>>
>> diff --git a/kernel/rcu/tree_nocb.h b/kernel/rcu/tree_nocb.h
>> index 0a5f0ef41484518..8b6dceeabde0b4d 100644
>> --- a/kernel/rcu/tree_nocb.h
>> +++ b/kernel/rcu/tree_nocb.h
>> @@ -1210,45 +1210,34 @@ EXPORT_SYMBOL_GPL(rcu_nocb_cpu_offload);
>>  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) */
>> +	const struct cpumask *cpumask = NULL;
>>  
>>  #if defined(CONFIG_NO_HZ_FULL)
>> -	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 (tick_nohz_full_running && !cpumask_empty(tick_nohz_full_mask))
>> +		cpumask = tick_nohz_full_mask;
>> +#endif
>> +
>> +#if defined(CONFIG_RCU_NOCB_CPU_DEFAULT_ALL)
>> +	if (!rcu_state.nocb_is_setup && !cpumask)
>> +		cpumask = cpu_possible_mask;
>> +#endif
> 
> Good, and it can even use a simple condition:
> 
> if (IS_ENABLED(CONFIG_RCU_NOCB_CPU_DEFAULT_ALL) &&
>     !rcu_state.nocb_is_setup && !cpumask)
>     cpumask = cpu_possible_mask;

Okay, I'll revise it in v7.

> 
> 
> Acked-by: Frederic Weisbecker <frederic@kernel.org>
> 
> 
>>  
>> -	if (need_rcu_nocb_mask) {
>> +	if (cpumask) {
>>  		if (!cpumask_available(rcu_nocb_mask)) {
>>  			if (!zalloc_cpumask_var(&rcu_nocb_mask, GFP_KERNEL)) {
>>  				pr_info("rcu_nocb_mask allocation failed, callback offloading disabled.\n");
>>  				return;
>>  			}
>>  		}
>> +
>> +		cpumask_or(rcu_nocb_mask, rcu_nocb_mask, cpumask);
>>  		rcu_state.nocb_is_setup = true;
>>  	}
>>  
>>  	if (!rcu_state.nocb_is_setup)
>>  		return;
>>  
>> -#if defined(CONFIG_NO_HZ_FULL)
>> -	if (tick_nohz_full_running)
>> -		cpumask_or(rcu_nocb_mask, rcu_nocb_mask, tick_nohz_full_mask);
>> -#endif /* #if defined(CONFIG_NO_HZ_FULL) */
>> -
>> -	if (offload_all)
>> -		cpumask_setall(rcu_nocb_mask);
>> -
>>  	if (!cpumask_subset(rcu_nocb_mask, cpu_possible_mask)) {
>>  		pr_info("\tNote: kernel parameter 'rcu_nocbs=', 'nohz_full', or 'isolcpus=' contains nonexistent CPUs.\n");
>>  		cpumask_and(rcu_nocb_mask, cpu_possible_mask,
>> -- 
>> 2.25.1
>>
> .
>
diff mbox series

Patch

diff --git a/kernel/rcu/tree_nocb.h b/kernel/rcu/tree_nocb.h
index 0a5f0ef41484518..8b6dceeabde0b4d 100644
--- a/kernel/rcu/tree_nocb.h
+++ b/kernel/rcu/tree_nocb.h
@@ -1210,45 +1210,34 @@  EXPORT_SYMBOL_GPL(rcu_nocb_cpu_offload);
 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) */
+	const struct cpumask *cpumask = NULL;
 
 #if defined(CONFIG_NO_HZ_FULL)
-	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 (tick_nohz_full_running && !cpumask_empty(tick_nohz_full_mask))
+		cpumask = tick_nohz_full_mask;
+#endif
+
+#if defined(CONFIG_RCU_NOCB_CPU_DEFAULT_ALL)
+	if (!rcu_state.nocb_is_setup && !cpumask)
+		cpumask = cpu_possible_mask;
+#endif
 
-	if (need_rcu_nocb_mask) {
+	if (cpumask) {
 		if (!cpumask_available(rcu_nocb_mask)) {
 			if (!zalloc_cpumask_var(&rcu_nocb_mask, GFP_KERNEL)) {
 				pr_info("rcu_nocb_mask allocation failed, callback offloading disabled.\n");
 				return;
 			}
 		}
+
+		cpumask_or(rcu_nocb_mask, rcu_nocb_mask, cpumask);
 		rcu_state.nocb_is_setup = true;
 	}
 
 	if (!rcu_state.nocb_is_setup)
 		return;
 
-#if defined(CONFIG_NO_HZ_FULL)
-	if (tick_nohz_full_running)
-		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,