All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 for-next 0/5] 5.15 cleanups and optimisations
@ 2021-08-15  9:40 Pavel Begunkov
  2021-08-15  9:40 ` [PATCH v2 1/9] io_uring: optimise iowq refcounting Pavel Begunkov
                   ` (9 more replies)
  0 siblings, 10 replies; 11+ messages in thread
From: Pavel Begunkov @ 2021-08-15  9:40 UTC (permalink / raw)
  To: Jens Axboe, io-uring

Some improvements after killing refcounting, and other cleanups.
With 2/2 with will be only tracking reqs with 
file->f_op == &io_uring_fops, which is nice.

6-9 optimise the generic path as well as linked timeouts.

v2: s/io_req_refcount/io_req_set_refcount (Jens)
    6-9 are added

Pavel Begunkov (9):
  io_uring: optimise iowq refcounting
  io_uring: don't inflight-track linked timeouts
  io_uring: optimise initial ltimeout refcounting
  io_uring: kill not necessary resubmit switch
  io_uring: deduplicate cancellation code
  io_uring: kill REQ_F_LTIMEOUT_ACTIVE
  io_uring: simplify io_prep_linked_timeout
  io_uring: cancel not-armed linked touts separately
  io_uring: optimise io_prep_linked_timeout()

 fs/io_uring.c | 165 ++++++++++++++++++++++++++++----------------------
 1 file changed, 94 insertions(+), 71 deletions(-)

-- 
2.32.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2 1/9] io_uring: optimise iowq refcounting
  2021-08-15  9:40 [PATCH v2 for-next 0/5] 5.15 cleanups and optimisations Pavel Begunkov
@ 2021-08-15  9:40 ` Pavel Begunkov
  2021-08-15  9:40 ` [PATCH v2 2/9] io_uring: don't inflight-track linked timeouts Pavel Begunkov
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Pavel Begunkov @ 2021-08-15  9:40 UTC (permalink / raw)
  To: Jens Axboe, io-uring

If a requests is forwarded into io-wq, there is a good chance it hasn't
been refcounted yet and we can save one req_ref_get() by setting the
refcount number to the right value directly.

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 fs/io_uring.c | 25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/fs/io_uring.c b/fs/io_uring.c
index 51c4166f68b5..761bfb56ed3b 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -1115,14 +1115,19 @@ static inline void req_ref_get(struct io_kiocb *req)
 	atomic_inc(&req->refs);
 }
 
-static inline void io_req_refcount(struct io_kiocb *req)
+static inline void __io_req_set_refcount(struct io_kiocb *req, int nr)
 {
 	if (!(req->flags & REQ_F_REFCOUNT)) {
 		req->flags |= REQ_F_REFCOUNT;
-		atomic_set(&req->refs, 1);
+		atomic_set(&req->refs, nr);
 	}
 }
 
+static inline void io_req_set_refcount(struct io_kiocb *req)
+{
+	__io_req_set_refcount(req, 1);
+}
+
 static inline void io_req_set_rsrc_node(struct io_kiocb *req)
 {
 	struct io_ring_ctx *ctx = req->ctx;
@@ -1304,8 +1309,8 @@ static struct io_kiocb *__io_prep_linked_timeout(struct io_kiocb *req)
 		return NULL;
 
 	/* linked timeouts should have two refs once prep'ed */
-	io_req_refcount(req);
-	io_req_refcount(nxt);
+	io_req_set_refcount(req);
+	io_req_set_refcount(nxt);
 	req_ref_get(nxt);
 
 	nxt->timeout.head = req;
@@ -5231,7 +5236,7 @@ static int io_arm_poll_handler(struct io_kiocb *req)
 	req->apoll = apoll;
 	req->flags |= REQ_F_POLLED;
 	ipt.pt._qproc = io_async_queue_proc;
-	io_req_refcount(req);
+	io_req_set_refcount(req);
 
 	ret = __io_arm_poll_handler(req, &apoll->poll, &ipt, mask,
 					io_async_wake);
@@ -5419,7 +5424,7 @@ static int io_poll_add_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe
 	if (flags & ~IORING_POLL_ADD_MULTI)
 		return -EINVAL;
 
-	io_req_refcount(req);
+	io_req_set_refcount(req);
 	poll->events = io_poll_parse_events(sqe, flags);
 	return 0;
 }
@@ -6311,9 +6316,11 @@ static void io_wq_submit_work(struct io_wq_work *work)
 	struct io_kiocb *timeout;
 	int ret = 0;
 
-	io_req_refcount(req);
-	/* will be dropped by ->io_free_work() after returning to io-wq */
-	req_ref_get(req);
+	/* one will be dropped by ->io_free_work() after returning to io-wq */
+	if (!(req->flags & REQ_F_REFCOUNT))
+		__io_req_set_refcount(req, 2);
+	else
+		req_ref_get(req);
 
 	timeout = io_prep_linked_timeout(req);
 	if (timeout)
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 2/9] io_uring: don't inflight-track linked timeouts
  2021-08-15  9:40 [PATCH v2 for-next 0/5] 5.15 cleanups and optimisations Pavel Begunkov
  2021-08-15  9:40 ` [PATCH v2 1/9] io_uring: optimise iowq refcounting Pavel Begunkov
@ 2021-08-15  9:40 ` Pavel Begunkov
  2021-08-15  9:40 ` [PATCH v2 3/9] io_uring: optimise initial ltimeout refcounting Pavel Begunkov
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Pavel Begunkov @ 2021-08-15  9:40 UTC (permalink / raw)
  To: Jens Axboe, io-uring

Tracking linked timeouts as infligh was needed to make sure that io-wq
is not destroyed by io_uring_cancel_generic() racing with
io_async_cancel_one() accessing it. Now, cancellations issued by linked
timeouts are done in the task context, so it's already synchronised.

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 fs/io_uring.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/fs/io_uring.c b/fs/io_uring.c
index 761bfb56ed3b..fde76d502fff 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -5699,8 +5699,6 @@ static int io_timeout_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe,
 
 	data->mode = io_translate_timeout_mode(flags);
 	hrtimer_init(&data->timer, CLOCK_MONOTONIC, data->mode);
-	if (is_timeout_link)
-		io_req_track_inflight(req);
 	return 0;
 }
 
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 3/9] io_uring: optimise initial ltimeout refcounting
  2021-08-15  9:40 [PATCH v2 for-next 0/5] 5.15 cleanups and optimisations Pavel Begunkov
  2021-08-15  9:40 ` [PATCH v2 1/9] io_uring: optimise iowq refcounting Pavel Begunkov
  2021-08-15  9:40 ` [PATCH v2 2/9] io_uring: don't inflight-track linked timeouts Pavel Begunkov
@ 2021-08-15  9:40 ` Pavel Begunkov
  2021-08-15  9:40 ` [PATCH v2 4/9] io_uring: kill not necessary resubmit switch Pavel Begunkov
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Pavel Begunkov @ 2021-08-15  9:40 UTC (permalink / raw)
  To: Jens Axboe, io-uring

Linked timeouts are never refcounted when it comes to the first call to
__io_prep_linked_timeout(), so save an io_ref_get() and set the desired
value directly.

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 fde76d502fff..d2b968c8111f 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -1310,8 +1310,7 @@ static struct io_kiocb *__io_prep_linked_timeout(struct io_kiocb *req)
 
 	/* linked timeouts should have two refs once prep'ed */
 	io_req_set_refcount(req);
-	io_req_set_refcount(nxt);
-	req_ref_get(nxt);
+	__io_req_set_refcount(nxt, 2);
 
 	nxt->timeout.head = req;
 	nxt->flags |= REQ_F_LTIMEOUT_ACTIVE;
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 4/9] io_uring: kill not necessary resubmit switch
  2021-08-15  9:40 [PATCH v2 for-next 0/5] 5.15 cleanups and optimisations Pavel Begunkov
                   ` (2 preceding siblings ...)
  2021-08-15  9:40 ` [PATCH v2 3/9] io_uring: optimise initial ltimeout refcounting Pavel Begunkov
@ 2021-08-15  9:40 ` Pavel Begunkov
  2021-08-15  9:40 ` [PATCH v2 5/9] io_uring: deduplicate cancellation code Pavel Begunkov
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Pavel Begunkov @ 2021-08-15  9:40 UTC (permalink / raw)
  To: Jens Axboe, io-uring

773af69121ecc ("io_uring: always reissue from task_work context") makes
all resubmission to be made from task_work, so we don't need that hack
with resubmit/not-resubmit switch anymore.

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 fs/io_uring.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/fs/io_uring.c b/fs/io_uring.c
index d2b968c8111f..fb3b07c0f15a 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -2291,7 +2291,7 @@ static inline bool io_run_task_work(void)
  * Find and free completed poll iocbs
  */
 static void io_iopoll_complete(struct io_ring_ctx *ctx, unsigned int *nr_events,
-			       struct list_head *done, bool resubmit)
+			       struct list_head *done)
 {
 	struct req_batch rb;
 	struct io_kiocb *req;
@@ -2306,7 +2306,7 @@ static void io_iopoll_complete(struct io_ring_ctx *ctx, unsigned int *nr_events,
 		req = list_first_entry(done, struct io_kiocb, inflight_entry);
 		list_del(&req->inflight_entry);
 
-		if (READ_ONCE(req->result) == -EAGAIN && resubmit &&
+		if (READ_ONCE(req->result) == -EAGAIN &&
 		    !(req->flags & REQ_F_DONT_REISSUE)) {
 			req->iopoll_completed = 0;
 			io_req_task_queue_reissue(req);
@@ -2329,7 +2329,7 @@ static void io_iopoll_complete(struct io_ring_ctx *ctx, unsigned int *nr_events,
 }
 
 static int io_do_iopoll(struct io_ring_ctx *ctx, unsigned int *nr_events,
-			long min, bool resubmit)
+			long min)
 {
 	struct io_kiocb *req, *tmp;
 	LIST_HEAD(done);
@@ -2369,7 +2369,7 @@ static int io_do_iopoll(struct io_ring_ctx *ctx, unsigned int *nr_events,
 	}
 
 	if (!list_empty(&done))
-		io_iopoll_complete(ctx, nr_events, &done, resubmit);
+		io_iopoll_complete(ctx, nr_events, &done);
 
 	return 0;
 }
@@ -2387,7 +2387,7 @@ static void io_iopoll_try_reap_events(struct io_ring_ctx *ctx)
 	while (!list_empty(&ctx->iopoll_list)) {
 		unsigned int nr_events = 0;
 
-		io_do_iopoll(ctx, &nr_events, 0, false);
+		io_do_iopoll(ctx, &nr_events, 0);
 
 		/* let it sleep and repeat later if can't complete a request */
 		if (nr_events == 0)
@@ -2449,7 +2449,7 @@ static int io_iopoll_check(struct io_ring_ctx *ctx, long min)
 			    list_empty(&ctx->iopoll_list))
 				break;
 		}
-		ret = io_do_iopoll(ctx, &nr_events, min, true);
+		ret = io_do_iopoll(ctx, &nr_events, min);
 	} while (!ret && nr_events < min && !need_resched());
 out:
 	mutex_unlock(&ctx->uring_lock);
@@ -6855,7 +6855,7 @@ static int __io_sq_thread(struct io_ring_ctx *ctx, bool cap_entries)
 
 		mutex_lock(&ctx->uring_lock);
 		if (!list_empty(&ctx->iopoll_list))
-			io_do_iopoll(ctx, &nr_events, 0, true);
+			io_do_iopoll(ctx, &nr_events, 0);
 
 		/*
 		 * Don't submit if refs are dying, good for io_uring_register(),
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 5/9] io_uring: deduplicate cancellation code
  2021-08-15  9:40 [PATCH v2 for-next 0/5] 5.15 cleanups and optimisations Pavel Begunkov
                   ` (3 preceding siblings ...)
  2021-08-15  9:40 ` [PATCH v2 4/9] io_uring: kill not necessary resubmit switch Pavel Begunkov
@ 2021-08-15  9:40 ` Pavel Begunkov
  2021-08-15  9:40 ` [PATCH v2 6/9] io_uring: kill REQ_F_LTIMEOUT_ACTIVE Pavel Begunkov
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Pavel Begunkov @ 2021-08-15  9:40 UTC (permalink / raw)
  To: Jens Axboe, io-uring

IORING_OP_ASYNC_CANCEL and IORING_OP_LINK_TIMEOUT have enough of
overlap, so extract a helper for request cancellation and use in both.
Also, removes some amount of ugliness because of success_ret.

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 fs/io_uring.c | 46 ++++++++++++++++++----------------------------
 1 file changed, 18 insertions(+), 28 deletions(-)

diff --git a/fs/io_uring.c b/fs/io_uring.c
index fb3b07c0f15a..a0d081dca389 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -5790,32 +5790,24 @@ static int io_async_cancel_one(struct io_uring_task *tctx, u64 user_data,
 	return ret;
 }
 
-static void io_async_find_and_cancel(struct io_ring_ctx *ctx,
-				     struct io_kiocb *req, __u64 sqe_addr,
-				     int success_ret)
+static int io_try_cancel_userdata(struct io_kiocb *req, u64 sqe_addr)
+	__acquires(&req->ctx->completion_lock)
 {
+	struct io_ring_ctx *ctx = req->ctx;
 	int ret;
 
+	WARN_ON_ONCE(req->task != current);
+
 	ret = io_async_cancel_one(req->task->io_uring, sqe_addr, ctx);
 	spin_lock(&ctx->completion_lock);
 	if (ret != -ENOENT)
-		goto done;
+		return ret;
 	spin_lock_irq(&ctx->timeout_lock);
 	ret = io_timeout_cancel(ctx, sqe_addr);
 	spin_unlock_irq(&ctx->timeout_lock);
 	if (ret != -ENOENT)
-		goto done;
-	ret = io_poll_cancel(ctx, sqe_addr, false);
-done:
-	if (!ret)
-		ret = success_ret;
-	io_cqring_fill_event(ctx, req->user_data, ret, 0);
-	io_commit_cqring(ctx);
-	spin_unlock(&ctx->completion_lock);
-	io_cqring_ev_posted(ctx);
-
-	if (ret < 0)
-		req_set_fail(req);
+		return ret;
+	return io_poll_cancel(ctx, sqe_addr, false);
 }
 
 static int io_async_cancel_prep(struct io_kiocb *req,
@@ -5839,17 +5831,7 @@ static int io_async_cancel(struct io_kiocb *req, unsigned int issue_flags)
 	struct io_tctx_node *node;
 	int ret;
 
-	/* tasks should wait for their io-wq threads, so safe w/o sync */
-	ret = io_async_cancel_one(req->task->io_uring, sqe_addr, ctx);
-	spin_lock(&ctx->completion_lock);
-	if (ret != -ENOENT)
-		goto done;
-	spin_lock_irq(&ctx->timeout_lock);
-	ret = io_timeout_cancel(ctx, sqe_addr);
-	spin_unlock_irq(&ctx->timeout_lock);
-	if (ret != -ENOENT)
-		goto done;
-	ret = io_poll_cancel(ctx, sqe_addr, false);
+	ret = io_try_cancel_userdata(req, sqe_addr);
 	if (ret != -ENOENT)
 		goto done;
 	spin_unlock(&ctx->completion_lock);
@@ -6416,9 +6398,17 @@ static void io_req_task_link_timeout(struct io_kiocb *req)
 {
 	struct io_kiocb *prev = req->timeout.prev;
 	struct io_ring_ctx *ctx = req->ctx;
+	int ret;
 
 	if (prev) {
-		io_async_find_and_cancel(ctx, req, prev->user_data, -ETIME);
+		ret = io_try_cancel_userdata(req, prev->user_data);
+		if (!ret)
+			ret = -ETIME;
+		io_cqring_fill_event(ctx, req->user_data, ret, 0);
+		io_commit_cqring(ctx);
+		spin_unlock(&ctx->completion_lock);
+		io_cqring_ev_posted(ctx);
+
 		io_put_req(prev);
 		io_put_req(req);
 	} else {
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 6/9] io_uring: kill REQ_F_LTIMEOUT_ACTIVE
  2021-08-15  9:40 [PATCH v2 for-next 0/5] 5.15 cleanups and optimisations Pavel Begunkov
                   ` (4 preceding siblings ...)
  2021-08-15  9:40 ` [PATCH v2 5/9] io_uring: deduplicate cancellation code Pavel Begunkov
@ 2021-08-15  9:40 ` Pavel Begunkov
  2021-08-15  9:40 ` [PATCH v2 7/9] io_uring: simplify io_prep_linked_timeout Pavel Begunkov
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Pavel Begunkov @ 2021-08-15  9:40 UTC (permalink / raw)
  To: Jens Axboe, io-uring

Instead of handling double consecutive linked timeouts through tricky
flag combinations, just check the submit_state.link during timeout_prep
and fail that case in advance.

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 fs/io_uring.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/fs/io_uring.c b/fs/io_uring.c
index a0d081dca389..ece69b1217c8 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -705,7 +705,6 @@ enum {
 	REQ_F_NEED_CLEANUP_BIT,
 	REQ_F_POLLED_BIT,
 	REQ_F_BUFFER_SELECTED_BIT,
-	REQ_F_LTIMEOUT_ACTIVE_BIT,
 	REQ_F_COMPLETE_INLINE_BIT,
 	REQ_F_REISSUE_BIT,
 	REQ_F_DONT_REISSUE_BIT,
@@ -750,8 +749,6 @@ enum {
 	REQ_F_POLLED		= BIT(REQ_F_POLLED_BIT),
 	/* buffer already selected */
 	REQ_F_BUFFER_SELECTED	= BIT(REQ_F_BUFFER_SELECTED_BIT),
-	/* linked timeout is active, i.e. prepared by link's head */
-	REQ_F_LTIMEOUT_ACTIVE	= BIT(REQ_F_LTIMEOUT_ACTIVE_BIT),
 	/* completion is deferred through io_comp_state */
 	REQ_F_COMPLETE_INLINE	= BIT(REQ_F_COMPLETE_INLINE_BIT),
 	/* caller should reissue async */
@@ -1313,7 +1310,6 @@ static struct io_kiocb *__io_prep_linked_timeout(struct io_kiocb *req)
 	__io_req_set_refcount(nxt, 2);
 
 	nxt->timeout.head = req;
-	nxt->flags |= REQ_F_LTIMEOUT_ACTIVE;
 	req->flags |= REQ_F_LINK_TIMEOUT;
 	return nxt;
 }
@@ -1893,11 +1889,7 @@ static bool io_kill_linked_timeout(struct io_kiocb *req)
 {
 	struct io_kiocb *link = req->link;
 
-	/*
-	 * Can happen if a linked timeout fired and link had been like
-	 * req -> link t-out -> link t-out [-> ...]
-	 */
-	if (link && (link->flags & REQ_F_LTIMEOUT_ACTIVE)) {
+	if (link && link->opcode == IORING_OP_LINK_TIMEOUT) {
 		struct io_timeout_data *io = link->async_data;
 
 		io_remove_next_linked(req);
@@ -5698,6 +5690,15 @@ static int io_timeout_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe,
 
 	data->mode = io_translate_timeout_mode(flags);
 	hrtimer_init(&data->timer, CLOCK_MONOTONIC, data->mode);
+
+	if (is_timeout_link) {
+		struct io_submit_link *link = &req->ctx->submit_state.link;
+
+		if (!link->head)
+			return -EINVAL;
+		if (link->last->opcode == IORING_OP_LINK_TIMEOUT)
+			return -EINVAL;
+	}
 	return 0;
 }
 
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 7/9] io_uring: simplify io_prep_linked_timeout
  2021-08-15  9:40 [PATCH v2 for-next 0/5] 5.15 cleanups and optimisations Pavel Begunkov
                   ` (5 preceding siblings ...)
  2021-08-15  9:40 ` [PATCH v2 6/9] io_uring: kill REQ_F_LTIMEOUT_ACTIVE Pavel Begunkov
@ 2021-08-15  9:40 ` Pavel Begunkov
  2021-08-15  9:40 ` [PATCH v2 8/9] io_uring: cancel not-armed linked touts separately Pavel Begunkov
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Pavel Begunkov @ 2021-08-15  9:40 UTC (permalink / raw)
  To: Jens Axboe, io-uring

The link test in io_prep_linked_timeout() is pretty bulky, replace it
with a flag. It's better for normal path and linked requests, and also
will be used further for request failing.

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 fs/io_uring.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/fs/io_uring.c b/fs/io_uring.c
index ece69b1217c8..abd9df563d3d 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -710,6 +710,7 @@ enum {
 	REQ_F_DONT_REISSUE_BIT,
 	REQ_F_CREDS_BIT,
 	REQ_F_REFCOUNT_BIT,
+	REQ_F_ARM_LTIMEOUT_BIT,
 	/* keep async read/write and isreg together and in order */
 	REQ_F_NOWAIT_READ_BIT,
 	REQ_F_NOWAIT_WRITE_BIT,
@@ -765,6 +766,8 @@ enum {
 	REQ_F_CREDS		= BIT(REQ_F_CREDS_BIT),
 	/* skip refcounting if not set */
 	REQ_F_REFCOUNT		= BIT(REQ_F_REFCOUNT_BIT),
+	/* there is a linked timeout that has to be armed */
+	REQ_F_ARM_LTIMEOUT	= BIT(REQ_F_ARM_LTIMEOUT_BIT),
 };
 
 struct async_poll {
@@ -1300,23 +1303,18 @@ static void io_req_track_inflight(struct io_kiocb *req)
 
 static struct io_kiocb *__io_prep_linked_timeout(struct io_kiocb *req)
 {
-	struct io_kiocb *nxt = req->link;
-
-	if (req->flags & REQ_F_LINK_TIMEOUT)
-		return NULL;
+	req->flags &= ~REQ_F_ARM_LTIMEOUT;
+	req->flags |= REQ_F_LINK_TIMEOUT;
 
 	/* linked timeouts should have two refs once prep'ed */
 	io_req_set_refcount(req);
-	__io_req_set_refcount(nxt, 2);
-
-	nxt->timeout.head = req;
-	req->flags |= REQ_F_LINK_TIMEOUT;
-	return nxt;
+	__io_req_set_refcount(req->link, 2);
+	return req->link;
 }
 
 static inline struct io_kiocb *io_prep_linked_timeout(struct io_kiocb *req)
 {
-	if (likely(!req->link || req->link->opcode != IORING_OP_LINK_TIMEOUT))
+	if (likely(!(req->flags & REQ_F_ARM_LTIMEOUT)))
 		return NULL;
 	return __io_prep_linked_timeout(req);
 }
@@ -5698,6 +5696,8 @@ static int io_timeout_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe,
 			return -EINVAL;
 		if (link->last->opcode == IORING_OP_LINK_TIMEOUT)
 			return -EINVAL;
+		req->timeout.head = link->last;
+		link->last->flags |= REQ_F_ARM_LTIMEOUT;
 	}
 	return 0;
 }
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 8/9] io_uring: cancel not-armed linked touts separately
  2021-08-15  9:40 [PATCH v2 for-next 0/5] 5.15 cleanups and optimisations Pavel Begunkov
                   ` (6 preceding siblings ...)
  2021-08-15  9:40 ` [PATCH v2 7/9] io_uring: simplify io_prep_linked_timeout Pavel Begunkov
@ 2021-08-15  9:40 ` Pavel Begunkov
  2021-08-15  9:40 ` [PATCH v2 9/9] io_uring: optimise io_prep_linked_timeout() Pavel Begunkov
  2021-08-15 15:17 ` [PATCH v2 for-next 0/5] 5.15 cleanups and optimisations Jens Axboe
  9 siblings, 0 replies; 11+ messages in thread
From: Pavel Begunkov @ 2021-08-15  9:40 UTC (permalink / raw)
  To: Jens Axboe, io-uring

Adjust io_disarm_next(), so it can detect if there is a linked but
not-yet-armed timeout and complete/cancel it separately. Will be used in
the following patch.

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 fs/io_uring.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/fs/io_uring.c b/fs/io_uring.c
index abd9df563d3d..9b6ed088d8d5 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -1034,6 +1034,9 @@ static const struct io_op_def io_op_defs[] = {
 	[IORING_OP_UNLINKAT] = {},
 };
 
+/* requests with any of those set should undergo io_disarm_next() */
+#define IO_DISARM_MASK (REQ_F_ARM_LTIMEOUT | REQ_F_LINK_TIMEOUT | REQ_F_FAIL)
+
 static bool io_disarm_next(struct io_kiocb *req);
 static void io_uring_del_tctx_node(unsigned long index);
 static void io_uring_try_cancel_requests(struct io_ring_ctx *ctx,
@@ -1686,7 +1689,7 @@ static void io_req_complete_post(struct io_kiocb *req, long res,
 	 */
 	if (req_ref_put_and_test(req)) {
 		if (req->flags & (REQ_F_LINK | REQ_F_HARDLINK)) {
-			if (req->flags & (REQ_F_LINK_TIMEOUT | REQ_F_FAIL))
+			if (req->flags & IO_DISARM_MASK)
 				io_disarm_next(req);
 			if (req->link) {
 				io_req_task_queue(req->link);
@@ -1924,7 +1927,17 @@ static bool io_disarm_next(struct io_kiocb *req)
 {
 	bool posted = false;
 
-	if (likely(req->flags & REQ_F_LINK_TIMEOUT)) {
+	if (req->flags & REQ_F_ARM_LTIMEOUT) {
+		struct io_kiocb *link = req->link;
+
+		if (link && link->opcode == IORING_OP_LINK_TIMEOUT) {
+			io_remove_next_linked(req);
+			io_cqring_fill_event(link->ctx, link->user_data,
+					     -ECANCELED, 0);
+			io_put_req_deferred(link);
+			posted = true;
+		}
+	} else if (req->flags & REQ_F_LINK_TIMEOUT) {
 		struct io_ring_ctx *ctx = req->ctx;
 
 		spin_lock_irq(&ctx->timeout_lock);
@@ -1949,7 +1962,7 @@ static struct io_kiocb *__io_req_find_next(struct io_kiocb *req)
 	 * dependencies to the next request. In case of failure, fail the rest
 	 * of the chain.
 	 */
-	if (req->flags & (REQ_F_LINK_TIMEOUT | REQ_F_FAIL)) {
+	if (req->flags & IO_DISARM_MASK) {
 		struct io_ring_ctx *ctx = req->ctx;
 		bool posted;
 
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 9/9] io_uring: optimise io_prep_linked_timeout()
  2021-08-15  9:40 [PATCH v2 for-next 0/5] 5.15 cleanups and optimisations Pavel Begunkov
                   ` (7 preceding siblings ...)
  2021-08-15  9:40 ` [PATCH v2 8/9] io_uring: cancel not-armed linked touts separately Pavel Begunkov
@ 2021-08-15  9:40 ` Pavel Begunkov
  2021-08-15 15:17 ` [PATCH v2 for-next 0/5] 5.15 cleanups and optimisations Jens Axboe
  9 siblings, 0 replies; 11+ messages in thread
From: Pavel Begunkov @ 2021-08-15  9:40 UTC (permalink / raw)
  To: Jens Axboe, io-uring

Linked timeout handling during issuing is heavy, it adds extra
instructions and forces to save the next linked timeout before
io_issue_sqe().

Follwing the same reasoning as in refcounting patches, a request can't
be freed by the time it returns from io_issue_sqe(), so now we don't
need to do io_prep_linked_timeout() in advance, and it can be delayed to
colder paths optimising the generic path.

Also, it should also save quite a lot for requests with linked timeouts
and completed inline on timeout spinlocking + hrtimer_start() +
hrtimer_try_to_cancel() and so on.

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 fs/io_uring.c | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/fs/io_uring.c b/fs/io_uring.c
index 9b6ed088d8d5..2313b39efbbe 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -1304,6 +1304,11 @@ static void io_req_track_inflight(struct io_kiocb *req)
 	}
 }
 
+static inline void io_unprep_linked_timeout(struct io_kiocb *req)
+{
+	req->flags &= ~REQ_F_LINK_TIMEOUT;
+}
+
 static struct io_kiocb *__io_prep_linked_timeout(struct io_kiocb *req)
 {
 	req->flags &= ~REQ_F_ARM_LTIMEOUT;
@@ -6483,7 +6488,7 @@ static void io_queue_linked_timeout(struct io_kiocb *req)
 static void __io_queue_sqe(struct io_kiocb *req)
 	__must_hold(&req->ctx->uring_lock)
 {
-	struct io_kiocb *linked_timeout = io_prep_linked_timeout(req);
+	struct io_kiocb *linked_timeout;
 	int ret;
 
 issue_sqe:
@@ -6501,10 +6506,19 @@ static void __io_queue_sqe(struct io_kiocb *req)
 			state->compl_reqs[state->compl_nr++] = req;
 			if (state->compl_nr == ARRAY_SIZE(state->compl_reqs))
 				io_submit_flush_completions(ctx);
+			return;
 		}
+
+		linked_timeout = io_prep_linked_timeout(req);
+		if (linked_timeout)
+			io_queue_linked_timeout(linked_timeout);
 	} else if (ret == -EAGAIN && !(req->flags & REQ_F_NOWAIT)) {
+		linked_timeout = io_prep_linked_timeout(req);
+
 		switch (io_arm_poll_handler(req)) {
 		case IO_APOLL_READY:
+			if (linked_timeout)
+				io_unprep_linked_timeout(req);
 			goto issue_sqe;
 		case IO_APOLL_ABORTED:
 			/*
@@ -6514,11 +6528,12 @@ static void __io_queue_sqe(struct io_kiocb *req)
 			io_queue_async_work(req);
 			break;
 		}
+
+		if (linked_timeout)
+			io_queue_linked_timeout(linked_timeout);
 	} else {
 		io_req_complete_failed(req, ret);
 	}
-	if (linked_timeout)
-		io_queue_linked_timeout(linked_timeout);
 }
 
 static inline void io_queue_sqe(struct io_kiocb *req)
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 for-next 0/5] 5.15 cleanups and optimisations
  2021-08-15  9:40 [PATCH v2 for-next 0/5] 5.15 cleanups and optimisations Pavel Begunkov
                   ` (8 preceding siblings ...)
  2021-08-15  9:40 ` [PATCH v2 9/9] io_uring: optimise io_prep_linked_timeout() Pavel Begunkov
@ 2021-08-15 15:17 ` Jens Axboe
  9 siblings, 0 replies; 11+ messages in thread
From: Jens Axboe @ 2021-08-15 15:17 UTC (permalink / raw)
  To: Pavel Begunkov, io-uring

On 8/15/21 3:40 AM, Pavel Begunkov wrote:
> Some improvements after killing refcounting, and other cleanups.
> With 2/2 with will be only tracking reqs with 
> file->f_op == &io_uring_fops, which is nice.
> 
> 6-9 optimise the generic path as well as linked timeouts.
> 
> v2: s/io_req_refcount/io_req_set_refcount (Jens)
>     6-9 are added

Looks good - applied, thanks.

-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2021-08-15 15:17 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-15  9:40 [PATCH v2 for-next 0/5] 5.15 cleanups and optimisations Pavel Begunkov
2021-08-15  9:40 ` [PATCH v2 1/9] io_uring: optimise iowq refcounting Pavel Begunkov
2021-08-15  9:40 ` [PATCH v2 2/9] io_uring: don't inflight-track linked timeouts Pavel Begunkov
2021-08-15  9:40 ` [PATCH v2 3/9] io_uring: optimise initial ltimeout refcounting Pavel Begunkov
2021-08-15  9:40 ` [PATCH v2 4/9] io_uring: kill not necessary resubmit switch Pavel Begunkov
2021-08-15  9:40 ` [PATCH v2 5/9] io_uring: deduplicate cancellation code Pavel Begunkov
2021-08-15  9:40 ` [PATCH v2 6/9] io_uring: kill REQ_F_LTIMEOUT_ACTIVE Pavel Begunkov
2021-08-15  9:40 ` [PATCH v2 7/9] io_uring: simplify io_prep_linked_timeout Pavel Begunkov
2021-08-15  9:40 ` [PATCH v2 8/9] io_uring: cancel not-armed linked touts separately Pavel Begunkov
2021-08-15  9:40 ` [PATCH v2 9/9] io_uring: optimise io_prep_linked_timeout() Pavel Begunkov
2021-08-15 15:17 ` [PATCH v2 for-next 0/5] 5.15 cleanups and optimisations Jens Axboe

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.