On 26/02/2020 00:45, Pavel Begunkov wrote: > On 26/02/2020 00:25, Jens Axboe wrote: >> On 2/25/20 2:22 PM, Pavel Begunkov wrote: >>> On 25/02/2020 23:27, Jens Axboe wrote: >>>> If work completes inline, then we should pick up a dependent link item >>>> in __io_queue_sqe() as well. If we don't do so, we're forced to go async >>>> with that item, which is suboptimal. >>>> >>>> Signed-off-by: Jens Axboe <axboe@xxxxxxxxx> >>>> >>>> --- >>>> >>>> diff --git a/fs/io_uring.c b/fs/io_uring.c >>>> index ffd9bfa84d86..160cf1b0f478 100644 >>>> --- a/fs/io_uring.c >>>> +++ b/fs/io_uring.c >>>> @@ -4531,8 +4531,15 @@ static void io_wq_submit_work(struct io_wq_work **workptr) >>>> } while (1); >>>> } >>>> >>>> - /* drop submission reference */ >>>> - io_put_req(req); >>>> + /* >>>> + * Drop submission reference. In case the handler already dropped the >>>> + * completion reference, then it didn't pick up any potential link >>>> + * work. If 'nxt' isn't set, try and do that here. >>>> + */ >>>> + if (nxt) >>> >>> It can't even get here, because of the submission ref, isn't it? would the >>> following do? >>> >>> - io_put_req(req); >>> + io_put_req_find_next(req, &nxt); >> >> I don't think it can, let me make that change. And test. >> >>> BTW, as I mentioned before, it appears to me, we don't even need completion ref >>> as it always pinned by the submission ref. I'll resurrect the patches doing >>> that, but after your poll work will land. >> >> We absolutely do need two references, unfortunately. Otherwise we could complete >> the io_kiocb deep down the stack through the callback. > > And I need your knowledge here to not make mistakes :) > I remember the conversation about the necessity of submission ref, that's to > make sure it won't be killed in the middle of block layer, etc. But what about > removing the completion ref then? > > E.g. io_read(), as I see all its work is bound by lifetime of io_read() call, > so it's basically synchronous from the caller perspective. In other words, it > can't complete req after it returned from io_read(). And that would mean it's > save to have only submission ref after dealing with poll and other edge cases. > > Do I miss something? Hmm, just started to question myself, whether handlers can be not as synchronous as described... -- Pavel Begunkov
