qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Jens Axboe <axboe@kernel.dk>
To: Stefan Hajnoczi <stefanha@redhat.com>,
	Dominique Martinet <dominique.martinet@atmark-techno.com>
Cc: Stefano Garzarella <sgarzare@redhat.com>,
	Aarushi Mehta <mehta.aaru20@gmail.com>,
	Julia Suvorova <jusual@redhat.com>, Kevin Wolf <kwolf@redhat.com>,
	Hanna Reitz <hreitz@redhat.com>,
	qemu-block@nongnu.org, qemu-devel@nongnu.org,
	Filipe Manana <fdmanana@kernel.org>,
	io-uring@vger.kernel.org
Subject: Re: [PATCH v2] io_uring: fix short read slow path
Date: Tue, 5 Jul 2022 13:23:48 -0600	[thread overview]
Message-ID: <e9bbbeb5-c6b9-8d19-9593-b2c9187a5d98@kernel.dk> (raw)
In-Reply-To: <YsQ8aM3/ZT+Bs7nC@stefanha-x1.localdomain>

On 7/5/22 7:28 AM, Stefan Hajnoczi wrote:
> On Fri, Jul 01, 2022 at 07:52:31AM +0900, Dominique Martinet wrote:
>> Stefano Garzarella wrote on Thu, Jun 30, 2022 at 05:49:21PM +0200:
>>>> so when we ask for more we issue an extra short reads, making sure we go
>>>> through the two short reads path.
>>>> (Unfortunately I wasn't quite sure what to fiddle with to issue short
>>>> reads in the first place, I tried cutting one of the iovs short in
>>>> luring_do_submit() but I must not have been doing it properly as I ended
>>>> up with 0 return values which are handled by filling in with 0 (reads
>>>> after eof) and that didn't work well)
>>>
>>> Do you remember the kernel version where you first saw these problems?
>>
>> Since you're quoting my paragraph about testing two short reads, I've
>> never seen any that I know of; but there's also no reason these couldn't
>> happen.
>>
>> Single short reads have been happening for me with O_DIRECT (cache=none)
>> on btrfs for a while, but unfortunately I cannot remember which was the
>> first kernel I've seen this on -- I think rather than a kernel update it
>> was due to file manipulations that made the file eligible for short
>> reads in the first place (I started running deduplication on the backing
>> file)
>>
>> The older kernel I have installed right now is 5.16 and that can
>> reproduce it --  I'll give my laptop some work over the weekend to test
>> still maintained stable branches if that's useful.
> 
> Hi Dominique,
> Linux 5.16 contains commit 9d93a3f5a0c ("io_uring: punt short reads to
> async context"). The comment above QEMU's luring_resubmit_short_read()
> claims that short reads are a bug that was fixed by Linux commit
> 9d93a3f5a0c.
> 
> If the comment is inaccurate it needs to be fixed. Maybe short writes
> need to be handled too.
> 
> I have CCed Jens and the io_uring mailing list to clarify:
> 1. Are short IORING_OP_READV reads possible on files/block devices?
> 2. Are short IORING_OP_WRITEV writes possible on files/block devices?

In general we try very hard to avoid them, but if eg we get a short read
or write from blocking context (eg io-wq), then io_uring does return
that. There's really not much we can do here, it seems futile to retry
IO which was issued just like it would've been from a normal blocking
syscall yet it is still short.

-- 
Jens Axboe



  reply	other threads:[~2022-07-05 19:27 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-29  4:49 [PATCH] io_uring: fix short read slow path corruptions Dominique Martinet
2022-06-29  5:23 ` Dominique Martinet
2022-06-29  8:46   ` Kevin Wolf
2022-06-29 10:22     ` Dominique Martinet
2022-06-30  1:01 ` [PATCH v2] io_uring: fix short read slow path Dominique Martinet
2022-06-30 15:43   ` Hanna Reitz
2022-06-30 15:49   ` Stefano Garzarella
2022-06-30 22:52     ` Dominique Martinet
2022-07-01  1:33       ` Dominique Martinet
2022-07-05 13:28       ` Stefan Hajnoczi
2022-07-05 19:23         ` Jens Axboe [this message]
2022-07-06  7:16           ` Stefan Hajnoczi
2022-07-05 22:52         ` Dominique Martinet
2022-07-06  7:17           ` Stefan Hajnoczi
2022-07-06  7:26             ` Dominique Martinet
2022-07-06  7:51               ` Stefan Hajnoczi
2022-07-05 13:34   ` Stefan Hajnoczi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=e9bbbeb5-c6b9-8d19-9593-b2c9187a5d98@kernel.dk \
    --to=axboe@kernel.dk \
    --cc=dominique.martinet@atmark-techno.com \
    --cc=fdmanana@kernel.org \
    --cc=hreitz@redhat.com \
    --cc=io-uring@vger.kernel.org \
    --cc=jusual@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=mehta.aaru20@gmail.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=sgarzare@redhat.com \
    --cc=stefanha@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).