From e08102d507f34e6591de521a4c2587c6f02c7996 Mon Sep 17 00:00:00 2001 From: Pavel Begunkov Date: Thu, 22 Oct 2020 16:43:08 +0100 Subject: io_uring: remove opcode check on ltimeout kill __io_kill_linked_timeout() already checks for REQ_F_LTIMEOUT_ACTIVE and it's set only for linked timeouts. No need to verify next request's opcode. Signed-off-by: Pavel Begunkov Signed-off-by: Jens Axboe --- fs/io_uring.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'fs') diff --git a/fs/io_uring.c b/fs/io_uring.c index d40717f8647b..db7ad9e61146 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -1872,8 +1872,7 @@ static bool __io_kill_linked_timeout(struct io_kiocb *req) if (list_empty(&req->link_list)) return false; link = list_first_entry(&req->link_list, struct io_kiocb, link_list); - if (link->opcode != IORING_OP_LINK_TIMEOUT) - return false; + /* * Can happen if a linked timeout fired and link had been like * req -> link t-out -> link t-out [-> ...] -- cgit v1.2.3 From ac877d2edd094e161801d72b49cfb56c5fc860fb Mon Sep 17 00:00:00 2001 From: Pavel Begunkov Date: Thu, 22 Oct 2020 16:43:09 +0100 Subject: io_uring: don't adjust LINK_HEAD in cancel ltimeout An armed linked timeout can never be a head of a link, so we don't need to clear REQ_F_LINK_HEAD for it. Signed-off-by: Pavel Begunkov Signed-off-by: Jens Axboe --- fs/io_uring.c | 1 - 1 file changed, 1 deletion(-) (limited to 'fs') diff --git a/fs/io_uring.c b/fs/io_uring.c index db7ad9e61146..043652929aa9 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -1856,7 +1856,6 @@ static bool io_link_cancel_timeout(struct io_kiocb *req) if (ret != -1) { io_cqring_fill_event(req, -ECANCELED); io_commit_cqring(ctx); - req->flags &= ~REQ_F_LINK_HEAD; io_put_req_deferred(req, 1); return true; } -- cgit v1.2.3 From cdfcc3ee04599ce51e5c84432c177163637dd0e0 Mon Sep 17 00:00:00 2001 From: Pavel Begunkov Date: Thu, 22 Oct 2020 16:43:10 +0100 Subject: io_uring: always clear LINK_TIMEOUT after cancel Move REQ_F_LINK_TIMEOUT clearing out of __io_kill_linked_timeout() because it might return early and leave the flag set. It's not a problem, but may be confusing. Signed-off-by: Pavel Begunkov Signed-off-by: Jens Axboe --- fs/io_uring.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'fs') diff --git a/fs/io_uring.c b/fs/io_uring.c index 043652929aa9..552c27850c36 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -1881,7 +1881,6 @@ static bool __io_kill_linked_timeout(struct io_kiocb *req) list_del_init(&link->link_list); wake_ev = io_link_cancel_timeout(link); - req->flags &= ~REQ_F_LINK_TIMEOUT; return wake_ev; } @@ -1893,6 +1892,7 @@ static void io_kill_linked_timeout(struct io_kiocb *req) spin_lock_irqsave(&ctx->completion_lock, flags); wake_ev = __io_kill_linked_timeout(req); + req->flags &= ~REQ_F_LINK_TIMEOUT; spin_unlock_irqrestore(&ctx->completion_lock, flags); if (wake_ev) -- cgit v1.2.3 From c9abd7ad832b9eef06d887f4971894af5de617fd Mon Sep 17 00:00:00 2001 From: Pavel Begunkov Date: Thu, 22 Oct 2020 16:43:11 +0100 Subject: io_uring: don't defer put of cancelled ltimeout Inline io_link_cancel_timeout() and __io_kill_linked_timeout() into io_kill_linked_timeout(). That allows to easily move a put of a cancelled linked timeout out of completion_lock and to not deferring it. It is also much more readable when not scattered across three different functions. Signed-off-by: Pavel Begunkov Signed-off-by: Jens Axboe --- fs/io_uring.c | 58 ++++++++++++++++++++-------------------------------------- 1 file changed, 20 insertions(+), 38 deletions(-) (limited to 'fs') diff --git a/fs/io_uring.c b/fs/io_uring.c index 552c27850c36..f6cb2b62ce1a 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -1846,57 +1846,39 @@ static void __io_free_req(struct io_kiocb *req) percpu_ref_put(&ctx->refs); } -static bool io_link_cancel_timeout(struct io_kiocb *req) +static void io_kill_linked_timeout(struct io_kiocb *req) { - struct io_timeout_data *io = req->async_data; struct io_ring_ctx *ctx = req->ctx; - int ret; - - ret = hrtimer_try_to_cancel(&io->timer); - if (ret != -1) { - io_cqring_fill_event(req, -ECANCELED); - io_commit_cqring(ctx); - io_put_req_deferred(req, 1); - return true; - } - - return false; -} - -static bool __io_kill_linked_timeout(struct io_kiocb *req) -{ struct io_kiocb *link; - bool wake_ev; - - if (list_empty(&req->link_list)) - return false; - link = list_first_entry(&req->link_list, struct io_kiocb, link_list); + bool cancelled = false; + unsigned long flags; + spin_lock_irqsave(&ctx->completion_lock, flags); + link = list_first_entry_or_null(&req->link_list, struct io_kiocb, + link_list); /* * Can happen if a linked timeout fired and link had been like * req -> link t-out -> link t-out [-> ...] */ - if (!(link->flags & REQ_F_LTIMEOUT_ACTIVE)) - return false; - - list_del_init(&link->link_list); - wake_ev = io_link_cancel_timeout(link); - return wake_ev; -} - -static void io_kill_linked_timeout(struct io_kiocb *req) -{ - struct io_ring_ctx *ctx = req->ctx; - unsigned long flags; - bool wake_ev; + if (link && (link->flags & REQ_F_LTIMEOUT_ACTIVE)) { + struct io_timeout_data *io = link->async_data; + int ret; - spin_lock_irqsave(&ctx->completion_lock, flags); - wake_ev = __io_kill_linked_timeout(req); + list_del_init(&link->link_list); + ret = hrtimer_try_to_cancel(&io->timer); + if (ret != -1) { + io_cqring_fill_event(link, -ECANCELED); + io_commit_cqring(ctx); + cancelled = true; + } + } req->flags &= ~REQ_F_LINK_TIMEOUT; spin_unlock_irqrestore(&ctx->completion_lock, flags); - if (wake_ev) + if (cancelled) { io_cqring_ev_posted(ctx); + io_put_req(link); + } } static struct io_kiocb *io_req_link_next(struct io_kiocb *req) -- cgit v1.2.3 From feaadc4fc2ebdbd53ffed1735077725855a2af53 Mon Sep 17 00:00:00 2001 From: Pavel Begunkov Date: Thu, 22 Oct 2020 16:47:16 +0100 Subject: io_uring: don't miss setting IO_WQ_WORK_CONCURRENT Set IO_WQ_WORK_CONCURRENT for all REQ_F_FORCE_ASYNC requests, do that in that is also looks better. Signed-off-by: Pavel Begunkov Signed-off-by: Jens Axboe --- fs/io_uring.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) (limited to 'fs') diff --git a/fs/io_uring.c b/fs/io_uring.c index f6cb2b62ce1a..3606ea572e61 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -1365,6 +1365,9 @@ static void io_prep_async_work(struct io_kiocb *req) io_req_init_async(req); id = req->work.identity; + if (req->flags & REQ_F_FORCE_ASYNC) + req->work.flags |= IO_WQ_WORK_CONCURRENT; + if (req->flags & REQ_F_ISREG) { if (def->hash_reg_file || (ctx->flags & IORING_SETUP_IOPOLL)) io_wq_hash_work(&req->work, file_inode(req->file)); @@ -6245,13 +6248,6 @@ fail_req: if (unlikely(ret)) goto fail_req; } - - /* - * Never try inline submit of IOSQE_ASYNC is set, go straight - * to async execution. - */ - io_req_init_async(req); - req->work.flags |= IO_WQ_WORK_CONCURRENT; io_queue_async_work(req); } else { if (sqe) { -- cgit v1.2.3 From 9aaf354352f1142831457492790d6bfa9c883021 Mon Sep 17 00:00:00 2001 From: Pavel Begunkov Date: Thu, 22 Oct 2020 16:47:17 +0100 Subject: io_uring: simplify nxt propagation in io_queue_sqe Don't overuse goto's, complex control flow doesn't make compilers happy and makes code harder to read. Signed-off-by: Pavel Begunkov Signed-off-by: Jens Axboe --- fs/io_uring.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) (limited to 'fs') diff --git a/fs/io_uring.c b/fs/io_uring.c index 3606ea572e61..4d647d91dab2 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -6188,7 +6188,6 @@ again: */ if (ret == -EAGAIN && !(req->flags & REQ_F_NOWAIT)) { if (!io_arm_poll_handler(req)) { -punt: /* * Queued up for async execution, worker will release * submit reference when the iocb is actually submitted. @@ -6217,12 +6216,9 @@ punt: if (nxt) { req = nxt; - - if (req->flags & REQ_F_FORCE_ASYNC) { - linked_timeout = NULL; - goto punt; - } - goto again; + if (!(req->flags & REQ_F_FORCE_ASYNC)) + goto again; + io_queue_async_work(req); } exit: if (old_creds) -- cgit v1.2.3 From 0d63c148d6d9ac57c124b618f66269bb4558553b Mon Sep 17 00:00:00 2001 From: Pavel Begunkov Date: Thu, 22 Oct 2020 16:47:18 +0100 Subject: io_uring: simplify __io_queue_sqe() Restructure __io_queue_sqe() so it follows simple if/else if/else control flow. It's more readable and removes extra goto/labels. Signed-off-by: Pavel Begunkov Signed-off-by: Jens Axboe --- fs/io_uring.c | 28 +++++++++++----------------- 1 file changed, 11 insertions(+), 17 deletions(-) (limited to 'fs') diff --git a/fs/io_uring.c b/fs/io_uring.c index 4d647d91dab2..74dcc4471e9b 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -6162,7 +6162,6 @@ static struct io_kiocb *io_prep_linked_timeout(struct io_kiocb *req) static void __io_queue_sqe(struct io_kiocb *req, struct io_comp_state *cs) { struct io_kiocb *linked_timeout; - struct io_kiocb *nxt; const struct cred *old_creds = NULL; int ret; @@ -6197,30 +6196,25 @@ again: if (linked_timeout) io_queue_linked_timeout(linked_timeout); - goto exit; - } + } else if (likely(!ret)) { + /* drop submission reference */ + req = io_put_req_find_next(req); + if (linked_timeout) + io_queue_linked_timeout(linked_timeout); - if (unlikely(ret)) { + if (req) { + if (!(req->flags & REQ_F_FORCE_ASYNC)) + goto again; + io_queue_async_work(req); + } + } else { /* un-prep timeout, so it'll be killed as any other linked */ req->flags &= ~REQ_F_LINK_TIMEOUT; req_set_fail_links(req); io_put_req(req); io_req_complete(req, ret); - goto exit; } - /* drop submission reference */ - nxt = io_put_req_find_next(req); - if (linked_timeout) - io_queue_linked_timeout(linked_timeout); - - if (nxt) { - req = nxt; - if (!(req->flags & REQ_F_FORCE_ASYNC)) - goto again; - io_queue_async_work(req); - } -exit: if (old_creds) revert_creds(old_creds); } -- cgit v1.2.3 From c8b5e2600a2cfa1cdfbecf151afd67aee227381d Mon Sep 17 00:00:00 2001 From: Jens Axboe Date: Sun, 25 Oct 2020 13:53:26 -0600 Subject: io_uring: use type appropriate io_kiocb handler for double poll io_poll_double_wake() is called for both request types - both pure poll requests, and internal polls. This means that we should be using the right handler based on the request type. Use the one that the original caller already assigned for the waitqueue handling, that will always match the correct type. Cc: stable@vger.kernel.org # v5.8+ Reported-by: Pavel Begunkov Signed-off-by: Jens Axboe --- fs/io_uring.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'fs') diff --git a/fs/io_uring.c b/fs/io_uring.c index 74dcc4471e9b..2f6af230e86e 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -4959,8 +4959,10 @@ static int io_poll_double_wake(struct wait_queue_entry *wait, unsigned mode, /* make sure double remove sees this as being gone */ wait->private = NULL; spin_unlock(&poll->head->lock); - if (!done) - __io_async_wake(req, poll, mask, io_poll_task_func); + if (!done) { + /* use wait func handler, so it matches the rq type */ + poll->wait.func(&poll->wait, mode, sync, key); + } } refcount_dec(&req->refs); return 1; -- cgit v1.2.3