* [PATCH 1/3] io_uring: fix nested timeout locking on disarming
2022-04-20 12:40 [PATCH for-next 0/3] timeout fixes & improvements Pavel Begunkov
@ 2022-04-20 12:40 ` Pavel Begunkov
2022-04-20 12:40 ` [PATCH 2/3] io_uring: move tout locking in io_timeout_cancel() Pavel Begunkov
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Pavel Begunkov @ 2022-04-20 12:40 UTC (permalink / raw)
To: io-uring; +Cc: Jens Axboe, asml.silence, syzbot+57e67273f92d7f5f1931
WARNING: possible recursive locking detected
syz-executor162/3588 is trying to acquire lock:
ffff888011a453d8 (&ctx->timeout_lock){....}-{2:2}, at: spin_lock_irq include/linux/spinlock.h:379 [inline]
ffff888011a453d8 (&ctx->timeout_lock){....}-{2:2}, at: io_disarm_next+0x545/0xaa0 fs/io_uring.c:2452
but task is already holding lock:
ffff888011a453d8 (&ctx->timeout_lock){....}-{2:2}, at: spin_lock_irq include/linux/spinlock.h:379 [inline]
ffff888011a453d8 (&ctx->timeout_lock){....}-{2:2}, at: io_kill_timeouts+0x4c/0x227 fs/io_uring.c:10432
Call Trace:
<TASK>
...
spin_lock_irq include/linux/spinlock.h:379 [inline]
io_disarm_next+0x545/0xaa0 fs/io_uring.c:2452
__io_req_complete_post+0x794/0xd90 fs/io_uring.c:2200
io_kill_timeout fs/io_uring.c:1815 [inline]
io_kill_timeout+0x210/0x21d fs/io_uring.c:1803
io_kill_timeouts+0xe2/0x227 fs/io_uring.c:10435
io_ring_ctx_wait_and_kill+0x1eb/0x360 fs/io_uring.c:10462
io_uring_release+0x42/0x46 fs/io_uring.c:10483
__fput+0x277/0x9d0 fs/file_table.c:317
task_work_run+0xdd/0x1a0 kernel/task_work.c:164
...
Return tw deferred putting back, it's easier than looking after all
potential nested locking. However, instead of filling an CQE on the spot
as it was before delay it as well by io_req_complete_post() via tw.
Reported-by: [email protected]
Fixes: 78bfbdd1a497 ("io_uring: kill io_put_req_deferred()")
Signed-off-by: Pavel Begunkov <[email protected]>
---
fs/io_uring.c | 20 +++++++++++++++++---
1 file changed, 17 insertions(+), 3 deletions(-)
diff --git a/fs/io_uring.c b/fs/io_uring.c
index 3905b3ec87b8..2b9a3af9ff42 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -1214,6 +1214,7 @@ static int io_close_fixed(struct io_kiocb *req, unsigned int issue_flags);
static enum hrtimer_restart io_link_timeout_fn(struct hrtimer *timer);
static void io_eventfd_signal(struct io_ring_ctx *ctx);
+static void io_req_tw_post_queue(struct io_kiocb *req, s32 res, u32 cflags);
static struct kmem_cache *req_cachep;
@@ -1782,7 +1783,7 @@ static void io_kill_timeout(struct io_kiocb *req, int status)
atomic_set(&req->ctx->cq_timeouts,
atomic_read(&req->ctx->cq_timeouts) + 1);
list_del_init(&req->timeout.list);
- __io_req_complete_post(req, status, 0);
+ io_req_tw_post_queue(req, status, 0);
}
}
@@ -2367,7 +2368,7 @@ static bool io_kill_linked_timeout(struct io_kiocb *req)
link->timeout.head = NULL;
if (hrtimer_try_to_cancel(&io->timer) != -1) {
list_del(&link->timeout.list);
- __io_req_complete_post(link, -ECANCELED, 0);
+ io_req_tw_post_queue(link, -ECANCELED, 0);
return true;
}
}
@@ -2413,7 +2414,7 @@ static bool io_disarm_next(struct io_kiocb *req)
req->flags &= ~REQ_F_ARM_LTIMEOUT;
if (link && link->opcode == IORING_OP_LINK_TIMEOUT) {
io_remove_next_linked(req);
- __io_req_complete_post(link, -ECANCELED, 0);
+ io_req_tw_post_queue(link, -ECANCELED, 0);
posted = true;
}
} else if (req->flags & REQ_F_LINK_TIMEOUT) {
@@ -2632,6 +2633,19 @@ static void io_req_task_work_add(struct io_kiocb *req, bool priority)
}
}
+static void io_req_tw_post(struct io_kiocb *req, bool *locked)
+{
+ io_req_complete_post(req, req->cqe.res, req->cqe.flags);
+}
+
+static void io_req_tw_post_queue(struct io_kiocb *req, s32 res, u32 cflags)
+{
+ req->cqe.res = res;
+ req->cqe.flags = cflags;
+ req->io_task_work.func = io_req_tw_post;
+ io_req_task_work_add(req, false);
+}
+
static void io_req_task_cancel(struct io_kiocb *req, bool *locked)
{
/* not needed for normal modes, but SQPOLL depends on it */
--
2.36.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH 2/3] io_uring: move tout locking in io_timeout_cancel()
2022-04-20 12:40 [PATCH for-next 0/3] timeout fixes & improvements Pavel Begunkov
2022-04-20 12:40 ` [PATCH 1/3] io_uring: fix nested timeout locking on disarming Pavel Begunkov
@ 2022-04-20 12:40 ` Pavel Begunkov
2022-04-20 12:40 ` [PATCH 3/3] io_uring: refactor io_disarm_next() locking Pavel Begunkov
2022-04-20 22:24 ` [PATCH for-next 0/3] timeout fixes & improvements Jens Axboe
3 siblings, 0 replies; 5+ messages in thread
From: Pavel Begunkov @ 2022-04-20 12:40 UTC (permalink / raw)
To: io-uring; +Cc: Jens Axboe, asml.silence
Move ->timeout_lock grabbing inside of io_timeout_cancel(), so
we can do io_req_task_queue_fail() outside of the lock. It's much nicer
than relying on triple nested locking.
Signed-off-by: Pavel Begunkov <[email protected]>
---
fs/io_uring.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/fs/io_uring.c b/fs/io_uring.c
index 2b9a3af9ff42..ce5941537b64 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -6489,7 +6489,11 @@ static int io_timeout_cancel(struct io_ring_ctx *ctx, __u64 user_data)
__must_hold(&ctx->completion_lock)
__must_hold(&ctx->timeout_lock)
{
- struct io_kiocb *req = io_timeout_extract(ctx, user_data);
+ struct io_kiocb *req;
+
+ spin_lock_irq(&ctx->timeout_lock);
+ req = io_timeout_extract(ctx, user_data);
+ spin_unlock_irq(&ctx->timeout_lock);
if (IS_ERR(req))
return PTR_ERR(req);
@@ -6608,9 +6612,7 @@ static int io_timeout_remove(struct io_kiocb *req, unsigned int issue_flags)
if (!(req->timeout_rem.flags & IORING_TIMEOUT_UPDATE)) {
spin_lock(&ctx->completion_lock);
- spin_lock_irq(&ctx->timeout_lock);
ret = io_timeout_cancel(ctx, tr->addr);
- spin_unlock_irq(&ctx->timeout_lock);
spin_unlock(&ctx->completion_lock);
} else {
enum hrtimer_mode mode = io_translate_timeout_mode(tr->flags);
@@ -6796,10 +6798,7 @@ static int io_try_cancel_userdata(struct io_kiocb *req, u64 sqe_addr)
ret = io_poll_cancel(ctx, sqe_addr, false);
if (ret != -ENOENT)
goto out;
-
- spin_lock_irq(&ctx->timeout_lock);
ret = io_timeout_cancel(ctx, sqe_addr);
- spin_unlock_irq(&ctx->timeout_lock);
out:
spin_unlock(&ctx->completion_lock);
return ret;
--
2.36.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH 3/3] io_uring: refactor io_disarm_next() locking
2022-04-20 12:40 [PATCH for-next 0/3] timeout fixes & improvements Pavel Begunkov
2022-04-20 12:40 ` [PATCH 1/3] io_uring: fix nested timeout locking on disarming Pavel Begunkov
2022-04-20 12:40 ` [PATCH 2/3] io_uring: move tout locking in io_timeout_cancel() Pavel Begunkov
@ 2022-04-20 12:40 ` Pavel Begunkov
2022-04-20 22:24 ` [PATCH for-next 0/3] timeout fixes & improvements Jens Axboe
3 siblings, 0 replies; 5+ messages in thread
From: Pavel Begunkov @ 2022-04-20 12:40 UTC (permalink / raw)
To: io-uring; +Cc: Jens Axboe, asml.silence
Split timeout handling into removal + failing, so we can reduce
spinlocking time and remove another instance of triple nested locking.
Signed-off-by: Pavel Begunkov <[email protected]>
---
fs/io_uring.c | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/fs/io_uring.c b/fs/io_uring.c
index ce5941537b64..1572edf97c06 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -2355,7 +2355,7 @@ static inline void io_remove_next_linked(struct io_kiocb *req)
nxt->link = NULL;
}
-static bool io_kill_linked_timeout(struct io_kiocb *req)
+static struct io_kiocb *io_disarm_linked_timeout(struct io_kiocb *req)
__must_hold(&req->ctx->completion_lock)
__must_hold(&req->ctx->timeout_lock)
{
@@ -2368,11 +2368,10 @@ static bool io_kill_linked_timeout(struct io_kiocb *req)
link->timeout.head = NULL;
if (hrtimer_try_to_cancel(&io->timer) != -1) {
list_del(&link->timeout.list);
- io_req_tw_post_queue(link, -ECANCELED, 0);
- return true;
+ return link;
}
}
- return false;
+ return NULL;
}
static void io_fail_links(struct io_kiocb *req)
@@ -2406,11 +2405,11 @@ static void io_fail_links(struct io_kiocb *req)
static bool io_disarm_next(struct io_kiocb *req)
__must_hold(&req->ctx->completion_lock)
{
+ struct io_kiocb *link = NULL;
bool posted = false;
if (req->flags & REQ_F_ARM_LTIMEOUT) {
- struct io_kiocb *link = req->link;
-
+ link = req->link;
req->flags &= ~REQ_F_ARM_LTIMEOUT;
if (link && link->opcode == IORING_OP_LINK_TIMEOUT) {
io_remove_next_linked(req);
@@ -2421,8 +2420,12 @@ static bool io_disarm_next(struct io_kiocb *req)
struct io_ring_ctx *ctx = req->ctx;
spin_lock_irq(&ctx->timeout_lock);
- posted = io_kill_linked_timeout(req);
+ link = io_disarm_linked_timeout(req);
spin_unlock_irq(&ctx->timeout_lock);
+ if (link) {
+ posted = true;
+ io_req_tw_post_queue(link, -ECANCELED, 0);
+ }
}
if (unlikely((req->flags & REQ_F_FAIL) &&
!(req->flags & REQ_F_HARDLINK))) {
--
2.36.0
^ permalink raw reply related [flat|nested] 5+ messages in thread