[PATCH] io_uring/futex: return a wait's data to the cache when it fails to arm

[PATCH] io_uring/futex: return a wait's data to the cache when it fails to arm

[Austin Schuh]

io_futex_wait() takes its io_futex_data from ctx->futex_cache, and a
completed wait gives it back in io_futex_complete().  A wait that fails
to arm kfree()s it instead.

The usual failure is -EAGAIN from futex_wait_setup(): the futex word no
longer holds the expected value.  That is routine for a waiter that
re-arms with the value it last read, since the word changes whenever
there is something to wake for.  Each such failure takes an object out
of the cache for good, so the cache drains and from then on waits
allocate.

The cache exists so that arming a wait doesn't have to go to the slab
allocator, and that matters most to the waiters that re-arm the most:
event loops, often real-time ones.  A kmalloc() or kfree() that misses
the per-CPU slab takes the node's list_lock, shared with every other
user of that kmalloc size on the system, and may go on to the page
allocator.

On PREEMPT_RT list_lock is a sleeping lock.  A real-time task that
finds it held waits for the holder, which may be any task on any CPU,
to get to run and release it.  Priority inheritance boosts the holder
only up to the waiter's priority, so the waiter still sits behind
whatever outranks that on the holder's CPU, such as threaded interrupt
handlers.  Re-arming a futex wait should not depend on any of that.

Put the data back in the cache on that path too, while the submit lock
still protects it.

In an event loop re-arming one IORING_OP_FUTEX_WAIT per message under
load, 0.39 arms per message failed with -EAGAIN, and io_futex_wait() did
0.39 kmalloc()s and kfree()s per message.  With this change, 100,000
waits that all fail with -EAGAIN do one kmalloc() in total.

Fixes: 194bb58c6090 ("io_uring: add support for futex wake and wait")
Assisted-by: LLM
Signed-off-by: Austin Schuh <austin.linux@gmail.com>
---

Notes:
    Based on for-7.4/io_uring (5b01d5955ac2).  Built for arm64 with
    PREEMPT_RT, W=1, no new warnings.  The numbers are from an equivalent
    change on 6.18.39-rt on a Jetson Orin Nano, counting kmem:kmalloc/kfree
    by call site with bpftrace while a test loops on IORING_OP_FUTEX_WAIT
    with a value the futex word doesn't hold.  Happy to turn that into
    a liburing test if it's useful.

 io_uring/futex.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/io_uring/futex.c b/io_uring/futex.c
index 9203998654ff..9e4e63ee7bc1 100644
--- a/io_uring/futex.c
+++ b/io_uring/futex.c
@@ -353,12 +353,16 @@ int io_futex_wait(struct io_kiocb *req, unsigned int issue_flags)
 	}
 
 done_unlock:
+	if (ifd) {
+		/* As in io_futex_complete(), or failed arms would drain it. */
+		io_cache_free(&ctx->futex_cache, ifd);
+		io_req_async_data_clear(req, 0);
+	}
 	io_ring_submit_unlock(ctx, issue_flags);
 done:
 	if (ret < 0)
 		req_set_fail(req);
 	io_req_set_res(req, ret, 0);
-	io_req_async_data_free(req);
 	return IOU_COMPLETE;
 }
 

base-commit: 5b01d5955ac206e38097f777eff6ab2a8b948c41
-- 
2.47.3

Re: [PATCH] io_uring/futex: return a wait's data to the cache when it fails to arm

[Jens Axboe]

On 10/19/26 5:12 PM, Austin Schuh wrote:
> io_futex_wait() takes its io_futex_data from ctx->futex_cache, and a
> completed wait gives it back in io_futex_complete().  A wait that fails
> to arm kfree()s it instead.
> 
> The usual failure is -EAGAIN from futex_wait_setup(): the futex word no
> longer holds the expected value.  That is routine for a waiter that
> re-arms with the value it last read, since the word changes whenever
> there is something to wake for.  Each such failure takes an object out
> of the cache for good, so the cache drains and from then on waits
> allocate.
> 
> The cache exists so that arming a wait doesn't have to go to the slab
> allocator, and that matters most to the waiters that re-arm the most:
> event loops, often real-time ones.  A kmalloc() or kfree() that misses
> the per-CPU slab takes the node's list_lock, shared with every other
> user of that kmalloc size on the system, and may go on to the page
> allocator.
> 
> On PREEMPT_RT list_lock is a sleeping lock.  A real-time task that
> finds it held waits for the holder, which may be any task on any CPU,
> to get to run and release it.  Priority inheritance boosts the holder
> only up to the waiter's priority, so the waiter still sits behind
> whatever outranks that on the holder's CPU, such as threaded interrupt
> handlers.  Re-arming a futex wait should not depend on any of that.
> 
> Put the data back in the cache on that path too, while the submit lock
> still protects it.
> 
> In an event loop re-arming one IORING_OP_FUTEX_WAIT per message under
> load, 0.39 arms per message failed with -EAGAIN, and io_futex_wait() did
> 0.39 kmalloc()s and kfree()s per message.  With this change, 100,000
> waits that all fail with -EAGAIN do one kmalloc() in total.

Patches like this have been posted before, and my question is
always "is this a common case?"? Reading this:

> Notes:
>     Based on for-7.4/io_uring (5b01d5955ac2).  Built for arm64 with
>     PREEMPT_RT, W=1, no new warnings.  The numbers are from an equivalent
>     change on 6.18.39-rt on a Jetson Orin Nano, counting kmem:kmalloc/kfree
>     by call site with bpftrace while a test loops on IORING_OP_FUTEX_WAIT
>     with a value the futex word doesn't hold.  Happy to turn that into
>     a liburing test if it's useful.

makes me very much think "nope, not at all", as it's a deliberately
failing test case. And yes if you keep issuing failing waits,
then it'll drain the cache. The cache is for the hot FAST path,
which is not an error path.

Hmm?

-- 
Jens Axboe