From: Jeff Moyer <jmoyer@redhat.com>
To: Andrew Morton <akpm@linuxfoundation.org>
Cc: Josef Bacik <josef@redhat.com>,
linux-kernel@vger.kernel.org,
btrfs-devel <btrfs-devel@oss.oracle.com>
Subject: [patch] O_DIRECT: fix the splitting up of contiguous I/O
Date: Thu, 12 Aug 2010 16:50:59 -0400 [thread overview]
Message-ID: <x49vd7f368c.fsf@segfault.boston.devel.redhat.com> (raw)
Hi,
commit c2c6ca4 (direct-io: do not merge logically non-contiguous
requests) introduced a bug whereby all O_DIRECT I/Os were submitted a
page at a time to the block layer. The problem is that the code
expected dio->block_in_file to correspond to the current page in the
dio. In fact, it corresponds to the previous page submitted via
submit_page_section. This was purely an oversight, as the
dio->cur_page_fs_offset field was introduced for just this purpose.
This patch simply uses the correct variable when calculating whether
there is a mismatch between contiguous logical blocks and contiguous
physical blocks (as described in the comments).
I also switched the if conditional following this check to an else if,
to ensure that we never call dio_bio_submit twice for the same dio (in
theory, this should not happen, anyway).
I've tested this by running blktrace and verifying that a 64KB I/O was
submitted as a single I/O. I also ran the patched kernel through
xfstests' aio tests using xfs, ext4 (with 1k and 4k block sizes) and
btrfs and verified that there were no regressions as compared to an
unpatched kernel.
Comments, as always, are welcome.
Cheers,
Jeff
Signed-off-by: Jeff Moyer <jmoyer@redhat.com>
Acked-by: Josef Bacik <jbacik@redhat.com>
diff --git a/fs/direct-io.c b/fs/direct-io.c
index 7600aac..445901c 100644
--- a/fs/direct-io.c
+++ b/fs/direct-io.c
@@ -632,7 +632,7 @@ static int dio_send_cur_page(struct dio *dio)
int ret = 0;
if (dio->bio) {
- loff_t cur_offset = dio->block_in_file << dio->blkbits;
+ loff_t cur_offset = dio->cur_page_fs_offset;
loff_t bio_next_offset = dio->logical_offset_in_bio +
dio->bio->bi_size;
@@ -657,7 +657,7 @@ static int dio_send_cur_page(struct dio *dio)
* Submit now if the underlying fs is about to perform a
* metadata read
*/
- if (dio->boundary)
+ else if (dio->boundary)
dio_bio_submit(dio);
}
next reply other threads:[~2010-08-12 20:51 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-08-12 20:50 Jeff Moyer [this message]
2010-09-02 14:25 ` Christoph Hellwig
2010-09-05 12:56 ` [Btrfs-devel] " Chris Samuel
2010-09-05 16:36 ` Andrew Morton
2010-09-06 6:29 ` Chris Samuel
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=x49vd7f368c.fsf@segfault.boston.devel.redhat.com \
--to=jmoyer@redhat.com \
--cc=akpm@linuxfoundation.org \
--cc=btrfs-devel@oss.oracle.com \
--cc=josef@redhat.com \
--cc=linux-kernel@vger.kernel.org \
/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
Powered by JetHome