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
