Re: [PATCH liburing 1/2] io_uring_enter: add timeout support

[Jiufei Xue <jiufei.xue@linux.alibaba.com>]

On 2020/7/30 下午11:28, Jens Axboe wrote:
> On 7/29/20 8:32 PM, Jiufei Xue wrote:
>> Hi Jens,
>>
>> On 2020/7/30 上午1:51, Jens Axboe wrote:
>>> On 7/29/20 4:10 AM, Jiufei Xue wrote:
>>>> Kernel can handle timeout when feature IORING_FEAT_GETEVENTS_TIMEOUT
>>>> supported. Add two new interfaces: io_uring_wait_cqes2(),
>>>> io_uring_wait_cqe_timeout2() for applications to use this feature.
>>>
>>> Why add new new interfaces, when the old ones already pass in the
>>> timeout? Surely they could just use this new feature, instead of the
>>> internal timeout, if it's available?
>>>
>> Applications use the old one may not call io_uring_submit() because
>> io_uring_wait_cqes() will do it. So I considered to add a new one.
> 
> Not sure I see how that's a problem - previously, you could not do that
> either, if you were doing separate submit/complete threads. So this
> doesn't really add any new restrictions. The app can check for the
> feature flag to see if it's safe to do so now.
>Yes, new applications can check for the feature flag. What about the existing
apps? The existing applications which do not separate submit/complete
threads may use io_uring_wait_cqes() or io_uring_wait_cqe_timeout() without
submiting the requests. No one will do that now when the feature is supported.


>>>> diff --git a/src/include/liburing.h b/src/include/liburing.h
>>>> index 0505a4f..6176a63 100644
>>>> --- a/src/include/liburing.h
>>>> +++ b/src/include/liburing.h
>>>> @@ -56,6 +56,7 @@ struct io_uring {
>>>>  	struct io_uring_sq sq;
>>>>  	struct io_uring_cq cq;
>>>>  	unsigned flags;
>>>> +	unsigned features;
>>>>  	int ring_fd;
>>>>  };
>>>
>>> This breaks the API, as it changes the size of the ring...
>>>
>> Oh, yes, I haven't considering that before. So could I add this feature
>> bit to io_uring.flags. Any suggestion?
> 
> Either that, or we add this (and add pad that we can use later) and just
> say that for the next release you have to re-compile against the lib.
> That will break existing applications, unless they are recompiled... But
> it might not be a bad idea to do so, just so we can pad io_uring out a
> little bit to provide for future flexibility.
>
Agree about that. So should we increase the major version after adding the
feature flag and some pad?

Thanks,
Jiufei