Thread (9 messages) 9 messages, 2 authors, 2021-12-07

Re: [PATCH 3/3] intel_idle: fix cpuidle_device unregistration

From: "Rafael J. Wysocki" <rafael@kernel.org>
Date: 2021-11-24 13:22:55

On Wed, Oct 27, 2021 at 10:07 AM Zhang Rui [off-list ref] wrote:
cpuidle_device is allocated as percpu data, and it is registered for every
CPU that has ever been onlined.
When unregistering, checking current online CPUs is not sufficient,
because some cpu may be offlined later with its cpuidle_device registered.
But the unregistration happens only in the error code path of
intel_idle_init(), doesn't it?

While I agree that doing a for_each_present_cpu() walk for that is
more prudent', I'm not sure if that makes any difference in practice.
quoted hunk ↗ jump to hunk
Fix this by using for_each_present_cpu() instead, and unregistering all
the cpuidle_devices that have been registered.

Signed-off-by: Zhang Rui <rui.zhang@intel.com>
---
 drivers/idle/intel_idle.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
index e7f2a5f85bf9..9e916e2adc89 100644
--- a/drivers/idle/intel_idle.c
+++ b/drivers/idle/intel_idle.c
@@ -1687,8 +1687,13 @@ static void __init intel_idle_cpuidle_unregister(struct cpuidle_driver *drv)

        if (intel_idle_cpuhp_state > 0)
                cpuhp_remove_state(intel_idle_cpuhp_state);
-       for_each_online_cpu(i)
-               cpuidle_unregister_device(per_cpu_ptr(intel_idle_cpuidle_devices, i));
+       for_each_present_cpu(i) {
+               struct cpuidle_device *dev;
+
+               dev = per_cpu_ptr(intel_idle_cpuidle_devices, i);
+               if (dev->registered)
dev->registered is checked by cpuidle_unregister_device().
+                       cpuidle_unregister_device(dev);
+       }
        cpuidle_unregister_driver(drv);
        free_percpu(intel_idle_cpuidle_devices);
 }
--
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help