Patchwork [4/5] ext4: flush journal when switching from journal data mode

login
register
mail settings
Submitter Yongqiang Yang
Date Nov. 15, 2011, 8:07 a.m.
Message ID <1321344474-14707-4-git-send-email-xiaoqiangnk@gmail.com>
Download mbox | patch
Permalink /patch/125726/
State Accepted
Headers show

Comments

Yongqiang Yang - Nov. 15, 2011, 8:07 a.m.
When switching from journal data mode, the data blocks
in journal will have no revoke record.  Thus, data could be
corrupted during replay.  However, there is no such problem in
switching to journal data mode.  So we flush journal only in
the case that swithes from journal data mode.

Signed-off-by: Yongqiang Yang <xiaoqiangnk@gmail.com>
---
 fs/ext4/inode.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)
Theodore Ts'o - Dec. 28, 2011, 6:56 p.m.
On Tue, Nov 15, 2011 at 04:07:53PM +0800, Yongqiang Yang wrote:
> When switching from journal data mode, the data blocks
> in journal will have no revoke record.  Thus, data could be
> corrupted during replay.  However, there is no such problem in
> switching to journal data mode.  So we flush journal only in
> the case that swithes from journal data mode.
> 
> Signed-off-by: Yongqiang Yang <xiaoqiangnk@gmail.com>

Applied, with a slightly different (and more explanatory commit
message):

ext4: flush journal when switching from data=journal mode

From: Yongqiang Yang <xiaoqiangnk@gmail.com>

It's necessary to flush the journal when switching away from
data=journal mode.  This is because there are no revoke records when
we are data blocks are journalled, which are required in the other
journal modes.

However, it is not necessary to flush the journal when switching into
data=journal mode, and flushing the journal is expensive.  So let's
avoid it in that case.

Signed-off-by: Yongqiang Yang <xiaoqiangnk@gmail.com>
Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>

						- Ted
--
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
Darrick J. Wong - Dec. 29, 2011, 9:01 p.m.
On Wed, Dec 28, 2011 at 01:56:43PM -0500, Ted Ts'o wrote:
> On Tue, Nov 15, 2011 at 04:07:53PM +0800, Yongqiang Yang wrote:
> > When switching from journal data mode, the data blocks
> > in journal will have no revoke record.  Thus, data could be
> > corrupted during replay.  However, there is no such problem in
> > switching to journal data mode.  So we flush journal only in
> > the case that swithes from journal data mode.
> > 
> > Signed-off-by: Yongqiang Yang <xiaoqiangnk@gmail.com>
> 
> Applied, with a slightly different (and more explanatory commit
> message):
> 
> ext4: flush journal when switching from data=journal mode
> 
> From: Yongqiang Yang <xiaoqiangnk@gmail.com>
> 
> It's necessary to flush the journal when switching away from
> data=journal mode.  This is because there are no revoke records when
> we are data blocks are journalled,

Minor nit, but did you mean "when data blocks are journalled" ?

--D
> which are required in the other journal modes.
> 
> However, it is not necessary to flush the journal when switching into
> data=journal mode, and flushing the journal is expensive.  So let's
> avoid it in that case.
> 
> Signed-off-by: Yongqiang Yang <xiaoqiangnk@gmail.com>
> Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
> 
> 						- Ted
> --
> 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
> 

--
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
Yongqiang Yang - Dec. 30, 2011, 2:43 p.m.
On Fri, Dec 30, 2011 at 5:01 AM, Darrick J. Wong <djwong@us.ibm.com> wrote:
> On Wed, Dec 28, 2011 at 01:56:43PM -0500, Ted Ts'o wrote:
>> On Tue, Nov 15, 2011 at 04:07:53PM +0800, Yongqiang Yang wrote:
>> > When switching from journal data mode, the data blocks
>> > in journal will have no revoke record.  Thus, data could be
>> > corrupted during replay.  However, there is no such problem in
>> > switching to journal data mode.  So we flush journal only in
>> > the case that swithes from journal data mode.
>> >
>> > Signed-off-by: Yongqiang Yang <xiaoqiangnk@gmail.com>
>>
>> Applied, with a slightly different (and more explanatory commit
>> message):
>>
>> ext4: flush journal when switching from data=journal mode
>>
>> From: Yongqiang Yang <xiaoqiangnk@gmail.com>
>>
>> It's necessary to flush the journal when switching away from
>> data=journal mode.  This is because there are no revoke records when
>> we are data blocks are journalled,
>
> Minor nit, but did you mean "when data blocks are journalled" ?
Yes!  chattr +j can change journal mode of a file to data mode.

Yongqiang.
>
> --D
>> which are required in the other journal modes.
>>
>> However, it is not necessary to flush the journal when switching into
>> data=journal mode, and flushing the journal is expensive.  So let's
>> avoid it in that case.
>>
>> Signed-off-by: Yongqiang Yang <xiaoqiangnk@gmail.com>
>> Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
>>
>>                                               - Ted
>> --
>> 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
>>
>
Theodore Ts'o - Dec. 30, 2011, 2:57 p.m.
On Thu, Dec 29, 2011 at 01:01:53PM -0800, Darrick J. Wong wrote:
> > It's necessary to flush the journal when switching away from
> > data=journal mode.  This is because there are no revoke records when
> > we are data blocks are journalled,
> 
> Minor nit, but did you mean "when data blocks are journalled" ?

Thanks, I've made this fix in commit description.

	     	       	      - Ted
--
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

Patch

diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 384f8a7..755f6c7 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -4689,7 +4689,6 @@  int ext4_change_inode_journal_flag(struct inode *inode, int val)
 	}
 
 	jbd2_journal_lock_updates(journal);
-	jbd2_journal_flush(journal);
 
 	/*
 	 * OK, there are no updates running now, and all cached data is
@@ -4699,10 +4698,12 @@  int ext4_change_inode_journal_flag(struct inode *inode, int val)
 	 * the inode's in-core data-journaling state flag now.
 	 */
 
-	if (val)
+	if (val) {
 		ext4_set_inode_flag(inode, EXT4_INODE_JOURNAL_DATA);
-	else
+	} else {
+		jbd2_journal_flush(journal);
 		ext4_clear_inode_flag(inode, EXT4_INODE_JOURNAL_DATA);
+	}
 	ext4_set_aops(inode);
 
 	jbd2_journal_unlock_updates(journal);