linux-kernel - Re: [PATCH v2] cpufreq/cppc: Take policy->cur into judge when set target

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]

Date: Wed, 29 May 2024 14:53:41 +0800
From: Riwen Lu <luriwen@...mail.com>
To: Viresh Kumar <viresh.kumar@...aro.org>
Cc: rafael@...nel.org, linux-pm@...r.kernel.org,
 linux-kernel@...r.kernel.org, Riwen Lu <luriwen@...inos.cn>
Subject: Re: [PATCH v2] cpufreq/cppc: Take policy->cur into judge when set
 target

在 2024/5/29 13:36, Viresh Kumar 写道:
> On 29-05-24, 11:22, Riwen Lu wrote:
>> From: Riwen Lu <luriwen@...inos.cn>
>>
>> There is a case that desired_perf is exactly the same with the old perf,
>> but the actual current freq is not. Add a judgment condition to return
>> only when the three values are exactly the same.
>>
>> This happened in S3 while the cpufreq governor is set to powersave.
>> During resume process, the CPU frequency is adjusted to the highest
>> perf. For the booting CPU, there's a warning that "CPU frequency out of
>> sync:", because the policy->cur is the lowest_perf while the actual
>> current frequency is the highest_perf that obtained via
>> cppc_cpufreq_get_rate(), then set policy->cur to highest_perf. The
>> governor->limits() intent to configure the CPU frequency to lowest_perf
>> and the governor->target() returned because the desired_perf is equal to
>> cpu->perf_ctrls.desired_perf leaving the actual current frequency and
>> policy->cur are remain the highest_perf. Add a judgement that if
>> policy->cur is the same with desired_perf to decide whther to return.
>>
>> Signed-off-by: Riwen Lu <luriwen@...inos.cn>
>>
>> ---
>> v1 -> v2:
>>   - Update commit message and email.
>> ---
>>   drivers/cpufreq/cppc_cpufreq.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
>> index 15f1d41920a3..802f7c7c0ad8 100644
>> --- a/drivers/cpufreq/cppc_cpufreq.c
>> +++ b/drivers/cpufreq/cppc_cpufreq.c
>> @@ -296,7 +296,8 @@ static int cppc_cpufreq_set_target(struct cpufreq_policy *policy,
>>   
>>   	desired_perf = cppc_khz_to_perf(&cpu_data->perf_caps, target_freq);
>>   	/* Return if it is exactly the same perf */
>> -	if (desired_perf == cpu_data->perf_ctrls.desired_perf)
>> +	if (desired_perf == cpu_data->perf_ctrls.desired_perf &&
>> +	    desired_perf == policy->cur)
> 
>  From my earlier understanding, desired_perf is a derived interpretation of the
> frequency and isn't an actual frequency value which can be compared with
> policy->cur.
> 
> Shouldn't we compare policy->cur with target_freq instead ? If yes, than the
> core must already be doing that somewhere I guess.
> 
Yes, you are right， I didn't think it through. In this circumstance, the 
policy->cur is the highest frequency, desired_perf converted from 
target_freq is the same with cpu_data->perf_ctrls.desired_perf which 
shouldn't.