linux-kernel - Re: [PATCH v2] cpufreq: Fix per-policy boost behavior after CPU hotplug

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date: Tue, 25 Jun 2024 15:35:27 +0530
From: Viresh Kumar <viresh.kumar@...aro.org>
To: PoShao Chen <poshao.chen@...iatek.com>
Cc: ccj.yeh@...iatek.com, ching-hao.hsu@...iatek.com,
	clive.lin@...iatek.com, linux-kernel@...r.kernel.org,
	linux-pm@...r.kernel.org, rafael@...nel.org
Subject: Re: [PATCH v2] cpufreq: Fix per-policy boost behavior after CPU
 hotplug

On 17-06-24, 13:48, PoShao Chen wrote:
> On Thu, 2024-06-13 at 14:50 +0530, Viresh Kumar wrote: 	 
> > Please try this instead:
> > 
> > diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> > index 7c6879efe9ef..bd9fe2b0f032 100644
> > --- a/drivers/cpufreq/cpufreq.c
> > +++ b/drivers/cpufreq/cpufreq.c
> > @@ -43,6 +43,9 @@ static LIST_HEAD(cpufreq_policy_list);
> >  #define for_each_inactive_policy(__policy)             \
> >         for_each_suitable_policy(__policy, false)
> > 
> > +#define for_each_policy(__policy)                       \
> > +       list_for_each_entry(__policy, &cpufreq_policy_list,
> > policy_list)
> > +
> >  /* Iterate over governors */
> >  static LIST_HEAD(cpufreq_governor_list);
> >  #define for_each_governor(__governor)                          \
> > @@ -2815,7 +2818,7 @@ int cpufreq_boost_trigger_state(int state)
> >         write_unlock_irqrestore(&cpufreq_driver_lock, flags);
> > 
> >         cpus_read_lock();
> > -       for_each_active_policy(policy) {
> > +       for_each_policy(policy) {
> >                 policy->boost_enabled = state;
> >                 ret = cpufreq_driver->set_boost(policy, state);
> >                 if (ret) {
> 
> Thank you for the suggestion. However, calling ->set_boost when
> the policy is inactive will fail in two ways:
> 
> 1. policy->freq_table will be NULL.
> 2. freq_qos_update_request will fail to refresh the frequency limit.

What about just this then ?

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index a45aac17c20f..8e92ba7dda4b 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1430,9 +1430,6 @@ static int cpufreq_online(unsigned int cpu)
                        goto out_free_policy;
                }

-               /* Let the per-policy boost flag mirror the cpufreq_driver boost during init */
-               policy->boost_enabled = cpufreq_boost_enabled() && policy_has_boost_freq(policy);
-
                /*
                 * The initialization has succeeded and the policy is online.
                 * If there is a problem with its frequency table, take it
@@ -1446,6 +1443,9 @@ static int cpufreq_online(unsigned int cpu)
                cpumask_copy(policy->related_cpus, policy->cpus);
        }

+       /* Let the per-policy boost flag mirror the cpufreq_driver boost during init */
+       policy->boost_enabled = cpufreq_boost_enabled() && policy_has_boost_freq(policy);
+
        /*
         * affected cpus must always be the one, which are online. We aren't
         * managing offline cpus here.



So the cpufreq core sets the policy boost to whatever the global boost
is at the time CPU comes online.

This won't call cppc_cpufreq_set_boost() (I think that's the driver
you are using ?) though. The freq_qos_update_request() thing we do
there is driver specific and the driver itself needs to take care of
this, perhaps in the online() callback.

-- 
viresh