Re: [RESEND PATCH 2/2] ext3: flush journal only when journal mode is changed from journaled

Jan Kara
2011-11-15T13:33:27Z

On Tue 15-11-11 16:04:49, Yongqiang Yang wrote:
> To prevent data from corruption during repaly, we need to flush journal
> before changing journal mode from journaled. However flushing journal is
> not needed when changing journal to journaled.
We flush the journal for other reasons as well - switching inodes
->i_aops is rather complex thing so a filesystem should better be in a
quiescent state. You'd definitely at least need to commit the running
transaction otherwise a hell would break loose if you tried to modify a
buffer both as ordered data and as metadata in one transaction.

So I strongly prefer to keep the code as is unless you can really
convincingly *prove* that nothing breaks and show benefit of such change.

Honza
>
> Signed-off-by: Yongqiang Yang <xiaoqiangnk@gmail.com>
> ---
> fs/ext3/inode.c | 7 ++++---
> 1 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/fs/ext3/inode.c b/fs/ext3/inode.c
> index 04da6ac..5577358 100644
> --- a/fs/ext3/inode.c
> +++ b/fs/ext3/inode.c
> @@ -3546,7 +3546,6 @@ int ext3_change_inode_journal_flag(struct inode *inode, int val)
> return -EROFS;
>
> journal_lock_updates(journal);
> - journal_flush(journal);
>
> /*
> * OK, there are no updates running now, and all cached data is
> @@ -3556,10 +3555,12 @@ int ext3_change_inode_journal_flag(struct inode *inode, int val)
> * the inode's in-core data-journaling state flag now.
> */
>
> - if (val)
> + if (val) {
> EXT3_I(inode)->i_flags |= EXT3_JOURNAL_DATA_FL;
> - else
> + } else {
> + journal_flush(journal);
> EXT3_I(inode)->i_flags &= ~EXT3_JOURNAL_DATA_FL;
> + }
> ext3_set_aops(inode);
>
> journal_unlock_updates(journal);
> --
> 1.7.5.1
>
--
Jan Kara <jack@suse.cz>
SUSE Labs, CR