qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PULL 0/2] Block patches
@ 2022-07-07  8:12 Stefan Hajnoczi
  2022-07-07  8:12 ` [PULL 1/2] io_uring: fix short read slow path Stefan Hajnoczi
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Stefan Hajnoczi @ 2022-07-07  8:12 UTC (permalink / raw)
  To: qemu-devel
  Cc: Julia Suvorova, qemu-block, Stefano Garzarella, Hanna Reitz,
	Kevin Wolf, Aarushi Mehta, Stefan Hajnoczi, Richard Henderson

The following changes since commit 8e9398e3b1a860b8c29c670c1b6c36afe8d87849:

  Merge tag 'pull-ppc-20220706' of https://gitlab.com/danielhb/qemu into staging (2022-07-07 06:21:05 +0530)

are available in the Git repository at:

  https://gitlab.com/stefanha/qemu.git tags/block-pull-request

for you to fetch changes up to be6a166fde652589761cf70471bcde623e9bd72a:

  block/io_uring: clarify that short reads can happen (2022-07-07 09:04:15 +0100)

----------------------------------------------------------------
Pull request

----------------------------------------------------------------

Dominique Martinet (1):
  io_uring: fix short read slow path

Stefan Hajnoczi (1):
  block/io_uring: clarify that short reads can happen

 block/io_uring.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

-- 
2.36.1



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

* [PULL 1/2] io_uring: fix short read slow path
  2022-07-07  8:12 [PULL 0/2] Block patches Stefan Hajnoczi
@ 2022-07-07  8:12 ` Stefan Hajnoczi
  2022-07-07  8:12 ` [PULL 2/2] block/io_uring: clarify that short reads can happen Stefan Hajnoczi
  2022-07-07 10:51 ` [PULL 0/2] Block patches Richard Henderson
  2 siblings, 0 replies; 4+ messages in thread
From: Stefan Hajnoczi @ 2022-07-07  8:12 UTC (permalink / raw)
  To: qemu-devel
  Cc: Julia Suvorova, qemu-block, Stefano Garzarella, Hanna Reitz,
	Kevin Wolf, Aarushi Mehta, Stefan Hajnoczi, Richard Henderson,
	Dominique Martinet

From: Dominique Martinet <dominique.martinet@atmark-techno.com>

sqeq.off here is the offset to read within the disk image, so obviously
not 'nread' (the amount we just read), but as the author meant to write
its current value incremented by the amount we just read.

Normally recent versions of linux will not issue short reads,
but it can happen so we should fix this.

This lead to weird image corruptions when short read happened

Fixes: 6663a0a33764 ("block/io_uring: implements interfaces for io_uring")
Link: https://lkml.kernel.org/r/YrrFGO4A1jS0GI0G@atmark-techno.com
Signed-off-by: Dominique Martinet <dominique.martinet@atmark-techno.com>
Message-Id: <20220630010137.2518851-1-dominique.martinet@atmark-techno.com>
Reviewed-by: Hanna Reitz <hreitz@redhat.com>
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/io_uring.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/block/io_uring.c b/block/io_uring.c
index d48e472e74..b238661740 100644
--- a/block/io_uring.c
+++ b/block/io_uring.c
@@ -89,7 +89,7 @@ static void luring_resubmit_short_read(LuringState *s, LuringAIOCB *luringcb,
     trace_luring_resubmit_short_read(s, luringcb, nread);
 
     /* Update read position */
-    luringcb->total_read = nread;
+    luringcb->total_read += nread;
     remaining = luringcb->qiov->size - luringcb->total_read;
 
     /* Shorten qiov */
@@ -103,7 +103,7 @@ static void luring_resubmit_short_read(LuringState *s, LuringAIOCB *luringcb,
                       remaining);
 
     /* Update sqe */
-    luringcb->sqeq.off = nread;
+    luringcb->sqeq.off += nread;
     luringcb->sqeq.addr = (__u64)(uintptr_t)luringcb->resubmit_qiov.iov;
     luringcb->sqeq.len = luringcb->resubmit_qiov.niov;
 
-- 
2.36.1



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

* [PULL 2/2] block/io_uring: clarify that short reads can happen
  2022-07-07  8:12 [PULL 0/2] Block patches Stefan Hajnoczi
  2022-07-07  8:12 ` [PULL 1/2] io_uring: fix short read slow path Stefan Hajnoczi
@ 2022-07-07  8:12 ` Stefan Hajnoczi
  2022-07-07 10:51 ` [PULL 0/2] Block patches Richard Henderson
  2 siblings, 0 replies; 4+ messages in thread
From: Stefan Hajnoczi @ 2022-07-07  8:12 UTC (permalink / raw)
  To: qemu-devel
  Cc: Julia Suvorova, qemu-block, Stefano Garzarella, Hanna Reitz,
	Kevin Wolf, Aarushi Mehta, Stefan Hajnoczi, Richard Henderson,
	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>
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
Message-id: 20220706080341.1206476-1-stefanha@redhat.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] 4+ messages in thread

* Re: [PULL 0/2] Block patches
  2022-07-07  8:12 [PULL 0/2] Block patches Stefan Hajnoczi
  2022-07-07  8:12 ` [PULL 1/2] io_uring: fix short read slow path Stefan Hajnoczi
  2022-07-07  8:12 ` [PULL 2/2] block/io_uring: clarify that short reads can happen Stefan Hajnoczi
@ 2022-07-07 10:51 ` Richard Henderson
  2 siblings, 0 replies; 4+ messages in thread
From: Richard Henderson @ 2022-07-07 10:51 UTC (permalink / raw)
  To: Stefan Hajnoczi, qemu-devel
  Cc: Julia Suvorova, qemu-block, Stefano Garzarella, Hanna Reitz,
	Kevin Wolf, Aarushi Mehta

On 7/7/22 13:42, Stefan Hajnoczi wrote:
> The following changes since commit 8e9398e3b1a860b8c29c670c1b6c36afe8d87849:
> 
>    Merge tag 'pull-ppc-20220706' of https://gitlab.com/danielhb/qemu into staging (2022-07-07 06:21:05 +0530)
> 
> are available in the Git repository at:
> 
>    https://gitlab.com/stefanha/qemu.git tags/block-pull-request
> 
> for you to fetch changes up to be6a166fde652589761cf70471bcde623e9bd72a:
> 
>    block/io_uring: clarify that short reads can happen (2022-07-07 09:04:15 +0100)
> 
> ----------------------------------------------------------------
> Pull request

Applied, thanks.  Please update https://wiki.qemu.org/ChangeLog/7.1 as appropriate.


r~


> 
> ----------------------------------------------------------------
> 
> Dominique Martinet (1):
>    io_uring: fix short read slow path
> 
> Stefan Hajnoczi (1):
>    block/io_uring: clarify that short reads can happen
> 
>   block/io_uring.c | 12 ++++--------
>   1 file changed, 4 insertions(+), 8 deletions(-)
> 



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

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

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-07-07  8:12 [PULL 0/2] Block patches Stefan Hajnoczi
2022-07-07  8:12 ` [PULL 1/2] io_uring: fix short read slow path Stefan Hajnoczi
2022-07-07  8:12 ` [PULL 2/2] block/io_uring: clarify that short reads can happen Stefan Hajnoczi
2022-07-07 10:51 ` [PULL 0/2] Block patches Richard Henderson

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