* [PATCH 0/2] Fix BUG_ON and hang in ext4_da_writepages() in presence of IO errors @ 2011-03-16 20:52 Jan Kara 2011-03-16 20:52 ` [PATCH 1/2] ext4: Fix BUG_ON in ext4_da_block_invalidatepages() Jan Kara 2011-03-16 20:52 ` [PATCH 2/2] ext4: Redirty page which could not be added to current extent in __mpage_da_writepage() Jan Kara 0 siblings, 2 replies; 5+ messages in thread From: Jan Kara @ 2011-03-16 20:52 UTC (permalink / raw) To: tytso; +Cc: linux-ext4 Hi Ted, I've noticed ext4 crashes when ext4_da_writepages() is confronted with IO errors. The two patches in this series fix the issues for me so now the system survives when the device is removed from under it. Could you please apply them? Thanks. Honza ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/2] ext4: Fix BUG_ON in ext4_da_block_invalidatepages() 2011-03-16 20:52 [PATCH 0/2] Fix BUG_ON and hang in ext4_da_writepages() in presence of IO errors Jan Kara @ 2011-03-16 20:52 ` Jan Kara 2011-03-16 21:02 ` Curt Wohlgemuth 2011-03-16 20:52 ` [PATCH 2/2] ext4: Redirty page which could not be added to current extent in __mpage_da_writepage() Jan Kara 1 sibling, 1 reply; 5+ messages in thread From: Jan Kara @ 2011-03-16 20:52 UTC (permalink / raw) To: tytso; +Cc: linux-ext4, Jan Kara 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 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] ext4: Fix BUG_ON in ext4_da_block_invalidatepages() 2011-03-16 20:52 ` [PATCH 1/2] ext4: Fix BUG_ON in ext4_da_block_invalidatepages() Jan Kara @ 2011-03-16 21:02 ` Curt Wohlgemuth 2011-03-16 21:11 ` Jan Kara 0 siblings, 1 reply; 5+ messages in thread From: Curt Wohlgemuth @ 2011-03-16 21:02 UTC (permalink / raw) To: Jan Kara; +Cc: tytso, linux-ext4 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] ext4: Fix BUG_ON in ext4_da_block_invalidatepages() 2011-03-16 21:02 ` Curt Wohlgemuth @ 2011-03-16 21:11 ` Jan Kara 0 siblings, 0 replies; 5+ messages in thread From: Jan Kara @ 2011-03-16 21:11 UTC (permalink / raw) To: Curt Wohlgemuth; +Cc: Jan Kara, tytso, linux-ext4 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 > > -- Jan Kara <jack@suse.cz> SUSE Labs, CR -- 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 2/2] ext4: Redirty page which could not be added to current extent in __mpage_da_writepage() 2011-03-16 20:52 [PATCH 0/2] Fix BUG_ON and hang in ext4_da_writepages() in presence of IO errors Jan Kara 2011-03-16 20:52 ` [PATCH 1/2] ext4: Fix BUG_ON in ext4_da_block_invalidatepages() Jan Kara @ 2011-03-16 20:52 ` Jan Kara 1 sibling, 0 replies; 5+ messages in thread From: Jan Kara @ 2011-03-16 20:52 UTC (permalink / raw) To: tytso; +Cc: linux-ext4, Jan Kara When a page cannot be added to current extent in __mpage_da_writepage() we map current extent and send it for IO. Currently, mpage_da_submit_io() also redirtied and unlocked this page but it's not clear whether this is just a lucky accident or a well hidden intent. Actually, we get this wrong in the case when ext4_map_blocks() fails because of EIO and thus mpage_da_submit_io() doesn't get called and the page is left locked leading to deadlocks. Fix the issue by explicitely redirtying and unlocking the page in __mpage_da_writepage() whenever we see the page could not be added to the current extent. Signed-off-by: Jan Kara <jack@suse.cz> --- fs/ext4/inode.c | 8 +++++--- 1 files changed, 5 insertions(+), 3 deletions(-) diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c index 337d9ca..aea9963 100644 --- a/fs/ext4/inode.c +++ b/fs/ext4/inode.c @@ -2465,6 +2465,7 @@ static int __mpage_da_writepage(struct page *page, */ if (mpd->next_page != mpd->first_page) { mpage_da_map_and_submit(mpd); +redirty_page: /* * skip rest of the page in the page_vec */ @@ -2477,6 +2478,7 @@ static int __mpage_da_writepage(struct page *page, * Start next extent of pages ... */ mpd->first_page = page->index; + mpd->next_page = page->index + 1; /* * ... and blocks @@ -2486,7 +2488,6 @@ static int __mpage_da_writepage(struct page *page, mpd->b_blocknr = 0; } - mpd->next_page = page->index + 1; logical = (sector_t) page->index << (PAGE_CACHE_SHIFT - inode->i_blkbits); @@ -2494,7 +2495,7 @@ static int __mpage_da_writepage(struct page *page, mpage_add_bh_to_extent(mpd, logical, PAGE_CACHE_SIZE, (1 << BH_Dirty) | (1 << BH_Uptodate)); if (mpd->io_done) - return MPAGE_DA_EXTENT_TAIL; + goto redirty_page; } else { /* * Page with regular buffer heads, just add all dirty ones @@ -2514,7 +2515,7 @@ static int __mpage_da_writepage(struct page *page, bh->b_size, bh->b_state); if (mpd->io_done) - return MPAGE_DA_EXTENT_TAIL; + goto redirty_page; } else if (buffer_dirty(bh) && (buffer_mapped(bh))) { /* * mapped dirty buffer. We need to update @@ -2530,6 +2531,7 @@ static int __mpage_da_writepage(struct page *page, logical++; } while ((bh = bh->b_this_page) != head); } + mpd->next_page = page->index + 1; return 0; } -- 1.7.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2011-03-16 21:11 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-03-16 20:52 [PATCH 0/2] Fix BUG_ON and hang in ext4_da_writepages() in presence of IO errors Jan Kara 2011-03-16 20:52 ` [PATCH 1/2] ext4: Fix BUG_ON in ext4_da_block_invalidatepages() Jan Kara 2011-03-16 21:02 ` Curt Wohlgemuth 2011-03-16 21:11 ` Jan Kara 2011-03-16 20:52 ` [PATCH 2/2] ext4: Redirty page which could not be added to current extent in __mpage_da_writepage() Jan Kara
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).