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 thediff --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