| Message ID | 1321344289-13125-1-git-send-email-xiaoqiangnk@gmail.com |
|---|---|
| State | New, archived |
| Headers | show |
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 >
On Tue, Nov 15, 2011 at 9:33 PM, Jan Kara <jack@suse.cz> wrote: > 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. Well. The only benefit is that we can switch journal mode of a file from ordered to journaled without flushing journal, thus, it is much faster. Before switching journal mode of a file from ordered to journaled, the metadata has been in journal while data not. After switching, both metadata and data will be go to journal. I am not sure if there is any problem:-). I am not very insistent on the patch. It seems that the benefit can be ignored. Yongqiang. > > 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 >
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);
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. Signed-off-by: Yongqiang Yang <xiaoqiangnk@gmail.com> --- fs/ext3/inode.c | 7 ++++--- 1 files changed, 4 insertions(+), 3 deletions(-)