Messages in this thread | | | Date | Wed, 20 Mar 2024 10:53:56 +0530 | From | Dhruva Gole <> | Subject | Re: [PATCH] cpufreq: Use a smaller freq for the policy->max when verify |
| |
Hi,
On Mar 19, 2024 at 16:01:53 +0800, Xuewen Yan wrote: > When driver use the cpufreq_frequency_table_verify() as the > cpufreq_driver->verify's callback. It may cause the policy->max > bigger than the freq_qos's max freq. > > Just as follow: > > unisoc:/sys/devices/system/cpu/cpufreq/policy0 # cat scaling_available_frequencies > 614400 768000 988000 1228800 1469000 1586000 1690000 1833000 2002000 2093000 > > unisoc:/sys/devices/system/cpu/cpufreq/policy0 # echo 1900000 > scaling_max_freq > unisoc:/sys/devices/system/cpu/cpufreq/policy0 # echo 1900000 > scaling_min_freq > unisoc:/sys/devices/system/cpu/cpufreq/policy0 # cat scaling_max_freq > 2002000 > unisoc:/sys/devices/system/cpu/cpufreq/policy0 # cat scaling_min_freq > 2002000 > > When user set the qos_min and qos_max as the same value, and the value > is not in the freq-table, the above scenario will occur. > > This is because in cpufreq_frequency_table_verify() func, when it can not > find the freq in table, it will change the policy->max to be a bigger freq, > as above, because there is no 1.9G in the freq-table, the policy->max would > be set to 2.002G. As a result, the cpufreq_policy->max is bigger than the > user's qos_max. This is unreasonable.
That's a good catch! Never thought of this.
> > So use a smaller freq when can not find the freq in fre-table, to prevent > the policy->max exceed the qos's max freq. > > Signed-off-by: Xuewen Yan <xuewen.yan@unisoc.com> > --- > drivers/cpufreq/freq_table.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/cpufreq/freq_table.c b/drivers/cpufreq/freq_table.c > index c4d4643b6ca6..1d98b8cf1688 100644 > --- a/drivers/cpufreq/freq_table.c > +++ b/drivers/cpufreq/freq_table.c > @@ -70,7 +70,7 @@ int cpufreq_frequency_table_verify(struct cpufreq_policy_data *policy, > struct cpufreq_frequency_table *table) > { > struct cpufreq_frequency_table *pos; > - unsigned int freq, next_larger = ~0; > + unsigned int freq, prev_smaller = 0; > bool found = false; > > pr_debug("request for verification of policy (%u - %u kHz) for cpu %u\n", > @@ -86,12 +86,12 @@ int cpufreq_frequency_table_verify(struct cpufreq_policy_data *policy, > break; > } > > - if ((next_larger > freq) && (freq > policy->max)) > - next_larger = freq; > + if ((prev_smaller < freq) && (freq <= policy->max)) > + prev_smaller = freq; > } > > if (!found) { > - policy->max = next_larger; > + policy->max = prev_smaller; > cpufreq_verify_within_cpu_limits(policy);
LGTM! Reviewed-by: Dhruva Gole <d-gole@ti.com>
-- Best regards, Dhruva
| |