io-uring.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] io_uring: retry in case of short read on block device
@ 2021-08-21 15:07 Ming Lei
  2021-08-31  9:39 ` Ming Lei
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Ming Lei @ 2021-08-21 15:07 UTC (permalink / raw)
  To: Jens Axboe, io-uring; +Cc: Ming Lei, Pavel Begunkov

In case of buffered reading from block device, when short read happens,
we should retry to read more, otherwise the IO will be completed
partially, for example, the following fio expects to read 2MB, but it
can only read 1M or less bytes:

    fio --name=onessd --filename=/dev/nvme0n1 --filesize=2M \
	--rw=randread --bs=2M --direct=0 --overwrite=0 --numjobs=1 \
	--iodepth=1 --time_based=0 --runtime=2 --ioengine=io_uring \
	--registerfiles --fixedbufs --gtod_reduce=1 --group_reporting

Fix the issue by allowing short read retry for block device, which sets
FMODE_BUF_RASYNC really.

Fixes: 9a173346bd9e ("io_uring: fix short read retries for non-reg files")
Cc: Pavel Begunkov <asml.silence@gmail.com>
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 fs/io_uring.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/fs/io_uring.c b/fs/io_uring.c
index bf548af0426c..bbcd1a9e75e5 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -3268,6 +3268,12 @@ static inline int io_iter_do_read(struct io_kiocb *req, struct iov_iter *iter)
 		return -EINVAL;
 }
 
+static bool need_read_all(struct io_kiocb *req)
+{
+	return req->flags & REQ_F_ISREG ||
+		S_ISBLK(file_inode(req->file)->i_mode);
+}
+
 static int io_read(struct io_kiocb *req, unsigned int issue_flags)
 {
 	struct iovec inline_vecs[UIO_FASTIOV], *iovec = inline_vecs;
@@ -3322,7 +3328,7 @@ static int io_read(struct io_kiocb *req, unsigned int issue_flags)
 	} else if (ret == -EIOCBQUEUED) {
 		goto out_free;
 	} else if (ret <= 0 || ret == io_size || !force_nonblock ||
-		   (req->flags & REQ_F_NOWAIT) || !(req->flags & REQ_F_ISREG)) {
+		   (req->flags & REQ_F_NOWAIT) || !need_read_all(req)) {
 		/* read all, failed, already did sync or don't want to retry */
 		goto done;
 	}
-- 
2.31.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] io_uring: retry in case of short read on block device
  2021-08-21 15:07 [PATCH] io_uring: retry in case of short read on block device Ming Lei
@ 2021-08-31  9:39 ` Ming Lei
  2021-08-31 10:07 ` Pavel Begunkov
  2021-08-31 12:24 ` Jens Axboe
  2 siblings, 0 replies; 4+ messages in thread
From: Ming Lei @ 2021-08-31  9:39 UTC (permalink / raw)
  To: Jens Axboe, io-uring; +Cc: Pavel Begunkov

On Sat, Aug 21, 2021 at 11:07:51PM +0800, Ming Lei wrote:
> In case of buffered reading from block device, when short read happens,
> we should retry to read more, otherwise the IO will be completed
> partially, for example, the following fio expects to read 2MB, but it
> can only read 1M or less bytes:
> 
>     fio --name=onessd --filename=/dev/nvme0n1 --filesize=2M \
> 	--rw=randread --bs=2M --direct=0 --overwrite=0 --numjobs=1 \
> 	--iodepth=1 --time_based=0 --runtime=2 --ioengine=io_uring \
> 	--registerfiles --fixedbufs --gtod_reduce=1 --group_reporting
> 
> Fix the issue by allowing short read retry for block device, which sets
> FMODE_BUF_RASYNC really.
> 
> Fixes: 9a173346bd9e ("io_uring: fix short read retries for non-reg files")
> Cc: Pavel Begunkov <asml.silence@gmail.com>
> Signed-off-by: Ming Lei <ming.lei@redhat.com>

Hello Jens and Pavel,

Any comments on this fix?


Thanks, 
Ming


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] io_uring: retry in case of short read on block device
  2021-08-21 15:07 [PATCH] io_uring: retry in case of short read on block device Ming Lei
  2021-08-31  9:39 ` Ming Lei
@ 2021-08-31 10:07 ` Pavel Begunkov
  2021-08-31 12:24 ` Jens Axboe
  2 siblings, 0 replies; 4+ messages in thread
From: Pavel Begunkov @ 2021-08-31 10:07 UTC (permalink / raw)
  To: Ming Lei, Jens Axboe, io-uring

On 8/21/21 4:07 PM, Ming Lei wrote:
> In case of buffered reading from block device, when short read happens,
> we should retry to read more, otherwise the IO will be completed
> partially, for example, the following fio expects to read 2MB, but it
> can only read 1M or less bytes:
> 
>     fio --name=onessd --filename=/dev/nvme0n1 --filesize=2M \
> 	--rw=randread --bs=2M --direct=0 --overwrite=0 --numjobs=1 \
> 	--iodepth=1 --time_based=0 --runtime=2 --ioengine=io_uring \
> 	--registerfiles --fixedbufs --gtod_reduce=1 --group_reporting
> 
> Fix the issue by allowing short read retry for block device, which sets
> FMODE_BUF_RASYNC really.

Should note that overhead on touching inode shouldn't be of concern at
this point, so all looks good

Reviewed-by: Pavel Begunkov <asml.silence@gmail.com>


> Fixes: 9a173346bd9e ("io_uring: fix short read retries for non-reg files")
> Cc: Pavel Begunkov <asml.silence@gmail.com>
> Signed-off-by: Ming Lei <ming.lei@redhat.com>
> ---
>  fs/io_uring.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/io_uring.c b/fs/io_uring.c
> index bf548af0426c..bbcd1a9e75e5 100644
> --- a/fs/io_uring.c
> +++ b/fs/io_uring.c
> @@ -3268,6 +3268,12 @@ static inline int io_iter_do_read(struct io_kiocb *req, struct iov_iter *iter)
>  		return -EINVAL;
>  }
>  
> +static bool need_read_all(struct io_kiocb *req)
> +{
> +	return req->flags & REQ_F_ISREG ||
> +		S_ISBLK(file_inode(req->file)->i_mode);
> +}
> +
>  static int io_read(struct io_kiocb *req, unsigned int issue_flags)
>  {
>  	struct iovec inline_vecs[UIO_FASTIOV], *iovec = inline_vecs;
> @@ -3322,7 +3328,7 @@ static int io_read(struct io_kiocb *req, unsigned int issue_flags)
>  	} else if (ret == -EIOCBQUEUED) {
>  		goto out_free;
>  	} else if (ret <= 0 || ret == io_size || !force_nonblock ||
> -		   (req->flags & REQ_F_NOWAIT) || !(req->flags & REQ_F_ISREG)) {
> +		   (req->flags & REQ_F_NOWAIT) || !need_read_all(req)) {
>  		/* read all, failed, already did sync or don't want to retry */
>  		goto done;
>  	}
> 

-- 
Pavel Begunkov

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] io_uring: retry in case of short read on block device
  2021-08-21 15:07 [PATCH] io_uring: retry in case of short read on block device Ming Lei
  2021-08-31  9:39 ` Ming Lei
  2021-08-31 10:07 ` Pavel Begunkov
@ 2021-08-31 12:24 ` Jens Axboe
  2 siblings, 0 replies; 4+ messages in thread
From: Jens Axboe @ 2021-08-31 12:24 UTC (permalink / raw)
  To: Ming Lei, io-uring; +Cc: Pavel Begunkov

On 8/21/21 9:07 AM, Ming Lei wrote:
> In case of buffered reading from block device, when short read happens,
> we should retry to read more, otherwise the IO will be completed
> partially, for example, the following fio expects to read 2MB, but it
> can only read 1M or less bytes:
> 
>     fio --name=onessd --filename=/dev/nvme0n1 --filesize=2M \
> 	--rw=randread --bs=2M --direct=0 --overwrite=0 --numjobs=1 \
> 	--iodepth=1 --time_based=0 --runtime=2 --ioengine=io_uring \
> 	--registerfiles --fixedbufs --gtod_reduce=1 --group_reporting
> 
> Fix the issue by allowing short read retry for block device, which sets
> FMODE_BUF_RASYNC really.

Applied, thanks Ming.

-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-08-31 12:24 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-21 15:07 [PATCH] io_uring: retry in case of short read on block device Ming Lei
2021-08-31  9:39 ` Ming Lei
2021-08-31 10:07 ` Pavel Begunkov
2021-08-31 12:24 ` Jens Axboe

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).