Re: [PATCH] cpufreq: conservative: Fix incorrect frequency decrease due to stale target

[Viresh Kumar]

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 ?

-- 
viresh