* [PATCH] PM: sleep: annotate RCU list iterations
@ 2025-08-26 10:55 Johannes Berg
2025-08-26 11:41 ` Rafael J. Wysocki
0 siblings, 1 reply; 2+ messages in thread
From: Johannes Berg @ 2025-08-26 10:55 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Pavel Machek, Len Brown, Greg Kroah-Hartman, Danilo Krummrich,
linux-pm, linux-kernel, Johannes Berg
From: Johannes Berg <johannes.berg@intel.com>
These iterations require the read lock, otherwise RCU
lockdep will splat:
=============================
WARNING: suspicious RCU usage
6.17.0-rc3-00014-g31419c045d64 #6 Tainted: G O
-----------------------------
drivers/base/power/main.c:1333 RCU-list traversed in non-reader section!!
other info that might help us debug this:
rcu_scheduler_active = 2, debug_locks = 1
5 locks held by rtcwake/547:
#0: 00000000643ab418 (sb_writers#6){.+.+}-{0:0}, at: file_start_write+0x2b/0x3a
#1: 0000000067a0ca88 (&of->mutex#2){+.+.}-{4:4}, at: kernfs_fop_write_iter+0x181/0x24b
#2: 00000000631eac40 (kn->active#3){.+.+}-{0:0}, at: kernfs_fop_write_iter+0x191/0x24b
#3: 00000000609a1308 (system_transition_mutex){+.+.}-{4:4}, at: pm_suspend+0xaf/0x30b
#4: 0000000060c0fdb0 (device_links_srcu){.+.+}-{0:0}, at: device_links_read_lock+0x75/0x98
stack backtrace:
CPU: 0 UID: 0 PID: 547 Comm: rtcwake Tainted: G O 6.17.0-rc3-00014-g31419c045d64 #6 VOLUNTARY
Tainted: [O]=OOT_MODULE
Stack:
223721b3a80 6089eac6 00000001 00000001
ffffff00 6089eac6 00000535 6086e528
721b3ac0 6003c294 00000000 60031fc0
Call Trace:
[<600407ed>] show_stack+0x10e/0x127
[<6003c294>] dump_stack_lvl+0x77/0xc6
[<6003c2fd>] dump_stack+0x1a/0x20
[<600bc2f8>] lockdep_rcu_suspicious+0x116/0x13e
[<603d8ea1>] dpm_async_suspend_superior+0x117/0x17e
[<603d980f>] device_suspend+0x528/0x541
[<603da24b>] dpm_suspend+0x1a2/0x267
[<603da837>] dpm_suspend_start+0x5d/0x72
[<600ca0c9>] suspend_devices_and_enter+0xab/0x736
[...]
Add the fourth argument to the iteration to annotate
this and avoid the splat.
Fixes: 06799631d522 ("PM: sleep: Make async suspend handle suppliers like parents")
Fixes: ed18738fff02 ("PM: sleep: Make async resume handle consumers like children")
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
Honestly, not sure, maybe this should just be without _rcu?
---
drivers/base/power/main.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
index dbf5456cd891..e80175486be7 100644
--- a/drivers/base/power/main.c
+++ b/drivers/base/power/main.c
@@ -675,7 +675,8 @@ static void dpm_async_resume_subordinate(struct device *dev, async_func_t func)
idx = device_links_read_lock();
/* Start processing the device's "async" consumers. */
- list_for_each_entry_rcu(link, &dev->links.consumers, s_node)
+ list_for_each_entry_rcu(link, &dev->links.consumers, s_node,
+ device_links_read_lock_held())
if (READ_ONCE(link->status) != DL_STATE_DORMANT)
dpm_async_with_cleanup(link->consumer, func);
@@ -1330,7 +1331,8 @@ static void dpm_async_suspend_superior(struct device *dev, async_func_t func)
idx = device_links_read_lock();
/* Start processing the device's "async" suppliers. */
- list_for_each_entry_rcu(link, &dev->links.suppliers, c_node)
+ list_for_each_entry_rcu(link, &dev->links.suppliers, c_node,
+ device_links_read_lock_held())
if (READ_ONCE(link->status) != DL_STATE_DORMANT)
dpm_async_with_cleanup(link->supplier, func);
--
2.51.0
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] PM: sleep: annotate RCU list iterations
2025-08-26 10:55 [PATCH] PM: sleep: annotate RCU list iterations Johannes Berg
@ 2025-08-26 11:41 ` Rafael J. Wysocki
0 siblings, 0 replies; 2+ messages in thread
From: Rafael J. Wysocki @ 2025-08-26 11:41 UTC (permalink / raw)
To: Johannes Berg
Cc: Rafael J. Wysocki, Pavel Machek, Len Brown, Greg Kroah-Hartman,
Danilo Krummrich, linux-pm, linux-kernel, Johannes Berg
On Tue, Aug 26, 2025 at 12:55 PM Johannes Berg
<johannes@sipsolutions.net> wrote:
>
> From: Johannes Berg <johannes.berg@intel.com>
>
> These iterations require the read lock, otherwise RCU
> lockdep will splat:
>
> =============================
> WARNING: suspicious RCU usage
> 6.17.0-rc3-00014-g31419c045d64 #6 Tainted: G O
> -----------------------------
> drivers/base/power/main.c:1333 RCU-list traversed in non-reader section!!
>
> other info that might help us debug this:
>
> rcu_scheduler_active = 2, debug_locks = 1
> 5 locks held by rtcwake/547:
> #0: 00000000643ab418 (sb_writers#6){.+.+}-{0:0}, at: file_start_write+0x2b/0x3a
> #1: 0000000067a0ca88 (&of->mutex#2){+.+.}-{4:4}, at: kernfs_fop_write_iter+0x181/0x24b
> #2: 00000000631eac40 (kn->active#3){.+.+}-{0:0}, at: kernfs_fop_write_iter+0x191/0x24b
> #3: 00000000609a1308 (system_transition_mutex){+.+.}-{4:4}, at: pm_suspend+0xaf/0x30b
> #4: 0000000060c0fdb0 (device_links_srcu){.+.+}-{0:0}, at: device_links_read_lock+0x75/0x98
>
> stack backtrace:
> CPU: 0 UID: 0 PID: 547 Comm: rtcwake Tainted: G O 6.17.0-rc3-00014-g31419c045d64 #6 VOLUNTARY
> Tainted: [O]=OOT_MODULE
> Stack:
> 223721b3a80 6089eac6 00000001 00000001
> ffffff00 6089eac6 00000535 6086e528
> 721b3ac0 6003c294 00000000 60031fc0
> Call Trace:
> [<600407ed>] show_stack+0x10e/0x127
> [<6003c294>] dump_stack_lvl+0x77/0xc6
> [<6003c2fd>] dump_stack+0x1a/0x20
> [<600bc2f8>] lockdep_rcu_suspicious+0x116/0x13e
> [<603d8ea1>] dpm_async_suspend_superior+0x117/0x17e
> [<603d980f>] device_suspend+0x528/0x541
> [<603da24b>] dpm_suspend+0x1a2/0x267
> [<603da837>] dpm_suspend_start+0x5d/0x72
> [<600ca0c9>] suspend_devices_and_enter+0xab/0x736
> [...]
>
> Add the fourth argument to the iteration to annotate
> this and avoid the splat.
Yeah, good catch!
> Fixes: 06799631d522 ("PM: sleep: Make async suspend handle suppliers like parents")
> Fixes: ed18738fff02 ("PM: sleep: Make async resume handle consumers like children")
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>
> ---
> Honestly, not sure, maybe this should just be without _rcu?
> ---
> drivers/base/power/main.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
> index dbf5456cd891..e80175486be7 100644
> --- a/drivers/base/power/main.c
> +++ b/drivers/base/power/main.c
> @@ -675,7 +675,8 @@ static void dpm_async_resume_subordinate(struct device *dev, async_func_t func)
> idx = device_links_read_lock();
>
> /* Start processing the device's "async" consumers. */
> - list_for_each_entry_rcu(link, &dev->links.consumers, s_node)
> + list_for_each_entry_rcu(link, &dev->links.consumers, s_node,
> + device_links_read_lock_held())
There is a macro for this already in main.c, it is called
list_for_each_entry_rcu_locked().
> if (READ_ONCE(link->status) != DL_STATE_DORMANT)
> dpm_async_with_cleanup(link->consumer, func);
>
> @@ -1330,7 +1331,8 @@ static void dpm_async_suspend_superior(struct device *dev, async_func_t func)
> idx = device_links_read_lock();
>
> /* Start processing the device's "async" suppliers. */
> - list_for_each_entry_rcu(link, &dev->links.suppliers, c_node)
> + list_for_each_entry_rcu(link, &dev->links.suppliers, c_node,
> + device_links_read_lock_held())
> if (READ_ONCE(link->status) != DL_STATE_DORMANT)
> dpm_async_with_cleanup(link->supplier, func);
>
> --
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2025-08-26 11:42 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-26 10:55 [PATCH] PM: sleep: annotate RCU list iterations Johannes Berg
2025-08-26 11:41 ` Rafael J. Wysocki
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).