[PATCH 5/6] cpufreq: Set policy->min and max as real QoS constraints

Pierre Gondois posted 6 patches 2 weeks ago
[PATCH 5/6] cpufreq: Set policy->min and max as real QoS constraints
Posted by Pierre Gondois 2 weeks ago
cpufreq_set_policy() will ultimately override the policy min/max
values written in the .init() callback through:
cpufreq_policy_online()
\-cpufreq_init_policy()
  \-cpufreq_set_policy()
    \-/* Set policy->min/max */
Thus the policy min/max values provided are only temporary.

There is an exception if CPUFREQ_NEED_INITIAL_FREQ_CHECK is set and:
cpufreq_policy_online()
\-cpufreq_init_policy()
  \-__cpufreq_driver_target()
    \-cpufreq_driver->target()
is called. In this case, some drivers use the policy min/max
values in their .target() callback before they are overridden.

Check cpufreq drivers:
- if their .target() callback doesn't use policy->min or max,
  remove the initialization
- assuming policy->min or max values were populated as constraints,
  set them as QoS real constraints in cpufreq_policy_online()

Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
---
 drivers/cpufreq/amd-pstate.c      | 24 ++++++++++++------------
 drivers/cpufreq/cpufreq-nforce2.c |  4 ++--
 drivers/cpufreq/cpufreq.c         | 16 ++++++++++++++--
 drivers/cpufreq/gx-suspmod.c      |  9 ++++-----
 drivers/cpufreq/intel_pstate.c    |  3 ---
 drivers/cpufreq/pcc-cpufreq.c     |  8 ++++----
 drivers/cpufreq/pxa3xx-cpufreq.c  |  4 ++--
 drivers/cpufreq/virtual-cpufreq.c |  6 +++---
 8 files changed, 41 insertions(+), 33 deletions(-)

diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index 310d5938cbdf6..aaafbe9b26cae 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -1003,12 +1003,12 @@ static int amd_pstate_cpu_init(struct cpufreq_policy *policy)
 
 	perf = READ_ONCE(cpudata->perf);
 
-	policy->cpuinfo.min_freq = policy->min = perf_to_freq(perf,
-							      cpudata->nominal_freq,
-							      perf.lowest_perf);
-	policy->cpuinfo.max_freq = policy->max = perf_to_freq(perf,
-							      cpudata->nominal_freq,
-							      perf.highest_perf);
+	policy->cpuinfo.min_freq = perf_to_freq(perf,
+						cpudata->nominal_freq,
+						perf.lowest_perf);
+	policy->cpuinfo.max_freq = perf_to_freq(perf,
+						cpudata->nominal_freq,
+						perf.highest_perf);
 
 	ret = amd_pstate_cppc_enable(policy);
 	if (ret)
@@ -1485,12 +1485,12 @@ static int amd_pstate_epp_cpu_init(struct cpufreq_policy *policy)
 
 	perf = READ_ONCE(cpudata->perf);
 
-	policy->cpuinfo.min_freq = policy->min = perf_to_freq(perf,
-							      cpudata->nominal_freq,
-							      perf.lowest_perf);
-	policy->cpuinfo.max_freq = policy->max = perf_to_freq(perf,
-							      cpudata->nominal_freq,
-							      perf.highest_perf);
+	policy->cpuinfo.min_freq = perf_to_freq(perf,
+						cpudata->nominal_freq,
+						perf.lowest_perf);
+	policy->cpuinfo.max_freq = perf_to_freq(perf,
+						cpudata->nominal_freq,
+						perf.highest_perf);
 	policy->driver_data = cpudata;
 
 	ret = amd_pstate_cppc_enable(policy);
diff --git a/drivers/cpufreq/cpufreq-nforce2.c b/drivers/cpufreq/cpufreq-nforce2.c
index fbbbe501cf2dc..831102522ad64 100644
--- a/drivers/cpufreq/cpufreq-nforce2.c
+++ b/drivers/cpufreq/cpufreq-nforce2.c
@@ -355,8 +355,8 @@ static int nforce2_cpu_init(struct cpufreq_policy *policy)
 		min_fsb = NFORCE2_MIN_FSB;
 
 	/* cpuinfo and default policy values */
-	policy->min = policy->cpuinfo.min_freq = min_fsb * fid * 100;
-	policy->max = policy->cpuinfo.max_freq = max_fsb * fid * 100;
+	policy->cpuinfo.min_freq = min_fsb * fid * 100;
+	policy->cpuinfo.max_freq = max_fsb * fid * 100;
 
 	return 0;
 }
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 43bf6aed90e49..209e2673ca50b 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1455,6 +1455,18 @@ static int cpufreq_policy_online(struct cpufreq_policy *policy,
 	cpumask_and(policy->cpus, policy->cpus, cpu_online_mask);
 
 	if (new_policy) {
+		unsigned int min, max;
+
+		/*
+		 * If the driver has set policy->min or max,
+		 * use the value as a QoS request.
+		 */
+		min = max(FREQ_QOS_MIN_DEFAULT_VALUE, policy->min);
+		if (policy->max)
+			max = min(FREQ_QOS_MAX_DEFAULT_VALUE, policy->max);
+		else
+			max = FREQ_QOS_MAX_DEFAULT_VALUE;
+
 		for_each_cpu(j, policy->related_cpus) {
 			per_cpu(cpufreq_cpu_data, j) = policy;
 			add_cpu_dev_symlink(policy, j, get_cpu_device(j));
@@ -1469,7 +1481,7 @@ static int cpufreq_policy_online(struct cpufreq_policy *policy,
 
 		ret = freq_qos_add_request(&policy->constraints,
 					   policy->min_freq_req, FREQ_QOS_MIN,
-					   FREQ_QOS_MIN_DEFAULT_VALUE);
+					   min);
 		if (ret < 0) {
 			/*
 			 * So we don't call freq_qos_remove_request() for an
@@ -1489,7 +1501,7 @@ static int cpufreq_policy_online(struct cpufreq_policy *policy,
 
 		ret = freq_qos_add_request(&policy->constraints,
 					   policy->max_freq_req, FREQ_QOS_MAX,
-					   FREQ_QOS_MAX_DEFAULT_VALUE);
+					   max);
 		if (ret < 0) {
 			policy->max_freq_req = NULL;
 			goto out_destroy_policy;
diff --git a/drivers/cpufreq/gx-suspmod.c b/drivers/cpufreq/gx-suspmod.c
index 75b3ef7ec6796..57999b8d51fa2 100644
--- a/drivers/cpufreq/gx-suspmod.c
+++ b/drivers/cpufreq/gx-suspmod.c
@@ -397,7 +397,7 @@ static int cpufreq_gx_target(struct cpufreq_policy *policy,
 
 static int cpufreq_gx_cpu_init(struct cpufreq_policy *policy)
 {
-	unsigned int maxfreq;
+	unsigned int minfreq, maxfreq;
 
 	if (!policy || policy->cpu != 0)
 		return -ENODEV;
@@ -418,11 +418,10 @@ static int cpufreq_gx_cpu_init(struct cpufreq_policy *policy)
 	policy->cpu = 0;
 
 	if (max_duration < POLICY_MIN_DIV)
-		policy->min = maxfreq / max_duration;
+		minfreq = maxfreq / max_duration;
 	else
-		policy->min = maxfreq / POLICY_MIN_DIV;
-	policy->max = maxfreq;
-	policy->cpuinfo.min_freq = maxfreq / max_duration;
+		minfreq = maxfreq / POLICY_MIN_DIV;
+	policy->cpuinfo.min_freq = minfreq;
 	policy->cpuinfo.max_freq = maxfreq;
 
 	return 0;
diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index ec4abe3745736..bf2f7524d04a9 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -3047,9 +3047,6 @@ static int __intel_pstate_cpu_init(struct cpufreq_policy *policy)
 	policy->cpuinfo.max_freq = READ_ONCE(global.no_turbo) ?
 			cpu->pstate.max_freq : cpu->pstate.turbo_freq;
 
-	policy->min = policy->cpuinfo.min_freq;
-	policy->max = policy->cpuinfo.max_freq;
-
 	intel_pstate_init_acpi_perf_limits(policy);
 
 	policy->fast_switch_possible = true;
diff --git a/drivers/cpufreq/pcc-cpufreq.c b/drivers/cpufreq/pcc-cpufreq.c
index ac2e90a65f0c4..231edfe8cabaa 100644
--- a/drivers/cpufreq/pcc-cpufreq.c
+++ b/drivers/cpufreq/pcc-cpufreq.c
@@ -551,13 +551,13 @@ static int pcc_cpufreq_cpu_init(struct cpufreq_policy *policy)
 		goto out;
 	}
 
-	policy->max = policy->cpuinfo.max_freq =
+	policy->cpuinfo.max_freq =
 		ioread32(&pcch_hdr->nominal) * 1000;
-	policy->min = policy->cpuinfo.min_freq =
+	policy->cpuinfo.min_freq =
 		ioread32(&pcch_hdr->minimum_frequency) * 1000;
 
-	pr_debug("init: policy->max is %d, policy->min is %d\n",
-		policy->max, policy->min);
+	pr_debug("init: max_freq is %d, min_freq is %d\n",
+		 policy->cpuinfo.max_freq, policy->cpuinfo.min_freq);
 out:
 	return result;
 }
diff --git a/drivers/cpufreq/pxa3xx-cpufreq.c b/drivers/cpufreq/pxa3xx-cpufreq.c
index 4afa48d172dbe..f53b9d7edc76a 100644
--- a/drivers/cpufreq/pxa3xx-cpufreq.c
+++ b/drivers/cpufreq/pxa3xx-cpufreq.c
@@ -185,8 +185,8 @@ static int pxa3xx_cpufreq_init(struct cpufreq_policy *policy)
 	int ret = -EINVAL;
 
 	/* set default policy and cpuinfo */
-	policy->min = policy->cpuinfo.min_freq = 104000;
-	policy->max = policy->cpuinfo.max_freq =
+	policy->cpuinfo.min_freq = 104000;
+	policy->cpuinfo.max_freq =
 		(cpu_is_pxa320()) ? 806000 : 624000;
 	policy->cpuinfo.transition_latency = 1000; /* FIXME: 1 ms, assumed */
 
diff --git a/drivers/cpufreq/virtual-cpufreq.c b/drivers/cpufreq/virtual-cpufreq.c
index 6ffa16d239b2b..6f7dbef7fda96 100644
--- a/drivers/cpufreq/virtual-cpufreq.c
+++ b/drivers/cpufreq/virtual-cpufreq.c
@@ -164,10 +164,10 @@ static int virt_cpufreq_get_freq_info(struct cpufreq_policy *policy)
 		policy->cpuinfo.min_freq = 1;
 		policy->cpuinfo.max_freq = virt_cpufreq_get_perftbl_entry(policy->cpu, 0);
 
-		policy->min = policy->cpuinfo.min_freq;
-		policy->max = policy->cpuinfo.max_freq;
+		policy->cpuinfo.min_freq;
+		policy->cpuinfo.max_freq;
 
-		policy->cur = policy->max;
+		policy->cur = policy->cpuinfo.max_freq;
 		return 0;
 	}
 
-- 
2.43.0
Re: [PATCH 5/6] cpufreq: Set policy->min and max as real QoS constraints
Posted by kernel test robot 1 week, 4 days ago
Hi Pierre,

kernel test robot noticed the following build warnings:

[auto build test WARNING on rafael-pm/linux-next]
[also build test WARNING on rafael-pm/bleeding-edge linus/master v6.19-rc7 next-20260128]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Pierre-Gondois/cpufreq-Remove-per-CPU-QoS-constraint/20260126-182440
base:   https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
patch link:    https://lore.kernel.org/r/20260126101826.94030-6-pierre.gondois%40arm.com
patch subject: [PATCH 5/6] cpufreq: Set policy->min and max as real QoS constraints
config: arm-allyesconfig (https://download.01.org/0day-ci/archive/20260129/202601291700.LY8K4K9v-lkp@intel.com/config)
compiler: arm-linux-gnueabi-gcc (GCC) 15.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260129/202601291700.LY8K4K9v-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202601291700.LY8K4K9v-lkp@intel.com/

All warnings (new ones prefixed by >>):

   drivers/cpufreq/virtual-cpufreq.c: In function 'virt_cpufreq_get_freq_info':
>> drivers/cpufreq/virtual-cpufreq.c:167:32: warning: statement with no effect [-Wunused-value]
     167 |                 policy->cpuinfo.min_freq;
         |                 ~~~~~~~~~~~~~~~^~~~~~~~~
   drivers/cpufreq/virtual-cpufreq.c:168:32: warning: statement with no effect [-Wunused-value]
     168 |                 policy->cpuinfo.max_freq;
         |                 ~~~~~~~~~~~~~~~^~~~~~~~~


vim +167 drivers/cpufreq/virtual-cpufreq.c

   155	
   156	static int virt_cpufreq_get_freq_info(struct cpufreq_policy *policy)
   157	{
   158		struct cpufreq_frequency_table *table;
   159		u32 num_perftbl_entries, idx;
   160	
   161		num_perftbl_entries = per_cpu(perftbl_num_entries, policy->cpu);
   162	
   163		if (num_perftbl_entries == 1) {
   164			policy->cpuinfo.min_freq = 1;
   165			policy->cpuinfo.max_freq = virt_cpufreq_get_perftbl_entry(policy->cpu, 0);
   166	
 > 167			policy->cpuinfo.min_freq;
   168			policy->cpuinfo.max_freq;
   169	
   170			policy->cur = policy->cpuinfo.max_freq;
   171			return 0;
   172		}
   173	
   174		table = kcalloc(num_perftbl_entries + 1, sizeof(*table), GFP_KERNEL);
   175		if (!table)
   176			return -ENOMEM;
   177	
   178		for (idx = 0; idx < num_perftbl_entries; idx++)
   179			table[idx].frequency = virt_cpufreq_get_perftbl_entry(policy->cpu, idx);
   180	
   181		table[idx].frequency = CPUFREQ_TABLE_END;
   182		policy->freq_table = table;
   183	
   184		return 0;
   185	}
   186	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Re: [PATCH 5/6] cpufreq: Set policy->min and max as real QoS constraints
Posted by kernel test robot 1 week, 4 days ago
Hi Pierre,

kernel test robot noticed the following build warnings:

[auto build test WARNING on rafael-pm/linux-next]
[also build test WARNING on rafael-pm/bleeding-edge linus/master v6.19-rc7 next-20260128]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Pierre-Gondois/cpufreq-Remove-per-CPU-QoS-constraint/20260126-182440
base:   https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
patch link:    https://lore.kernel.org/r/20260126101826.94030-6-pierre.gondois%40arm.com
patch subject: [PATCH 5/6] cpufreq: Set policy->min and max as real QoS constraints
config: riscv-allyesconfig (https://download.01.org/0day-ci/archive/20260129/202601291343.bghsD4zH-lkp@intel.com/config)
compiler: clang version 16.0.6 (https://github.com/llvm/llvm-project 7cbf1a2591520c2491aa35339f227775f4d3adf6)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260129/202601291343.bghsD4zH-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202601291343.bghsD4zH-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/cpufreq/virtual-cpufreq.c:167:19: warning: expression result unused [-Wunused-value]
                   policy->cpuinfo.min_freq;
                   ~~~~~~~~~~~~~~~ ^~~~~~~~
   drivers/cpufreq/virtual-cpufreq.c:168:19: warning: expression result unused [-Wunused-value]
                   policy->cpuinfo.max_freq;
                   ~~~~~~~~~~~~~~~ ^~~~~~~~
   2 warnings generated.


vim +167 drivers/cpufreq/virtual-cpufreq.c

   155	
   156	static int virt_cpufreq_get_freq_info(struct cpufreq_policy *policy)
   157	{
   158		struct cpufreq_frequency_table *table;
   159		u32 num_perftbl_entries, idx;
   160	
   161		num_perftbl_entries = per_cpu(perftbl_num_entries, policy->cpu);
   162	
   163		if (num_perftbl_entries == 1) {
   164			policy->cpuinfo.min_freq = 1;
   165			policy->cpuinfo.max_freq = virt_cpufreq_get_perftbl_entry(policy->cpu, 0);
   166	
 > 167			policy->cpuinfo.min_freq;
   168			policy->cpuinfo.max_freq;
   169	
   170			policy->cur = policy->cpuinfo.max_freq;
   171			return 0;
   172		}
   173	
   174		table = kcalloc(num_perftbl_entries + 1, sizeof(*table), GFP_KERNEL);
   175		if (!table)
   176			return -ENOMEM;
   177	
   178		for (idx = 0; idx < num_perftbl_entries; idx++)
   179			table[idx].frequency = virt_cpufreq_get_perftbl_entry(policy->cpu, idx);
   180	
   181		table[idx].frequency = CPUFREQ_TABLE_END;
   182		policy->freq_table = table;
   183	
   184		return 0;
   185	}
   186	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki