Commit b7902803 authored by Rafael J. Wysocki's avatar Rafael J. Wysocki
Browse files

cpufreq: Fix setting policy limits when frequency tables are used 

Commit 7491cdf4 ("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: 7491cdf4 ("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: default avatarStephan Gerhold <stephan.gerhold@linaro.org>
Signed-off-by: default avatarRafael J. Wysocki <rafael.j.wysocki@intel.com>
Tested-by: default avatarStephan Gerhold <stephan.gerhold@linaro.org>
Reviewed-by: default avatarLifeng Zheng <zhenglifeng1@huawei.com>
Link: https://patch.msgid.link/5896780.DvuYhMxLoT@rjwysocki.net
parent 3d592249

drivers/cpufreq/cpufreq.c

+14 −8
Original line number Diff line number Diff line
@@ -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);

drivers/cpufreq/cpufreq_ondemand.c

+2 −1
Original line number Diff line number Diff line
@@ -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;

drivers/cpufreq/freq_table.c

+3 −3
Original line number Diff line number Diff line
@@ -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;

include/linux/cpufreq.h

+54 −29
Original line number Diff line number Diff line
@@ -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,

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,

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,

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;

	}