| Message ID | 20240319080153.3263-1-xuewen.yan@unisoc.com (mailing list archive) |
|---|---|
| State | In Next |
| Delegated to: | Rafael Wysocki |
| Headers | show |
| Series | cpufreq: Use a smaller freq for the policy->max when verify | expand |
On 19-03-24, 16:01, 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. > > So use a smaller freq when can not find the freq in fre-table, to prevent freq-table > 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(-) Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
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>
On Wed, Mar 20, 2024 at 4:21 AM Viresh Kumar <viresh.kumar@linaro.org> wrote: > > On 19-03-24, 16:01, 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. > > > > So use a smaller freq when can not find the freq in fre-table, to prevent > > freq-table > > > 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(-) > > Acked-by: Viresh Kumar <viresh.kumar@linaro.org> Applied as 6.10 material, thanks!
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); }
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. 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(-)