2011-02-18 21:31:53

by Curt Wohlgemuth

[permalink] [raw]
Subject: [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(-)

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



2011-02-18 21:31:53

by Curt Wohlgemuth

[permalink] [raw]
Subject: [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(-)

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


2011-02-26 18:54:10

by Theodore Ts'o

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

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

Thanks, added to the ext4 patch queue.

- Ted

2011-02-26 18:54:21

by Theodore Ts'o

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

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

Thanks, added to the ext4 patch queue.

- Ted