Re: [PATCH 2/2] io_uring: add timeout update

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

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