On 2020/8/4 下午12:50, Jens Axboe wrote: > On 8/3/20 7:29 PM, Jiufei Xue wrote: >> >> Hi Jens, >> On 2020/8/4 上午12:41, Jens Axboe wrote: >>> 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? >>> >> how about io_uring_wait_cqe_timeout_nolock()? That means applications can use >> the new APIs without synchronization. > > But even applications that don't share the ring across submit/complete > threads will want to use the new interface, if supported by the kernel. > Yes, if they share, they must use it - but even if they don't, it's > likely going to be a more logical interface for them. > > So I don't think that _nolock() really conveys that very well, but at > the same time I don't have any great suggestions. > > io_uring_wait_cqe_timeout_direct()? Or we could go simpler and just call > it io_uring_wait_cqe_timeout_r(), which is a familiar theme from libc > that is applied to thread safe implementations. > > I'll ponder this a bit... > As suggested by Stefan, applications can pass a flag, say IORING_SETUP_GETEVENTS_TIMEOUT to initialize the ring to indicate they want to use the new feature. Function io_uring_wait_cqes() need to submit the timeout sqe neither the kernel is not supported nor applications do not want to use the new feature. So we do not need to add a new API. Thanks, Jiufei
