linux-kernel - Re: [PATCH v1 02/10] cpufreq: Consolidate some code in cpufreq

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250401083230.zkqy3i4uo3yeyw76@vireshk-i7>
Date: Tue, 1 Apr 2025 14:02:30 +0530
From: Viresh Kumar <viresh.kumar@...aro.org>
To: "Rafael J. Wysocki" <rjw@...ysocki.net>
Cc: Linux PM <linux-pm@...r.kernel.org>,
	LKML <linux-kernel@...r.kernel.org>,
	Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>,
	Mario Limonciello <mario.limonciello@....com>,
	Sudeep Holla <sudeep.holla@....com>
Subject: Re: [PATCH v1 02/10] cpufreq: Consolidate some code in
 cpufreq_online()

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

-- 
viresh