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? -- Pavel Begunkov
Attachment:
signature.asc
Description: OpenPGP digital signature
