Re: [PATCH 0/2] cpufreq: governor: Apply limits with target_freq instead of policy->cur

[Jie Zhan <zhanjie9@hisilicon.com>]

On 3/6/2026 5:21 PM, zhenglifeng (A) wrote:
> On 3/5/2026 8:19 PM, Jie Zhan wrote:
>>
>> On 2/10/2026 7:54 PM, Lifeng Zheng wrote:
>>> The motivation for this patchset cames from a test on our platform:
>>>
>>> With conservative governor and some pressure on CPU, the frequency rapidly
>>> reach the max supported frequency, such as 2GHz.
>>>
>>> Later, some frequency division strategies on our platform were triggered
>>> and the actual frequency become 500MHz -- 1/4 of the OS distribution
>>> frequency.
>>>
>>> At that time, if someone excecutes 'cat cpuinfo_cur_freq', the actual
>>> frequency will become 250MHz -- 1/4 of the min supported frequency.
>>>
>>> After the platform recovering from the frequency division, the frequency
>>> will stay on 1GHz, until the pressure disappear.
>>>
>>> The reason this happens is that in cpufreq_verify_current_freq(), if
>>> policy->cur != new_freq, policy->update will be queued, which will
>>> ultimately lead to a call to cpufreq_policy_apply_limits(), and update the
>>> target frequency to policy->min. And then in cs_dbs_update(), since the
>>> pressure never vanish, it will always hit the following branches:
>>>
>>> 	if (load > dbs_data->up_threshold) {
>>> 		dbs_info->down_skip = 0;
>>>
>>> 		/* if we are already at full speed then break out early */
>>> 		if (requested_freq == policy->max)
>>> 			goto out;
>>>
>>> Therefore, the target frequency will always remain at the lowest frequency.
>> I feel like this is a common issue that some special handling in the
>> governor should happen when the frequency limits changes, i.e.
>> governor->limits() gets called.  See 'limits_changed' or 'need_freq_update'
>> in the schedutil governor.
>>
>> Do you think it's reasonable to mark such a flag in 'cpufreq_policy' when
>> its limits changes, and any governor can use that for their own code?
> 
> Yes. This is why I said 'The branching conditions in cs_dbs_update() may
> not be strict enough'. However, using policy->cur to decide the target freq
> is unreasonable to me. So this patch set is still meaningful I think.
> 
Ok, ignore this.  I was thinking of extracting such a flag/variable to a
common struct shared among ondemand, conservative, and scheduitl.  But this
is unrealistic at the moment, considering the governor data between DBS
(ondemand, conservative) and schedutil is totally different.

>>
>> Jie
>>>
>>> The branching conditions in cs_dbs_update() may not be strict enough, but
>>> the root cause of this problem is that the target frequency was updated
>>> when querying cpuinfo_cur_freq. For ondemand and schedutil governor,
>>> although the frequency will not always remain at the lowest level without
>>> rising, will still be min_freq in a short period of time when the query
>>> action occurs.
>>>
>>> Using the freq requested by the governor to decide whether to update the
>>> target frequency is more reasonable in cpufreq_policy_apply_limits().
>>>
>>> Lifeng Zheng (2):
>>>   cpufreq: governor: Move requested_freq to policy_dbs_info
>>>   cpufreq: governor: Apply limits with requested_freq or next_freq
>>>
>>>  drivers/cpufreq/cpufreq_conservative.c | 14 ++++----------
>>>  drivers/cpufreq/cpufreq_governor.c     |  3 ++-
>>>  drivers/cpufreq/cpufreq_governor.h     | 12 ++++++++++++
>>>  drivers/cpufreq/cpufreq_ondemand.c     | 10 +++++-----
>>>  include/linux/cpufreq.h                |  7 ++++---
>>>  kernel/sched/cpufreq_schedutil.c       |  4 ++--
>>>  6 files changed, 29 insertions(+), 21 deletions(-)
>>>
>