cleanup io_put/free
cover cleanup io_put/free
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(-)
Signature · 1 line
2.24.0
PATCH 1/4 io_uring: remove io_free_req_find_next()
There is only one one-liner user of io_free_req_find_next(). Inline it.
--- fs/io_uring.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-)
fs/io_uring.c +2 -7
@@ -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); }/*
Signature · 1 line
2.24.0
PATCH 2/4 io_uring: pass only !null to io_req_find_next()
In reply to Pavel BegunkovMake io_req_find_next() and io_req_link_next() to accept only non-null nxt, and handle it in callers.
--- fs/io_uring.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)
fs/io_uring.c +7 -2
@@ -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); }/*
Signature · 1 line
2.24.0
PATCH 3/4 io_uring: simplify io_req_link_next()
In reply to 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.
--- fs/io_uring.c | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-)
fs/io_uring.c +1 -10
@@ -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 ; }
Signature · 1 line
2.24.0
PATCH 4/4 io_uring: only !null ptr to io_issue_sqe()
In reply to Pavel BegunkovPass 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).
--- fs/io_uring.c | 28 ++++++++++++---------------- 1 file changed, 12 insertions(+), 16 deletions(-)
fs/io_uring.c +12 -16
@@ -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 */
Signature · 1 line
2.24.0