[PATCH] rcu/nocb: Delete local variable 'need_rcu_nocb_mask' in rcu_init_nohz()

[PATCH] rcu/nocb: Delete local variable 'need_rcu_nocb_mask' in rcu_init_nohz()

[Zhen Lei]

The local variable 'need_rcu_nocb_mask' is true only if CONFIG_NO_HZ_FULL
is defined. So branch "if (need_rcu_nocb_mask)" can be moved within the
scope of "#if defined(CONFIG_NO_HZ_FULL)". At this point, using variable
'need_rcu_nocb_mask' is not necessary, so delete it.

No functional changes, but the code looks a little more concise.

Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
---
 kernel/rcu/tree_nocb.h | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/kernel/rcu/tree_nocb.h b/kernel/rcu/tree_nocb.h
index 636d0546a4e932e..1e334e217f0afb7 100644
--- a/kernel/rcu/tree_nocb.h
+++ b/kernel/rcu/tree_nocb.h
@@ -1165,15 +1165,10 @@ EXPORT_SYMBOL_GPL(rcu_nocb_cpu_offload);
 void __init rcu_init_nohz(void)
 {
 	int cpu;
-	bool need_rcu_nocb_mask = false;
 	struct rcu_data *rdp;
 
 #if defined(CONFIG_NO_HZ_FULL)
-	if (tick_nohz_full_running && !cpumask_empty(tick_nohz_full_mask))
-		need_rcu_nocb_mask = true;
-#endif /* #if defined(CONFIG_NO_HZ_FULL) */
-
-	if (need_rcu_nocb_mask) {
+	if (tick_nohz_full_running && !cpumask_empty(tick_nohz_full_mask)) {
 		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");
@@ -1182,6 +1177,7 @@ void __init rcu_init_nohz(void)
 		}
 		rcu_nocb_is_setup = true;
 	}
+#endif /* #if defined(CONFIG_NO_HZ_FULL) */
 
 	if (!rcu_nocb_is_setup)
 		return;
-- 
2.26.0.106.g9fadedd

Re: [PATCH] rcu/nocb: Delete local variable 'need_rcu_nocb_mask' in rcu_init_nohz()

[Frederic Weisbecker]

On Tue, Apr 26, 2022 at 03:36:26PM +0800, Zhen Lei wrote:
> The local variable 'need_rcu_nocb_mask' is true only if CONFIG_NO_HZ_FULL
> is defined. So branch "if (need_rcu_nocb_mask)" can be moved within the
> scope of "#if defined(CONFIG_NO_HZ_FULL)". At this point, using variable
> 'need_rcu_nocb_mask' is not necessary, so delete it.
> 
> No functional changes, but the code looks a little more concise.
> 
> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
> ---
>  kernel/rcu/tree_nocb.h | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/kernel/rcu/tree_nocb.h b/kernel/rcu/tree_nocb.h
> index 636d0546a4e932e..1e334e217f0afb7 100644
> --- a/kernel/rcu/tree_nocb.h
> +++ b/kernel/rcu/tree_nocb.h
> @@ -1165,15 +1165,10 @@ EXPORT_SYMBOL_GPL(rcu_nocb_cpu_offload);
>  void __init rcu_init_nohz(void)
>  {
>  	int cpu;
> -	bool need_rcu_nocb_mask = false;
>  	struct rcu_data *rdp;
>  
>  #if defined(CONFIG_NO_HZ_FULL)
> -	if (tick_nohz_full_running && !cpumask_empty(tick_nohz_full_mask))
> -		need_rcu_nocb_mask = true;
> -#endif /* #if defined(CONFIG_NO_HZ_FULL) */
> -
> -	if (need_rcu_nocb_mask) {
> +	if (tick_nohz_full_running && !cpumask_empty(tick_nohz_full_mask)) {

Thanks! I suspect though that the introduction of
CONFIG_RCU_NOCB_CPU_DEFAULT_ALL has made this patch unecessary.


>  		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");
> @@ -1182,6 +1177,7 @@ void __init rcu_init_nohz(void)
>  		}
>  		rcu_nocb_is_setup = true;
>  	}
> +#endif /* #if defined(CONFIG_NO_HZ_FULL) */
>  
>  	if (!rcu_nocb_is_setup)
>  		return;
> -- 
> 2.26.0.106.g9fadedd
> 

Re: [PATCH] rcu/nocb: Delete local variable 'need_rcu_nocb_mask' in rcu_init_nohz()

[Paul E. McKenney]

On Tue, Apr 26, 2022 at 03:36:26PM +0800, Zhen Lei wrote:
> The local variable 'need_rcu_nocb_mask' is true only if CONFIG_NO_HZ_FULL
> is defined. So branch "if (need_rcu_nocb_mask)" can be moved within the
> scope of "#if defined(CONFIG_NO_HZ_FULL)". At this point, using variable
> 'need_rcu_nocb_mask' is not necessary, so delete it.
> 
> No functional changes, but the code looks a little more concise.
> 
> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>

First, please accept my apologies for the late reply and for the
overly active spam filters.

One question below.

							Thanx, Paul

> ---
>  kernel/rcu/tree_nocb.h | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/kernel/rcu/tree_nocb.h b/kernel/rcu/tree_nocb.h
> index 636d0546a4e932e..1e334e217f0afb7 100644
> --- a/kernel/rcu/tree_nocb.h
> +++ b/kernel/rcu/tree_nocb.h
> @@ -1165,15 +1165,10 @@ EXPORT_SYMBOL_GPL(rcu_nocb_cpu_offload);
>  void __init rcu_init_nohz(void)
>  {
>  	int cpu;
> -	bool need_rcu_nocb_mask = false;
>  	struct rcu_data *rdp;
>  
>  #if defined(CONFIG_NO_HZ_FULL)
> -	if (tick_nohz_full_running && !cpumask_empty(tick_nohz_full_mask))
> -		need_rcu_nocb_mask = true;
> -#endif /* #if defined(CONFIG_NO_HZ_FULL) */
> -
> -	if (need_rcu_nocb_mask) {

Could you please test this on a kernel built with CONFIG_NO_HZ_FULL=n
and CONFIG_RCU_NOCB_CPU=y?  If that works, please add an explanation
of why it works to the commit log above and repost the patch.

> +	if (tick_nohz_full_running && !cpumask_empty(tick_nohz_full_mask)) {
>  		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");
> @@ -1182,6 +1177,7 @@ void __init rcu_init_nohz(void)
>  		}
>  		rcu_nocb_is_setup = true;
>  	}
> +#endif /* #if defined(CONFIG_NO_HZ_FULL) */
>  
>  	if (!rcu_nocb_is_setup)
>  		return;
> -- 
> 2.26.0.106.g9fadedd
> 

Re: [PATCH] rcu/nocb: Delete local variable 'need_rcu_nocb_mask' in rcu_init_nohz()

[Leizhen (ThunderTown)]

On 2022/5/13 23:05, Paul E. McKenney wrote:
> On Tue, Apr 26, 2022 at 03:36:26PM +0800, Zhen Lei wrote:
>> The local variable 'need_rcu_nocb_mask' is true only if CONFIG_NO_HZ_FULL
>> is defined. So branch "if (need_rcu_nocb_mask)" can be moved within the
>> scope of "#if defined(CONFIG_NO_HZ_FULL)". At this point, using variable
>> 'need_rcu_nocb_mask' is not necessary, so delete it.
>>
>> No functional changes, but the code looks a little more concise.
>>
>> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
> 
> First, please accept my apologies for the late reply and for the
> overly active spam filters.
> 
> One question below.
> 
> 							Thanx, Paul
> 
>> ---
>>  kernel/rcu/tree_nocb.h | 8 ++------
>>  1 file changed, 2 insertions(+), 6 deletions(-)
>>
>> diff --git a/kernel/rcu/tree_nocb.h b/kernel/rcu/tree_nocb.h
>> index 636d0546a4e932e..1e334e217f0afb7 100644
>> --- a/kernel/rcu/tree_nocb.h
>> +++ b/kernel/rcu/tree_nocb.h
>> @@ -1165,15 +1165,10 @@ EXPORT_SYMBOL_GPL(rcu_nocb_cpu_offload);
>>  void __init rcu_init_nohz(void)
>>  {
>>  	int cpu;
>> -	bool need_rcu_nocb_mask = false;
>>  	struct rcu_data *rdp;
>>  
>>  #if defined(CONFIG_NO_HZ_FULL)
>> -	if (tick_nohz_full_running && !cpumask_empty(tick_nohz_full_mask))
>> -		need_rcu_nocb_mask = true;
>> -#endif /* #if defined(CONFIG_NO_HZ_FULL) */
>> -
>> -	if (need_rcu_nocb_mask) {
> 
> Could you please test this on a kernel built with CONFIG_NO_HZ_FULL=n
> and CONFIG_RCU_NOCB_CPU=y?  If that works, please add an explanation

OK, I will test it.

> of why it works to the commit log above and repost the patch.

Sorry, If I had written pseudocode in the commit log, it would have been clear, as below:

-----------------before-----------------
        bool need_rcu_nocb_mask = false;

#if defined(CONFIG_NO_HZ_FULL)
        if (tick_nohz_full_running && !cpumask_empty(tick_nohz_full_mask))
                need_rcu_nocb_mask = true;      <----	//Move it here, then delete all 'need_rcu_nocb_mask' related code
#endif /* #if defined(CONFIG_NO_HZ_FULL) */          |
                                                     |
        if (need_rcu_nocb_mask) {                    |  //Can only be true above
                ...                             -----
        }

-----------------after-----------------
#if defined(CONFIG_NO_HZ_FULL)
        if (tick_nohz_full_running && !cpumask_empty(tick_nohz_full_mask)) {
                ...
	}
#endif /* #if defined(CONFIG_NO_HZ_FULL) */

I'm doing clean up on the premise that the original code is correct.


> 
>> +	if (tick_nohz_full_running && !cpumask_empty(tick_nohz_full_mask)) {
>>  		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");
>> @@ -1182,6 +1177,7 @@ void __init rcu_init_nohz(void)
>>  		}
>>  		rcu_nocb_is_setup = true;
>>  	}
>> +#endif /* #if defined(CONFIG_NO_HZ_FULL) */
>>  
>>  	if (!rcu_nocb_is_setup)
>>  		return;
>> -- 
>> 2.26.0.106.g9fadedd
>>
> .
> 

-- 
Regards,
  Zhen Lei