[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAB8ipk_VX2VPm706Jwa1=8NSA7_btWL2ieXmBgHr2JcULEP76g@mail.gmail.com>
Date: Wed, 4 Feb 2026 10:56:49 +0800
From: Xuewen Yan <xuewen.yan94@...il.com>
To: "Rafael J. Wysocki" <rafael@...nel.org>
Cc: Linux PM <linux-pm@...r.kernel.org>, LKML <linux-kernel@...r.kernel.org>,
Ulf Hansson <ulf.hansson@...aro.org>, Saravana Kannan <saravanak@...gle.com>,
Xuewen Yan <xuewen.yan@...soc.com>
Subject: Re: [PATCH v1] PM: sleep: core: Clear device async state upfront
during suspend
Hi Rafael,
On Wed, Feb 4, 2026 at 4:38 AM Rafael J. Wysocki <rafael@...nel.org> wrote:
>
> From: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
>
> In all of the system suspend transition phases, async state of all
> devices needs to be cleared before starting async processing for any of
> them because the latter may race with power.work_in_progress updates for
> the device's parent or suppliers and if it touches bit fields from the
> same group (for example, power.must_resume or power.wakeup_path), bit
> field corruption is possible.
>
> Rearrange the code accordingly.
Could we use the following patch:
diff --git a/include/linux/pm.h b/include/linux/pm.h
index 98a899858ece..afcaaa37a812 100644
--- a/include/linux/pm.h
+++ b/include/linux/pm.h
@@ -681,10 +681,10 @@ struct dev_pm_info {
struct list_head entry;
struct completion completion;
struct wakeup_source *wakeup;
+ bool work_in_progress; /* Owned by
the PM core */
bool wakeup_path:1;
bool syscore:1;
bool no_pm_callbacks:1; /* Owned by
the PM core */
- bool work_in_progress:1; /* Owned by
the PM core */
bool smart_suspend:1; /* Owned by
the PM core */
bool must_resume:1; /* Owned by
the PM core */
bool may_skip_resume:1; /* Set by subsystems */
Due to byte alignment, the size of struct dev_pm_info remains unchanged,
while also preventing concurrency issues between work_in_progress and
other variables.
Additionally, with this modification, there’s no need to traverse the
device list twice.
BR
---
xuewen
>
> Fixes: aa7a9275ab81 ("PM: sleep: Suspend async parents after suspending children")
> Fixes: 443046d1ad66 ("PM: sleep: Make suspend of devices more asynchronous")
> Reported-by: Xuewen Yan <xuewen.yan@...soc.com>
> Closes: https://lore.kernel.org/linux-pm/20260203063459.12808-1-xuewen.yan@unisoc.com/
> Cc: All applicable <stable@...r.kernel.org>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
> ---
> drivers/base/power/main.c | 33 ++++++++++++++++++++++++++++++---
> 1 file changed, 30 insertions(+), 3 deletions(-)
>
> --- a/drivers/base/power/main.c
> +++ b/drivers/base/power/main.c
> @@ -1527,11 +1527,20 @@ static int dpm_noirq_suspend_devices(pm_
> mutex_lock(&dpm_list_mtx);
>
> /*
> + * Clear the async state for all devices upfront to prevent the
> + * power.work_in_progress updates from racing with power.must_resume
> + * updates carried out by dpm_superior_set_must_resume(), since these
> + * flags belong to the same group of bit fields and they should not be
> + * updated at the same time without synchronization.
> + */
> + list_for_each_entry_reverse(dev, &dpm_late_early_list, power.entry)
> + dpm_clear_async_state(dev);
> +
> + /*
> * Start processing "async" leaf devices upfront so they don't need to
> * wait for the "sync" devices they don't depend on.
> */
> list_for_each_entry_reverse(dev, &dpm_late_early_list, power.entry) {
> - dpm_clear_async_state(dev);
> if (dpm_leaf_device(dev))
> dpm_async_with_cleanup(dev, async_suspend_noirq);
> }
> @@ -1732,11 +1741,20 @@ int dpm_suspend_late(pm_message_t state)
> mutex_lock(&dpm_list_mtx);
>
> /*
> + * Clear the async state for all devices upfront to prevent the
> + * power.work_in_progress updates from racing with power.wakeup_path
> + * updates carried out by dpm_propagate_wakeup_to_parent(), since these
> + * flags belong to the same group of bit fields and they should not be
> + * updated at the same time without synchronization.
> + */
> + list_for_each_entry_reverse(dev, &dpm_suspended_list, power.entry)
> + dpm_clear_async_state(dev);
> +
> + /*
> * Start processing "async" leaf devices upfront so they don't need to
> * wait for the "sync" devices they don't depend on.
> */
> list_for_each_entry_reverse(dev, &dpm_suspended_list, power.entry) {
> - dpm_clear_async_state(dev);
> if (dpm_leaf_device(dev))
> dpm_async_with_cleanup(dev, async_suspend_late);
> }
> @@ -2023,11 +2041,20 @@ int dpm_suspend(pm_message_t state)
> mutex_lock(&dpm_list_mtx);
>
> /*
> + * Clear the async state for all devices upfront to prevent the
> + * power.work_in_progress updates from racing with power.wakeup_path
> + * updates carried out by dpm_propagate_wakeup_to_parent(), since these
> + * flags belong to the same group of bit fields and they should not be
> + * updated at the same time without synchronization.
> + */
> + list_for_each_entry_reverse(dev, &dpm_prepared_list, power.entry)
> + dpm_clear_async_state(dev);
> +
> + /*
> * Start processing "async" leaf devices upfront so they don't need to
> * wait for the "sync" devices they don't depend on.
> */
> list_for_each_entry_reverse(dev, &dpm_prepared_list, power.entry) {
> - dpm_clear_async_state(dev);
> if (dpm_leaf_device(dev))
> dpm_async_with_cleanup(dev, async_suspend);
> }
>
>
>
>
Powered by blists - more mailing lists