[PATCH 0/4] clean up linked timeouts

[PATCH 0/4] clean up linked timeouts

[Pavel Begunkov]

These are preparations for future patches but looks good by itself,
so sending upfront.

Pavel Begunkov (4):
  io_uring: remove opcode check on ltimeout kill
  io_uring: dont adjust LINK_HEAD in cancel ltimeout
  io_uring: always clear LINK_TIMEOUT after cancel
  io_uring: don't defer put of cancelled ltimeout

 fs/io_uring.c | 60 +++++++++++++++++----------------------------------
 1 file changed, 20 insertions(+), 40 deletions(-)

-- 
2.24.0

[PATCH 1/4] io_uring: remove opcode check on ltimeout kill

[Pavel Begunkov]

__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 <asml.silence@gmail.com>
---
 fs/io_uring.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/fs/io_uring.c b/fs/io_uring.c
index 29170bbdd708..8d9a017b564a 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 [-> ...]
-- 
2.24.0

[PATCH 2/4] io_uring: dont adjust LINK_HEAD in cancel ltimeout

[Pavel Begunkov]

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 <asml.silence@gmail.com>
---
 fs/io_uring.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/fs/io_uring.c b/fs/io_uring.c
index 8d9a017b564a..5e327d620b8f 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;
 	}
-- 
2.24.0

[PATCH 3/4] io_uring: always clear LINK_TIMEOUT after cancel

[Pavel Begunkov]

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 <asml.silence@gmail.com>
---
 fs/io_uring.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/io_uring.c b/fs/io_uring.c
index 5e327d620b8f..d9ac45f850be 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)
-- 
2.24.0

[PATCH 4/4] io_uring: don't defer put of cancelled ltimeout

[Pavel Begunkov]

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 <asml.silence@gmail.com>
---
 fs/io_uring.c | 58 ++++++++++++++++++---------------------------------
 1 file changed, 20 insertions(+), 38 deletions(-)

diff --git a/fs/io_uring.c b/fs/io_uring.c
index d9ac45f850be..5a0fa2889fcb 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)
-- 
2.24.0