Linux Power Management development
 help / color / mirror / Atom feed
From: "Rafael J. Wysocki" <rjw@rjwysocki.net>
To: Linux PM <linux-pm@vger.kernel.org>
Cc: LKML <linux-kernel@vger.kernel.org>,
	Mario Limonciello <mario.limonciello@amd.com>,
	Chris Bainbridge <chris.bainbridge@gmail.com>,
	Ulf Hansson <ulf.hansson@linaro.org>,
	Saravana Kannan <saravanak@google.com>,
	Sudeep Holla <sudeep.holla@arm.com>
Subject: [PATCH v1 3/3] PM: sleep: Add locking to dpm_async_resume_children()
Date: Tue, 03 Jun 2025 18:21:57 +0200	[thread overview]
Message-ID: <13779172.uLZWGnKmhe@rjwysocki.net> (raw)
In-Reply-To: <6166108.lOV4Wx5bFT@rjwysocki.net>

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Commit 0cbef962ce1f ("PM: sleep: Resume children after resuming the
parent") introduced a subtle concurrency issue that may lead to a kernel
crash if system suspend is aborted and may also slow down asynchronous
device resume otherwise.

Namely, the initial list walks in dpm_noirq_resume_devices(),
dpm_resume_early(), and dpm_resume() call dpm_clear_async_state() for
every device and attepmt to asynchronously resume it if it has no
children (so it is a "root" device).  The asynchronous resume of a
root device triggers an attempt to asynchronously resume its children
which may take place before calling dpm_clear_async_state() for them
due to the lack of synchronization between dpm_async_resume_children()
and the code calling dpm_clear_async_state().  If this happens, the
dpm_clear_async_state() that comes in late, will clear
power.work_in_progress for the given device after it has been set by
__dpm_async(), so the suspend callback will be allowed to run once
again for the same device during the same transition.  This leads to
a whole range of interesting breakage.

Fortunately, if the suspend transition is not aborted, power.work_in_progress
is set by it for all devices, so dpm_async_resume_children() will not
schedule asynchronous resume for them until dpm_clear_async_state()
clears that flag, but this means missing an opportunity to start the
resume of those devices earlier.

Address the above issue by adding dpm_list_mtx locking to
dpm_async_resume_children(), so it will wait for the entire initial
list walk and the invocation of dpm_clear_async_state() for all devices
to be completed before scheduling any new asynchronous resume callbacks.

Fixes: 0cbef962ce1f ("PM: sleep: Resume children after resuming the parent")
Reported-and-tested-by: Chris Bainbridge <chris.bainbridge@gmail.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/base/power/main.c |    7 +++++++
 1 file changed, 7 insertions(+)

--- a/drivers/base/power/main.c
+++ b/drivers/base/power/main.c
@@ -638,6 +638,13 @@
 static void dpm_async_resume_children(struct device *dev, async_func_t func)
 {
 	/*
+	 * Prevent racing with dpm_clear_async_state() during initial list
+	 * walks in dpm_noirq_resume_devices(), dpm_resume_early(), and
+	 * dpm_resume().
+	 */
+	guard(mutex)(&dpm_list_mtx);
+
+	/*
 	 * Start processing "async" children of the device unless it's been
 	 * started already for them.
 	 *




  parent reply	other threads:[~2025-06-03 16:22 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-06-03 16:16 [PATCH v1 0/3] PM: sleep: Fixes related to aborted suspend handling Rafael J. Wysocki
2025-06-03 16:17 ` [PATCH v1 1/3] PM: sleep: Fix list splicing in device suspend error paths Rafael J. Wysocki
2025-06-03 16:19 ` [PATCH v1 2/3] PM: sleep: Fix power.is_suspended cleanup for direct-complete devices Rafael J. Wysocki
2025-06-03 16:21 ` Rafael J. Wysocki [this message]
2025-06-03 16:27   ` [PATCH v1 3/3] PM: sleep: Add locking to dpm_async_resume_children() Limonciello, Mario
2025-06-03 16:31     ` Rafael J. Wysocki
2025-06-03 16:28 ` [PATCH v1 0/3] PM: sleep: Fixes related to aborted suspend handling Limonciello, Mario

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=13779172.uLZWGnKmhe@rjwysocki.net \
    --to=rjw@rjwysocki.net \
    --cc=chris.bainbridge@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=mario.limonciello@amd.com \
    --cc=saravanak@google.com \
    --cc=sudeep.holla@arm.com \
    --cc=ulf.hansson@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox