On 6/23/20 5:57 AM, Pavel Begunkov wrote: > On 23/06/2020 05:18, Jens Axboe wrote: >> On 6/22/20 8:07 PM, Jens Axboe wrote: >>> On 6/22/20 4:16 PM, Pavel Begunkov wrote: >>>> io_do_iopoll() won't do anything with a request unless >>>> req->iopoll_completed is set. So io_complete_rw_iopoll() has to set >>>> it, otherwise io_do_iopoll() will poll a file again and again even >>>> though the request of interest was completed long ago. >>> >>> I need to look at this again, because with this change, I previously >>> got various use-after-free. I haven't seen any issues with it, but >>> I agree, from a quick look that I'm not quite sure how it's currently >>> not causing hangs. Yet I haven't seen any, with targeted -EAGAIN >>> testing. > > Can io_complete_rw_iopoll() get -EAGAIN after being successfully enqueued > (i.e. EIOCBQUEUED)? It's reliably fails for me, because my hacked nullblk > _can_ (i.e. probabilistically returns BLK_STS_AGAIN from ->iopoll()). Yes it can. The primary example would be a polled bio that gets split, into let's say 4 bio's. First one queues fine, but one of the subsequent ones run into request allocation failures and it gets marked as -EAGAIN. >> Ah I think I know what it is - if we run into: >> >> if (req->result == -EAGAIN) >> return -EAGAIN >> >> in io_issue_sqe() and race with it, we'll reissue twice potentially. >> So the above isn't quite enough, we'll need something a bit broader. > > I see, I'll deal with it. Thanks! -- Jens Axboe
