qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] block/io_uring: clarify that short reads can happen
@ 2022-07-06  8:03 Stefan Hajnoczi
  2022-07-07  6:49 ` Stefano Garzarella
  2022-07-07  8:11 ` Stefan Hajnoczi
  0 siblings, 2 replies; 3+ messages in thread
From: Stefan Hajnoczi @ 2022-07-06  8:03 UTC (permalink / raw)
  To: qemu-devel
  Cc: Stefan Hajnoczi, Kevin Wolf, Julia Suvorova, qemu-block,
	Hanna Reitz, Aarushi Mehta, Stefano Garzarella,
	Dominique Martinet

Jens Axboe has confirmed that short reads are rare but can happen:
https://lore.kernel.org/io-uring/YsU%2FCGkl9ZXUI+Tj@stefanha-x1.localdomain/T/#m729963dc577d709b709c191922e98ec79d7eef54

The luring_resubmit_short_read() comment claimed they were only due to a
specific io_uring bug that was fixed in Linux commit 9d93a3f5a0c
("io_uring: punt short reads to async context"), which is wrong.
Dominique Martinet found that a btrfs bug also causes short reads. There
may be more kernel code paths that result in short reads.

Let's consider short reads fair game.

Cc: Dominique Martinet <dominique.martinet@atmark-techno.com>
Based-on: <20220630010137.2518851-1-dominique.martinet@atmark-techno.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/io_uring.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/block/io_uring.c b/block/io_uring.c
index b238661740..f8a19fd97f 100644
--- a/block/io_uring.c
+++ b/block/io_uring.c
@@ -73,12 +73,8 @@ static void luring_resubmit(LuringState *s, LuringAIOCB *luringcb)
 /**
  * luring_resubmit_short_read:
  *
- * Before Linux commit 9d93a3f5a0c ("io_uring: punt short reads to async
- * context") a buffered I/O request with the start of the file range in the
- * page cache could result in a short read.  Applications need to resubmit the
- * remaining read request.
- *
- * This is a slow path but recent kernels never take it.
+ * Short reads are rare but may occur. The remaining read request needs to be
+ * resubmitted.
  */
 static void luring_resubmit_short_read(LuringState *s, LuringAIOCB *luringcb,
                                        int nread)
-- 
2.36.1



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

* Re: [PATCH] block/io_uring: clarify that short reads can happen
  2022-07-06  8:03 [PATCH] block/io_uring: clarify that short reads can happen Stefan Hajnoczi
@ 2022-07-07  6:49 ` Stefano Garzarella
  2022-07-07  8:11 ` Stefan Hajnoczi
  1 sibling, 0 replies; 3+ messages in thread
From: Stefano Garzarella @ 2022-07-07  6:49 UTC (permalink / raw)
  To: Stefan Hajnoczi
  Cc: qemu-devel, Kevin Wolf, Julia Suvorova, qemu-block, Hanna Reitz,
	Aarushi Mehta, Dominique Martinet

On Wed, Jul 06, 2022 at 09:03:41AM +0100, Stefan Hajnoczi wrote:
>Jens Axboe has confirmed that short reads are rare but can happen:
>https://lore.kernel.org/io-uring/YsU%2FCGkl9ZXUI+Tj@stefanha-x1.localdomain/T/#m729963dc577d709b709c191922e98ec79d7eef54
>
>The luring_resubmit_short_read() comment claimed they were only due to a
>specific io_uring bug that was fixed in Linux commit 9d93a3f5a0c
>("io_uring: punt short reads to async context"), which is wrong.
>Dominique Martinet found that a btrfs bug also causes short reads. There
>may be more kernel code paths that result in short reads.
>
>Let's consider short reads fair game.
>
>Cc: Dominique Martinet <dominique.martinet@atmark-techno.com>
>Based-on: <20220630010137.2518851-1-dominique.martinet@atmark-techno.com>
>Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
>---
> block/io_uring.c | 8 ++------
> 1 file changed, 2 insertions(+), 6 deletions(-)
>
>diff --git a/block/io_uring.c b/block/io_uring.c
>index b238661740..f8a19fd97f 100644
>--- a/block/io_uring.c
>+++ b/block/io_uring.c
>@@ -73,12 +73,8 @@ static void luring_resubmit(LuringState *s, LuringAIOCB *luringcb)
> /**
>  * luring_resubmit_short_read:
>  *
>- * Before Linux commit 9d93a3f5a0c ("io_uring: punt short reads to async
>- * context") a buffered I/O request with the start of the file range in the
>- * page cache could result in a short read.  Applications need to resubmit the
>- * remaining read request.
>- *
>- * This is a slow path but recent kernels never take it.
>+ * Short reads are rare but may occur. The remaining read request needs to be
>+ * resubmitted.
>  */
> static void luring_resubmit_short_read(LuringState *s, LuringAIOCB *luringcb,
>                                        int nread)
>-- 
>2.36.1
>

Thanks for fixing the comment!

Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>




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

* Re: [PATCH] block/io_uring: clarify that short reads can happen
  2022-07-06  8:03 [PATCH] block/io_uring: clarify that short reads can happen Stefan Hajnoczi
  2022-07-07  6:49 ` Stefano Garzarella
@ 2022-07-07  8:11 ` Stefan Hajnoczi
  1 sibling, 0 replies; 3+ messages in thread
From: Stefan Hajnoczi @ 2022-07-07  8:11 UTC (permalink / raw)
  To: qemu-devel
  Cc: Kevin Wolf, Julia Suvorova, qemu-block, Hanna Reitz,
	Aarushi Mehta, Stefano Garzarella, Dominique Martinet

[-- Attachment #1: Type: text/plain, Size: 1048 bytes --]

On Wed, Jul 06, 2022 at 09:03:41AM +0100, Stefan Hajnoczi wrote:
> Jens Axboe has confirmed that short reads are rare but can happen:
> https://lore.kernel.org/io-uring/YsU%2FCGkl9ZXUI+Tj@stefanha-x1.localdomain/T/#m729963dc577d709b709c191922e98ec79d7eef54
> 
> The luring_resubmit_short_read() comment claimed they were only due to a
> specific io_uring bug that was fixed in Linux commit 9d93a3f5a0c
> ("io_uring: punt short reads to async context"), which is wrong.
> Dominique Martinet found that a btrfs bug also causes short reads. There
> may be more kernel code paths that result in short reads.
> 
> Let's consider short reads fair game.
> 
> Cc: Dominique Martinet <dominique.martinet@atmark-techno.com>
> Based-on: <20220630010137.2518851-1-dominique.martinet@atmark-techno.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  block/io_uring.c | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)

Thanks, applied to my block tree:
https://gitlab.com/stefanha/qemu/commits/block

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

end of thread, other threads:[~2022-07-07  8:22 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-07-06  8:03 [PATCH] block/io_uring: clarify that short reads can happen Stefan Hajnoczi
2022-07-07  6:49 ` Stefano Garzarella
2022-07-07  8:11 ` Stefan Hajnoczi

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).