Re: [PATCH v1 02/10] cpufreq: Consolidate some code in cpufreq_online()

[Viresh Kumar]

On 28-03-25, 21:40, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
> 
> Notice that the policy->cpu update in cpufreq_policy_alloc() can be
> moved to cpufreq_online() and then it can be carried out under the
> policy rwsem, along with the clearing of policy->governor (unnecessary
> in the "new policy" code branch, but also not harmful).  If this is
> done, the bottom parts of the "if (policy)" branches become identical
> and they can be collapsed and moved below the conditional.
> 
> Modify the code accordingly which makes it somewhat easier to follow.
> 
> No intentional functional impact.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
> ---
>  drivers/cpufreq/cpufreq.c |   10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -1306,7 +1306,6 @@
>  	init_waitqueue_head(&policy->transition_wait);
>  	INIT_WORK(&policy->update, handle_update);
>  
> -	policy->cpu = cpu;
>  	return policy;
>  
>  err_min_qos_notifier:
> @@ -1394,17 +1393,18 @@
>  
>  		/* This is the only online CPU for the policy.  Start over. */
>  		new_policy = false;
> -		down_write(&policy->rwsem);
> -		policy->cpu = cpu;
> -		policy->governor = NULL;
>  	} else {
>  		new_policy = true;
>  		policy = cpufreq_policy_alloc(cpu);
>  		if (!policy)
>  			return -ENOMEM;
> -		down_write(&policy->rwsem);
>  	}
>  
> +	down_write(&policy->rwsem);
> +
> +	policy->cpu = cpu;
> +	policy->governor = NULL;
> +
>  	if (!new_policy && cpufreq_driver->online) {
>  		/* Recover policy->cpus using related_cpus */
>  		cpumask_copy(policy->cpus, policy->related_cpus);

Acked-by: Viresh Kumar <viresh.kumar@xxxxxxxxxx>

-- 
viresh