[RFC PATCH 0/4] cleanup io_put/free

[RFC PATCH 0/4] cleanup io_put/free

[Pavel Begunkov]

Nested checking @nxt for non-nullness in io_put_req_find_next() and
friends is a nuisance. Make them accept only non-null @nxt obliging
callers to io_queue_async_work() if they don't need @nxt.

The patchset is a bit rusty (rebased), but still relevant. The part
removing io_wq_current_is_worker() (PATCH 2/4) looks good, but would
like someone to check the assumption.

P.S. it depends on the patch with renaming __io_submit_sqe().


Pavel Begunkov (4):
  io_uring: remove io_free_req_find_next()
  io_uring: pass only !null to io_req_find_next()
  io_uring: simplify io_req_link_next()
  io_uring: only !null ptr to io_issue_sqe()

 fs/io_uring.c | 51 +++++++++++++++++++--------------------------------
 1 file changed, 19 insertions(+), 32 deletions(-)

-- 
2.24.0

[PATCH 1/4] io_uring: remove io_free_req_find_next()

[Pavel Begunkov]

There is only one one-liner user of io_free_req_find_next(). Inline it.

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

diff --git a/fs/io_uring.c b/fs/io_uring.c
index 86d1d8f272ae..07b48ce3ccab 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -979,15 +979,10 @@ static void io_req_find_next(struct io_kiocb *req, struct io_kiocb **nxt)
 	}
 }
 
-static void io_free_req_find_next(struct io_kiocb *req, struct io_kiocb **nxt)
-{
-	io_req_find_next(req, nxt);
-	__io_free_req(req);
-}
-
 static void io_free_req(struct io_kiocb *req)
 {
-	io_free_req_find_next(req, NULL);
+	io_req_find_next(req, NULL);
+	__io_free_req(req);
 }
 
 /*
-- 
2.24.0

[PATCH 2/4] io_uring: pass only !null to io_req_find_next()

[Pavel Begunkov]

Make io_req_find_next() and io_req_link_next() to accept only non-null
nxt, and handle it in callers.

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

diff --git a/fs/io_uring.c b/fs/io_uring.c
index 07b48ce3ccab..63ef603d5fa0 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -903,7 +903,7 @@ static void io_req_link_next(struct io_kiocb *req, struct io_kiocb **nxtptr)
 		 * in this context instead of having to queue up new async work.
 		 */
 		if (nxt) {
-			if (nxtptr && io_wq_current_is_worker())
+			if (io_wq_current_is_worker())
 				*nxtptr = nxt;
 			else
 				io_queue_async_work(nxt);
@@ -981,8 +981,13 @@ static void io_req_find_next(struct io_kiocb *req, struct io_kiocb **nxt)
 
 static void io_free_req(struct io_kiocb *req)
 {
-	io_req_find_next(req, NULL);
+	struct io_kiocb *nxt = NULL;
+
+	io_req_find_next(req, &nxt);
 	__io_free_req(req);
+
+	if (nxt)
+		io_queue_async_work(nxt);
 }
 
 /*
-- 
2.24.0

[PATCH 3/4] io_uring: simplify io_req_link_next()

[Pavel Begunkov]

"if (nxt)" is always true, as it was checked in the while's condition.
io_wq_current_is_worker() is unnecessary, as non-async callers don't
pass nxt, so io_queue_async_work() will be called for them anyway.

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

diff --git a/fs/io_uring.c b/fs/io_uring.c
index 63ef603d5fa0..1ab045e5d663 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -898,16 +898,7 @@ static void io_req_link_next(struct io_kiocb *req, struct io_kiocb **nxtptr)
 			nxt->flags |= REQ_F_LINK;
 		}
 
-		/*
-		 * If we're in async work, we can continue processing the chain
-		 * in this context instead of having to queue up new async work.
-		 */
-		if (nxt) {
-			if (io_wq_current_is_worker())
-				*nxtptr = nxt;
-			else
-				io_queue_async_work(nxt);
-		}
+		*nxtptr = nxt;
 		break;
 	}
 
-- 
2.24.0

[PATCH 4/4] io_uring: only !null ptr to io_issue_sqe()

[Pavel Begunkov]

Pass only non-null @nxt to io_issue_sqe() and handle it at the caller's
side. And propagate it.

- kiocb_done() is only called from io_read() and io_write(), which are
only called from io_issue_sqe(), so it's @nxt != NULL

- io_put_req_find_next() is called either with explicitly non-null local
nxt, or from one of the functions in io_issue_sqe() switch (or their
callees).

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

diff --git a/fs/io_uring.c b/fs/io_uring.c
index 1ab045e5d663..3e8efe8f064d 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -985,21 +985,13 @@ static void io_free_req(struct io_kiocb *req)
  * Drop reference to request, return next in chain (if there is one) if this
  * was the last reference to this request.
  */
+__attribute__((nonnull))
 static void io_put_req_find_next(struct io_kiocb *req, struct io_kiocb **nxtptr)
 {
-	struct io_kiocb *nxt = NULL;
-
-	io_req_find_next(req, &nxt);
+	io_req_find_next(req, nxtptr);
 
 	if (refcount_dec_and_test(&req->refs))
 		__io_free_req(req);
-
-	if (nxt) {
-		if (nxtptr)
-			*nxtptr = nxt;
-		else
-			io_queue_async_work(nxt);
-	}
 }
 
 static void io_put_req(struct io_kiocb *req)
@@ -1482,7 +1474,7 @@ static inline void io_rw_done(struct kiocb *kiocb, ssize_t ret)
 static void kiocb_done(struct kiocb *kiocb, ssize_t ret, struct io_kiocb **nxt,
 		       bool in_async)
 {
-	if (in_async && ret >= 0 && nxt && kiocb->ki_complete == io_complete_rw)
+	if (in_async && ret >= 0 && kiocb->ki_complete == io_complete_rw)
 		*nxt = __io_complete_rw(kiocb, ret);
 	else
 		io_rw_done(kiocb, ret);
@@ -2570,6 +2562,7 @@ static int io_req_defer(struct io_kiocb *req)
 	return -EIOCBQUEUED;
 }
 
+__attribute__((nonnull))
 static int io_issue_sqe(struct io_kiocb *req, struct io_kiocb **nxt,
 			bool force_nonblock)
 {
@@ -2884,10 +2877,13 @@ static struct io_kiocb *io_prep_linked_timeout(struct io_kiocb *req)
 
 static void __io_queue_sqe(struct io_kiocb *req)
 {
-	struct io_kiocb *nxt = io_prep_linked_timeout(req);
+	struct io_kiocb *linked_timeout = io_prep_linked_timeout(req);
+	struct io_kiocb *nxt = NULL;
 	int ret;
 
-	ret = io_issue_sqe(req, NULL, true);
+	ret = io_issue_sqe(req, &nxt, true);
+	if (nxt)
+		io_queue_async_work(nxt);
 
 	/*
 	 * We async punt it if the file wasn't marked NOWAIT, or if the file
@@ -2923,11 +2919,11 @@ static void __io_queue_sqe(struct io_kiocb *req)
 	/* drop submission reference */
 	io_put_req(req);
 
-	if (nxt) {
+	if (linked_timeout) {
 		if (!ret)
-			io_queue_linked_timeout(nxt);
+			io_queue_linked_timeout(linked_timeout);
 		else
-			io_put_req(nxt);
+			io_put_req(linked_timeout);
 	}
 
 	/* and drop final reference, if we failed */
-- 
2.24.0