Re: [ISSUE] The time cost of IOSQE_IO_LINK

[Jens Axboe <axboe@kernel.dk>]

On 2/16/20 12:06 PM, Pavel Begunkov wrote:
> On 15/02/2020 09:01, Jens Axboe wrote:
>> diff --git a/fs/io_uring.c b/fs/io_uring.c
>> index fb94b8bac638..530dcd91fa53 100644
>> @@ -4630,6 +4753,14 @@ static void __io_queue_sqe(struct io_kiocb *req, const struct io_uring_sqe *sqe)
>>  	 */
>>  	if (ret == -EAGAIN && (!(req->flags & REQ_F_NOWAIT) ||
>>  	    (req->flags & REQ_F_MUST_PUNT))) {
>> +
>> +		if (io_arm_poll_handler(req, &retry_count)) {
>> +			if (retry_count == 1)
>> +				goto issue;
> 
> Better to sqe=NULL before retrying, so it won't re-read sqe and try to
> init the req twice.

Good point, that should get cleared after issue.

> Also, the second sync-issue may -EAGAIN again, and as I remember,
> read/write/etc will try to copy iovec into req->io. But iovec is
> already in req->io, so it will self memcpy(). Not a good thing.

I'll look into those details, that has indeed reared its head before.

>> +			else if (!retry_count)
>> +				goto done_req;
>> +			INIT_IO_WORK(&req->work, io_wq_submit_work);
> 
> It's not nice to reset it as this:
> - prep() could set some work.flags
> - custom work.func is more performant (adds extra switch)
> - some may rely on specified work.func to be called. e.g. close(), even though
> it doesn't participate in the scheme

It's totally a hack as-is for the "can't do it, go async". I did clean
this up a bit (if you check the git version, it's changed quite a bit),
but it's still a mess in terms of that and ->work vs union ownership.
The commit message also has a note about that.

So more work needed in that area for sure.

-- 
Jens Axboe