In this series I've collected the patches that prepare ext2 for BKL removal.
It consist mostly of cleanups and additionally introduces a spinlock to protect
some of the superblock's fields against concurrent access. I've addressed the
feedback kindly provided by Ogawa-san by moving the ext2_write_super() out of
ext2_setup_super().

These patches have been part of the BKL removal series that I have posted in
November 2009 already. Since this is more than just removing the usage of the
big lock I repost it separately for inclusion. This series, at least the last
patch that includes the s_lock, needs to be merged before Frederics bkl-removal
branch, if he merges the rest of my patches there.

Thanks,
Jan

Jan Blunck (5):
  ext2: Use ext2_clear_super_error() in ext2_sync_fs()
  ext2: Set the write time in ext2_sync_fs()
  ext2: Remove duplicate code from ext2_sync_fs()
  ext2: Move ext2_write_super() out of ext2_setup_super()
  ext2: Add ext2_sb_info s_lock spinlock

 fs/ext2/inode.c            |    2 +
 fs/ext2/super.c            |   63 +++++++++++++++++++++++++-------------------
 include/linux/ext2_fs_sb.h |    6 ++++
 3 files changed, 44 insertions(+), 27 deletions(-)

--

[ message continues ]

Move ext2_write_super() out of ext2_setup_super() as a preparation for the
next patch that adds a new lock for superblock fields.

Signed-off-by: Jan Blunck <jblunck@suse.de>
---
 fs/ext2/super.c |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/fs/ext2/super.c b/fs/ext2/super.c
index 40fc8d5..b01c491 100644
--- a/fs/ext2/super.c
+++ b/fs/ext2/super.c
@@ -606,7 +606,6 @@ static int ext2_setup_super (struct super_block * sb,
 	if (!le16_to_cpu(es->s_max_mnt_count))
 		es->s_max_mnt_count = cpu_to_le16(EXT2_DFL_MAX_MNT_COUNT);
 	le16_add_cpu(&es->s_mnt_count, 1);
-	ext2_write_super(sb);
 	if (test_opt (sb, DEBUG))
 		ext2_msg(sb, KERN_INFO, "%s, %s, bs=%lu, fs=%lu, gc=%lu, "
 			"bpg=%lu, ipg=%lu, mo=%04lx]",
@@ -1079,7 +1078,9 @@ static int ext2_fill_super(struct super_block *sb, void *data, int silent)
 	if (EXT2_HAS_COMPAT_FEATURE(sb, EXT3_FEATURE_COMPAT_HAS_JOURNAL))
 		ext2_msg(sb, KERN_WARNING,
 			"warning: mounting ext3 filesystem as ext2");
-	ext2_setup_super (sb, es, sb->s_flags & MS_RDONLY);
+	if (ext2_setup_super (sb, es, sb->s_flags & MS_RDONLY))
+		sb->s_flags != MS_RDONLY;
+	ext2_write_super(sb);
 	return 0;
 
 cantfind_ext2:
@@ -1249,6 +1250,7 @@ static int ext2_remount (struct super_block * sb, int * flags, char * data)
 		 */
 		es->s_state = cpu_to_le16(sbi->s_mount_state);
 		es->s_mtime = cpu_to_le32(get_seconds());
+		ext2_sync_super(sb, es);
 	} else {
 		__le32 ret = EXT2_HAS_RO_COMPAT_FEATURE(sb,
 					       ~EXT2_FEATURE_RO_COMPAT_SUPP);
@@ -1268,8 +1270,8 @@ static int ext2_remount (struct super_block * sb, int * flags, char * data)
 		sbi->s_mount_state = le16_to_cpu(es->s_state);
 		if (!ext2_setup_super (sb, es, 0))
 			sb->s_flags &= ~MS_RDONLY;
+		ext2_write_super(sb);
 	}
-	ext2_sync_super(sb, es);
 	unlock_kernel();
 	return 0;
 restore_opts:
-- 
1.6.4.2

--

[ message continues ]

^^ Should be |=, shouldn't it?

										Honza
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR
--

[ message continues ]

Add a spinlock that protects against concurrent modifications of
s_mount_state, s_blocks_last, s_overhead_last and the content of the
superblock's buffer pointed to by sbi->s_es. This is a preparation patch
for removing the BKL from ext2 in the next patch.

Signed-off-by: Jan Blunck <jblunck@suse.de>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: Jan Kara <jack@suse.cz>
Cc: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
---
 fs/ext2/inode.c            |    2 ++
 fs/ext2/super.c            |   31 +++++++++++++++++++++++++++++--
 include/linux/ext2_fs_sb.h |    6 ++++++
 3 files changed, 37 insertions(+), 2 deletions(-)

diff --git a/fs/ext2/inode.c b/fs/ext2/inode.c
index fc13cc1..5d15442 100644
--- a/fs/ext2/inode.c
+++ b/fs/ext2/inode.c
@@ -1407,9 +1407,11 @@ static int __ext2_write_inode(struct inode *inode, int do_sync)
 				* created, add a flag to the superblock.
 				*/
 				lock_kernel();
+				spin_lock(&EXT2_SB(sb)->s_lock);
 				ext2_update_dynamic_rev(sb);
 				EXT2_SET_RO_COMPAT_FEATURE(sb,
 					EXT2_FEATURE_RO_COMPAT_LARGE_FILE);
+				spin_unlock(&EXT2_SB(sb)->s_lock);
 				unlock_kernel();
 				ext2_write_super(sb);
 			}
diff --git a/fs/ext2/super.c b/fs/ext2/super.c
index b01c491..34d7a62 100644
--- a/fs/ext2/super.c
+++ b/fs/ext2/super.c
@@ -52,8 +52,10 @@ void ext2_error (struct super_block * sb, const char * function,
 	struct ext2_super_block *es = sbi->s_es;
 
 	if (!(sb->s_flags & MS_RDONLY)) {
+		spin_lock(&sbi->s_lock);
 		sbi->s_mount_state |= EXT2_ERROR_FS;
 		es->s_state |= cpu_to_le16(EXT2_ERROR_FS);
+		spin_unlock(&sbi->s_lock);
 		ext2_sync_super(sb, es);
 	}
 
@@ -84,6 +86,9 @@ void ext2_msg(struct super_block *sb, const char *prefix,
 	va_end(args);
 }
 
+/*
+ * This must be called with sbi->s_lock held.
+ */
 void ext2_update_dynamic_rev(struct super_block *sb)
 {
 	struct ext2_super_block *es = EXT2_SB(sb)->s_es;
@@ -124,7 +129,9 @@ static void ext2_put_super (struct super_block * sb)
 	if (!(sb->s_flags & MS_RDONLY)) {
 ...
[ message continues ]

Looking at this - probably we should protect by this lock also setting of
a feature in ext2_xattr_update_super_block(). It's an unrelated bugfix but
  Why exactly do you have in the above? Probably because of consistent
view of mount options? You should comment about that in the changelo and
  Could you please fold in ext2_commit_super? It's used only here and it's
name looks a bit scary to be called under the spinlock...

									Honza
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR
--

[ message continues ]

Right. Is there actually a reason why we don't update the s_free_blocks_count
and s_free_inodes_count when we found the filesystem did not have the
EXT2_VALID_FS set? Otherwise I could use ext2_sync_super() in both and make it
honor the wait flag by just calling sync_dirty_buffer() when it is necessary.

What do you think?

diff --git a/fs/ext2/super.c b/fs/ext2/super.c
index 40fc8d5..8187061 100644
--- a/fs/ext2/super.c
+++ b/fs/ext2/super.c
@@ -39,7 +39,7 @@
 #include "xip.h"
 
 static void ext2_sync_super(struct super_block *sb,
-			    struct ext2_super_block *es);
+			    struct ext2_super_block *es, int wait);
 static int ext2_remount (struct super_block * sb, int * flags, char * data);
 static int ext2_statfs (struct dentry * dentry, struct kstatfs * buf);
 static int ext2_sync_fs(struct super_block *sb, int wait);
@@ -54,7 +54,7 @@ void ext2_error (struct super_block * sb, const char * function,
 	if (!(sb->s_flags & MS_RDONLY)) {
 		sbi->s_mount_state |= EXT2_ERROR_FS;
 		es->s_state |= cpu_to_le16(EXT2_ERROR_FS);
-		ext2_sync_super(sb, es);
+		ext2_sync_super(sb, es, 1);
 	}
 
 	va_start(args, fmt);
@@ -125,7 +125,7 @@ static void ext2_put_super (struct super_block * sb)
 		struct ext2_super_block *es = sbi->s_es;
 
 		es->s_state = cpu_to_le16(sbi->s_mount_state);
-		ext2_sync_super(sb, es);
+		ext2_sync_super(sb, es, 1);
 	}
 	db_count = sbi->s_gdb_count;
 	for (i = 0; i < db_count; i++)
@@ -1127,23 +1127,16 @@ static void ext2_clear_super_error(struct super_block *sb)
 	}
 }
 
-static void ext2_commit_super (struct super_block * sb,
-			       struct ext2_super_block * es)
-{
-	ext2_clear_super_error(sb);
-	es->s_wtime = cpu_to_le32(get_seconds());
-	mark_buffer_dirty(EXT2_SB(sb)->s_sbh);
-	sb->s_dirt = 0;
-}
-
-static void ext2_sync_super(struct super_block *sb, struct ext2_super_block *es)
+static void ext2_sync_super(struct super_block *sb, struct ext2_super_block *es,
+			    int wait)
 {
 	ext2_clear_super_error(sb);
 ...
[ message continues ]

It looks like this is all about .35 material.

This is going to be hard to have a separate tree to pushdown/remove
the bkl in the mount path if it depends on the ext2 tree.

What about putting that with the fs bkl removal tree? Would that conflict
with other changes in the ext2 tree?

Thanks.

--

[ message continues ]

The pushdown can be in a separate tree that can be merged independently,
just the final patch removing it in do_new_mount needs to wait
for everything else to get merged first. We have some other patches,
in particular the one that makes CONFIG_BKL modular that have to
wait for other patches and should probably just be applied separately
rather than merged from a bisectable git tree.

	Arnd
--

[ message continues ]

Ok, no problem then.

Thanks.

--

[ message continues ]