On Thu 21-03-13 17:43:52, Kazuya Mio wrote:
> 2013/03/20 4:31, Jan Kara wrote:
> > I'm not sure I understand. Looking into dio_send_cur_page() it seems may
> >prematurely submit the bio if sdio->boundary is set - in that case we
> >should probably first try to add the page to the bio and submit the bio
> >only after that. Is that what you mean?
>
> I think the direct I/O works for each page into buffer_head by the following
> three steps:
> 1. submit sdio->bio if sdio->boudary is set
> 2. add sdio->cur_page to sdio->bio by dio_new_bio() or dio_bio_add_page()
> 3. set the curret page to sdio->cur_page in submit_page_section()
>
> It is true that dio_send_cur_page() submits the bio if sdio->boudary is set.
> However, at this time, this bio does not contain sdio->cur_page and
> the current page do_direct_IO() passed.
Sorry for not getting to you earlier. So we agree in our analysis. Do you
agree with the attached patch? Does it help your workload?
Honza
--
Jan Kara <jack@xxxxxxx>
SUSE Labs, CR
>From d9ef6ae45c80b298f7f3b718a101956071709b02 Mon Sep 17 00:00:00 2001
From: Jan Kara <jack@xxxxxxx>
Date: Fri, 29 Mar 2013 18:05:01 +0100
Subject: [PATCH] direct-io: Submit bio after boundary buffer is added to it
Currently, dio_send_cur_page() submits bio before current page
(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@xxxxxxxxxxxxx>
Signed-off-by: Jan Kara <jack@xxxxxxx>
---
fs/direct-io.c | 12 ++++++------
1 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/fs/direct-io.c b/fs/direct-io.c
index e666854..2ccde31 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) {
@@ -694,6 +688,12 @@ static inline int dio_send_cur_page(struct dio *dio, struct dio_submit *sdio,
BUG_ON(ret != 0);
}
}
+ /*
+ * Submit now if the underlying fs is about to perform a
+ * metadata read
+ */
+ if (sdio->boundary)
+ dio_bio_submit(dio, sdio);
out:
return ret;
}
--
1.7.1
