* [PATCH v2] cpufreq: Fix setting policy limits when frequency tables are used
@ 2025-04-25 11:36 Rafael J. Wysocki
2025-04-25 13:21 ` Stephan Gerhold
2025-04-27 2:25 ` zhenglifeng (A)
0 siblings, 2 replies; 6+ messages in thread
From: Rafael J. Wysocki @ 2025-04-25 11:36 UTC (permalink / raw)
To: Linux PM
Cc: Christian Loehle, LKML, Viresh Kumar, Srinivas Pandruvada,
Mario Limonciello, Sultan Alsawaf, Stephan Gerhold,
Rafael J. Wysocki
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Commit 7491cdf46b5c ("cpufreq: Avoid using inconsistent policy->min and
policy->max") overlooked the fact that policy->min and policy->max were
accessed directly in cpufreq_frequency_table_target() and in the
functions called by it. Consequently, the changes made by that commit
led to problems with setting policy limits.
Address this by passing the target frequency limits to __resolve_freq()
and cpufreq_frequency_table_target() and propagating them to the
functions called by the latter.
Fixes: 7491cdf46b5c ("cpufreq: Avoid using inconsistent policy->min and policy->max")
Link: https://lore.kernel.org/linux-pm/aAplED3IA_J0eZN0@linaro.org/
Reported-by: Stephan Gerhold <stephan.gerhold@linaro.org>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
The v1 is here: https://lore.kernel.org/linux-pm/12665363.O9o76ZdvQC@rjwysocki.net/
v1 -> v2:
* Do clamp_val(target_freq, min, max) before checking freq_table against
NULL in __resolve_freq().
* Update comment in cpufreq_frequency_table_target() to match the new code.
---
drivers/cpufreq/cpufreq.c | 22 ++++++---
drivers/cpufreq/cpufreq_ondemand.c | 3 -
drivers/cpufreq/freq_table.c | 6 +-
include/linux/cpufreq.h | 83 ++++++++++++++++++++++++-------------
4 files changed, 73 insertions(+), 41 deletions(-)
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -491,14 +491,18 @@
EXPORT_SYMBOL_GPL(cpufreq_disable_fast_switch);
static unsigned int __resolve_freq(struct cpufreq_policy *policy,
- unsigned int target_freq, unsigned int relation)
+ unsigned int target_freq,
+ unsigned int min, unsigned int max,
+ unsigned int relation)
{
unsigned int idx;
+ target_freq = clamp_val(target_freq, min, max);
+
if (!policy->freq_table)
return target_freq;
- idx = cpufreq_frequency_table_target(policy, target_freq, relation);
+ idx = cpufreq_frequency_table_target(policy, target_freq, min, max, relation);
policy->cached_resolved_idx = idx;
policy->cached_target_freq = target_freq;
return policy->freq_table[idx].frequency;
@@ -532,8 +536,7 @@
if (unlikely(min > max))
min = max;
- return __resolve_freq(policy, clamp_val(target_freq, min, max),
- CPUFREQ_RELATION_LE);
+ return __resolve_freq(policy, target_freq, min, max, CPUFREQ_RELATION_LE);
}
EXPORT_SYMBOL_GPL(cpufreq_driver_resolve_freq);
@@ -2351,8 +2354,8 @@
if (cpufreq_disabled())
return -ENODEV;
- target_freq = clamp_val(target_freq, policy->min, policy->max);
- target_freq = __resolve_freq(policy, target_freq, relation);
+ target_freq = __resolve_freq(policy, target_freq, policy->min,
+ policy->max, relation);
pr_debug("target for CPU %u: %u kHz, relation %u, requested %u kHz\n",
policy->cpu, target_freq, relation, old_target_freq);
@@ -2650,8 +2653,11 @@
* compiler optimizations around them because they may be accessed
* concurrently by cpufreq_driver_resolve_freq() during the update.
*/
- WRITE_ONCE(policy->max, __resolve_freq(policy, new_data.max, CPUFREQ_RELATION_H));
- new_data.min = __resolve_freq(policy, new_data.min, CPUFREQ_RELATION_L);
+ WRITE_ONCE(policy->max, __resolve_freq(policy, new_data.max,
+ new_data.min, new_data.max,
+ CPUFREQ_RELATION_H));
+ new_data.min = __resolve_freq(policy, new_data.min, new_data.min,
+ new_data.max, CPUFREQ_RELATION_L);
WRITE_ONCE(policy->min, new_data.min > policy->max ? policy->max : new_data.min);
trace_cpu_frequency_limits(policy);
--- a/drivers/cpufreq/cpufreq_ondemand.c
+++ b/drivers/cpufreq/cpufreq_ondemand.c
@@ -76,7 +76,8 @@
return freq_next;
}
- index = cpufreq_frequency_table_target(policy, freq_next, relation);
+ index = cpufreq_frequency_table_target(policy, freq_next, policy->min,
+ policy->max, relation);
freq_req = freq_table[index].frequency;
freq_reduc = freq_req * od_tuners->powersave_bias / 1000;
freq_avg = freq_req - freq_reduc;
--- a/drivers/cpufreq/freq_table.c
+++ b/drivers/cpufreq/freq_table.c
@@ -115,8 +115,8 @@
EXPORT_SYMBOL_GPL(cpufreq_generic_frequency_table_verify);
int cpufreq_table_index_unsorted(struct cpufreq_policy *policy,
- unsigned int target_freq,
- unsigned int relation)
+ unsigned int target_freq, unsigned int min,
+ unsigned int max, unsigned int relation)
{
struct cpufreq_frequency_table optimal = {
.driver_data = ~0,
@@ -147,7 +147,7 @@
cpufreq_for_each_valid_entry_idx(pos, table, i) {
freq = pos->frequency;
- if ((freq < policy->min) || (freq > policy->max))
+ if (freq < min || freq > max)
continue;
if (freq == target_freq) {
optimal.driver_data = i;
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -788,8 +788,8 @@
int cpufreq_generic_frequency_table_verify(struct cpufreq_policy_data *policy);
int cpufreq_table_index_unsorted(struct cpufreq_policy *policy,
- unsigned int target_freq,
- unsigned int relation);
+ unsigned int target_freq, unsigned int min,
+ unsigned int max, unsigned int relation);
int cpufreq_frequency_table_get_index(struct cpufreq_policy *policy,
unsigned int freq);
@@ -852,12 +852,12 @@
return best;
}
-/* Works only on sorted freq-tables */
-static inline int cpufreq_table_find_index_l(struct cpufreq_policy *policy,
- unsigned int target_freq,
- bool efficiencies)
+static inline int find_index_l(struct cpufreq_policy *policy,
+ unsigned int target_freq,
+ unsigned int min, unsigned int max,
+ bool efficiencies)
{
- target_freq = clamp_val(target_freq, policy->min, policy->max);
+ target_freq = clamp_val(target_freq, min, max);
if (policy->freq_table_sorted == CPUFREQ_TABLE_SORTED_ASCENDING)
return cpufreq_table_find_index_al(policy, target_freq,
@@ -867,6 +867,14 @@
efficiencies);
}
+/* Works only on sorted freq-tables */
+static inline int cpufreq_table_find_index_l(struct cpufreq_policy *policy,
+ unsigned int target_freq,
+ bool efficiencies)
+{
+ return find_index_l(policy, target_freq, policy->min, policy->max, efficiencies);
+}
+
/* Find highest freq at or below target in a table in ascending order */
static inline int cpufreq_table_find_index_ah(struct cpufreq_policy *policy,
unsigned int target_freq,
@@ -920,12 +928,12 @@
return best;
}
-/* Works only on sorted freq-tables */
-static inline int cpufreq_table_find_index_h(struct cpufreq_policy *policy,
- unsigned int target_freq,
- bool efficiencies)
+static inline int find_index_h(struct cpufreq_policy *policy,
+ unsigned int target_freq,
+ unsigned int min, unsigned int max,
+ bool efficiencies)
{
- target_freq = clamp_val(target_freq, policy->min, policy->max);
+ target_freq = clamp_val(target_freq, min, max);
if (policy->freq_table_sorted == CPUFREQ_TABLE_SORTED_ASCENDING)
return cpufreq_table_find_index_ah(policy, target_freq,
@@ -935,6 +943,14 @@
efficiencies);
}
+/* Works only on sorted freq-tables */
+static inline int cpufreq_table_find_index_h(struct cpufreq_policy *policy,
+ unsigned int target_freq,
+ bool efficiencies)
+{
+ return find_index_h(policy, target_freq, policy->min, policy->max, efficiencies);
+}
+
/* Find closest freq to target in a table in ascending order */
static inline int cpufreq_table_find_index_ac(struct cpufreq_policy *policy,
unsigned int target_freq,
@@ -1005,12 +1021,12 @@
return best;
}
-/* Works only on sorted freq-tables */
-static inline int cpufreq_table_find_index_c(struct cpufreq_policy *policy,
- unsigned int target_freq,
- bool efficiencies)
+static inline int find_index_c(struct cpufreq_policy *policy,
+ unsigned int target_freq,
+ unsigned int min, unsigned int max,
+ bool efficiencies)
{
- target_freq = clamp_val(target_freq, policy->min, policy->max);
+ target_freq = clamp_val(target_freq, min, max);
if (policy->freq_table_sorted == CPUFREQ_TABLE_SORTED_ASCENDING)
return cpufreq_table_find_index_ac(policy, target_freq,
@@ -1020,7 +1036,17 @@
efficiencies);
}
-static inline bool cpufreq_is_in_limits(struct cpufreq_policy *policy, int idx)
+/* Works only on sorted freq-tables */
+static inline int cpufreq_table_find_index_c(struct cpufreq_policy *policy,
+ unsigned int target_freq,
+ bool efficiencies)
+{
+ return find_index_c(policy, target_freq, policy->min, policy->max, efficiencies);
+}
+
+static inline bool cpufreq_is_in_limits(struct cpufreq_policy *policy,
+ unsigned int min, unsigned int max,
+ int idx)
{
unsigned int freq;
@@ -1029,11 +1055,13 @@
freq = policy->freq_table[idx].frequency;
- return freq == clamp_val(freq, policy->min, policy->max);
+ return freq == clamp_val(freq, min, max);
}
static inline int cpufreq_frequency_table_target(struct cpufreq_policy *policy,
unsigned int target_freq,
+ unsigned int min,
+ unsigned int max,
unsigned int relation)
{
bool efficiencies = policy->efficiencies_available &&
@@ -1044,29 +1072,26 @@
relation &= ~CPUFREQ_RELATION_E;
if (unlikely(policy->freq_table_sorted == CPUFREQ_TABLE_UNSORTED))
- return cpufreq_table_index_unsorted(policy, target_freq,
- relation);
+ return cpufreq_table_index_unsorted(policy, target_freq, min,
+ max, relation);
retry:
switch (relation) {
case CPUFREQ_RELATION_L:
- idx = cpufreq_table_find_index_l(policy, target_freq,
- efficiencies);
+ idx = find_index_l(policy, target_freq, min, max, efficiencies);
break;
case CPUFREQ_RELATION_H:
- idx = cpufreq_table_find_index_h(policy, target_freq,
- efficiencies);
+ idx = find_index_h(policy, target_freq, min, max, efficiencies);
break;
case CPUFREQ_RELATION_C:
- idx = cpufreq_table_find_index_c(policy, target_freq,
- efficiencies);
+ idx = find_index_c(policy, target_freq, min, max, efficiencies);
break;
default:
WARN_ON_ONCE(1);
return 0;
}
- /* Limit frequency index to honor policy->min/max */
- if (!cpufreq_is_in_limits(policy, idx) && efficiencies) {
+ /* Limit frequency index to honor min and max */
+ if (!cpufreq_is_in_limits(policy, min, max, idx) && efficiencies) {
efficiencies = false;
goto retry;
}
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] cpufreq: Fix setting policy limits when frequency tables are used
2025-04-25 11:36 [PATCH v2] cpufreq: Fix setting policy limits when frequency tables are used Rafael J. Wysocki
@ 2025-04-25 13:21 ` Stephan Gerhold
2025-04-25 13:30 ` Rafael J. Wysocki
2025-04-27 2:25 ` zhenglifeng (A)
1 sibling, 1 reply; 6+ messages in thread
From: Stephan Gerhold @ 2025-04-25 13:21 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Linux PM, Christian Loehle, LKML, Viresh Kumar,
Srinivas Pandruvada, Mario Limonciello, Sultan Alsawaf,
Rafael J. Wysocki, Johan Hovold
On Fri, Apr 25, 2025 at 01:36:21PM +0200, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> Commit 7491cdf46b5c ("cpufreq: Avoid using inconsistent policy->min and
> policy->max") overlooked the fact that policy->min and policy->max were
> accessed directly in cpufreq_frequency_table_target() and in the
> functions called by it. Consequently, the changes made by that commit
> led to problems with setting policy limits.
>
> Address this by passing the target frequency limits to __resolve_freq()
> and cpufreq_frequency_table_target() and propagating them to the
> functions called by the latter.
>
> Fixes: 7491cdf46b5c ("cpufreq: Avoid using inconsistent policy->min and policy->max")
> Link: https://lore.kernel.org/linux-pm/aAplED3IA_J0eZN0@linaro.org/
> Reported-by: Stephan Gerhold <stephan.gerhold@linaro.org>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Thanks a lot for the quick fix! It works for me. After the CPU frequency
was throttled due to high temperature and the device has cooled down,
the CPU frequency goes back to maximum again.
Tested-by: Stephan Gerhold <stephan.gerhold@linaro.org>
> ---
>
> The v1 is here: https://lore.kernel.org/linux-pm/12665363.O9o76ZdvQC@rjwysocki.net/
>
> v1 -> v2:
> * Do clamp_val(target_freq, min, max) before checking freq_table against
> NULL in __resolve_freq().
> * Update comment in cpufreq_frequency_table_target() to match the new code.
>
> ---
> drivers/cpufreq/cpufreq.c | 22 ++++++---
> drivers/cpufreq/cpufreq_ondemand.c | 3 -
> drivers/cpufreq/freq_table.c | 6 +-
> include/linux/cpufreq.h | 83 ++++++++++++++++++++++++-------------
> 4 files changed, 73 insertions(+), 41 deletions(-)
>
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -491,14 +491,18 @@
> EXPORT_SYMBOL_GPL(cpufreq_disable_fast_switch);
>
> static unsigned int __resolve_freq(struct cpufreq_policy *policy,
> - unsigned int target_freq, unsigned int relation)
> + unsigned int target_freq,
> + unsigned int min, unsigned int max,
> + unsigned int relation)
> {
> unsigned int idx;
>
> + target_freq = clamp_val(target_freq, min, max);
> +
> if (!policy->freq_table)
> return target_freq;
>
> - idx = cpufreq_frequency_table_target(policy, target_freq, relation);
> + idx = cpufreq_frequency_table_target(policy, target_freq, min, max, relation);
> policy->cached_resolved_idx = idx;
> policy->cached_target_freq = target_freq;
> return policy->freq_table[idx].frequency;
> @@ -532,8 +536,7 @@
> if (unlikely(min > max))
> min = max;
>
> - return __resolve_freq(policy, clamp_val(target_freq, min, max),
> - CPUFREQ_RELATION_LE);
> + return __resolve_freq(policy, target_freq, min, max, CPUFREQ_RELATION_LE);
> }
> EXPORT_SYMBOL_GPL(cpufreq_driver_resolve_freq);
>
> @@ -2351,8 +2354,8 @@
> if (cpufreq_disabled())
> return -ENODEV;
>
> - target_freq = clamp_val(target_freq, policy->min, policy->max);
> - target_freq = __resolve_freq(policy, target_freq, relation);
> + target_freq = __resolve_freq(policy, target_freq, policy->min,
> + policy->max, relation);
>
> pr_debug("target for CPU %u: %u kHz, relation %u, requested %u kHz\n",
> policy->cpu, target_freq, relation, old_target_freq);
> @@ -2650,8 +2653,11 @@
> * compiler optimizations around them because they may be accessed
> * concurrently by cpufreq_driver_resolve_freq() during the update.
> */
> - WRITE_ONCE(policy->max, __resolve_freq(policy, new_data.max, CPUFREQ_RELATION_H));
> - new_data.min = __resolve_freq(policy, new_data.min, CPUFREQ_RELATION_L);
> + WRITE_ONCE(policy->max, __resolve_freq(policy, new_data.max,
> + new_data.min, new_data.max,
> + CPUFREQ_RELATION_H));
> + new_data.min = __resolve_freq(policy, new_data.min, new_data.min,
> + new_data.max, CPUFREQ_RELATION_L);
> WRITE_ONCE(policy->min, new_data.min > policy->max ? policy->max : new_data.min);
>
> trace_cpu_frequency_limits(policy);
> --- a/drivers/cpufreq/cpufreq_ondemand.c
> +++ b/drivers/cpufreq/cpufreq_ondemand.c
> @@ -76,7 +76,8 @@
> return freq_next;
> }
>
> - index = cpufreq_frequency_table_target(policy, freq_next, relation);
> + index = cpufreq_frequency_table_target(policy, freq_next, policy->min,
> + policy->max, relation);
> freq_req = freq_table[index].frequency;
> freq_reduc = freq_req * od_tuners->powersave_bias / 1000;
> freq_avg = freq_req - freq_reduc;
> --- a/drivers/cpufreq/freq_table.c
> +++ b/drivers/cpufreq/freq_table.c
> @@ -115,8 +115,8 @@
> EXPORT_SYMBOL_GPL(cpufreq_generic_frequency_table_verify);
>
> int cpufreq_table_index_unsorted(struct cpufreq_policy *policy,
> - unsigned int target_freq,
> - unsigned int relation)
> + unsigned int target_freq, unsigned int min,
> + unsigned int max, unsigned int relation)
> {
> struct cpufreq_frequency_table optimal = {
> .driver_data = ~0,
> @@ -147,7 +147,7 @@
> cpufreq_for_each_valid_entry_idx(pos, table, i) {
> freq = pos->frequency;
>
> - if ((freq < policy->min) || (freq > policy->max))
> + if (freq < min || freq > max)
> continue;
> if (freq == target_freq) {
> optimal.driver_data = i;
> --- a/include/linux/cpufreq.h
> +++ b/include/linux/cpufreq.h
> @@ -788,8 +788,8 @@
> int cpufreq_generic_frequency_table_verify(struct cpufreq_policy_data *policy);
>
> int cpufreq_table_index_unsorted(struct cpufreq_policy *policy,
> - unsigned int target_freq,
> - unsigned int relation);
> + unsigned int target_freq, unsigned int min,
> + unsigned int max, unsigned int relation);
> int cpufreq_frequency_table_get_index(struct cpufreq_policy *policy,
> unsigned int freq);
>
> @@ -852,12 +852,12 @@
> return best;
> }
>
> -/* Works only on sorted freq-tables */
> -static inline int cpufreq_table_find_index_l(struct cpufreq_policy *policy,
> - unsigned int target_freq,
> - bool efficiencies)
> +static inline int find_index_l(struct cpufreq_policy *policy,
> + unsigned int target_freq,
> + unsigned int min, unsigned int max,
> + bool efficiencies)
> {
> - target_freq = clamp_val(target_freq, policy->min, policy->max);
> + target_freq = clamp_val(target_freq, min, max);
>
> if (policy->freq_table_sorted == CPUFREQ_TABLE_SORTED_ASCENDING)
> return cpufreq_table_find_index_al(policy, target_freq,
> @@ -867,6 +867,14 @@
> efficiencies);
> }
>
> +/* Works only on sorted freq-tables */
> +static inline int cpufreq_table_find_index_l(struct cpufreq_policy *policy,
> + unsigned int target_freq,
> + bool efficiencies)
> +{
> + return find_index_l(policy, target_freq, policy->min, policy->max, efficiencies);
> +}
> +
> /* Find highest freq at or below target in a table in ascending order */
> static inline int cpufreq_table_find_index_ah(struct cpufreq_policy *policy,
> unsigned int target_freq,
> @@ -920,12 +928,12 @@
> return best;
> }
>
> -/* Works only on sorted freq-tables */
> -static inline int cpufreq_table_find_index_h(struct cpufreq_policy *policy,
> - unsigned int target_freq,
> - bool efficiencies)
> +static inline int find_index_h(struct cpufreq_policy *policy,
> + unsigned int target_freq,
> + unsigned int min, unsigned int max,
> + bool efficiencies)
> {
> - target_freq = clamp_val(target_freq, policy->min, policy->max);
> + target_freq = clamp_val(target_freq, min, max);
>
> if (policy->freq_table_sorted == CPUFREQ_TABLE_SORTED_ASCENDING)
> return cpufreq_table_find_index_ah(policy, target_freq,
> @@ -935,6 +943,14 @@
> efficiencies);
> }
>
> +/* Works only on sorted freq-tables */
> +static inline int cpufreq_table_find_index_h(struct cpufreq_policy *policy,
> + unsigned int target_freq,
> + bool efficiencies)
> +{
> + return find_index_h(policy, target_freq, policy->min, policy->max, efficiencies);
> +}
> +
> /* Find closest freq to target in a table in ascending order */
> static inline int cpufreq_table_find_index_ac(struct cpufreq_policy *policy,
> unsigned int target_freq,
> @@ -1005,12 +1021,12 @@
> return best;
> }
>
> -/* Works only on sorted freq-tables */
> -static inline int cpufreq_table_find_index_c(struct cpufreq_policy *policy,
> - unsigned int target_freq,
> - bool efficiencies)
> +static inline int find_index_c(struct cpufreq_policy *policy,
> + unsigned int target_freq,
> + unsigned int min, unsigned int max,
> + bool efficiencies)
> {
> - target_freq = clamp_val(target_freq, policy->min, policy->max);
> + target_freq = clamp_val(target_freq, min, max);
>
> if (policy->freq_table_sorted == CPUFREQ_TABLE_SORTED_ASCENDING)
> return cpufreq_table_find_index_ac(policy, target_freq,
> @@ -1020,7 +1036,17 @@
> efficiencies);
> }
>
> -static inline bool cpufreq_is_in_limits(struct cpufreq_policy *policy, int idx)
> +/* Works only on sorted freq-tables */
> +static inline int cpufreq_table_find_index_c(struct cpufreq_policy *policy,
> + unsigned int target_freq,
> + bool efficiencies)
> +{
> + return find_index_c(policy, target_freq, policy->min, policy->max, efficiencies);
> +}
> +
> +static inline bool cpufreq_is_in_limits(struct cpufreq_policy *policy,
> + unsigned int min, unsigned int max,
> + int idx)
> {
> unsigned int freq;
>
> @@ -1029,11 +1055,13 @@
>
> freq = policy->freq_table[idx].frequency;
>
> - return freq == clamp_val(freq, policy->min, policy->max);
> + return freq == clamp_val(freq, min, max);
> }
>
> static inline int cpufreq_frequency_table_target(struct cpufreq_policy *policy,
> unsigned int target_freq,
> + unsigned int min,
> + unsigned int max,
> unsigned int relation)
> {
> bool efficiencies = policy->efficiencies_available &&
> @@ -1044,29 +1072,26 @@
> relation &= ~CPUFREQ_RELATION_E;
>
> if (unlikely(policy->freq_table_sorted == CPUFREQ_TABLE_UNSORTED))
> - return cpufreq_table_index_unsorted(policy, target_freq,
> - relation);
> + return cpufreq_table_index_unsorted(policy, target_freq, min,
> + max, relation);
> retry:
> switch (relation) {
> case CPUFREQ_RELATION_L:
> - idx = cpufreq_table_find_index_l(policy, target_freq,
> - efficiencies);
> + idx = find_index_l(policy, target_freq, min, max, efficiencies);
> break;
> case CPUFREQ_RELATION_H:
> - idx = cpufreq_table_find_index_h(policy, target_freq,
> - efficiencies);
> + idx = find_index_h(policy, target_freq, min, max, efficiencies);
> break;
> case CPUFREQ_RELATION_C:
> - idx = cpufreq_table_find_index_c(policy, target_freq,
> - efficiencies);
> + idx = find_index_c(policy, target_freq, min, max, efficiencies);
> break;
> default:
> WARN_ON_ONCE(1);
> return 0;
> }
>
> - /* Limit frequency index to honor policy->min/max */
> - if (!cpufreq_is_in_limits(policy, idx) && efficiencies) {
> + /* Limit frequency index to honor min and max */
> + if (!cpufreq_is_in_limits(policy, min, max, idx) && efficiencies) {
> efficiencies = false;
> goto retry;
> }
>
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] cpufreq: Fix setting policy limits when frequency tables are used
2025-04-25 13:21 ` Stephan Gerhold
@ 2025-04-25 13:30 ` Rafael J. Wysocki
0 siblings, 0 replies; 6+ messages in thread
From: Rafael J. Wysocki @ 2025-04-25 13:30 UTC (permalink / raw)
To: Stephan Gerhold
Cc: Rafael J. Wysocki, Linux PM, Christian Loehle, LKML,
Viresh Kumar, Srinivas Pandruvada, Mario Limonciello,
Sultan Alsawaf, Rafael J. Wysocki, Johan Hovold
On Fri, Apr 25, 2025 at 3:21 PM Stephan Gerhold
<stephan.gerhold@linaro.org> wrote:
>
> On Fri, Apr 25, 2025 at 01:36:21PM +0200, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >
> > Commit 7491cdf46b5c ("cpufreq: Avoid using inconsistent policy->min and
> > policy->max") overlooked the fact that policy->min and policy->max were
> > accessed directly in cpufreq_frequency_table_target() and in the
> > functions called by it. Consequently, the changes made by that commit
> > led to problems with setting policy limits.
> >
> > Address this by passing the target frequency limits to __resolve_freq()
> > and cpufreq_frequency_table_target() and propagating them to the
> > functions called by the latter.
> >
> > Fixes: 7491cdf46b5c ("cpufreq: Avoid using inconsistent policy->min and policy->max")
> > Link: https://lore.kernel.org/linux-pm/aAplED3IA_J0eZN0@linaro.org/
> > Reported-by: Stephan Gerhold <stephan.gerhold@linaro.org>
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> Thanks a lot for the quick fix! It works for me. After the CPU frequency
> was throttled due to high temperature and the device has cooled down,
> the CPU frequency goes back to maximum again.
>
> Tested-by: Stephan Gerhold <stephan.gerhold@linaro.org>
Thanks for the quick turnaround!
I want it to spend at least a couple of days in linux-next and I'd
like to give people a chance to review it in case something is still
missing, so my current plan is to push it for -rc5.
Thank you!
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] cpufreq: Fix setting policy limits when frequency tables are used
2025-04-25 11:36 [PATCH v2] cpufreq: Fix setting policy limits when frequency tables are used Rafael J. Wysocki
2025-04-25 13:21 ` Stephan Gerhold
@ 2025-04-27 2:25 ` zhenglifeng (A)
2025-04-27 11:41 ` Rafael J. Wysocki
1 sibling, 1 reply; 6+ messages in thread
From: zhenglifeng (A) @ 2025-04-27 2:25 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Linux PM, Christian Loehle, LKML, Viresh Kumar,
Srinivas Pandruvada, Mario Limonciello, Sultan Alsawaf,
Stephan Gerhold, Rafael J. Wysocki
On 2025/4/25 19:36, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> Commit 7491cdf46b5c ("cpufreq: Avoid using inconsistent policy->min and
> policy->max") overlooked the fact that policy->min and policy->max were
> accessed directly in cpufreq_frequency_table_target() and in the
> functions called by it. Consequently, the changes made by that commit
> led to problems with setting policy limits.
>
> Address this by passing the target frequency limits to __resolve_freq()
> and cpufreq_frequency_table_target() and propagating them to the
> functions called by the latter.
>
> Fixes: 7491cdf46b5c ("cpufreq: Avoid using inconsistent policy->min and policy->max")
> Link: https://lore.kernel.org/linux-pm/aAplED3IA_J0eZN0@linaro.org/
> Reported-by: Stephan Gerhold <stephan.gerhold@linaro.org>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>
> The v1 is here: https://lore.kernel.org/linux-pm/12665363.O9o76ZdvQC@rjwysocki.net/
>
> v1 -> v2:
> * Do clamp_val(target_freq, min, max) before checking freq_table against
> NULL in __resolve_freq().
> * Update comment in cpufreq_frequency_table_target() to match the new code.
>
> ---
> drivers/cpufreq/cpufreq.c | 22 ++++++---
> drivers/cpufreq/cpufreq_ondemand.c | 3 -
> drivers/cpufreq/freq_table.c | 6 +-
> include/linux/cpufreq.h | 83 ++++++++++++++++++++++++-------------
> 4 files changed, 73 insertions(+), 41 deletions(-)
>
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -491,14 +491,18 @@
> EXPORT_SYMBOL_GPL(cpufreq_disable_fast_switch);
>
> static unsigned int __resolve_freq(struct cpufreq_policy *policy,
> - unsigned int target_freq, unsigned int relation)
> + unsigned int target_freq,
> + unsigned int min, unsigned int max,
> + unsigned int relation)
> {
> unsigned int idx;
>
> + target_freq = clamp_val(target_freq, min, max);
> +
> if (!policy->freq_table)
> return target_freq;
>
> - idx = cpufreq_frequency_table_target(policy, target_freq, relation);
> + idx = cpufreq_frequency_table_target(policy, target_freq, min, max, relation);
> policy->cached_resolved_idx = idx;
> policy->cached_target_freq = target_freq;
> return policy->freq_table[idx].frequency;
> @@ -532,8 +536,7 @@
> if (unlikely(min > max))
> min = max;
>
> - return __resolve_freq(policy, clamp_val(target_freq, min, max),
> - CPUFREQ_RELATION_LE);
> + return __resolve_freq(policy, target_freq, min, max, CPUFREQ_RELATION_LE);
> }
> EXPORT_SYMBOL_GPL(cpufreq_driver_resolve_freq);
>
> @@ -2351,8 +2354,8 @@
> if (cpufreq_disabled())
> return -ENODEV;
>
> - target_freq = clamp_val(target_freq, policy->min, policy->max);
> - target_freq = __resolve_freq(policy, target_freq, relation);
> + target_freq = __resolve_freq(policy, target_freq, policy->min,
> + policy->max, relation);
>
> pr_debug("target for CPU %u: %u kHz, relation %u, requested %u kHz\n",
> policy->cpu, target_freq, relation, old_target_freq);
> @@ -2650,8 +2653,11 @@
> * compiler optimizations around them because they may be accessed
> * concurrently by cpufreq_driver_resolve_freq() during the update.
> */
> - WRITE_ONCE(policy->max, __resolve_freq(policy, new_data.max, CPUFREQ_RELATION_H));
> - new_data.min = __resolve_freq(policy, new_data.min, CPUFREQ_RELATION_L);
> + WRITE_ONCE(policy->max, __resolve_freq(policy, new_data.max,
> + new_data.min, new_data.max,
> + CPUFREQ_RELATION_H));
> + new_data.min = __resolve_freq(policy, new_data.min, new_data.min,
> + new_data.max, CPUFREQ_RELATION_L);
> WRITE_ONCE(policy->min, new_data.min > policy->max ? policy->max : new_data.min);
It might be better like:
- WRITE_ONCE(policy->max, __resolve_freq(policy, new_data.max, CPUFREQ_RELATION_H));
- new_data.min = __resolve_freq(policy, new_data.min, CPUFREQ_RELATION_L);
- WRITE_ONCE(policy->min, new_data.min > policy->max ? policy->max : new_data.min);
+ WRITE_ONCE(policy->max, __resolve_freq(policy, new_data.max,
+ new_data.min, new_data.max,
+ CPUFREQ_RELATION_H));
+ WRITE_ONCE(policy->min, __resolve_freq(policy, new_data.min,
+ new_data.min, policy->max,
+ CPUFREQ_RELATION_L));
>
> trace_cpu_frequency_limits(policy);
> --- a/drivers/cpufreq/cpufreq_ondemand.c
> +++ b/drivers/cpufreq/cpufreq_ondemand.c
> @@ -76,7 +76,8 @@
> return freq_next;
> }
>
> - index = cpufreq_frequency_table_target(policy, freq_next, relation);
> + index = cpufreq_frequency_table_target(policy, freq_next, policy->min,
> + policy->max, relation);
> freq_req = freq_table[index].frequency;
> freq_reduc = freq_req * od_tuners->powersave_bias / 1000;
> freq_avg = freq_req - freq_reduc;
> --- a/drivers/cpufreq/freq_table.c
> +++ b/drivers/cpufreq/freq_table.c
> @@ -115,8 +115,8 @@
> EXPORT_SYMBOL_GPL(cpufreq_generic_frequency_table_verify);
>
> int cpufreq_table_index_unsorted(struct cpufreq_policy *policy,
> - unsigned int target_freq,
> - unsigned int relation)
> + unsigned int target_freq, unsigned int min,
> + unsigned int max, unsigned int relation)
> {
> struct cpufreq_frequency_table optimal = {
> .driver_data = ~0,
> @@ -147,7 +147,7 @@
> cpufreq_for_each_valid_entry_idx(pos, table, i) {
> freq = pos->frequency;
>
> - if ((freq < policy->min) || (freq > policy->max))
> + if (freq < min || freq > max)
> continue;
> if (freq == target_freq) {
> optimal.driver_data = i;
> --- a/include/linux/cpufreq.h
> +++ b/include/linux/cpufreq.h
> @@ -788,8 +788,8 @@
> int cpufreq_generic_frequency_table_verify(struct cpufreq_policy_data *policy);
>
> int cpufreq_table_index_unsorted(struct cpufreq_policy *policy,
> - unsigned int target_freq,
> - unsigned int relation);
> + unsigned int target_freq, unsigned int min,
> + unsigned int max, unsigned int relation);
> int cpufreq_frequency_table_get_index(struct cpufreq_policy *policy,
> unsigned int freq);
>
> @@ -852,12 +852,12 @@
> return best;
> }
>
> -/* Works only on sorted freq-tables */
> -static inline int cpufreq_table_find_index_l(struct cpufreq_policy *policy,
> - unsigned int target_freq,
> - bool efficiencies)
> +static inline int find_index_l(struct cpufreq_policy *policy,
> + unsigned int target_freq,
> + unsigned int min, unsigned int max,
> + bool efficiencies)
> {
> - target_freq = clamp_val(target_freq, policy->min, policy->max);
> + target_freq = clamp_val(target_freq, min, max);
>
> if (policy->freq_table_sorted == CPUFREQ_TABLE_SORTED_ASCENDING)
> return cpufreq_table_find_index_al(policy, target_freq,
> @@ -867,6 +867,14 @@
> efficiencies);
> }
>
> +/* Works only on sorted freq-tables */
> +static inline int cpufreq_table_find_index_l(struct cpufreq_policy *policy,
> + unsigned int target_freq,
> + bool efficiencies)
> +{
> + return find_index_l(policy, target_freq, policy->min, policy->max, efficiencies);
> +}
> +
> /* Find highest freq at or below target in a table in ascending order */
> static inline int cpufreq_table_find_index_ah(struct cpufreq_policy *policy,
> unsigned int target_freq,
> @@ -920,12 +928,12 @@
> return best;
> }
>
> -/* Works only on sorted freq-tables */
> -static inline int cpufreq_table_find_index_h(struct cpufreq_policy *policy,
> - unsigned int target_freq,
> - bool efficiencies)
> +static inline int find_index_h(struct cpufreq_policy *policy,
> + unsigned int target_freq,
> + unsigned int min, unsigned int max,
> + bool efficiencies)
> {
> - target_freq = clamp_val(target_freq, policy->min, policy->max);
> + target_freq = clamp_val(target_freq, min, max);
>
> if (policy->freq_table_sorted == CPUFREQ_TABLE_SORTED_ASCENDING)
> return cpufreq_table_find_index_ah(policy, target_freq,
> @@ -935,6 +943,14 @@
> efficiencies);
> }
>
> +/* Works only on sorted freq-tables */
> +static inline int cpufreq_table_find_index_h(struct cpufreq_policy *policy,
> + unsigned int target_freq,
> + bool efficiencies)
> +{
> + return find_index_h(policy, target_freq, policy->min, policy->max, efficiencies);
> +}
> +
> /* Find closest freq to target in a table in ascending order */
> static inline int cpufreq_table_find_index_ac(struct cpufreq_policy *policy,
> unsigned int target_freq,
> @@ -1005,12 +1021,12 @@
> return best;
> }
>
> -/* Works only on sorted freq-tables */
> -static inline int cpufreq_table_find_index_c(struct cpufreq_policy *policy,
> - unsigned int target_freq,
> - bool efficiencies)
> +static inline int find_index_c(struct cpufreq_policy *policy,
> + unsigned int target_freq,
> + unsigned int min, unsigned int max,
> + bool efficiencies)
> {
> - target_freq = clamp_val(target_freq, policy->min, policy->max);
> + target_freq = clamp_val(target_freq, min, max);
>
> if (policy->freq_table_sorted == CPUFREQ_TABLE_SORTED_ASCENDING)
> return cpufreq_table_find_index_ac(policy, target_freq,
> @@ -1020,7 +1036,17 @@
> efficiencies);
> }
>
> -static inline bool cpufreq_is_in_limits(struct cpufreq_policy *policy, int idx)
> +/* Works only on sorted freq-tables */
> +static inline int cpufreq_table_find_index_c(struct cpufreq_policy *policy,
> + unsigned int target_freq,
> + bool efficiencies)
> +{
> + return find_index_c(policy, target_freq, policy->min, policy->max, efficiencies);
> +}
> +
> +static inline bool cpufreq_is_in_limits(struct cpufreq_policy *policy,
> + unsigned int min, unsigned int max,
> + int idx)
> {
> unsigned int freq;
>
> @@ -1029,11 +1055,13 @@
>
> freq = policy->freq_table[idx].frequency;
>
> - return freq == clamp_val(freq, policy->min, policy->max);
> + return freq == clamp_val(freq, min, max);
> }
>
> static inline int cpufreq_frequency_table_target(struct cpufreq_policy *policy,
> unsigned int target_freq,
> + unsigned int min,
> + unsigned int max,
> unsigned int relation)
> {
> bool efficiencies = policy->efficiencies_available &&
> @@ -1044,29 +1072,26 @@
> relation &= ~CPUFREQ_RELATION_E;
>
> if (unlikely(policy->freq_table_sorted == CPUFREQ_TABLE_UNSORTED))
> - return cpufreq_table_index_unsorted(policy, target_freq,
> - relation);
> + return cpufreq_table_index_unsorted(policy, target_freq, min,
> + max, relation);
> retry:
> switch (relation) {
> case CPUFREQ_RELATION_L:
> - idx = cpufreq_table_find_index_l(policy, target_freq,
> - efficiencies);
> + idx = find_index_l(policy, target_freq, min, max, efficiencies);
> break;
> case CPUFREQ_RELATION_H:
> - idx = cpufreq_table_find_index_h(policy, target_freq,
> - efficiencies);
> + idx = find_index_h(policy, target_freq, min, max, efficiencies);
> break;
> case CPUFREQ_RELATION_C:
> - idx = cpufreq_table_find_index_c(policy, target_freq,
> - efficiencies);
> + idx = find_index_c(policy, target_freq, min, max, efficiencies);
> break;
> default:
> WARN_ON_ONCE(1);
> return 0;
> }
>
> - /* Limit frequency index to honor policy->min/max */
> - if (!cpufreq_is_in_limits(policy, idx) && efficiencies) {
> + /* Limit frequency index to honor min and max */
> + if (!cpufreq_is_in_limits(policy, min, max, idx) && efficiencies) {
> efficiencies = false;
> goto retry;
> }
>
>
>
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] cpufreq: Fix setting policy limits when frequency tables are used
2025-04-27 2:25 ` zhenglifeng (A)
@ 2025-04-27 11:41 ` Rafael J. Wysocki
2025-04-28 1:44 ` zhenglifeng (A)
0 siblings, 1 reply; 6+ messages in thread
From: Rafael J. Wysocki @ 2025-04-27 11:41 UTC (permalink / raw)
To: zhenglifeng (A)
Cc: Rafael J. Wysocki, Linux PM, Christian Loehle, LKML,
Viresh Kumar, Srinivas Pandruvada, Mario Limonciello,
Sultan Alsawaf, Stephan Gerhold, Rafael J. Wysocki
On Sun, Apr 27, 2025 at 4:26 AM zhenglifeng (A) <zhenglifeng1@huawei.com> wrote:
>
> On 2025/4/25 19:36, Rafael J. Wysocki wrote:
>
> > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >
> > Commit 7491cdf46b5c ("cpufreq: Avoid using inconsistent policy->min and
> > policy->max") overlooked the fact that policy->min and policy->max were
> > accessed directly in cpufreq_frequency_table_target() and in the
> > functions called by it. Consequently, the changes made by that commit
> > led to problems with setting policy limits.
> >
> > Address this by passing the target frequency limits to __resolve_freq()
> > and cpufreq_frequency_table_target() and propagating them to the
> > functions called by the latter.
> >
> > Fixes: 7491cdf46b5c ("cpufreq: Avoid using inconsistent policy->min and policy->max")
> > Link: https://lore.kernel.org/linux-pm/aAplED3IA_J0eZN0@linaro.org/
> > Reported-by: Stephan Gerhold <stephan.gerhold@linaro.org>
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > ---
> >
> > The v1 is here: https://lore.kernel.org/linux-pm/12665363.O9o76ZdvQC@rjwysocki.net/
> >
> > v1 -> v2:
> > * Do clamp_val(target_freq, min, max) before checking freq_table against
> > NULL in __resolve_freq().
> > * Update comment in cpufreq_frequency_table_target() to match the new code.
> >
> > ---
> > drivers/cpufreq/cpufreq.c | 22 ++++++---
> > drivers/cpufreq/cpufreq_ondemand.c | 3 -
> > drivers/cpufreq/freq_table.c | 6 +-
> > include/linux/cpufreq.h | 83 ++++++++++++++++++++++++-------------
> > 4 files changed, 73 insertions(+), 41 deletions(-)
> >
> > --- a/drivers/cpufreq/cpufreq.c
> > +++ b/drivers/cpufreq/cpufreq.c
> > @@ -491,14 +491,18 @@
> > EXPORT_SYMBOL_GPL(cpufreq_disable_fast_switch);
> >
> > static unsigned int __resolve_freq(struct cpufreq_policy *policy,
> > - unsigned int target_freq, unsigned int relation)
> > + unsigned int target_freq,
> > + unsigned int min, unsigned int max,
> > + unsigned int relation)
> > {
> > unsigned int idx;
> >
> > + target_freq = clamp_val(target_freq, min, max);
> > +
> > if (!policy->freq_table)
> > return target_freq;
> >
> > - idx = cpufreq_frequency_table_target(policy, target_freq, relation);
> > + idx = cpufreq_frequency_table_target(policy, target_freq, min, max, relation);
> > policy->cached_resolved_idx = idx;
> > policy->cached_target_freq = target_freq;
> > return policy->freq_table[idx].frequency;
> > @@ -532,8 +536,7 @@
> > if (unlikely(min > max))
> > min = max;
> >
> > - return __resolve_freq(policy, clamp_val(target_freq, min, max),
> > - CPUFREQ_RELATION_LE);
> > + return __resolve_freq(policy, target_freq, min, max, CPUFREQ_RELATION_LE);
> > }
> > EXPORT_SYMBOL_GPL(cpufreq_driver_resolve_freq);
> >
> > @@ -2351,8 +2354,8 @@
> > if (cpufreq_disabled())
> > return -ENODEV;
> >
> > - target_freq = clamp_val(target_freq, policy->min, policy->max);
> > - target_freq = __resolve_freq(policy, target_freq, relation);
> > + target_freq = __resolve_freq(policy, target_freq, policy->min,
> > + policy->max, relation);
> >
> > pr_debug("target for CPU %u: %u kHz, relation %u, requested %u kHz\n",
> > policy->cpu, target_freq, relation, old_target_freq);
> > @@ -2650,8 +2653,11 @@
> > * compiler optimizations around them because they may be accessed
> > * concurrently by cpufreq_driver_resolve_freq() during the update.
> > */
> > - WRITE_ONCE(policy->max, __resolve_freq(policy, new_data.max, CPUFREQ_RELATION_H));
> > - new_data.min = __resolve_freq(policy, new_data.min, CPUFREQ_RELATION_L);
> > + WRITE_ONCE(policy->max, __resolve_freq(policy, new_data.max,
> > + new_data.min, new_data.max,
> > + CPUFREQ_RELATION_H));
> > + new_data.min = __resolve_freq(policy, new_data.min, new_data.min,
> > + new_data.max, CPUFREQ_RELATION_L);
> > WRITE_ONCE(policy->min, new_data.min > policy->max ? policy->max : new_data.min);
>
> It might be better like:
>
> - WRITE_ONCE(policy->max, __resolve_freq(policy, new_data.max, CPUFREQ_RELATION_H));
> - new_data.min = __resolve_freq(policy, new_data.min, CPUFREQ_RELATION_L);
> - WRITE_ONCE(policy->min, new_data.min > policy->max ? policy->max : new_data.min);
> + WRITE_ONCE(policy->max, __resolve_freq(policy, new_data.max,
> + new_data.min, new_data.max,
> + CPUFREQ_RELATION_H));
> + WRITE_ONCE(policy->min, __resolve_freq(policy, new_data.min,
> + new_data.min, policy->max,
> + CPUFREQ_RELATION_L));
>
Not really because policy->max may be less than new_data.min at this
point AFAICS.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] cpufreq: Fix setting policy limits when frequency tables are used
2025-04-27 11:41 ` Rafael J. Wysocki
@ 2025-04-28 1:44 ` zhenglifeng (A)
0 siblings, 0 replies; 6+ messages in thread
From: zhenglifeng (A) @ 2025-04-28 1:44 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Rafael J. Wysocki, Linux PM, Christian Loehle, LKML,
Viresh Kumar, Srinivas Pandruvada, Mario Limonciello,
Sultan Alsawaf, Stephan Gerhold, Rafael J. Wysocki
On 2025/4/27 19:41, Rafael J. Wysocki wrote:
> On Sun, Apr 27, 2025 at 4:26 AM zhenglifeng (A) <zhenglifeng1@huawei.com> wrote:
>>
>> On 2025/4/25 19:36, Rafael J. Wysocki wrote:
>>
>>> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>>>
>>> Commit 7491cdf46b5c ("cpufreq: Avoid using inconsistent policy->min and
>>> policy->max") overlooked the fact that policy->min and policy->max were
>>> accessed directly in cpufreq_frequency_table_target() and in the
>>> functions called by it. Consequently, the changes made by that commit
>>> led to problems with setting policy limits.
>>>
>>> Address this by passing the target frequency limits to __resolve_freq()
>>> and cpufreq_frequency_table_target() and propagating them to the
>>> functions called by the latter.
>>>
>>> Fixes: 7491cdf46b5c ("cpufreq: Avoid using inconsistent policy->min and policy->max")
>>> Link: https://lore.kernel.org/linux-pm/aAplED3IA_J0eZN0@linaro.org/
>>> Reported-by: Stephan Gerhold <stephan.gerhold@linaro.org>
>>> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>>> ---
>>>
>>> The v1 is here: https://lore.kernel.org/linux-pm/12665363.O9o76ZdvQC@rjwysocki.net/
>>>
>>> v1 -> v2:
>>> * Do clamp_val(target_freq, min, max) before checking freq_table against
>>> NULL in __resolve_freq().
>>> * Update comment in cpufreq_frequency_table_target() to match the new code.
>>>
>>> ---
>>> drivers/cpufreq/cpufreq.c | 22 ++++++---
>>> drivers/cpufreq/cpufreq_ondemand.c | 3 -
>>> drivers/cpufreq/freq_table.c | 6 +-
>>> include/linux/cpufreq.h | 83 ++++++++++++++++++++++++-------------
>>> 4 files changed, 73 insertions(+), 41 deletions(-)
>>>
>>> --- a/drivers/cpufreq/cpufreq.c
>>> +++ b/drivers/cpufreq/cpufreq.c
>>> @@ -491,14 +491,18 @@
>>> EXPORT_SYMBOL_GPL(cpufreq_disable_fast_switch);
>>>
>>> static unsigned int __resolve_freq(struct cpufreq_policy *policy,
>>> - unsigned int target_freq, unsigned int relation)
>>> + unsigned int target_freq,
>>> + unsigned int min, unsigned int max,
>>> + unsigned int relation)
>>> {
>>> unsigned int idx;
>>>
>>> + target_freq = clamp_val(target_freq, min, max);
>>> +
>>> if (!policy->freq_table)
>>> return target_freq;
>>>
>>> - idx = cpufreq_frequency_table_target(policy, target_freq, relation);
>>> + idx = cpufreq_frequency_table_target(policy, target_freq, min, max, relation);
>>> policy->cached_resolved_idx = idx;
>>> policy->cached_target_freq = target_freq;
>>> return policy->freq_table[idx].frequency;
>>> @@ -532,8 +536,7 @@
>>> if (unlikely(min > max))
>>> min = max;
>>>
>>> - return __resolve_freq(policy, clamp_val(target_freq, min, max),
>>> - CPUFREQ_RELATION_LE);
>>> + return __resolve_freq(policy, target_freq, min, max, CPUFREQ_RELATION_LE);
>>> }
>>> EXPORT_SYMBOL_GPL(cpufreq_driver_resolve_freq);
>>>
>>> @@ -2351,8 +2354,8 @@
>>> if (cpufreq_disabled())
>>> return -ENODEV;
>>>
>>> - target_freq = clamp_val(target_freq, policy->min, policy->max);
>>> - target_freq = __resolve_freq(policy, target_freq, relation);
>>> + target_freq = __resolve_freq(policy, target_freq, policy->min,
>>> + policy->max, relation);
>>>
>>> pr_debug("target for CPU %u: %u kHz, relation %u, requested %u kHz\n",
>>> policy->cpu, target_freq, relation, old_target_freq);
>>> @@ -2650,8 +2653,11 @@
>>> * compiler optimizations around them because they may be accessed
>>> * concurrently by cpufreq_driver_resolve_freq() during the update.
>>> */
>>> - WRITE_ONCE(policy->max, __resolve_freq(policy, new_data.max, CPUFREQ_RELATION_H));
>>> - new_data.min = __resolve_freq(policy, new_data.min, CPUFREQ_RELATION_L);
>>> + WRITE_ONCE(policy->max, __resolve_freq(policy, new_data.max,
>>> + new_data.min, new_data.max,
>>> + CPUFREQ_RELATION_H));
>>> + new_data.min = __resolve_freq(policy, new_data.min, new_data.min,
>>> + new_data.max, CPUFREQ_RELATION_L);
>>> WRITE_ONCE(policy->min, new_data.min > policy->max ? policy->max : new_data.min);
>>
>> It might be better like:
>>
>> - WRITE_ONCE(policy->max, __resolve_freq(policy, new_data.max, CPUFREQ_RELATION_H));
>> - new_data.min = __resolve_freq(policy, new_data.min, CPUFREQ_RELATION_L);
>> - WRITE_ONCE(policy->min, new_data.min > policy->max ? policy->max : new_data.min);
>> + WRITE_ONCE(policy->max, __resolve_freq(policy, new_data.max,
>> + new_data.min, new_data.max,
>> + CPUFREQ_RELATION_H));
>> + WRITE_ONCE(policy->min, __resolve_freq(policy, new_data.min,
>> + new_data.min, policy->max,
>> + CPUFREQ_RELATION_L));
>>
>
> Not really because policy->max may be less than new_data.min at this
> point AFAICS.
I see.
Reviewed-by: Lifeng Zheng <zhenglifeng1@huawei.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-04-28 1:44 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-04-25 11:36 [PATCH v2] cpufreq: Fix setting policy limits when frequency tables are used Rafael J. Wysocki
2025-04-25 13:21 ` Stephan Gerhold
2025-04-25 13:30 ` Rafael J. Wysocki
2025-04-27 2:25 ` zhenglifeng (A)
2025-04-27 11:41 ` Rafael J. Wysocki
2025-04-28 1:44 ` zhenglifeng (A)
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®