On 7/12/23 2:51?AM, Peter Zijlstra wrote: > On Tue, Jul 11, 2023 at 06:47:01PM -0600, Jens Axboe wrote: >> Add support for FUTEX_WAKE/WAIT primitives. >> >> IORING_OP_FUTEX_WAKE is mix of FUTEX_WAKE and FUTEX_WAKE_BITSET, as >> it does support passing in a bitset. >> >> Similary, IORING_OP_FUTEX_WAIT is a mix of FUTEX_WAIT and >> FUTEX_WAIT_BITSET. >> >> FUTEX_WAKE is straight forward, as we can always just do those inline. >> FUTEX_WAIT will queue the futex with an appropriate callback, and >> that callback will in turn post a CQE when it has triggered. >> >> Cancelations are supported, both from the application point-of-view, >> but also to be able to cancel pending waits if the ring exits before >> all events have occurred. >> >> This is just the barebones wait/wake support. PI or REQUEUE support is >> not added at this point, unclear if we might look into that later. >> >> Likewise, explicit timeouts are not supported either. It is expected >> that users that need timeouts would do so via the usual io_uring >> mechanism to do that using linked timeouts. >> >> Signed-off-by: Jens Axboe <axboe@xxxxxxxxx> > > I'm not sure I'm qualified to review this :/ I really don't know > anything about how io-uring works. And the above doesn't really begin to > explain things. It's definitely catered to someone who knows how that works already, I feel like it'd be a bit beyond the scope of a commit message like that to explain io_uring internals. Then we'd have to do that every time we add a new request type. But I can certainly expand it a bit and hopefully make it a bit clearer. I'll do that. >> +static void io_futex_wake_fn(struct wake_q_head *wake_q, struct futex_q *q) >> +{ >> + struct io_futex_data *ifd = container_of(q, struct io_futex_data, q); >> + struct io_kiocb *req = ifd->req; >> + >> + __futex_unqueue(q); >> + smp_store_release(&q->lock_ptr, NULL); >> + >> + io_req_set_res(req, 0, 0); >> + req->io_task_work.func = io_futex_complete; >> + io_req_task_work_add(req); >> +} > > I'm noting the WARN from futex_wake_mark() went walk-about. True. > Perhaps something like so? I like that, sharing more code is always better. Should be a separate patch though I think, or folded into patch 2. Which would you prefer? I'll do a separate patch for now. -- Jens Axboe