On 2/25/20 2:52 PM, Pavel Begunkov wrote: > 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... It very much can complete the req after io_read() returns, that's what happens for any async disk request! By the time io_read() returns, the request could be completed, or it could just be in-flight. This is different from lots of the other opcodes, where the actual call returns completion sync (either success, or EAGAIN). -- Jens Axboe
