Skip to content

Commit a9c83a0

Browse files
committed
io_uring/timeout: flush timeouts outside of the timeout lock
syzbot reports that a recent fix causes nesting issues between the (now) raw timeoutlock and the eventfd locking: ============================= [ BUG: Invalid wait context ] 6.13.0-rc4-00080-g9828a4c0901f Rust-for-Linux#29 Not tainted ----------------------------- kworker/u32:0/68094 is trying to lock: ffff000014d7a520 (&ctx->wqh#2){..-.}-{3:3}, at: eventfd_signal_mask+0x64/0x180 other info that might help us debug this: context-{5:5} 6 locks held by kworker/u32:0/68094: #0: ffff0000c1d98148 ((wq_completion)iou_exit){+.+.}-{0:0}, at: process_one_work+0x4e8/0xfc0 Rust-for-Linux#1: ffff80008d927c78 ((work_completion)(&ctx->exit_work)){+.+.}-{0:0}, at: process_one_work+0x53c/0xfc0 Rust-for-Linux#2: ffff0000c59bc3d8 (&ctx->completion_lock){+.+.}-{3:3}, at: io_kill_timeouts+0x40/0x180 Rust-for-Linux#3: ffff0000c59bc358 (&ctx->timeout_lock){-.-.}-{2:2}, at: io_kill_timeouts+0x48/0x180 Rust-for-Linux#4: ffff800085127aa0 (rcu_read_lock){....}-{1:3}, at: rcu_lock_acquire+0x8/0x38 Rust-for-Linux#5: ffff800085127aa0 (rcu_read_lock){....}-{1:3}, at: rcu_lock_acquire+0x8/0x38 stack backtrace: CPU: 7 UID: 0 PID: 68094 Comm: kworker/u32:0 Not tainted 6.13.0-rc4-00080-g9828a4c0901f Rust-for-Linux#29 Hardware name: linux,dummy-virt (DT) Workqueue: iou_exit io_ring_exit_work Call trace: show_stack+0x1c/0x30 (C) __dump_stack+0x24/0x30 dump_stack_lvl+0x60/0x80 dump_stack+0x14/0x20 __lock_acquire+0x19f8/0x60c8 lock_acquire+0x1a4/0x540 _raw_spin_lock_irqsave+0x90/0xd0 eventfd_signal_mask+0x64/0x180 io_eventfd_signal+0x64/0x108 io_req_local_work_add+0x294/0x430 __io_req_task_work_add+0x1c0/0x270 io_kill_timeout+0x1f0/0x288 io_kill_timeouts+0xd4/0x180 io_uring_try_cancel_requests+0x2e8/0x388 io_ring_exit_work+0x150/0x550 process_one_work+0x5e8/0xfc0 worker_thread+0x7ec/0xc80 kthread+0x24c/0x300 ret_from_fork+0x10/0x20 because after the preempt-rt fix for the timeout lock nesting inside the io-wq lock, we now have the eventfd spinlock nesting inside the raw timeout spinlock. Rather than play whack-a-mole with other nesting on the timeout lock, split the deletion and killing of timeouts so queueing the task_work for the timeout cancelations can get done outside of the timeout lock. Reported-by: [email protected] Fixes: 020b40f ("io_uring: make ctx->timeout_lock a raw spinlock") Signed-off-by: Jens Axboe <[email protected]>
1 parent 38fc96a commit a9c83a0

File tree

1 file changed

+31
-14
lines changed

1 file changed

+31
-14
lines changed

io_uring/timeout.c

+31-14
Original file line numberDiff line numberDiff line change
@@ -85,29 +85,45 @@ static void io_timeout_complete(struct io_kiocb *req, struct io_tw_state *ts)
8585
io_req_task_complete(req, ts);
8686
}
8787

88-
static bool io_kill_timeout(struct io_kiocb *req, int status)
88+
static __cold bool io_flush_killed_timeouts(struct list_head *list, int err)
89+
{
90+
if (list_empty(list))
91+
return false;
92+
93+
while (!list_empty(list)) {
94+
struct io_timeout *timeout;
95+
struct io_kiocb *req;
96+
97+
timeout = list_first_entry(list, struct io_timeout, list);
98+
list_del_init(&timeout->list);
99+
req = cmd_to_io_kiocb(timeout);
100+
if (err)
101+
req_set_fail(req);
102+
io_req_queue_tw_complete(req, err);
103+
}
104+
105+
return true;
106+
}
107+
108+
static void io_kill_timeout(struct io_kiocb *req, struct list_head *list)
89109
__must_hold(&req->ctx->timeout_lock)
90110
{
91111
struct io_timeout_data *io = req->async_data;
92112

93113
if (hrtimer_try_to_cancel(&io->timer) != -1) {
94114
struct io_timeout *timeout = io_kiocb_to_cmd(req, struct io_timeout);
95115

96-
if (status)
97-
req_set_fail(req);
98116
atomic_set(&req->ctx->cq_timeouts,
99117
atomic_read(&req->ctx->cq_timeouts) + 1);
100-
list_del_init(&timeout->list);
101-
io_req_queue_tw_complete(req, status);
102-
return true;
118+
list_move_tail(&timeout->list, list);
103119
}
104-
return false;
105120
}
106121

107122
__cold void io_flush_timeouts(struct io_ring_ctx *ctx)
108123
{
109-
u32 seq;
110124
struct io_timeout *timeout, *tmp;
125+
LIST_HEAD(list);
126+
u32 seq;
111127

112128
raw_spin_lock_irq(&ctx->timeout_lock);
113129
seq = ctx->cached_cq_tail - atomic_read(&ctx->cq_timeouts);
@@ -131,10 +147,11 @@ __cold void io_flush_timeouts(struct io_ring_ctx *ctx)
131147
if (events_got < events_needed)
132148
break;
133149

134-
io_kill_timeout(req, 0);
150+
io_kill_timeout(req, &list);
135151
}
136152
ctx->cq_last_tm_flush = seq;
137153
raw_spin_unlock_irq(&ctx->timeout_lock);
154+
io_flush_killed_timeouts(&list, 0);
138155
}
139156

140157
static void io_req_tw_fail_links(struct io_kiocb *link, struct io_tw_state *ts)
@@ -661,7 +678,7 @@ __cold bool io_kill_timeouts(struct io_ring_ctx *ctx, struct io_uring_task *tctx
661678
bool cancel_all)
662679
{
663680
struct io_timeout *timeout, *tmp;
664-
int canceled = 0;
681+
LIST_HEAD(list);
665682

666683
/*
667684
* completion_lock is needed for io_match_task(). Take it before
@@ -672,11 +689,11 @@ __cold bool io_kill_timeouts(struct io_ring_ctx *ctx, struct io_uring_task *tctx
672689
list_for_each_entry_safe(timeout, tmp, &ctx->timeout_list, list) {
673690
struct io_kiocb *req = cmd_to_io_kiocb(timeout);
674691

675-
if (io_match_task(req, tctx, cancel_all) &&
676-
io_kill_timeout(req, -ECANCELED))
677-
canceled++;
692+
if (io_match_task(req, tctx, cancel_all))
693+
io_kill_timeout(req, &list);
678694
}
679695
raw_spin_unlock_irq(&ctx->timeout_lock);
680696
spin_unlock(&ctx->completion_lock);
681-
return canceled != 0;
697+
698+
return io_flush_killed_timeouts(&list, -ECANCELED);
682699
}

0 commit comments

Comments
 (0)