Re: [PATCH] cpufreq: conservative: Fix incorrect frequency decrease due to stale target
From: zhenglifeng (A)
Date: Wed May 20 2026 - 04:35:07 EST
On 5/20/2026 3:48 PM, Viresh Kumar wrote:
> On 11-05-26, 12:03, Zhongqiu Han wrote:
>> Just one small concern is that requested_freq may underflow as an
>> unsigned value; perhaps this could be improved by:
>>
>> - if (requested_freq > freq_step)
>> + if (requested_freq > policy->min + freq_step)
>> requested_freq -= freq_step;
>> else
>> requested_freq = policy->min;
>
> Looks fine.
>
>> or use min() func?
>>
>> Additionally, it seems that dropping the early‑exit checks also appears
>> to be a nice side effect fix for CPUFREQ_NEED_UPDATE_LIMITS drivers when
>> updating internal upper and lower frequency boundaries.
>>
>> As designed in commit 1c534352f47f ("cpufreq: Introduce
>> CPUFREQ_NEED_UPDATE_LIMITS driver flag"), a cpufreq driver may need to
>> update its internal frequency limits when policy min or max changes for
>> drivers setting CPUFREQ_NEED_UPDATE_LIMITS.
>>
>> However, the early‑exit in cs_dbs_update() can prevent
>> __cpufreq_driver_target() from ever being called.
>>
>> For example, when policy->min rises from 200 to 400 kHz while policy
>> ->cur is already at 400 kHz, under low load:
>>
>> cpufreq_policy_apply_limits():
>> policy->min(400) > policy->cur(400) ? NO -> driver not called
>>
>> cs_limits():
>> dbs_info->requested_freq = policy->cur = 400
>>
>> [next sampling period, low load]
>> cs_dbs_update():
>> requested_freq = 400
>> if (requested_freq == policy->min) /* 400 == 400 -> true */
>> goto out; /* __cpufreq_driver_target() never called */
>>
>> With the early‑exit removed, __cpufreq_driver_target() is called with
>> target == policy->cur, and CPUFREQ_NEED_UPDATE_LIMITS ensures the driver
>> is invoked to update its internal performance boundaries.
>
> I think we are all in agreement on this now ?
>
For CPUFREQ_NEED_UPDATE_LIMITS drivers, dropping this early-exit checks
means cpufreq_driver->target() will always be called every time when
cs_dbs_update() is called. Is it OK?