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

[Stefan Metzmacher <metze@samba.org>]

[-- Attachment #1.1: Type: text/plain, Size: 2155 bytes --]

Am 03.08.20 um 18:41 schrieb Jens Axboe:
> On 8/2/20 9:16 PM, Jiufei Xue wrote:
>> Hi Jens,
>>
>> On 2020/7/31 上午11:57, Jens Axboe wrote:
>>> Then why not just make the sqe-less timeout path flush existing requests,
>>> if it needs to? Seems a lot simpler than adding odd x2 variants, which
>>> won't really be clear.
>>>
>> Flushing the requests will access and modify the head of submit queue, that
>> may race with the submit thread. I think the reap thread should not touch
>> the submit queue when IORING_FEAT_GETEVENTS_TIMEOUT is supported.
> 
> Ahhh, that's the clue I was missing, yes that's a good point!
> 
>>> Chances are, if it's called with sq entries pending, the caller likely
>>> wants those submitted. Either the caller was aware and relying on that
>>> behavior, or the caller is simply buggy and has a case where it doesn't
>>> submit IO before waiting for completions.
>>>
>>
>> That is not true when the SQ/CQ handling are split in two different threads.
>> The reaping thread is not aware of the submit queue. It should only wait for
>> completion of the requests, such as below:
>>
>> submitting_thread:                   reaping_thread:
>>
>> io_uring_get_sqe()
>> io_uring_prep_nop()     
>>                                  io_uring_wait_cqe_timeout2()
>> io_uring_submit()
>>                                  woken if requests are completed or timeout
>>
>>
>> And if the SQ/CQ handling are in the same thread, applications should use the
>> old API if they do not want to submit the request themselves.
>>
>> io_uring_get_sqe
>> io_uring_prep_nop
>> io_uring_wait_cqe_timeout
> 
> Thanks, yes it's all clear to me now. I do wonder if we can't come up with
> something better than postfixing the functions with a 2, that seems kind of
> ugly and doesn't really convey to anyone what the difference is.
> 
> Any suggestions for better naming?

Isn't a bit in ring->flags enough? Instead of a new function?

Also the struct passed to the kernel should be a named one instead of
an anonymous struct defined in two places. Maybe a wrapping union would
be good...

metze



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]