[PATCH V5 3/9] kernel/cpu_pm: Add runtime PM support for CPUs
From: khilman@baylibre.com (Kevin Hilman)
Date: 2017-03-29 23:54:56
Also in:
linux-arm-msm, linux-pm
Ulf Hansson [off-list ref] writes:
On 3 March 2017 at 21:41, Lina Iyer [off-list ref] wrote:quoted
Notify runtime PM when the CPU is going to be powered off in the idle state. This allows for runtime PM suspend/resume of the CPU as well as its PM domain. Cc: Kevin Hilman <khilman@kernel.org> Cc: Rafael J. Wysocki <redacted> Signed-off-by: Lina Iyer <redacted>
[...]
quoted
@@ -99,6 +102,7 @@ int cpu_pm_enter(void) { int nr_calls; int ret = 0; + struct device *dev = get_cpu_device(smp_processor_id()); read_lock(&cpu_pm_notifier_lock); ret = cpu_pm_notify(CPU_PM_ENTER, -1, &nr_calls);@@ -110,6 +114,10 @@ int cpu_pm_enter(void) cpu_pm_notify(CPU_PM_ENTER_FAILED, nr_calls - 1, NULL); read_unlock(&cpu_pm_notifier_lock); + /* Notify Runtime PM that we are suspending the CPU */ + if (!ret && dev) + RCU_NONIDLE(pm_runtime_put_sync_suspend(dev)); +I am trying to understand how the runtime PM usage count becomes properly balanced. I believe you could you end up first calling a pm_runtime_put_sync_suspend(), without earlier having called pm_runtime_get*(). I am not sure though, but perhaps you can elaborate. Anyway, in patch2/9, where you enable runtime PM there is only a call to pm_runtime_set_active(), which doesn't increase the usage count. To me, it seems like that change also needs a pm_runtime_get_noresume().
IIUC, the CPU hotplug callback below (cpu_pm_cpu_starting) will do the first _get_sync(), and I'm assuming that will happen before any of the CPU PM notifiers get called, so I think the usecount will always be at least 1 by the time any CPU PM callbacks happen. [...]
quoted
+#ifdef CONFIG_HOTPLUG_CPU +static int cpu_pm_cpu_dying(unsigned int cpu) +{ + struct device *dev = get_cpu_device(cpu); + + if (dev) + pm_runtime_put_sync_suspend(dev); + + return 0; +} + +static int cpu_pm_cpu_starting(unsigned int cpu) +{ + struct device *dev = get_cpu_device(cpu); + + if (dev) + pm_runtime_get_sync(dev);I assume that according to my comment above, you somehow need to compensate for either of the cases when CONFIG_HOTPLUG_CPU is set or unset. Right?
Right, if for some reason CONFIG_HOTPLUG_CPU=n, we'll have a problem where there is never an initial _get() so the usecount will be zero when CPU PM notifiers get called the first time. Kevin