From: "Rafael J. Wysocki" rafael.j.wysocki@intel.com
[ Upstream commit 7b1b7961170e4fcad488755e5ffaaaf9bd527e8f ]
Refuse to register a cpuidle device if the given CPU has a cpuidle device already and print a message regarding it.
Without this, an attempt to register a new cpuidle device without unregistering the existing one leads to the removal of the existing cpuidle device without removing its sysfs interface.
Signed-off-by: Rafael J. Wysocki rafael.j.wysocki@intel.com Signed-off-by: Sasha Levin sashal@kernel.org ---
LLM Generated explanations, may be completely bogus:
YES
Explanation
- What it fixes - The patch adds an explicit per-CPU guard in the core registration path to prevent registering a second cpuidle device for a CPU that already has one. Specifically, it introduces an early check in __cpuidle_register_device: - drivers/cpuidle/cpuidle.c:641 - if (per_cpu(cpuidle_devices, cpu)) { pr_info(...); return -EEXIST; } - Before this, the code unconditionally replaced the per-CPU pointer with the new device: - drivers/cpuidle/cpuidle.c:657 - per_cpu(cpuidle_devices, cpu) = dev; - This “silent replacement” makes the prior device unreachable to the core (and duplicates entries on cpuidle_detected_devices), while its sysfs state remains present and bound to the old device object. The sysfs layer allocates a kobject that keeps a backpointer to the cpuidle_device: - drivers/cpuidle/sysfs.c:697 (cpuidle_add_sysfs) sets kdev->dev = dev and publishes it - drivers/cpuidle/sysfs.c:740 (cpuidle_remove_sysfs) tears it down for the same dev - If a new device is registered without first unregistering the old one, the old sysfs instance is never removed, leaving stale sysfs entries referencing the old cpuidle_device. That is at best user- visible breakage (stale sysfs) and at worst a lifetime hazard if that device is later freed by its owner.
- Why the change is correct and minimal-risk - The new guard is small, contained, and runs under the existing cpuidle_lock (as required by the function’s contract), so it’s race- safe with the unregister path. - The function comment already requires the lock; cpuidle_register_device holds it before calling __cpuidle_register_device (drivers/cpuidle/cpuidle.c:680). - It complements the existing check that only prevents double- registering the same struct (dev->registered): - drivers/cpuidle/cpuidle.c:682 - That check does not cover the case of a different struct cpuidle_device for the same CPU. The new per-CPU check closes that gap. - The behavior change is limited to returning -EEXIST instead of proceeding to corrupt state. Callers already treat non-zero returns as failure and back out cleanly (see drivers like ACPI, intel_idle, etc., which unregister the driver or bail on error). - No architectural changes, no new features, no ABI changes. The only user-visible change is a pr_info() when misuse occurs.
- Stable backport considerations - It fixes a real bug with observable user impact (stale sysfs interface) and potential lifetime issues. - The fix is tiny (7 insertions and one trivial local-variable use) and self-contained to drivers/cpuidle/cpuidle.c: no dependencies on new APIs, no cross-subsystem changes. - It aligns with stable rules: important bugfix, minimal risk, confined to the cpuidle core. - It leverages existing per-CPU tracking (include/linux/cpuidle.h:116) and existing unregister semantics that clear the pointer and dev->registered, so it should apply cleanly across maintained stable branches.
Conclusion: This is a clear, contained bug fix that prevents a subtle but serious state/lifetime problem in cpuidle registration. It is well- suited for stable backport.
drivers/cpuidle/cpuidle.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c index 0835da449db8b..56132e843c991 100644 --- a/drivers/cpuidle/cpuidle.c +++ b/drivers/cpuidle/cpuidle.c @@ -635,8 +635,14 @@ static void __cpuidle_device_init(struct cpuidle_device *dev) static int __cpuidle_register_device(struct cpuidle_device *dev) { struct cpuidle_driver *drv = cpuidle_get_cpu_driver(dev); + unsigned int cpu = dev->cpu; int i, ret;
+ if (per_cpu(cpuidle_devices, cpu)) { + pr_info("CPU%d: cpuidle device already registered\n", cpu); + return -EEXIST; + } + if (!try_module_get(drv->owner)) return -EINVAL;
@@ -648,7 +654,7 @@ static int __cpuidle_register_device(struct cpuidle_device *dev) dev->states_usage[i].disable |= CPUIDLE_STATE_DISABLED_BY_USER; }
- per_cpu(cpuidle_devices, dev->cpu) = dev; + per_cpu(cpuidle_devices, cpu) = dev; list_add(&dev->device_list, &cpuidle_detected_devices);
ret = cpuidle_coupled_register_device(dev);