io_uring/io-wq.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-)
When CONFIG_PREEMPT_RT enable, raw_spin_lock() will preempt_disable()
-> raw_spin_lock()
-> wake_up()
...
-> spin_lock_irqsave()
which will trigger:
BUG: sleeping function called from invalid context at kernel/locking/spinlock_rt.c:46
in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 985654, name: iou-wrk-983605
preempt_count: 1, expected: 0
RCU nest depth: 0, expected: 0
CPU: 3 PID: 985654 Comm: iou-wrk-983605 Tainted: G O 6.1.83-rt28-g19631eb82f21
stack:0 ppid:977479 flags:0x00000008
tgid:977553 prio:120 preempt:0x100000001 rcu_read_lock_nesting:0
used_cpu 3 wake_cpu 3 on_cpu 3 on_rq 1 migrate_dis 0
arrive:17423426040675 queued:0 prev_sum:33955300 sum_exec:33955300
Call trace:
dump_backtrace.part.0+0xdc/0xec
show_stack+0x1c/0x30
dump_stack_lvl+0xac/0xc4
dump_stack+0x14/0x30
__might_resched+0x13c/0x170
rt_spin_lock+0x34/0xc0
__wake_up_common_lock+0x68/0xd0
__wake_up+0x1c/0x24
io_worker_handle_work+0x5b0/0x600
io_wqe_worker+0xf4/0x310
ret_from_fork+0x10/0x20
Signed-off-by: Xiaochuan Li <chuanx2070@163.com>
---
Changes in v2:
io_uring/io-wq: fix lockdep warning by deferring hash wake up outside acct->lock
The stall wake up path in io_get_next_work() holds acct->lock while
calling wake_up() on the hash wait queue, which creates lock ordering
acct->lock -> hash->wait.lock and triggers lockdep circular dependency
warning.
The previous approach of temporarily dropping and retaking acct->lock
is racy and juggles the lock unnecessarily. Instead, add a need_wake
output flag to io_get_next_work() and defer the wake_up() to the outer
worker loop, after acct->lock has been released.
This preserves the calling convention that io_get_next_work() returns
with acct->lock held, removes the lock inversion, and avoids any racy
sleeper checks outside of the lock. Drop the wq_has_sleeper check as
bare wake_up is safe and the optimization is not worth the complexity.
io_uring/io-wq.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/io_uring/io-wq.c b/io_uring/io-wq.c
index 2ca223e47d41..7a167f409379 100644
--- a/io_uring/io-wq.c
+++ b/io_uring/io-wq.c
@@ -526,7 +526,8 @@ static bool io_wait_on_hash(struct io_wq *wq, unsigned int hash)
}
static struct io_wq_work *io_get_next_work(struct io_wq_acct *acct,
- struct io_wq *wq)
+ struct io_wq *wq
+ bool *need_wake)
__must_hold(acct->lock)
{
struct io_wq_work_node *node, *prev;
@@ -574,9 +575,8 @@ static struct io_wq_work *io_get_next_work(struct io_wq_acct *acct,
unstalled = io_wait_on_hash(wq, stall_hash);
raw_spin_lock(&acct->lock);
if (unstalled) {
+ *need_wake = true;
clear_bit(IO_ACCT_STALLED_BIT, &acct->flags);
- if (wq_has_sleeper(&wq->hash->wait))
- wake_up(&wq->hash->wait);
}
}
@@ -607,6 +607,7 @@ static void io_worker_handle_work(struct io_wq_acct *acct,
do {
struct io_wq_work *work;
+ bool need_wake = false;
/*
* If we got some work, mark us as busy. If we didn't, but
@@ -615,7 +616,7 @@ static void io_worker_handle_work(struct io_wq_acct *acct,
* can't make progress, any work completion or insertion will
* clear the stalled flag.
*/
- work = io_get_next_work(acct, wq);
+ work = io_get_next_work(acct, wq, &need_wake);
if (work) {
/*
* Make sure cancelation can find this, even before
@@ -631,6 +632,9 @@ static void io_worker_handle_work(struct io_wq_acct *acct,
raw_spin_unlock(&acct->lock);
+ if (need_wake)
+ wake_up(&wq->hash->wait);
+
if (!work)
break;
base-commit: a8c591ed6b672915e0be57843f943a2a723aff40
--
2.43.0Xiaochuan Li <chuanx2070@163.com> writes: > When CONFIG_PREEMPT_RT enable, raw_spin_lock() will preempt_disable() The subject prefix is wrong. It should be "io_uring/io-wq". Also, It might be that I just didn't find it, but was there a v1 of this patch? I can't find it on the list. > which will trigger: > BUG: sleeping function called from invalid context at kernel/locking/spinlock_rt.c:46 > in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 985654, name: iou-wrk-983605 > preempt_count: 1, expected: 0 > RCU nest depth: 0, expected: 0 > CPU: 3 PID: 985654 Comm: iou-wrk-983605 Tainted: G O 6.1.83-rt28-g19631eb82f21 > stack:0 ppid:977479 flags:0x00000008 > tgid:977553 prio:120 preempt:0x100000001 rcu_read_lock_nesting:0 > used_cpu 3 wake_cpu 3 on_cpu 3 on_rq 1 migrate_dis 0 > arrive:17423426040675 queued:0 prev_sum:33955300 sum_exec:33955300 > Call trace: > dump_backtrace.part.0+0xdc/0xec > show_stack+0x1c/0x30 > dump_stack_lvl+0xac/0xc4 > dump_stack+0x14/0x30 > __might_resched+0x13c/0x170 > rt_spin_lock+0x34/0xc0 > __wake_up_common_lock+0x68/0xd0 > __wake_up+0x1c/0x24 > io_worker_handle_work+0x5b0/0x600 > io_wqe_worker+0xf4/0x310 > ret_from_fork+0x10/0x20 > > Signed-off-by: Xiaochuan Li <chuanx2070@163.com> > --- > Changes in v2: > io_uring/io-wq: fix lockdep warning by deferring hash wake up outside acct->lock > > The stall wake up path in io_get_next_work() holds acct->lock while > calling wake_up() on the hash wait queue, which creates lock ordering > acct->lock -> hash->wait.lock and triggers lockdep circular dependency > warning. > > The previous approach of temporarily dropping and retaking acct->lock > is racy and juggles the lock unnecessarily. Instead, add a need_wake > output flag to io_get_next_work() and defer the wake_up() to the outer > worker loop, after acct->lock has been released. what previous approach? > > This preserves the calling convention that io_get_next_work() returns > with acct->lock held, removes the lock inversion, and avoids any racy > sleeper checks outside of the lock. Drop the wq_has_sleeper check as > bare wake_up is safe and the optimization is not worth the complexity. Either way, these paragraphs should be part of the commit message. By putting them after the ---, they get dropped at commit-time. -- Gabriel Krisman Bertazi
On 9/23/26 3:13 PM, Gabriel Krisman Bertazi wrote: > Xiaochuan Li <chuanx2070@163.com> writes: > >> When CONFIG_PREEMPT_RT enable, raw_spin_lock() will preempt_disable() > > The subject prefix is wrong. It should be "io_uring/io-wq". > > Also, It might be that I just didn't find it, but was there a v1 of this > patch? I can't find it on the list. There should be a v1 and a v2, off the same thread. >> which will trigger: >> BUG: sleeping function called from invalid context at kernel/locking/spinlock_rt.c:46 >> in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 985654, name: iou-wrk-983605 >> preempt_count: 1, expected: 0 >> RCU nest depth: 0, expected: 0 >> CPU: 3 PID: 985654 Comm: iou-wrk-983605 Tainted: G O 6.1.83-rt28-g19631eb82f21 >> stack:0 ppid:977479 flags:0x00000008 >> tgid:977553 prio:120 preempt:0x100000001 rcu_read_lock_nesting:0 >> used_cpu 3 wake_cpu 3 on_cpu 3 on_rq 1 migrate_dis 0 >> arrive:17423426040675 queued:0 prev_sum:33955300 sum_exec:33955300 >> Call trace: >> dump_backtrace.part.0+0xdc/0xec >> show_stack+0x1c/0x30 >> dump_stack_lvl+0xac/0xc4 >> dump_stack+0x14/0x30 >> __might_resched+0x13c/0x170 >> rt_spin_lock+0x34/0xc0 >> __wake_up_common_lock+0x68/0xd0 >> __wake_up+0x1c/0x24 >> io_worker_handle_work+0x5b0/0x600 >> io_wqe_worker+0xf4/0x310 >> ret_from_fork+0x10/0x20 >> >> Signed-off-by: Xiaochuan Li <chuanx2070@163.com> >> --- >> Changes in v2: >> io_uring/io-wq: fix lockdep warning by deferring hash wake up outside acct->lock >> >> The stall wake up path in io_get_next_work() holds acct->lock while >> calling wake_up() on the hash wait queue, which creates lock ordering >> acct->lock -> hash->wait.lock and triggers lockdep circular dependency >> warning. >> >> The previous approach of temporarily dropping and retaking acct->lock >> is racy and juggles the lock unnecessarily. Instead, add a need_wake >> output flag to io_get_next_work() and defer the wake_up() to the outer >> worker loop, after acct->lock has been released. > > what previous approach? > >> >> This preserves the calling convention that io_get_next_work() returns >> with acct->lock held, removes the lock inversion, and avoids any racy >> sleeper checks outside of the lock. Drop the wq_has_sleeper check as >> bare wake_up is safe and the optimization is not worth the complexity. > > Either way, these paragraphs should be part of the commit message. By > putting them after the ---, they get dropped at commit-time. I rewrote most of the commit message when applying, fwiw. You can find it in my tree. -- Jens Axboe
On 9/23/26 10:16 AM, Xiaochuan Li wrote:
> @@ -574,9 +575,8 @@ static struct io_wq_work *io_get_next_work(struct io_wq_acct *acct,
> unstalled = io_wait_on_hash(wq, stall_hash);
> raw_spin_lock(&acct->lock);
> if (unstalled) {
> + *need_wake = true;
> clear_bit(IO_ACCT_STALLED_BIT, &acct->flags);
> - if (wq_has_sleeper(&wq->hash->wait))
> - wake_up(&wq->hash->wait);
> }
> }
*need_wake = wq_has_sleeper(&wq->hash->wait);
? And probably retain the sme ordering with the bit clear.
Apart from that, I think it looks fine. The lock shuffling is somewhat
annoying and inefficient, an unfortunate side effect of the raw vs
normal spinlocks.
--
Jens Axboe
© 2016 - 2026 Red Hat, Inc.