Skip to content

Commit

Permalink
ext3: Fix deadlock in data=journal mode when fs is frozen
Browse files Browse the repository at this point in the history
When ext3 is used in data=journal mode, syncing filesystem makes sure
all the data is committed in the journal but the data doesn't have to be
checkpointed. ext3_freeze() then takes care of checkpointing all the
data so all buffer heads are clean but pages can still have dangling
dirty bits. So when flusher thread comes later when filesystem is
frozen, it tries to write back dirty pages, ext3_journalled_writepage()
tries to start a transaction and hangs waiting for frozen fs causing a
deadlock because a holder of s_umount semaphore may be waiting for
flusher thread to complete.

The fix is luckily relatively easy. We don't have to start a transaction
in ext3_journalled_writepage() when a page is just dirty (and doesn't
have PageChecked set) because in that case all buffers should be already
mapped (mapping must happen before writing a buffer to the journal) and
it is enough to write them out. This optimization also solves the deadlock
because block_write_full_page() will just find out there's no buffer to
write and do nothing.

Signed-off-by: Jan Kara <jack@suse.cz>
  • Loading branch information
Jan Kara committed May 22, 2014
1 parent 22e7478 commit 166418c
Showing 1 changed file with 17 additions and 16 deletions.
33 changes: 17 additions & 16 deletions fs/ext3/inode.c
Original file line number Diff line number Diff line change
Expand Up @@ -1716,17 +1716,17 @@ static int ext3_journalled_writepage(struct page *page,
WARN_ON_ONCE(IS_RDONLY(inode) &&
!(EXT3_SB(inode->i_sb)->s_mount_state & EXT3_ERROR_FS));

if (ext3_journal_current_handle())
goto no_write;

trace_ext3_journalled_writepage(page);
handle = ext3_journal_start(inode, ext3_writepage_trans_blocks(inode));
if (IS_ERR(handle)) {
ret = PTR_ERR(handle);
goto no_write;
}

if (!page_has_buffers(page) || PageChecked(page)) {
if (ext3_journal_current_handle())
goto no_write;

handle = ext3_journal_start(inode,
ext3_writepage_trans_blocks(inode));
if (IS_ERR(handle)) {
ret = PTR_ERR(handle);
goto no_write;
}
/*
* It's mmapped pagecache. Add buffers and journal it. There
* doesn't seem much point in redirtying the page here.
Expand All @@ -1749,17 +1749,18 @@ static int ext3_journalled_writepage(struct page *page,
atomic_set(&EXT3_I(inode)->i_datasync_tid,
handle->h_transaction->t_tid);
unlock_page(page);
err = ext3_journal_stop(handle);
if (!ret)
ret = err;
} else {
/*
* It may be a page full of checkpoint-mode buffers. We don't
* really know unless we go poke around in the buffer_heads.
* But block_write_full_page will do the right thing.
* It is a page full of checkpoint-mode buffers. Go and write
* them. They should have been already mapped when they went
* to the journal so provide NULL get_block function to catch
* errors.
*/
ret = block_write_full_page(page, ext3_get_block, wbc);
ret = block_write_full_page(page, NULL, wbc);
}
err = ext3_journal_stop(handle);
if (!ret)
ret = err;
out:
return ret;

Expand Down

0 comments on commit 166418c

Please sign in to comment.