Message ID | 1241152637-11986-2-git-send-email-tytso@mit.edu |
---|---|
State | Accepted, archived |
Headers | show |
On Fri, May 01, 2009 at 12:37:16AM -0400, Theodore Ts'o wrote: > The s_dirt flag wasn't completely handled correctly, but it didn't > really matter when journalling was enabled. It turns out that when > ext4 runs without a journal, we don't clear s_dirt in places where we > should have, with the result that the high-level write_super() > function was writing the superblock when it wasn't necessary. > > So we fix this by making ext4_commit_super() clear the s_dirt flag, > and removing many of the other places where s_dirt is manipulated. > When journalling is enabled, the s_dirt flag might be left set more > often, but s_dirt really doesn't matter when journalling is enabled. Btw, you might want to move all s_dirt setting into a wrapper, so that it never gets set for journal mode. That way we can avoid superflous calls into the filesystem in that default mode. -- 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
On Fri, May 01, 2009 at 03:02:55AM -0400, Christoph Hellwig wrote: > On Fri, May 01, 2009 at 12:37:16AM -0400, Theodore Ts'o wrote: > > The s_dirt flag wasn't completely handled correctly, but it didn't > > really matter when journalling was enabled. It turns out that when > > ext4 runs without a journal, we don't clear s_dirt in places where we > > should have, with the result that the high-level write_super() > > function was writing the superblock when it wasn't necessary. > > > > So we fix this by making ext4_commit_super() clear the s_dirt flag, > > and removing many of the other places where s_dirt is manipulated. > > When journalling is enabled, the s_dirt flag might be left set more > > often, but s_dirt really doesn't matter when journalling is enabled. > > Btw, you might want to move all s_dirt setting into a wrapper, so that > it never gets set for journal mode. That way we can avoid superflous > calls into the filesystem in that default mode. I thought about doing this, but currently the VFS core checks for the presence for the write_super method function before it checks for s_dirt, so it didn't seem to matter much whether or not s_dirt was set if the filesystem doesn't have a NULL write_super function. - 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
diff --git a/fs/ext4/super.c b/fs/ext4/super.c index ad4c9be..7c7a08a 100644 --- a/fs/ext4/super.c +++ b/fs/ext4/super.c @@ -3128,7 +3128,6 @@ static int ext4_load_journal(struct super_block *sb, if (journal_devnum && journal_devnum != le32_to_cpu(es->s_journal_dev)) { es->s_journal_dev = cpu_to_le32(journal_devnum); - sb->s_dirt = 1; /* Make sure we flush the recovery flag to disk. */ ext4_commit_super(sb, 1); @@ -3168,7 +3167,7 @@ static int ext4_commit_super(struct super_block *sb, int sync) &EXT4_SB(sb)->s_freeblocks_counter)); es->s_free_inodes_count = cpu_to_le32(percpu_counter_sum_positive( &EXT4_SB(sb)->s_freeinodes_counter)); - + sb->s_dirt = 0; BUFFER_TRACE(sbh, "marking dirty"); mark_buffer_dirty(sbh); if (sync) { @@ -3210,7 +3209,6 @@ static void ext4_mark_recovery_complete(struct super_block *sb, if (EXT4_HAS_INCOMPAT_FEATURE(sb, EXT4_FEATURE_INCOMPAT_RECOVER) && sb->s_flags & MS_RDONLY) { EXT4_CLEAR_INCOMPAT_FEATURE(sb, EXT4_FEATURE_INCOMPAT_RECOVER); - sb->s_dirt = 0; ext4_commit_super(sb, 1); } unlock_super(sb); @@ -3271,10 +3269,8 @@ int ext4_force_commit(struct super_block *sb) return 0; journal = EXT4_SB(sb)->s_journal; - if (journal) { - sb->s_dirt = 0; + if (journal) ret = ext4_journal_force_commit(journal); - } return ret; } @@ -3282,15 +3278,13 @@ int ext4_force_commit(struct super_block *sb) /* * Ext4 always journals updates to the superblock itself, so we don't * have to propagate any other updates to the superblock on disk at this - * point. (We can probably nuke this function altogether, and remove - * any mention to sb->s_dirt in all of fs/ext4; eventual cleanup...) + * point if the journalling is enabled. */ static void ext4_write_super(struct super_block *sb) { if (EXT4_SB(sb)->s_journal) { if (mutex_trylock(&sb->s_lock) != 0) BUG(); - sb->s_dirt = 0; } else { ext4_commit_super(sb, 1); } @@ -3302,7 +3296,6 @@ static int ext4_sync_fs(struct super_block *sb, int wait) tid_t target; trace_mark(ext4_sync_fs, "dev %s wait %d", sb->s_id, wait); - sb->s_dirt = 0; if (EXT4_SB(sb)->s_journal) { if (jbd2_journal_start_commit(EXT4_SB(sb)->s_journal, &target)) { @@ -3324,7 +3317,6 @@ static int ext4_freeze(struct super_block *sb) { int error = 0; journal_t *journal; - sb->s_dirt = 0; if (!(sb->s_flags & MS_RDONLY)) { journal = EXT4_SB(sb)->s_journal;
The s_dirt flag wasn't completely handled correctly, but it didn't really matter when journalling was enabled. It turns out that when ext4 runs without a journal, we don't clear s_dirt in places where we should have, with the result that the high-level write_super() function was writing the superblock when it wasn't necessary. So we fix this by making ext4_commit_super() clear the s_dirt flag, and removing many of the other places where s_dirt is manipulated. When journalling is enabled, the s_dirt flag might be left set more often, but s_dirt really doesn't matter when journalling is enabled. Signed-off-by: "Theodore Ts'o" <tytso@mit.edu> --- fs/ext4/super.c | 14 +++----------- 1 files changed, 3 insertions(+), 11 deletions(-)