On 4/28/21 3:39 PM, Jens Axboe wrote: > On 4/28/21 8:34 AM, Pavel Begunkov wrote: >> On 4/28/21 2:32 PM, Hao Xu wrote: >>> sqes are submitted by sqthread when it is leveraged, which means there >>> is IO latency when waking up sqthread. To wipe it out, submit limited >>> number of sqes in the original task context. >>> Tests result below: >> >> Frankly, it can be a nest of corner cases if not now then in the future, >> leading to a high maintenance burden. Hence, if we consider the change, >> I'd rather want to limit the userspace exposure, so it can be removed >> if needed. >> >> A noticeable change of behaviour here, as Hao recently asked, is that >> the ring can be passed to a task from a completely another thread group, >> and so the feature would execute from that context, not from the >> original/sqpoll one. >> >> Not sure IORING_ENTER_SQ_DEPUTY knob is needed, but at least can be >> ignored if the previous point is addressed. > > I mostly agree on that. The problem I see is that for most use cases, > the "submit from task itself if we need to enter the kernel" is > perfectly fine, and would probably be preferable. But there are also > uses cases that absolutely do not want to spend any extra cycles doing > submit, they are isolating the submission to sqpoll exclusively and that > is part of the win there. Based on that, I don't think it can be an > automatic kind of feature. Reasonable. > I do think the naming is kind of horrible. IORING_ENTER_SQ_SUBMIT_IDLE > would likely be better, or maybe even more verbose as > IORING_ENTER_SQ_SUBMIT_ON_IDLE. > > On top of that, I don't think an extra submit flag is a huge deal, I > don't imagine we'll end up with a ton of them. In fact, two have been > added related to sqpoll since the inception, out of the 3 total added > flags. I don't care about the flag itself, nor about performance as it's nicely under the SQPOLL check, but I rather want to leave a way to ignore the feature if we would (ever) need to disable it, either with flag or without it. > This is all independent of implementation detail and needed fixes to the > patch. > -- Pavel Begunkov
