RE: [PATCH] PM: sleep: core: Fix race condition in async noirq suspend

From: Jacky Bai

Date: Fri Feb 06 2026 - 19:16:34 EST


> Subject: Re: [PATCH] PM: sleep: core: Fix race condition in async noirq
> suspend
>
> On Fri, Feb 6, 2026 at 4:03 PM Jacky Bai <ping.bai@xxxxxxx> wrote:
> >
> > A race condition occurs during the noirq suspend stage when accessing
> > bitfields in struct dev_pm_info, causing system suspend to hang.
> >
> > During async noirq suspend, leaf devices are queued for asynchronous
> > processing while the main suspend thread continues clearing the
> > 'work_in_progress' flag of other devices, including potential parents
> > of those leaf devices.
> >
> > In struct dev_pm_info, 'work_in_progress' and 'must_resume' are
> > bitfields stored in the same byte. This creates a race when:
> >
> > 1. A leaf device's async thread sets its parent's 'must_resume' flag
> > via dpm_superior_set_must_resume()
> > 2. Simultaneously, the main thread clears the parent's
> > 'work_in_progress' flag
> >
> > Since bitfield operations are not atomic, concurrent modifications to
> > the same byte can corrupt both fields. This leaves the parent device
> > in an incorrect state, causing the system suspend to hang.
> >
> > Fix this by adding lock protection around the bitfield accesses to
> > ensure atomic read-modify-write operations.
> >
> > Fixes: 443046d1ad66 ("PM: sleep: Make suspend of devices more
> > asynchronous")
> > Signed-off-by: Jacky Bai <ping.bai@xxxxxxx>
>
> There is a fix for this already in my tree, see
>
Thx Rafael. The patch in your tree provide a non-locking way. My patch can be dropped.
I Will pick this patch Into our internal tree to fix the issue we meet.

BR
Jacky Bai
>
> > ---
> > drivers/base/power/main.c | 20 +++++++++++++++++---
> > 1 file changed, 17 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
> > index
> >
> 189de5250f256bdc1cdc33006b2c386d0794485f..561e24c257d779db51a0f0d
> 50dcf
> > ee61e98de64f 100644
> > --- a/drivers/base/power/main.c
> > +++ b/drivers/base/power/main.c
> > @@ -633,7 +633,9 @@ static bool __dpm_async(struct device *dev,
> async_func_t func)
> > if (!is_async(dev))
> > return false;
> >
> > + spin_lock_irq(&dev->power.lock);
> > dev->power.work_in_progress = true;
> > + spin_unlock_irq(&dev->power.lock);
> >
> > get_device(dev);
> >
> > @@ -656,8 +658,11 @@ static int dpm_async_with_cleanup(struct device
> > *dev, void *fn) {
> > guard(mutex)(&async_wip_mtx);
> >
> > - if (!__dpm_async(dev, fn))
> > + if (!__dpm_async(dev, fn)) {
> > + spin_lock_irq(&dev->power.lock);
> > dev->power.work_in_progress = false;
> > + spin_unlock_irq(&dev->power.lock);
> > + }
> >
> > return 0;
> > }
> > @@ -698,7 +703,10 @@ static void dpm_async_resume_subordinate(struct
> > device *dev, async_func_t func) static void
> > dpm_clear_async_state(struct device *dev) {
> > reinit_completion(&dev->power.completion);
> > +
> > + spin_lock_irq(&dev->power.lock);
> > dev->power.work_in_progress = false;
> > + spin_unlock_irq(&dev->power.lock);
> > }
> >
> > static bool dpm_root_device(struct device *dev) @@ -1407,13 +1415,19
> > @@ static void dpm_superior_set_must_resume(struct device *dev)
> > struct device_link *link;
> > int idx;
> >
> > - if (dev->parent)
> > + if (dev->parent) {
> > + spin_lock_irq(&dev->parent->power.lock);
> > dev->parent->power.must_resume = true;
> > + spin_unlock_irq(&dev->parent->power.lock);
> > + }
> >
> > idx = device_links_read_lock();
> >
> > - dev_for_each_link_to_supplier(link, dev)
> > + dev_for_each_link_to_supplier(link, dev) {
> > + spin_lock_irq(&link->supplier->power.lock);
> > link->supplier->power.must_resume = true;
> > + spin_unlock_irq(&link->supplier->power.lock);
> > + }
> >
> > device_links_read_unlock(idx); }
> >
> > ---
> > base-commit: 9845cf73f7db6094c0d8419d6adb848028f4a921
> > change-id: 20260206-noirq_async_suspend-645bd6b8305e
> >
> > Best regards,
> > --
> > Jacky Bai <ping.bai@xxxxxxx>
> >
> >