Re: [PATCH 3/7] io_uring: add support for futex wake and wait

[Jens Axboe]

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