Re: Large number of empty reads on 5.9-rc2 under moderate load

[Jens Axboe <axboe@kernel.dk>]

On 8/24/20 10:18 AM, Jens Axboe wrote:
> On 8/24/20 10:13 AM, Dmitry Shulyak wrote:
>> On Mon, 24 Aug 2020 at 19:10, Jens Axboe <axboe@kernel.dk> wrote:
>>>
>>> On 8/24/20 9:33 AM, Dmitry Shulyak wrote:
>>>> On Mon, 24 Aug 2020 at 17:45, Jens Axboe <axboe@kernel.dk> wrote:
>>>>>
>>>>> On 8/24/20 8:06 AM, Jens Axboe wrote:
>>>>>> On 8/24/20 5:09 AM, Dmitry Shulyak wrote:
>>>>>>> library that i am using https://github.com/dshulyak/uring
>>>>>>> It requires golang 1.14, if installed, benchmark can be run with:
>>>>>>> go test ./fs -run=xx -bench=BenchmarkReadAt/uring_8 -benchtime=1000000x
>>>>>>> go test ./fs -run=xx -bench=BenchmarkReadAt/uring_5 -benchtime=8000000x
>>>>>>>
>>>>>>> note that it will setup uring instance per cpu, with shared worker pool.
>>>>>>> it will take me too much time to implement repro in c, but in general
>>>>>>> i am simply submitting multiple concurrent
>>>>>>> read requests and watching read rate.
>>>>>>
>>>>>> I'm fine with trying your Go version, but I can into a bit of trouble:
>>>>>>
>>>>>> axboe@amd ~/g/go-uring (master)>
>>>>>> go test ./fs -run=xx -bench=BenchmarkReadAt/uring_8 -benchtime=1000000x
>>>>>> # github.com/dshulyak/uring/fixed
>>>>>> fixed/allocator.go:38:48: error: incompatible type for field 2 in struct construction (cannot use type uint64 as type syscall.Iovec_len_t)
>>>>>>    38 |  iovec := []syscall.Iovec{{Base: &mem[0], Len: uint64(size)}}
>>>>>>       |                                                ^
>>>>>> FAIL  github.com/dshulyak/uring/fs [build failed]
>>>>>> FAIL
>>>>>> axboe@amd ~/g/go-uring (master)> go version
>>>>>> go version go1.14.6 gccgo (Ubuntu 10.2.0-5ubuntu1~20.04) 10.2.0 linux/amd64
>>>>>
>>>>> Alright, got it working. What device are you running this on? And am I
>>>>> correct in assuming you get short reads, or rather 0 reads? What file
>>>>> system?
>>>>
>>>> Was going to look into this.
>>>> I am getting 0 reads. This is on some old kingston ssd, ext4.
>>>
>>> I can't seem to reproduce this. I do see some cqe->res == 0 completes,
>>> but those appear to be NOPs. And they trigger at the start and end. I'll
>>> keep poking.
>>
>> Nops are used for draining and closing rings at the end of benchmarks.
>> It also appears in the beginning because of the way golang runs
>> benchmarks...
> 
> OK, just checking if it was expected.
> 
> But I can reproduce it now, turns out I was running XFS and that doesn't
> trigger it. With ext4, I do see zero sized read completions. I'll keep
> poking.

Can you try with this? Looks like some cases will consume bytes from the
iterator even if they ultimately return an error. If we've consumed bytes
but need to trigger retry, ensure we revert the consumed bytes.


diff --git a/fs/io_uring.c b/fs/io_uring.c
index 91e2cc8414f9..609b4996a4e9 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -3152,6 +3152,8 @@ static int io_read(struct io_kiocb *req, bool force_nonblock,
 	} else if (ret == -EAGAIN) {
 		if (!force_nonblock)
 			goto done;
+		/* some cases will consume bytes even on error returns */
+		iov_iter_revert(iter, iov_count - iov_iter_count(iter));
 		ret = io_setup_async_rw(req, iovec, inline_vecs, iter, false);
 		if (ret)
 			goto out_free;
@@ -3293,6 +3295,8 @@ static int io_write(struct io_kiocb *req, bool force_nonblock,
 	if (!force_nonblock || ret2 != -EAGAIN) {
 		kiocb_done(kiocb, ret2, cs);
 	} else {
+		/* some cases will consume bytes even on error returns */
+		iov_iter_revert(iter, iov_count - iov_iter_count(iter));
 copy_iov:
 		ret = io_setup_async_rw(req, iovec, inline_vecs, iter, false);
 		if (!ret)

-- 
Jens Axboe