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

Viresh Kumar
2025-04-01T08:32:33Z

On 28-03-25, 21:40, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> 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@intel.com>
> ---
> 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@linaro.org>

--
viresh