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@xxxxxxxxxx>
> 
> 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