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... -- Jens Axboe
