cpufreq: Fix setting policy limits when frequency tables are used

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")
Cc: 5.16+ <stable@vger.kernel.org> # 5.16+
Closes: 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>
Tested-by: Stephan Gerhold <stephan.gerhold@linaro.org>
Reviewed-by: Lifeng Zheng <zhenglifeng1@huawei.com>
Link: https://patch.msgid.link/5896780.DvuYhMxLoT@rjwysocki.net

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index acf19b0042bb6aa8d02900e812459a9f22a9384e..f45ded62b0e082a8b0a686b7544128838e823e49 100644 (file)
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -536,14 +536,18 @@ void cpufreq_disable_fast_switch(struct cpufreq_policy *policy)
 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;
@@ -577,8 +581,7 @@ unsigned int cpufreq_driver_resolve_freq(struct cpufreq_policy *policy,
        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);
 
@@ -2397,8 +2400,8 @@ int __cpufreq_driver_target(struct cpufreq_policy *policy,
        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);
@@ -2727,8 +2730,11 @@ static int cpufreq_set_policy(struct cpufreq_policy *policy,
         * 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);

diff --git a/drivers/cpufreq/cpufreq_ondemand.c b/drivers/cpufreq/cpufreq_ondemand.c
index a7c38b8b3e789094106d7d09635b89b39fe12e9b..0e65d37c923113afcc772b5d29372fb65d1fc75b 100644 (file)
--- a/drivers/cpufreq/cpufreq_ondemand.c
+++ b/drivers/cpufreq/cpufreq_ondemand.c
@@ -76,7 +76,8 @@ static unsigned int generic_powersave_bias_target(struct cpufreq_policy *policy,
                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;

diff --git a/drivers/cpufreq/freq_table.c b/drivers/cpufreq/freq_table.c
index c03a91502f8480df9cd26b9fdfff261f4b4ab5d7..35de513af6c94e5ceb210847557d1bd99e149c27 100644 (file)
--- a/drivers/cpufreq/freq_table.c
+++ b/drivers/cpufreq/freq_table.c
@@ -115,8 +115,8 @@ int cpufreq_generic_frequency_table_verify(struct cpufreq_policy_data *policy)
 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 @@ int cpufreq_table_index_unsorted(struct cpufreq_policy *policy,
        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;

diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
index 400fee6427a5c0013c2cba601a0699545b9cde09..7a5b391dcc0176d20ecf2dadd6285ee0ee656514 100644 (file)
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -776,8 +776,8 @@ int cpufreq_frequency_table_verify(struct cpufreq_policy_data *policy,
 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);
 
@@ -840,12 +840,12 @@ static inline int cpufreq_table_find_index_dl(struct cpufreq_policy *policy,
        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,
@@ -855,6 +855,14 @@ static inline int cpufreq_table_find_index_l(struct cpufreq_policy *policy,
                                                   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,
@@ -908,12 +916,12 @@ static inline int cpufreq_table_find_index_dh(struct cpufreq_policy *policy,
        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,
@@ -923,6 +931,14 @@ static inline int cpufreq_table_find_index_h(struct cpufreq_policy *policy,
                                                   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,
@@ -993,12 +1009,12 @@ static inline int cpufreq_table_find_index_dc(struct cpufreq_policy *policy,
        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,
@@ -1008,7 +1024,17 @@ static inline int cpufreq_table_find_index_c(struct cpufreq_policy *policy,
                                                   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;
 
@@ -1017,11 +1043,13 @@ static inline bool cpufreq_is_in_limits(struct cpufreq_policy *policy, int idx)
 
        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 &&
@@ -1032,29 +1060,26 @@ static inline int cpufreq_frequency_table_target(struct cpufreq_policy *policy,
        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;
        }