Re: [PATCH v2 2/2] io_uring/timeout: immediate timeout arg

[Pavel Begunkov <asml.silence@gmail.com>]

On 2/27/26 19:39, Jens Axboe wrote:
> On 2/27/26 12:08 PM, Pavel Begunkov wrote:
>> On 2/27/26 14:08, Stefan Metzmacher wrote:
>>> Hi Pavel,
>>>
>>>>        if (unlikely(req->flags & (REQ_F_FIXED_FILE | REQ_F_BUFFER_SELECT)))
>>>>            return -EINVAL;
>>>> @@ -460,10 +461,20 @@ int io_timeout_remove_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
>>>>                return -EINVAL;
>>>>            if (tr->flags & IORING_LINK_TIMEOUT_UPDATE)
>>>>                tr->ltimeout = true;
>>>> -        if (tr->flags & ~(IORING_TIMEOUT_UPDATE_MASK|IORING_TIMEOUT_ABS))
>>>> +        if (tr->flags & ~(IORING_TIMEOUT_UPDATE_MASK |
>>>> +                  IORING_TIMEOUT_ABS |
>>>> +                  IORING_TIMEOUT_IMMEDIATE_ARG))
>>>>                return -EINVAL;
>>>> -        if (get_timespec64(&tr->ts, u64_to_user_ptr(READ_ONCE(sqe->addr2))))
>>>> +
>>>> +        arg = READ_ONCE(sqe->addr2);
>>>> +        if (tr->flags & IORING_TIMEOUT_IMMEDIATE_ARG) {
>>>> +            if (tr->flags & IORING_TIMEOUT_ABS)
>>>> +                return -EINVAL;
>>>> +            tr->ts = ns_to_timespec64(arg);
>>>
>>> I'm wondering if there is enough free space in a small sqe to hold a full timespec?
>>> So that there is no restriction for IORING_TIMEOUT_ABS...
>>
>> Well, u64 gives ~500 years in ns, it should be fine to just
>> allow the abs mode. We just need to make sure to zero check
>> the unused fields in case it'd need to be extended.
> 
> I don't think it's about length of it - if you can avoid the div by
> doing ns_to_timespec64(), that might be very useful? Would make

hrtimer_start(&data->timer, timespec64_to_ktime(data->ts), mode);
                                    ^^^

io_uring just needs to flip it and use ktime, but I left it for later.

> userspace simpler too potentially, and basically make the immediate mode
> _exactly_ the same as the non-immediate mode, it just delivers the
> __kernel_timespec in a different way.

I very much want to believe that everything about kernel_timespec has
some deep meaning, but I fail to see why they split it as sec/ns and
left invalid ranges for ns, why ns is signed, and why even after a
large revamp one of the fields doesn't use a fixed width type.
I'm not sure exactly like it is actually a good idea.

-- 
Pavel Begunkov