Patchwork [1/2] ext4: Fix BUG_ON in ext4_da_block_invalidatepages()

login
register
mail settings
Submitter Jan Kara
Date March 16, 2011, 8:52 p.m.
Message ID <1300308774-14140-2-git-send-email-jack@suse.cz>
Download mbox | patch
Permalink /patch/87303/
State Rejected
Headers show

Comments

Jan Kara - March 16, 2011, 8:52 p.m.
When an allocation of blocks fails in mpage_da_map_and_submit() e.g. because of
EIO we call ext4_da_block_invalidatepages() to invalidate pages we cannot write
but we fail to set mpd->io_done. Thus we continue searching for dirty pages and
add them to the current extent in mpd structure. Eventually we try to allocate
blocks for the extent again and strange things start happening. In particular
if the allocation fails again, we try to invalidate pages again and that
triggers BUG_ON in ext4_da_block_invalidatepages().

Fix the issue by setting mpd->io_done after the pages are invalidated.

Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/ext4/inode.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
Curt Wohlgemuth - March 16, 2011, 9:02 p.m.
Hi Jan:

This problem was fixed by the patch I sent on 18 Feb, titled

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

I guess this didn't make it into 2.6.38, but it's in the ext4 patch queue now.

Thanks,
Curt

On Wed, Mar 16, 2011 at 1:52 PM, Jan Kara <jack@suse.cz> wrote:
> When an allocation of blocks fails in mpage_da_map_and_submit() e.g. because of
> EIO we call ext4_da_block_invalidatepages() to invalidate pages we cannot write
> but we fail to set mpd->io_done. Thus we continue searching for dirty pages and
> add them to the current extent in mpd structure. Eventually we try to allocate
> blocks for the extent again and strange things start happening. In particular
> if the allocation fails again, we try to invalidate pages again and that
> triggers BUG_ON in ext4_da_block_invalidatepages().
>
> Fix the issue by setting mpd->io_done after the pages are invalidated.
>
> Signed-off-by: Jan Kara <jack@suse.cz>
> ---
>  fs/ext4/inode.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 9f7f9e4..337d9ca 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -2314,6 +2314,7 @@ 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);
> +               mpd->io_done = 1;
>                return;
>        }
>        BUG_ON(blks == 0);
> --
> 1.7.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jan Kara - March 16, 2011, 9:11 p.m.
Hi Curt,

On Wed 16-03-11 14:02:02, Curt Wohlgemuth wrote:
> This problem was fixed by the patch I sent on 18 Feb, titled
> 
>    [PATCH 1/2] ext4: mark multi-page IO complete on mapping failure
> 
> I guess this didn't make it into 2.6.38, but it's in the ext4 patch queue now.
  Ah, OK. And I see you also fixed the second issue although my patch IMHO
still makes sense as a cleanup...

								Honza

> On Wed, Mar 16, 2011 at 1:52 PM, Jan Kara <jack@suse.cz> wrote:
> > When an allocation of blocks fails in mpage_da_map_and_submit() e.g. because of
> > EIO we call ext4_da_block_invalidatepages() to invalidate pages we cannot write
> > but we fail to set mpd->io_done. Thus we continue searching for dirty pages and
> > add them to the current extent in mpd structure. Eventually we try to allocate
> > blocks for the extent again and strange things start happening. In particular
> > if the allocation fails again, we try to invalidate pages again and that
> > triggers BUG_ON in ext4_da_block_invalidatepages().
> >
> > Fix the issue by setting mpd->io_done after the pages are invalidated.
> >
> > Signed-off-by: Jan Kara <jack@suse.cz>
> > ---
> >  fs/ext4/inode.c |    1 +
> >  1 files changed, 1 insertions(+), 0 deletions(-)
> >
> > diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> > index 9f7f9e4..337d9ca 100644
> > --- a/fs/ext4/inode.c
> > +++ b/fs/ext4/inode.c
> > @@ -2314,6 +2314,7 @@ 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);
> > +               mpd->io_done = 1;
> >                return;
> >        }
> >        BUG_ON(blks == 0);
> > --
> > 1.7.1
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >

Patch

diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 9f7f9e4..337d9ca 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -2314,6 +2314,7 @@  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);
+		mpd->io_done = 1;
 		return;
 	}
 	BUG_ON(blks == 0);