On 7/25/23 7:00?AM, Peter Zijlstra wrote:
> On Fri, Jul 21, 2023 at 09:29:14AM -0600, Jens Axboe wrote:
>
>> FWIW, here's the io_uring incremental after that rebase. Update the
>> liburing futex branch as well, updating the prep helpers to take 64 bit
>> values for mask/val and also add the flags argument that was missing as
>> well. Only other addition was adding those 4 new patches instead of the
>> old 3 ones, and adding single patch that just moves FUTEX2_MASK to
>> futex.h.
>>
>> All checks out fine, tests pass and it works.
>>
>>
>> diff --git a/io_uring/futex.c b/io_uring/futex.c
>> index 93df54dffaa0..4c9f2c841b98 100644
>> --- a/io_uring/futex.c
>> +++ b/io_uring/futex.c
>> @@ -18,11 +18,11 @@ struct io_futex {
>> u32 __user *uaddr;
>> struct futex_waitv __user *uwaitv;
>> };
>> + unsigned long futex_val;
>> + unsigned long futex_mask;
>> unsigned long futexv_owned;
>> + u32 futex_flags;
>> + unsigned int futex_nr;
>> };
>>
>> struct io_futex_data {
>> @@ -171,15 +171,28 @@ bool io_futex_remove_all(struct io_ring_ctx *ctx, struct task_struct *task,
>> int io_futex_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
>> {
>> struct io_futex *iof = io_kiocb_to_cmd(req, struct io_futex);
>> + u32 flags;
>>
>> + if (unlikely(sqe->fd || sqe->buf_index || sqe->file_index))
>> return -EINVAL;
>>
>> iof->uaddr = u64_to_user_ptr(READ_ONCE(sqe->addr));
>> + iof->futex_val = READ_ONCE(sqe->addr2);
>> + iof->futex_mask = READ_ONCE(sqe->addr3);
>> + iof->futex_nr = READ_ONCE(sqe->len);
>> + if (iof->futex_nr && req->opcode != IORING_OP_FUTEX_WAITV)
>> + return -EINVAL;
>> +
>
> Hmm, would something like:
>
> if (req->opcode == IORING_OP_FUTEX_WAITV) {
> if (iof->futex_val && iof->futex_mask)
> return -EINVAL;
>
> /* sys_futex_waitv() doesn't take @flags as of yet */
> if (iof->futex_flags)
> return -EINVAL;
>
> if (!iof->futex_nr)
> return -EINVAL;
>
> } else {
> /* sys_futex_{wake,wait}() don't take @nr */
> if (iof->futex_nr)
> return -EINVAL;
>
> /* both take @flags and @mask */
> flags = READ_ONCE(sqe->futex_flags);
> if (flags & ~FUTEX2_MASK)
> return -EINVAL;
>
> iof->futex_flags = futex2_to_flags(flags);
> if (!futex_flags_valid(iof->futex_flags))
> return -EINVAL;
>
> if (!futex_validate_input(iof->futex_flags, iof->futex_mask))
> return -EINVAL;
>
> /* sys_futex_wait() takes @val */
> if (req->iocode == IORING_OP_FUTEX_WAIT) {
> if (!futex_validate_input(iof->futex_flags, iof->futex_val))
> return -EINVAL;
> } else {
> if (iof->futex_val)
> return -EINVAL;
> }
> }
>
> work? The waitv thing is significantly different from the other two.
I think I'll just have prep and prepv totally separate. It only makes
sense to share parts of them if one is a subset of the other. That'll
get rid of the odd conditionals and sectioning of it.
--
Jens Axboe
