public inbox for [email protected]
 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 <[email protected]>
---
 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 <[email protected]>
---
 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 <[email protected]>
---
 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 <[email protected]>
---
 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 <[email protected]>
---
 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 <[email protected]>
---
 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 <[email protected]>
---
 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 <[email protected]>
---
 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 <[email protected]>
---
 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox