[PATCH] io_uring/futex: Recycle ifd on failure

[PATCH] io_uring/futex: Recycle ifd on failure

[Gabriel Krisman Bertazi]

The failure path doesn't recycle the ifd on FUTEX_WAIT for no good
reason.  If we move the free above done_unlock, we are even already
holding the submit lock.  Recycle it.

Signed-off-by: Gabriel Krisman Bertazi <krisman@suse.de>
---
 io_uring/futex.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/io_uring/futex.c b/io_uring/futex.c
index eaee14242a3a..6389bd2f0399 100644
--- a/io_uring/futex.c
+++ b/io_uring/futex.c
@@ -321,20 +321,22 @@ int io_futex_wait(struct io_kiocb *req, unsigned int issue_flags)
 
 	ret = futex_wait_setup(iof->uaddr, iof->futex_val, iof->futex_flags,
 			       &ifd->q, NULL, NULL);
-	if (!ret) {
-		hlist_add_head(&req->hash_node, &ctx->futex_list);
-		io_ring_submit_unlock(ctx, issue_flags);
+	if (ret)
+		goto free_async;
 
-		return IOU_ISSUE_SKIP_COMPLETE;
-	}
+	hlist_add_head(&req->hash_node, &ctx->futex_list);
+	io_ring_submit_unlock(ctx, issue_flags);
 
+	return IOU_ISSUE_SKIP_COMPLETE;
+
+free_async:
+	io_cache_free(&ctx->futex_cache, ifd);
+	io_req_async_data_clear(req, 0);
 done_unlock:
 	io_ring_submit_unlock(ctx, issue_flags);
 done:
-	if (ret < 0)
-		req_set_fail(req);
+	req_set_fail(req);
 	io_req_set_res(req, ret, 0);
-	io_req_async_data_free(req);
 	return IOU_COMPLETE;
 }
 
-- 
2.55.0

Re: [PATCH] io_uring/futex: Recycle ifd on failure

[Jens Axboe]

On 9/2/26 11:27 AM, Gabriel Krisman Bertazi wrote:
> The failure path doesn't recycle the ifd on FUTEX_WAIT for no good
> reason.  If we move the free above done_unlock, we are even already
> holding the submit lock.  Recycle it.

Does it matter? For performance, it should not, unless you have a
benchmark that deliberately ends up with futex_wait_setup() failing.
Any normal/fast path will have all of these succeed.

Is the motivation just to unify the cleanup, or..?

-- 
Jens Axboe

Re: [PATCH] io_uring/futex: Recycle ifd on failure

[Gabriel Krisman Bertazi]

Jens Axboe <axboe@kernel.dk> writes:

> On 9/2/26 11:27 AM, Gabriel Krisman Bertazi wrote:
>> The failure path doesn't recycle the ifd on FUTEX_WAIT for no good
>> reason.  If we move the free above done_unlock, we are even already
>> holding the submit lock.  Recycle it.
>
> Does it matter? For performance, it should not, unless you have a
> benchmark that deliberately ends up with futex_wait_setup() failing.
> Any normal/fast path will have all of these succeed.
>
> Is the motivation just to unify the cleanup, or..?

Because recycling is good for the world :)

It is a cleanup.  A todo in my list is recycling objects as much as we
can. for instance, in the generic cleanup path we could be recycling for
various objects.  I'll send it as a larger series.

-- 
Gabriel Krisman Bertazi

Re: [PATCH] io_uring/futex: Recycle ifd on failure

[Gabriel Krisman Bertazi]

Jens Axboe <axboe@kernel.dk> writes:

> On 9/2/26 11:27 AM, Gabriel Krisman Bertazi wrote:
>> The failure path doesn't recycle the ifd on FUTEX_WAIT for no good
>> reason.  If we move the free above done_unlock, we are even already
>> holding the submit lock.  Recycle it.
>
> Does it matter? For performance, it should not, unless you have a
> benchmark that deliberately ends up with futex_wait_setup() failing.
> Any normal/fast path will have all of these succeed.
>
> Is the motivation just to unify the cleanup, or..?

Because recycling is good for the world :)

It is a cleanup.  A todo in my list is recycling objects as much as we
can. for instance, in the generic cleanup path we could be recycling for
various objects.  I'll send those as part of a larger series.

-- 
Gabriel Krisman Bertazi