ext4: mark multi-page IO complete on mapping failure

PATCH In review Curt Wohlgemuth 2 participants 4 messages Feb 18, 2011 – Feb 26, 2011 ext4 file system

Share & tools

Curt Wohlgemuth

author

2011-02-18 21:31 UTC #0 lore raw

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

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 <[email protected]>

---
 fs/ext4/inode.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

fs/ext4/inode.c +3 -0

index 9f7f9e4..1cff229 100644
@@ -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);

Signature · 1 line

1.7.3.1

Curt Wohlgemuth

author

2011-02-18 21:31 UTC #1 lore raw

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

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 <[email protected]>

---
 fs/ext4/inode.c |   11 ++++-------
 1 files changed, 4 insertions(+), 7 deletions(-)

fs/ext4/inode.c +4 -7

index 1cff229..18a3b90 100644
@@ -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;

Signature · 1 line

1.7.3.1

Ted Ts'o

maintainer

2011-02-26 18:54 UTC #2 lore raw

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

Quoted 13 lines · 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.

Quoted 7 lines

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 <[email protected]>

Thanks, added to the ext4 patch queue.

- Ted

Ted Ts'o

maintainer

2011-02-26 18:54 UTC #3 lore raw

In reply to Curt Wohlgemuth

Quoted 11 lines · 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().

Quoted 5 lines

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 <[email protected]>

Thanks, added to the ext4 patch queue.

- Ted