Re: [PATCH v5 2/6] cpufreq: Add boost_freq_req QoS request
From: Pierre Gondois
Date: Fri Mar 13 2026 - 11:40:01 EST
On 3/13/26 16:31, Rafael J. Wysocki wrote:
On Fri, Mar 13, 2026 at 3:30 PM Pierre Gondois <pierre.gondois@xxxxxxx> wrote:
But he has also said "+1" to this one. :-)
On 3/11/26 14:57, Rafael J. Wysocki wrote:
On Wed, Mar 11, 2026 at 2:52 PM Rafael J. Wysocki<rafael@xxxxxxxxxx> wrote:
On Wed, Feb 25, 2026 at 9:49 AM Pierre Gondois<pierre.gondois@xxxxxxx> wrote:
The Power Management Quality of Service (PM QoS) allows toAre you sure?
aggregate constraints from multiple entities. It is currently
used to manage the min/max frequency of a given policy.
Frequency constraints can come for instance from:
- Thermal framework: acpi_thermal_cpufreq_init()
- Firmware: _PPC objects: acpi_processor_ppc_init()
- User: by setting policyX/scaling_[min|max]_freq
The minimum of the max frequency constraints is used to compute
the resulting maximum allowed frequency.
When enabling boost frequencies, the same frequency request object
(policy->max_freq_req) as to handle requests from users is used.
As a result, when setting:
- scaling_max_freq
- boost
The last sysfs file used overwrites the request from the other
sysfs file.
To avoid this, create a per-policy boost_freq_req to save the boost
constraints instead of overwriting the last scaling_max_freq
constraint.
Signed-off-by: Pierre Gondois<pierre.gondois@xxxxxxx>
---
drivers/cpufreq/cpufreq.c | 37 ++++++++++++++++++++++++++++++++-----
include/linux/cpufreq.h | 1 +
2 files changed, 33 insertions(+), 5 deletions(-)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index db414c052658b..50467b938668a 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1359,17 +1359,21 @@ static void cpufreq_policy_free(struct cpufreq_policy *policy)
/* Cancel any pending policy->update work before freeing the policy. */
cancel_work_sync(&policy->update);
- if (policy->max_freq_req) {
+ if ((policy->max_freq_req && !policy->boost_supported) ||
+ policy->boost_freq_req) {
/*Instead of doing this, why don't you add 1 to the policy->min_freq_req
- * Remove max_freq_req after sending CPUFREQ_REMOVE_POLICY
- * notification, since CPUFREQ_CREATE_POLICY notification was
- * sent after adding max_freq_req earlier.
+ * Remove max/boost _freq_req after sending CPUFREQ_REMOVE_POLICY
+ * notification, since CPUFREQ_CREATE_POLICY notification was sent
+ * after adding max/boost _freq_req earlier.
*/
blocking_notifier_call_chain(&cpufreq_policy_notifier_list,
CPUFREQ_REMOVE_POLICY, policy);
- freq_qos_remove_request(policy->max_freq_req);
}
+ freq_qos_remove_request(policy->boost_freq_req);
+ kfree(policy->boost_freq_req);
+
+ freq_qos_remove_request(policy->max_freq_req);
freq_qos_remove_request(policy->min_freq_req);
kfree(policy->min_freq_req);
@@ -1479,6 +1483,29 @@ static int cpufreq_policy_online(struct cpufreq_policy *policy,
goto out_destroy_policy;
}
+ if (policy->boost_supported) {
+ policy->boost_freq_req = kzalloc(sizeof(*policy->boost_freq_req),
+ GFP_KERNEL);
allocation size when boost is supported? You'll destroy the policy if
the allocation fails anyway.
Yes right,
this would make the error handling much simpler to follow.
On the other hand Viresh seems to have acknowledged the logic,
ah right indeed
so whatever seems is better to both of you.I think we agree.