[PATCH] direct-io: Submit bio after boundary buffer is added to it

[PATCH] direct-io: Submit bio after boundary buffer is added to it

[Jan Kara]

Currently, dio_send_cur_page() submits bio before current page and
cached sdio->cur_page is added to the bio if sdio->boundary is set. This
is actually wrong because sdio->boundary means the current buffer is the
last one before metadata needs to be read. So we should rather submit
the bio after the current page is added to it.

Reported-by: Kazuya Mio <k-mio@sx.jp.nec.com>
Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/direct-io.c |   28 +++++++++++-----------------
 1 files changed, 11 insertions(+), 17 deletions(-)

 Andrew, can you add this patch the previous direct IO boundary handling fix?

diff --git a/fs/direct-io.c b/fs/direct-io.c
index e666854..c57add7 100644
--- a/fs/direct-io.c
+++ b/fs/direct-io.c
@@ -672,12 +672,6 @@ static inline int dio_send_cur_page(struct dio *dio, struct dio_submit *sdio,
 		if (sdio->final_block_in_bio != sdio->cur_page_block ||
 		    cur_offset != bio_next_offset)
 			dio_bio_submit(dio, sdio);
-		/*
-		 * Submit now if the underlying fs is about to perform a
-		 * metadata read
-		 */
-		else if (sdio->boundary)
-			dio_bio_submit(dio, sdio);
 	}
 
 	if (sdio->bio == NULL) {
@@ -737,16 +731,6 @@ submit_page_section(struct dio *dio, struct dio_submit *sdio, struct page *page,
 	    sdio->cur_page_block +
 	    (sdio->cur_page_len >> sdio->blkbits) == blocknr) {
 		sdio->cur_page_len += len;
-
-		/*
-		 * If sdio->boundary then we want to schedule the IO now to
-		 * avoid metadata seeks.
-		 */
-		if (sdio->boundary) {
-			ret = dio_send_cur_page(dio, sdio, map_bh);
-			page_cache_release(sdio->cur_page);
-			sdio->cur_page = NULL;
-		}
 		goto out;
 	}
 
@@ -758,7 +742,7 @@ submit_page_section(struct dio *dio, struct dio_submit *sdio, struct page *page,
 		page_cache_release(sdio->cur_page);
 		sdio->cur_page = NULL;
 		if (ret)
-			goto out;
+			return ret;
 	}
 
 	page_cache_get(page);		/* It is in dio */
@@ -768,6 +752,16 @@ submit_page_section(struct dio *dio, struct dio_submit *sdio, struct page *page,
 	sdio->cur_page_block = blocknr;
 	sdio->cur_page_fs_offset = sdio->block_in_file << sdio->blkbits;
 out:
+	/*
+	 * If sdio->boundary then we want to schedule the IO now to
+	 * avoid metadata seeks.
+	 */
+	if (sdio->boundary) {
+		ret = dio_send_cur_page(dio, sdio, map_bh);
+		dio_bio_submit(dio, sdio);
+		page_cache_release(sdio->cur_page);
+		sdio->cur_page = NULL;
+	}
 	return ret;
 }
 
-- 
1.7.1

Re: [PATCH] direct-io: Submit bio after boundary buffer is added to it

[Andrew Morton]

On Tue,  9 Apr 2013 17:42:49 +0200 Jan Kara <jack@suse.cz> wrote:

> Currently, dio_send_cur_page() submits bio before current page and
> cached sdio->cur_page is added to the bio if sdio->boundary is set. This
> is actually wrong because sdio->boundary means the current buffer is the
> last one before metadata needs to be read. So we should rather submit
> the bio after the current page is added to it.
> 
> Reported-by: Kazuya Mio <k-mio@sx.jp.nec.com>
> Signed-off-by: Jan Kara <jack@suse.cz>
> ---
>  fs/direct-io.c |   28 +++++++++++-----------------
>  1 files changed, 11 insertions(+), 17 deletions(-)
> 
>  Andrew, can you add this patch the previous direct IO boundary handling fix?

What is the user-visible impact of these two patches?

Re: [PATCH] direct-io: Submit bio after boundary buffer is added to it

[Jan Kara]

On Tue 09-04-13 13:47:58, Andrew Morton wrote:
> On Tue,  9 Apr 2013 17:42:49 +0200 Jan Kara <jack@suse.cz> wrote:
> 
> > Currently, dio_send_cur_page() submits bio before current page and
> > cached sdio->cur_page is added to the bio if sdio->boundary is set. This
> > is actually wrong because sdio->boundary means the current buffer is the
> > last one before metadata needs to be read. So we should rather submit
> > the bio after the current page is added to it.
> > 
> > Reported-by: Kazuya Mio <k-mio@sx.jp.nec.com>
> > Signed-off-by: Jan Kara <jack@suse.cz>
> > ---
> >  fs/direct-io.c |   28 +++++++++++-----------------
> >  1 files changed, 11 insertions(+), 17 deletions(-)
> > 
> >  Andrew, can you add this patch the previous direct IO boundary handling fix?
> 
> What is the user-visible impact of these two patches?
  I've measured about 10% throughput improvement of direct IO reads on ext3
with SATA harddrive (from 90 MB/s to 100 MB/s) with these patches applied.
With ramdisk, the improvement was about 3-fold (from 350 MB/s to 1.2 GB/s).
For other filesystems (such as ext4), the improvements won't be as visible
because the frequency of BH_Boundary flag being set is much smaller...

								Honza
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR