[PATCH 0/3] follow up cleanups

[PATCH 0/3] follow up cleanups

[Pavel Begunkov]

Random follow up cleanups. Patches 1-2 refactor io_req_complete_post(),
which became quite a monster over time.

Pavel Begunkov (3):
  io_uring: remove current check from complete_post
  io_uring: refactor io_req_complete_post()
  io_uring: clean up io_lockdep_assert_cq_locked

 io_uring/io_uring.c | 29 +++++++++++------------------
 io_uring/io_uring.h |  9 ++-------
 2 files changed, 13 insertions(+), 25 deletions(-)

-- 
2.43.0

[PATCH 1/3] io_uring: remove current check from complete_post

[Pavel Begunkov]

task_work execution is now always locked, and we shouldn't get into
io_req_complete_post() from them. That means that complete_post() is
always called out of the original task context and we don't even need to
check current.

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

diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
index 023fcf5d52c1..025709cadab9 100644
--- a/io_uring/io_uring.c
+++ b/io_uring/io_uring.c
@@ -978,7 +978,7 @@ void io_req_complete_post(struct io_kiocb *req, unsigned issue_flags)
 {
 	struct io_ring_ctx *ctx = req->ctx;
 
-	if (ctx->task_complete && ctx->submitter_task != current) {
+	if (ctx->task_complete) {
 		req->io_task_work.func = io_req_task_complete;
 		io_req_task_work_add(req);
 	} else if (!(issue_flags & IO_URING_F_UNLOCKED) ||
-- 
2.43.0

[PATCH 2/3] io_uring: refactor io_req_complete_post()

[Pavel Begunkov]

Make io_req_complete_post() to push all IORING_SETUP_IOPOLL requests
to task_work, it's much cleaner and should normally happen. We couldn't
do it before because there was a possibility of looping in

complete_post() -> tw -> complete_post() -> ...

Also, unexport the function and inline __io_req_complete_post().

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 io_uring/io_uring.c | 29 +++++++++++------------------
 io_uring/io_uring.h |  1 -
 2 files changed, 11 insertions(+), 19 deletions(-)

diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
index 025709cadab9..846d67a9c72e 100644
--- a/io_uring/io_uring.c
+++ b/io_uring/io_uring.c
@@ -926,11 +926,21 @@ bool io_req_post_cqe(struct io_kiocb *req, s32 res, u32 cflags)
 	return posted;
 }
 
-static void __io_req_complete_post(struct io_kiocb *req, unsigned issue_flags)
+static void io_req_complete_post(struct io_kiocb *req, unsigned issue_flags)
 {
 	struct io_ring_ctx *ctx = req->ctx;
 	struct io_rsrc_node *rsrc_node = NULL;
 
+	/*
+	 * Handle special CQ sync cases via task_work. DEFER_TASKRUN requires
+	 * the submitter task context, IOPOLL protects with uring_lock.
+	 */
+	if (ctx->task_complete || (ctx->flags & IORING_SETUP_IOPOLL)) {
+		req->io_task_work.func = io_req_task_complete;
+		io_req_task_work_add(req);
+		return;
+	}
+
 	io_cq_lock(ctx);
 	if (!(req->flags & REQ_F_CQE_SKIP)) {
 		if (!io_fill_cqe_req(ctx, req))
@@ -974,23 +984,6 @@ static void __io_req_complete_post(struct io_kiocb *req, unsigned issue_flags)
 	}
 }
 
-void io_req_complete_post(struct io_kiocb *req, unsigned issue_flags)
-{
-	struct io_ring_ctx *ctx = req->ctx;
-
-	if (ctx->task_complete) {
-		req->io_task_work.func = io_req_task_complete;
-		io_req_task_work_add(req);
-	} else if (!(issue_flags & IO_URING_F_UNLOCKED) ||
-		   !(ctx->flags & IORING_SETUP_IOPOLL)) {
-		__io_req_complete_post(req, issue_flags);
-	} else {
-		mutex_lock(&ctx->uring_lock);
-		__io_req_complete_post(req, issue_flags & ~IO_URING_F_UNLOCKED);
-		mutex_unlock(&ctx->uring_lock);
-	}
-}
-
 void io_req_defer_failed(struct io_kiocb *req, s32 res)
 	__must_hold(&ctx->uring_lock)
 {
diff --git a/io_uring/io_uring.h b/io_uring/io_uring.h
index 4bc96470e591..db6cab40bbbf 100644
--- a/io_uring/io_uring.h
+++ b/io_uring/io_uring.h
@@ -65,7 +65,6 @@ bool io_cqe_cache_refill(struct io_ring_ctx *ctx, bool overflow);
 void io_req_cqe_overflow(struct io_kiocb *req);
 int io_run_task_work_sig(struct io_ring_ctx *ctx);
 void io_req_defer_failed(struct io_kiocb *req, s32 res);
-void io_req_complete_post(struct io_kiocb *req, unsigned issue_flags);
 bool io_post_aux_cqe(struct io_ring_ctx *ctx, u64 user_data, s32 res, u32 cflags);
 bool io_req_post_cqe(struct io_kiocb *req, s32 res, u32 cflags);
 void __io_commit_cqring_flush(struct io_ring_ctx *ctx);
-- 
2.43.0

[PATCH 3/3] io_uring: clean up io_lockdep_assert_cq_locked

[Pavel Begunkov]

Move CONFIG_PROVE_LOCKING checks inside of io_lockdep_assert_cq_locked()
and kill the else branch.

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 io_uring/io_uring.h | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/io_uring/io_uring.h b/io_uring/io_uring.h
index db6cab40bbbf..85f4c8c1e846 100644
--- a/io_uring/io_uring.h
+++ b/io_uring/io_uring.h
@@ -119,9 +119,9 @@ enum {
 void io_eventfd_ops(struct rcu_head *rcu);
 void io_activate_pollwq(struct io_ring_ctx *ctx);
 
-#if defined(CONFIG_PROVE_LOCKING)
 static inline void io_lockdep_assert_cq_locked(struct io_ring_ctx *ctx)
 {
+#if defined(CONFIG_PROVE_LOCKING)
 	lockdep_assert(in_task());
 
 	if (ctx->flags & IORING_SETUP_IOPOLL) {
@@ -140,12 +140,8 @@ static inline void io_lockdep_assert_cq_locked(struct io_ring_ctx *ctx)
 		else
 			lockdep_assert(current == ctx->submitter_task);
 	}
-}
-#else
-static inline void io_lockdep_assert_cq_locked(struct io_ring_ctx *ctx)
-{
-}
 #endif
+}
 
 static inline void io_req_task_work_add(struct io_kiocb *req)
 {
-- 
2.43.0

Re: [PATCH 0/3] follow up cleanups

[Jens Axboe]

On Fri, 15 Mar 2024 21:45:59 +0000, Pavel Begunkov wrote:
> Random follow up cleanups. Patches 1-2 refactor io_req_complete_post(),
> which became quite a monster over time.
> 
> Pavel Begunkov (3):
>   io_uring: remove current check from complete_post
>   io_uring: refactor io_req_complete_post()
>   io_uring: clean up io_lockdep_assert_cq_locked
> 
> [...]

Applied, thanks!

[1/3] io_uring: remove current check from complete_post
      commit: ed245fa7d84f30810edf4d351ec4c483387651c3
[2/3] io_uring: refactor io_req_complete_post()
      commit: 4d441260e353cb41c718a7f638158af996dcc5db
[3/3] io_uring: clean up io_lockdep_assert_cq_locked
      commit: 1fffb93f8e102b109b351c1917af81e3ce4c65db

Best regards,
-- 
Jens Axboe