Re: [PATCH v5 2/6] cpufreq: Add boost_freq_req QoS request

[Rafael J. Wysocki]

On Wed, Feb 25, 2026 at 9:49 AM Pierre Gondois <pierre.gondois@xxxxxxx> wrote:
>
> The Power Management Quality of Service (PM QoS) allows to
> 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) {

Are you sure?

>                 /*
> -                * 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);

Instead of doing this, why don't you add 1 to the policy->min_freq_req
allocation size when boost is supported?  You'll destroy the policy if
the allocation fails anyway.

> +                       if (!policy->boost_freq_req) {
> +                               ret = -ENOMEM;
> +                               goto out_destroy_policy;
> +                       }
> +
> +                       ret = freq_qos_add_request(&policy->constraints,
> +                                                  policy->boost_freq_req,
> +                                                  FREQ_QOS_MAX,
> +                                                  FREQ_QOS_MAX_DEFAULT_VALUE);
> +                       if (ret < 0) {
> +                               /*
> +                                * So we don't call freq_qos_remove_request() for an
> +                                * uninitialized request.
> +                                */
> +                               kfree(policy->boost_freq_req);
> +                               policy->boost_freq_req = NULL;
> +                               goto out_destroy_policy;
> +                       }
> +               }

Then you can put this block before the policy->min_freq_req addition
and the corresponding update of cpufreq_policy_free() will be more
straightforward.

BTW, if I'm not mistaken, there is a bug in the current
cpufreq_policy_free() because it may attempt to remove a NULL freq QoS
request for policy->min_freq_req.  I'll send a patch for that later if
I can confirm it.

> +
>                 blocking_notifier_call_chain(&cpufreq_policy_notifier_list,
>                                 CPUFREQ_CREATE_POLICY, policy);
>         }
> diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
> index 0465d1e6f72ac..c292a6a19e4f5 100644
> --- a/include/linux/cpufreq.h
> +++ b/include/linux/cpufreq.h
> @@ -81,6 +81,7 @@ struct cpufreq_policy {
>         struct freq_constraints constraints;
>         struct freq_qos_request *min_freq_req;
>         struct freq_qos_request *max_freq_req;
> +       struct freq_qos_request *boost_freq_req;
>
>         struct cpufreq_frequency_table  *freq_table;
>         enum cpufreq_table_sorting freq_table_sorted;
> --