[PATCH v1] PM: runtime: Do not clear needs_force_resume with enabled runtime PM

Posted by Rafael J. Wysocki 4 hours ago

On Monday, December 15, 2025 1:21:42 PM CET ed.tsai@mediatek.com wrote:
> From: Ed Tsai <ed.tsai@mediatek.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@mediatek.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@intel.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@mediatek.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@intel.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

Re: [PATCH v1] PM: runtime: Do not clear needs_force_resume with enabled runtime PM

Posted by Ulf Hansson 2 hours ago

On Mon, 15 Dec 2025 at 15:21, Rafael J. Wysocki <rafael@kernel.org> wrote:
>
> On Monday, December 15, 2025 1:21:42 PM CET ed.tsai@mediatek.com wrote:
> > From: Ed Tsai <ed.tsai@mediatek.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@mediatek.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@intel.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@mediatek.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@intel.com>

Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>

Kind regards
Uffe

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