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

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

[Yongqiang Yang]

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(-)

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

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

[Jan Kara]

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

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

[Yongqiang Yang]

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
>



-- 
Best Wishes
Yongqiang Yang
--
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