From: Milan Broz <gmazyland@gmail.com>
To: Jens Axboe <axboe@kernel.dk>
Cc: linux-block <linux-block@vger.kernel.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
Ondrej Kozina <okozina@redhat.com>
Subject: Partial direct-io loop regression in 5.17-rc
Date: Fri, 4 Feb 2022 10:22:06 +0100 [thread overview]
Message-ID: <feb7e4b4-1a6f-71a7-0cdd-fda547408bea@gmail.com> (raw)
Hi Jens,
It seems that there is a regression in direct-io over loop for partial
direct-io reads (or perhaps even for other situations).
If I run this code (loop over 6M file, dd direct-io read with 4M blocks)
IMG=tst.img
LOOP=/dev/loop66
truncate -s 6M $IMG
losetup $LOOP $IMG
dd if=$LOOP of=/dev/null bs=4M iflag=direct
losetup -d $LOOP
on older kernel (<=5.16) it reads the whole file
6291456 bytes (6.3 MB, 6.0 MiB) copied, 0.201591 s, 31.2 MB/s
while on 5.17-rc (tested on today/s Linus' git) it reads only the full blocks:
4194304 bytes (4.2 MB, 4.0 MiB) copied, 0.201904 s, 20.8 MB/s
No error reported, exit code is 0.
I am not sure if the reproducer is intended behavior (there was
some discussion, though), but it is a major regression IMO.
If you do not want to support partial direct-io, it should return
an error then and not EOF (the last read returns 0)!
It was detected in our cryptsetup testsuite and we always use
direct aligned IO, but for the reported device sector size only (like 4k).
But here it breaks dd without any chance to detect error.
Bisect ends on your patch (and clean revert indeed fixes the issue):
ceaa762527f41a431b552bc000de4b626d2d8cb7 is the first bad commit
commit ceaa762527f41a431b552bc000de4b626d2d8cb7
Author: Jens Axboe <axboe@kernel.dk>
Date: Thu Oct 28 08:57:09 2021 -0600
block: move direct_IO into our own read_iter handler
Don't call into generic_file_read_iter() if we know it's O_DIRECT, just
set it up ourselves and call our own handler. This avoids an indirect call
for O_DIRECT.
Fall back to filemap_read() if we fail.
Signed-off-by: Jens Axboe <axboe@kernel.dk>
block/fops.c | 37 ++++++++++++++++++++++++++++++++-----
1 file changed, 32 insertions(+), 5 deletions(-)
Please could you check what's wrong here?
Thanks,
Milan
next reply other threads:[~2022-02-04 9:22 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-02-04 9:22 Milan Broz [this message]
2022-02-04 13:32 ` Jens Axboe
2022-02-04 15:03 ` Milan Broz
2022-02-04 15:33 ` Jens Axboe
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=feb7e4b4-1a6f-71a7-0cdd-fda547408bea@gmail.com \
--to=gmazyland@gmail.com \
--cc=axboe@kernel.dk \
--cc=linux-block@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=okozina@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
all inboxes | Powered by JetHome®