Thread (6 messages) 6 messages, 3 authors, 2021-06-18

Re: [PATCH V2 1/3] cpufreq: Add start_cpu() and stop_cpu() callbacks

From: "Rafael J. Wysocki" <rafael@kernel.org>
Date: 2021-06-17 13:33:40
Also in: linux-pm, lkml

On Wed, Jun 16, 2021 at 8:48 AM Viresh Kumar [off-list ref] wrote:
quoted hunk ↗ jump to hunk
On CPU hot-unplug, the cpufreq core doesn't call any driver specific
callback unless all the CPUs of a policy went away, in which case we
call stop_cpu() callback.

For the CPPC cpufreq driver, we need to perform per-cpu init/exit work
which that can't be performed from policy specific init()/exit()
callbacks.

This patch adds a new callback, start_cpu() and modifies the stop_cpu()
callback, to perform such CPU specific work.

These routines are called whenever a CPU is added or removed from a
policy.

Note that this also moves the setting of policy->cpus to online CPUs
only, outside of rwsem as we needed to call start_cpu() for online CPUs
only. This shouldn't have any side effects.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 Documentation/cpu-freq/cpu-drivers.rst |  7 +++++--
 drivers/cpufreq/cpufreq.c              | 19 +++++++++++++++----
 include/linux/cpufreq.h                |  5 ++++-
 3 files changed, 24 insertions(+), 7 deletions(-)
diff --git a/Documentation/cpu-freq/cpu-drivers.rst b/Documentation/cpu-freq/cpu-drivers.rst
index a697278ce190..15cfe42b4075 100644
--- a/Documentation/cpu-freq/cpu-drivers.rst
+++ b/Documentation/cpu-freq/cpu-drivers.rst
@@ -71,8 +71,11 @@ And optionally
  .exit - A pointer to a per-policy cleanup function called during
  CPU_POST_DEAD phase of cpu hotplug process.

- .stop_cpu - A pointer to a per-policy stop function called during
- CPU_DOWN_PREPARE phase of cpu hotplug process.
+ .start_cpu - A pointer to a per-policy per-cpu start function called
+ during CPU online phase.
+
+ .stop_cpu - A pointer to a per-policy per-cpu stop function called
+ during CPU offline phase.

  .suspend - A pointer to a per-policy suspend function which is called
  with interrupts disabled and _after_ the governor is stopped for the
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 802abc925b2a..128dfb1b0cdf 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1119,6 +1119,10 @@ static int cpufreq_add_policy_cpu(struct cpufreq_policy *policy, unsigned int cp

        cpumask_set_cpu(cpu, policy->cpus);

+       /* Do CPU specific initialization if required */
+       if (cpufreq_driver->start_cpu)
+               cpufreq_driver->start_cpu(policy, cpu);
+
        if (has_target()) {
                ret = cpufreq_start_governor(policy);
                if (ret)
@@ -1375,13 +1379,19 @@ static int cpufreq_online(unsigned int cpu)
                cpumask_copy(policy->related_cpus, policy->cpus);
        }

-       down_write(&policy->rwsem);
        /*
         * affected cpus must always be the one, which are online. We aren't
         * managing offline cpus here.
         */
        cpumask_and(policy->cpus, policy->cpus, cpu_online_mask);

+       /* Do CPU specific initialization if required */
+       if (cpufreq_driver->start_cpu) {
+               for_each_cpu(j, policy->cpus)
+                       cpufreq_driver->start_cpu(policy, j);
+       }
+
+       down_write(&policy->rwsem);
        if (new_policy) {
                for_each_cpu(j, policy->related_cpus) {
                        per_cpu(cpufreq_cpu_data, j) = policy;
@@ -1581,6 +1591,10 @@ static int cpufreq_offline(unsigned int cpu)
                policy->cpu = cpumask_any(policy->cpus);
        }

+       /* Do CPU specific de-initialization if required */
+       if (cpufreq_driver->stop_cpu)
+               cpufreq_driver->stop_cpu(policy, cpu);
+
        /* Start governor again for active policy */
        if (!policy_is_inactive(policy)) {
                if (has_target()) {
@@ -1597,9 +1611,6 @@ static int cpufreq_offline(unsigned int cpu)
                policy->cdev = NULL;
        }

-       if (cpufreq_driver->stop_cpu)
-               cpufreq_driver->stop_cpu(policy);
-
This should be a separate patch IMO, after you've migrated everyone to
->offline/->exit.

BTW, IMO it might be better to migrate ->stop_cpu to ->offline rather
than to ->exit.
quoted hunk ↗ jump to hunk
        if (has_target())
                cpufreq_exit_governor(policy);
diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
index 353969c7acd3..c281b3df4e2f 100644
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -371,7 +371,10 @@ struct cpufreq_driver {
        int             (*online)(struct cpufreq_policy *policy);
        int             (*offline)(struct cpufreq_policy *policy);
        int             (*exit)(struct cpufreq_policy *policy);
-       void            (*stop_cpu)(struct cpufreq_policy *policy);
+
+       /* CPU specific start/stop */
+       void            (*start_cpu)(struct cpufreq_policy *policy, unsigned int cpu);
+       void            (*stop_cpu)(struct cpufreq_policy *policy, unsigned int cpu);
        int             (*suspend)(struct cpufreq_policy *policy);
        int             (*resume)(struct cpufreq_policy *policy);

--
2.31.1.272.g89b43f80a514
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help