On 6/10/21 4:38 PM, Olivier Langlois wrote: > On Thu, 2021-06-10 at 10:03 +0100, Pavel Begunkov wrote: >> On 6/9/21 11:08 PM, Olivier Langlois wrote: >>> It is quite frequent that when an operation fails and returns >>> EAGAIN, >>> the data becomes available between that failure and the call to >>> vfs_poll() done by io_arm_poll_handler(). >>> >>> Detecting the situation and reissuing the operation is much faster >>> than going ahead and push the operation to the io-wq. >> >> The poll stuff is not perfect and definitely can be improved, >> but there are drawbacks, with this one fairness may suffer >> with higher submit batching and make lat worse for all >> but one request. >> >> I'll get to it and another poll related email later, >> probably next week. >> > Hi Pavel, > > I am looking forward to see the improved solution that you succeed > coming up with. > > However, I want to bring 1 detail to your attention in case that it > went unnoticed. > > If io_arm_poll_handler() returns false because vfs_poll() returns a non > zero value, reissuing the sqe will be attempted at most only 1 time > because req->flags will have REQ_F_POLLED and on the second time > io_arm_poll_handler() will be called, it will immediately return false. > > With this detail in mind, I honestly did not think that this would make > the function unfair for the other requests in a batch submission > compared to the cost of pushing the request to io-wq that possibly > includes an io worker thread creation. > > Does this detail can change your verdict? > If not, I would really be interested to know more about your fairness > concern. Right, but it still stalls other requests and IIRC there are people not liking the syscall already taking too long. Consider io_req_task_queue(), adds more overhead but will delay execution to the syscall exit. In any case, would be great to have numbers, e.g. to see if io_req_task_queue() is good enough, how often your problem takes places and how much it gives us. -- Pavel Begunkov
