On 30/11/2020 18:15, Jens Axboe wrote:
> On 11/29/20 10:12 AM, Pavel Begunkov wrote:
>> + tr->flags = READ_ONCE(sqe->timeout_flags);
>> + if (tr->flags) {
>> + if (!(tr->flags & IORING_TIMEOUT_UPDATE))
>> + return -EINVAL;
>> + if (tr->flags & ~(IORING_TIMEOUT_UPDATE|IORING_TIMEOUT_ABS))
>> + return -EINVAL;
>
> These flag comparisons are a bit obtuse - perhaps warrants a comment?
Ok, the one below should be more readable.
if (tr->flags & IORING_TIMEOUT_UPDATE) {
if (flags & ~ALLOWED_UPDATE_FLAGS)
return -EINVAL;
...
} else if (tr->flags) {
/* timeout removal doesn't support flags */
return -EINVAL;
}
>
>> + ret = __io_sq_thread_acquire_mm(req->ctx);
>> + if (ret)
>> + return ret;
>
> Why is this done manually?
mm is only needed in *prep(), so don't want IO_WQ_WORK_MM to put it
into req->work since it also affects timeout remove reqs.
>
>> diff --git a/include/uapi/linux/io_uring.h b/include/uapi/linux/io_uring.h
>> index 6bb8229de892..12a6443ea60d 100644
>> --- a/include/uapi/linux/io_uring.h
>> +++ b/include/uapi/linux/io_uring.h
>> @@ -151,6 +151,7 @@ enum {
>> * sqe->timeout_flags
>> */
>> #define IORING_TIMEOUT_ABS (1U << 0)
>> +#define IORING_TIMEOUT_UPDATE (1U << 31)
>
> Why bit 31?
Left bits for other potential timeout modes, don't know which though.
Can return it to bit 1.
--
Pavel Begunkov
