[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date: Tue, 15 Nov 2011 21:57:13 +0800
From: Yongqiang Yang <xiaoqiangnk@...il.com>
To: Jan Kara <jack@...e.cz>
Cc: linux-ext4@...r.kernel.org
Subject: Re: [RESEND PATCH 2/2] ext3: flush journal only when journal mode is
changed from journaled
On Tue, Nov 15, 2011 at 9:33 PM, Jan Kara <jack@...e.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@...il.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@...e.cz>
> SUSE Labs, CR
>
--
Best Wishes
Yongqiang Yang
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists