[PATCH 1/2] ext4: mark multi-page IO complete on mapping failure

[PATCH 1/2] ext4: mark multi-page IO complete on mapping failure

[Curt Wohlgemuth]

In mpage_da_map_and_submit(), if we have a delayed block
allocation failure from ext4_map_blocks(), we need to mark
the IO as complete, by setting

      mpd->io_done = 1;

Otherwise, we could end up submitting the pages in an outer
loop; since they are unlocked on mapping failure in
ext4_da_block_invalidatepages(), this will cause a bug check
in mpage_da_submit_io().

I tested this by injected failures into ext4_map_blocks().
Without this patch, a simple fsstress run will bug check;
with the patch, it works fine.

Signed-off-by: Curt Wohlgemuth <curtw@google.com>
---
 fs/ext4/inode.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 9f7f9e4..1cff229 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -2314,6 +2314,9 @@ static void mpage_da_map_and_submit(struct mpage_da_data *mpd)
 		/* invalidate all the pages */
 		ext4_da_block_invalidatepages(mpd, next,
 				mpd->b_size >> mpd->inode->i_blkbits);
+
+		/* Mark this page range as having been completed */
+		mpd->io_done = 1;
 		return;
 	}
 	BUG_ON(blks == 0);
-- 
1.7.3.1

[PATCH 2/2] ext4: fix ext4_da_block_invalidatepages() to handle page range properly

[Curt Wohlgemuth]

If ext4_da_block_invalidatepages() is called because of a
failure from ext4_map_blocks() in mpage_da_map_and_submit(),
it's supposed to clean up -- including unlock -- all the
pages in the mpd structure.  But these values may not match
up, even on a system in which block size == page size:

   mpd->b_blocknr != mpd->first_page
   mpd->b_size != (mpd->next_page - mpd->first_page)

ext4_da_block_invalidatepages() has been using b_blocknr and
b_size; this patch changes it to use first_page and
next_page.

Tested:  I injected a small number (5%) of failures in
ext4_map_blocks() in the case that the flags contain
EXT4_GET_BLOCKS_DELALLOC_RESERVE, and ran fsstress on this
kernel.  Without this patch, I got hung tasks every time.
With this patch, I see no hangs in many runs of fsstress.

Signed-off-by: Curt Wohlgemuth <curtw@google.com>
---
 fs/ext4/inode.c |   11 ++++-------
 1 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 1cff229..18a3b90 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -2165,8 +2165,7 @@ static int mpage_da_submit_io(struct mpage_da_data *mpd,
 	return ret;
 }
 
-static void ext4_da_block_invalidatepages(struct mpage_da_data *mpd,
-					sector_t logical, long blk_cnt)
+static void ext4_da_block_invalidatepages(struct mpage_da_data *mpd)
 {
 	int nr_pages, i;
 	pgoff_t index, end;
@@ -2174,9 +2173,8 @@ static void ext4_da_block_invalidatepages(struct mpage_da_data *mpd,
 	struct inode *inode = mpd->inode;
 	struct address_space *mapping = inode->i_mapping;
 
-	index = logical >> (PAGE_CACHE_SHIFT - inode->i_blkbits);
-	end   = (logical + blk_cnt - 1) >>
-				(PAGE_CACHE_SHIFT - inode->i_blkbits);
+	index = mpd->first_page;
+	end   = mpd->next_page - 1;
 	while (index <= end) {
 		nr_pages = pagevec_lookup(&pvec, mapping, index, PAGEVEC_SIZE);
 		if (nr_pages == 0)
@@ -2312,8 +2310,7 @@ static void mpage_da_map_and_submit(struct mpage_da_data *mpd)
 				ext4_print_free_blocks(mpd->inode);
 		}
 		/* invalidate all the pages */
-		ext4_da_block_invalidatepages(mpd, next,
-				mpd->b_size >> mpd->inode->i_blkbits);
+		ext4_da_block_invalidatepages(mpd);
 
 		/* Mark this page range as having been completed */
 		mpd->io_done = 1;
-- 
1.7.3.1

Re: [PATCH 2/2] ext4: fix ext4_da_block_invalidatepages() to handle page range properly

[Ted Ts'o]

On Fri, Feb 18, 2011 at 01:31:39PM -0800, Curt Wohlgemuth wrote:
> If ext4_da_block_invalidatepages() is called because of a
> failure from ext4_map_blocks() in mpage_da_map_and_submit(),
> it's supposed to clean up -- including unlock -- all the
> pages in the mpd structure.  But these values may not match
> up, even on a system in which block size == page size:
> 
>    mpd->b_blocknr != mpd->first_page
>    mpd->b_size != (mpd->next_page - mpd->first_page)
> 
> ext4_da_block_invalidatepages() has been using b_blocknr and
> b_size; this patch changes it to use first_page and
> next_page.
> 
> Tested:  I injected a small number (5%) of failures in
> ext4_map_blocks() in the case that the flags contain
> EXT4_GET_BLOCKS_DELALLOC_RESERVE, and ran fsstress on this
> kernel.  Without this patch, I got hung tasks every time.
> With this patch, I see no hangs in many runs of fsstress.
> 
> Signed-off-by: Curt Wohlgemuth <curtw@google.com>

Thanks, added to the ext4 patch queue.

						- Ted

Re: [PATCH 1/2] ext4: mark multi-page IO complete on mapping failure

[Ted Ts'o]

On Fri, Feb 18, 2011 at 01:31:38PM -0800, Curt Wohlgemuth wrote:
> In mpage_da_map_and_submit(), if we have a delayed block
> allocation failure from ext4_map_blocks(), we need to mark
> the IO as complete, by setting
> 
>       mpd->io_done = 1;
> 
> Otherwise, we could end up submitting the pages in an outer
> loop; since they are unlocked on mapping failure in
> ext4_da_block_invalidatepages(), this will cause a bug check
> in mpage_da_submit_io().
> 
> I tested this by injected failures into ext4_map_blocks().
> Without this patch, a simple fsstress run will bug check;
> with the patch, it works fine.
> 
> Signed-off-by: Curt Wohlgemuth <curtw@google.com>

Thanks, added to the ext4 patch queue.

					- Ted