[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <12807571.O9o76ZdvQC@rafael.j.wysocki>
Date: Mon, 15 Dec 2025 15:21:34 +0100
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Ed Tsai <ed.tsai@...iatek.com>, linux-pm@...r.kernel.org
Cc: Ulf Hansson <ulf.hansson@...aro.org>, linux-kernel@...r.kernel.org,
linux-mediatek@...ts.infradead.org, chun-hung.wu@...iatek.com,
freddy.shin@...iatek.com, "Rafael J. Wysocki" <rafael.j.wysocki@...el.com>
Subject:
[PATCH v1] PM: runtime: Do not clear needs_force_resume with enabled runtime
PM
On Monday, December 15, 2025 1:21:42 PM CET ed.tsai@...iatek.com wrote:
> From: Ed Tsai <ed.tsai@...iatek.com>
>
> needs_force_resume is a bitfield member in struct dev_pm_info and must
> be accessed under the same lock as its bitfield group.
>
> A real concurrent write was observed between needs_force_resume and
> idle_notification; stacks below:
>
> write needs_force_resume:
> pm_runtime_reinit+0x110/0x360
> really_probe+0xe0/0x464
> __driver_probe_device+0x9c/0x104
> driver_probe_device+0x3c/0x1a8
> __device_attach_driver+0x100/0x17c
> bus_for_each_drv+0x10c/0x168
> __device_attach_async_helper+0x7c/0xf4
> async_run_entry_fn+0x4c/0x1b4
> process_scheduled_works+0x1dc/0x498
> worker_thread+0x220/0x320
> kthread+0x150/0x27c
> ret_from_fork+0x10/0x20
>
> write idle_notification:
> rpm_idle+0x464/0x5f8
> __pm_runtime_idle+0x7c/0x170
> scsi_autopm_put_device+0x18/0x28
> scsi_sysfs_add_sdev+0x1a0/0x1d8
> scsi_probe_and_add_lun+0xbd8/0xcd0
> __scsi_add_device+0xb8/0x11c
> ufshcd_async_scan+0xb4/0x3a4
> async_run_entry_fn+0x4c/0x1b4
> process_scheduled_works+0x1dc/0x498
> worker_thread+0x220/0x320
> kthread+0x150/0x27c
> ret_from_fork+0x10/0x20
>
> Fixes: 89d9cec3b1e9 ("PM: runtime: Clear power.needs_force_resume in pm_runtime_reinit()")
> Signed-off-by: Ed Tsai <ed.tsai@...iatek.com>
> ---
> drivers/base/power/runtime.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c
> index 84676cc24221..3328543b1ed8 100644
> --- a/drivers/base/power/runtime.c
> +++ b/drivers/base/power/runtime.c
> @@ -1879,11 +1879,22 @@ void pm_runtime_reinit(struct device *dev)
> pm_runtime_put(dev->parent);
> }
> }
> +
> /*
> * Clear power.needs_force_resume in case it has been set by
> * pm_runtime_force_suspend() invoked from a driver remove callback.
> */
> + if (dev->power.irq_safe)
> + spin_lock(&dev->power.lock);
> + else
> + spin_lock_irq(&dev->power.lock);
> +
> dev->power.needs_force_resume = false;
> +
> + if (dev->power.irq_safe)
> + spin_unlock(&dev->power.lock);
> + else
> + spin_unlock_irq(&dev->power.lock);
> }
>
> /**
>
Thanks for the patch, but this happens because the flag is cleared when
runtime PM is enabled which shouldn't be necessary, so I'd prefer to make
the change below.
---
From: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
Subject: [PATCH v1] PM: runtime: Do not clear needs_force_resume with enabled runtime PM
Commit 89d9cec3b1e9 ("PM: runtime: Clear power.needs_force_resume in
pm_runtime_reinit()") added provisional clearing of power.needs_force_resume
to pm_runtime_reinit(), but it is done unconditionally which is a
mistake because pm_runtime_reinit() may race with driver probing
and removal [1].
To address this, notice that power.needs_force_resume should never
be set when runtime PM is enabled and so it only needs to be cleared
when runtime PM is disabled, and update pm_runtime_init() to only
clear that flag when runtime PM is disabled.
Fixes: 89d9cec3b1e9 ("PM: runtime: Clear power.needs_force_resume in pm_runtime_reinit()")
Reported-by: Ed Tsai <ed.tsai@...iatek.com>
Closes: https://lore.kernel.org/linux-pm/20251215122154.3180001-1-ed.tsai@mediatek.com/ [1]
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
---
drivers/base/power/runtime.c | 22 ++++++++++++----------
1 file changed, 12 insertions(+), 10 deletions(-)
--- a/drivers/base/power/runtime.c
+++ b/drivers/base/power/runtime.c
@@ -1878,16 +1878,18 @@ void pm_runtime_init(struct device *dev)
*/
void pm_runtime_reinit(struct device *dev)
{
- if (!pm_runtime_enabled(dev)) {
- if (dev->power.runtime_status == RPM_ACTIVE)
- pm_runtime_set_suspended(dev);
- if (dev->power.irq_safe) {
- spin_lock_irq(&dev->power.lock);
- dev->power.irq_safe = 0;
- spin_unlock_irq(&dev->power.lock);
- if (dev->parent)
- pm_runtime_put(dev->parent);
- }
+ if (pm_runtime_enabled(dev))
+ return;
+
+ if (dev->power.runtime_status == RPM_ACTIVE)
+ pm_runtime_set_suspended(dev);
+
+ if (dev->power.irq_safe) {
+ spin_lock_irq(&dev->power.lock);
+ dev->power.irq_safe = 0;
+ spin_unlock_irq(&dev->power.lock);
+ if (dev->parent)
+ pm_runtime_put(dev->parent);
}
/*
* Clear power.needs_force_resume in case it has been set by
Powered by blists - more mailing lists