[PATCH for-next 0/2] inline execution ltimeout optimisation

[PATCH for-next 0/2] inline execution ltimeout optimisation

[Pavel Begunkov]

Patches are not related but put touch adjacent lines. First we improve
a little bit on post issue iopoll checks, and then return back a long
lost linked timeout optimisation.

Pavel Begunkov (2):
  io_uring: don't check iopoll if request completes
  io_uring: optimise ltimeout for inline execution

 io_uring/io_uring.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

-- 
2.43.0

[PATCH for-next 1/2] io_uring: don't check iopoll if request completes

[Pavel Begunkov]

IOPOLL request should never return IOU_OK, so the following iopoll
queueing check in io_issue_sqe() after getting IOU_OK doesn't make any
sense as would never turn true. Let's optimise on that and return a bit
earlier. It's also much more resilient to potential bugs from
mischieving iopoll implementations.

Cc: stable@vger.kernel.org
Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 io_uring/io_uring.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
index 6ffd7216393b..21e646ef9654 100644
--- a/io_uring/io_uring.c
+++ b/io_uring/io_uring.c
@@ -1896,7 +1896,11 @@ static int io_issue_sqe(struct io_kiocb *req, unsigned int issue_flags)
 			io_req_complete_defer(req);
 		else
 			io_req_complete_post(req, issue_flags);
-	} else if (ret != IOU_ISSUE_SKIP_COMPLETE)
+
+		return 0;
+	}
+
+	if (ret != IOU_ISSUE_SKIP_COMPLETE)
 		return ret;
 
 	/* If the op doesn't have a file, we're not polling for it */
-- 
2.43.0

[PATCH for-next 2/2] io_uring: optimise ltimeout for inline execution

[Pavel Begunkov]

At one point in time we had an optimisation that would not spin up a
linked timeout timer when the master request successfully completes
inline (during the first nowait execution attempt). We somehow lost it,
so this patch restores it back.

Note, that it's fine using io_arm_ltimeout() after the io_issue_sqe()
completes the request because of delayed completion, but that that adds
unwanted overhead.

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

diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
index 21e646ef9654..6212f81ed887 100644
--- a/io_uring/io_uring.c
+++ b/io_uring/io_uring.c
@@ -1900,14 +1900,15 @@ static int io_issue_sqe(struct io_kiocb *req, unsigned int issue_flags)
 		return 0;
 	}
 
-	if (ret != IOU_ISSUE_SKIP_COMPLETE)
-		return ret;
-
-	/* If the op doesn't have a file, we're not polling for it */
-	if ((req->ctx->flags & IORING_SETUP_IOPOLL) && def->iopoll_queue)
-		io_iopoll_req_issued(req, issue_flags);
+	if (ret == IOU_ISSUE_SKIP_COMPLETE) {
+		ret = 0;
+		io_arm_ltimeout(req);
 
-	return 0;
+		/* If the op doesn't have a file, we're not polling for it */
+		if ((req->ctx->flags & IORING_SETUP_IOPOLL) && def->iopoll_queue)
+			io_iopoll_req_issued(req, issue_flags);
+	}
+	return ret;
 }
 
 int io_poll_issue(struct io_kiocb *req, struct io_tw_state *ts)
@@ -2078,9 +2079,7 @@ static inline void io_queue_sqe(struct io_kiocb *req)
 	 * We async punt it if the file wasn't marked NOWAIT, or if the file
 	 * doesn't support non-blocking read/write attempts
 	 */
-	if (likely(!ret))
-		io_arm_ltimeout(req);
-	else
+	if (unlikely(ret))
 		io_queue_async(req, ret);
 }
 
-- 
2.43.0

Re: [PATCH for-next 2/2] io_uring: optimise ltimeout for inline execution

[Pavel Begunkov]

On 12/1/23 00:38, Pavel Begunkov wrote:
> At one point in time we had an optimisation that would not spin up a
> linked timeout timer when the master request successfully completes
> inline (during the first nowait execution attempt). We somehow lost it,
> so this patch restores it back.
> 
> Note, that it's fine using io_arm_ltimeout() after the io_issue_sqe()
> completes the request because of delayed completion, but that that adds
> unwanted overhead.

Let's add:

Reported-by: Christian Mazakas <christian.mazakas@gmail.com>

> 
> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
> ---
>   io_uring/io_uring.c | 19 +++++++++----------
>   1 file changed, 9 insertions(+), 10 deletions(-)
> 
> diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
> index 21e646ef9654..6212f81ed887 100644
> --- a/io_uring/io_uring.c
> +++ b/io_uring/io_uring.c
> @@ -1900,14 +1900,15 @@ static int io_issue_sqe(struct io_kiocb *req, unsigned int issue_flags)
>   		return 0;
>   	}
>   
> -	if (ret != IOU_ISSUE_SKIP_COMPLETE)
> -		return ret;
> -
> -	/* If the op doesn't have a file, we're not polling for it */
> -	if ((req->ctx->flags & IORING_SETUP_IOPOLL) && def->iopoll_queue)
> -		io_iopoll_req_issued(req, issue_flags);
> +	if (ret == IOU_ISSUE_SKIP_COMPLETE) {
> +		ret = 0;
> +		io_arm_ltimeout(req);
>   
> -	return 0;
> +		/* If the op doesn't have a file, we're not polling for it */
> +		if ((req->ctx->flags & IORING_SETUP_IOPOLL) && def->iopoll_queue)
> +			io_iopoll_req_issued(req, issue_flags);
> +	}
> +	return ret;
>   }
>   
>   int io_poll_issue(struct io_kiocb *req, struct io_tw_state *ts)
> @@ -2078,9 +2079,7 @@ static inline void io_queue_sqe(struct io_kiocb *req)
>   	 * We async punt it if the file wasn't marked NOWAIT, or if the file
>   	 * doesn't support non-blocking read/write attempts
>   	 */
> -	if (likely(!ret))
> -		io_arm_ltimeout(req);
> -	else
> +	if (unlikely(ret))
>   		io_queue_async(req, ret);
>   }
>   

-- 
Pavel Begunkov

Re: [PATCH for-next 0/2] inline execution ltimeout optimisation

[Jens Axboe]

On Fri, 01 Dec 2023 00:38:51 +0000, Pavel Begunkov wrote:
> Patches are not related but put touch adjacent lines. First we improve
> a little bit on post issue iopoll checks, and then return back a long
> lost linked timeout optimisation.
> 
> Pavel Begunkov (2):
>   io_uring: don't check iopoll if request completes
>   io_uring: optimise ltimeout for inline execution
> 
> [...]

Applied, thanks!

[1/2] io_uring: don't check iopoll if request completes
      (no commit info)
[2/2] io_uring: optimise ltimeout for inline execution
      (no commit info)

Best regards,
-- 
Jens Axboe