Google Groups no longer supports new Usenet posts or subscriptions. Historical content remains viewable.
Dismiss

[PATCH 0/7] ext2: Preparation to remove BKL (v2)

3 views
Skip to first unread message

Jan Blunck

unread,
Apr 14, 2010, 8:40:01 AM4/14/10
to
This is a series of patches in preparation to the removal of the big kernel
lock from ext2. It consists mostly of cleanup patches to later introduce a
spinlock to protect some of the superblock's fields against concurrent access.
With the spinlock in place we don't need to use the big kernel lock anymore.

This series has been part of the BKL removal patches that I have posted in
November 2009 already. I've addressed the feedback kindly provided by Ogawa-san
by moving the ext2_write_super() out of ext2_setup_super().

Changes since v1:
- fix typo when setting MS_RDONLY
- take s_lock when setting EXT2_FEATURE_COMPAT_EXT_ATTR
- fold ext2_commit_super() into ext2_sync_super()
- include BKL removal patch

Thanks,
Jan

Jan Blunck (7):
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: Fold ext2_commit_super() into ext2_sync_super()
ext2: Move ext2_write_super() out of ext2_setup_super()
ext2: Add ext2_sb_info s_lock spinlock
BKL: Remove BKL from ext2 filesystem

fs/ext2/inode.c | 5 +-
fs/ext2/super.c | 99 ++++++++++++++++++--------------------------
fs/ext2/xattr.c | 2 +
include/linux/ext2_fs_sb.h | 9 ++++
4 files changed, 54 insertions(+), 61 deletions(-)

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Jan Blunck

unread,
Apr 14, 2010, 8:40:01 AM4/14/10
to
Depending in the state (valid or unchecked) of the filesystem either
ext2_sync_super() or ext2_commit_super() is called. If the filesystem is
currently valid (it is checked), we first mark it unchecked and afterwards
duplicate the work that ext2_sync_super() is doing later. Therefore this
patch removes the duplicate code and calls ext2_sync_super() directly after
marking the filesystem unchecked.

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

diff --git a/fs/ext2/super.c b/fs/ext2/super.c
index b205003..09a88bf 100644
--- a/fs/ext2/super.c
+++ b/fs/ext2/super.c
@@ -1163,16 +1163,9 @@ static int ext2_sync_fs(struct super_block *sb, int wait)
struct ext2_super_block *es = EXT2_SB(sb)->s_es;

lock_kernel();
- ext2_clear_super_error(sb);
-
if (es->s_state & cpu_to_le16(EXT2_VALID_FS)) {
ext2_debug("setting valid to 0\n");
es->s_state &= cpu_to_le16(~EXT2_VALID_FS);
- es->s_free_blocks_count =
- cpu_to_le32(ext2_count_free_blocks(sb));
- es->s_free_inodes_count =
- cpu_to_le32(ext2_count_free_inodes(sb));
- es->s_wtime = cpu_to_le32(get_seconds());
ext2_sync_super(sb, es);
} else {
ext2_commit_super(sb, es);
--
1.6.4.2

Jan Blunck

unread,
Apr 14, 2010, 8:40:02 AM4/14/10
to
This is probably a typo since the write time should actually be updated by
ext2_sync_fs() instead of the mount time.

Signed-off-by: Jan Blunck <jbl...@suse.de>
---

fs/ext2/super.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/fs/ext2/super.c b/fs/ext2/super.c
index 8e8b675..b205003 100644
--- a/fs/ext2/super.c
+++ b/fs/ext2/super.c
@@ -1172,7 +1172,7 @@ static int ext2_sync_fs(struct super_block *sb, int wait)
cpu_to_le32(ext2_count_free_blocks(sb));
es->s_free_inodes_count =
cpu_to_le32(ext2_count_free_inodes(sb));
- es->s_mtime = cpu_to_le32(get_seconds());
+ es->s_wtime = cpu_to_le32(get_seconds());


ext2_sync_super(sb, es);
} else {
ext2_commit_super(sb, es);
--
1.6.4.2

--

Jan Blunck

unread,
Apr 14, 2010, 8:40:02 AM4/14/10
to
Both function originally did similar things except that ext2_sync_super()
is returning after the call to sync_dirty_buffer(sbh). Therefore this
patch adds a wait flag to tell ext2_sync_super() if it has to call
sync_dirty_buffer() to wait for in-progress I/O to finish.

Signed-off-by: Jan Blunck <jbl...@suse.de>
---

fs/ext2/super.c | 28 +++++++++-------------------
1 files changed, 9 insertions(+), 19 deletions(-)

diff --git a/fs/ext2/super.c b/fs/ext2/super.c
index 09a88bf..a304c54 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);
es->s_free_blocks_count = cpu_to_le32(ext2_count_free_blocks(sb));
es->s_free_inodes_count = cpu_to_le32(ext2_count_free_inodes(sb));
es->s_wtime = cpu_to_le32(get_seconds());
mark_buffer_dirty(EXT2_SB(sb)->s_sbh);
- sync_dirty_buffer(EXT2_SB(sb)->s_sbh);
+ if (wait)
+ sync_dirty_buffer(EXT2_SB(sb)->s_sbh);
sb->s_dirt = 0;
}

@@ -1166,11 +1159,8 @@ static int ext2_sync_fs(struct super_block *sb, int wait)


if (es->s_state & cpu_to_le16(EXT2_VALID_FS)) {
ext2_debug("setting valid to 0\n");
es->s_state &= cpu_to_le16(~EXT2_VALID_FS);

- ext2_sync_super(sb, es);
- } else {
- ext2_commit_super(sb, es);
}
- sb->s_dirt = 0;
+ ext2_sync_super(sb, es, wait);
unlock_kernel();

return 0;
@@ -1268,7 +1258,7 @@ static int ext2_remount (struct super_block * sb, int * flags, char * data)
if (!ext2_setup_super (sb, es, 0))
sb->s_flags &= ~MS_RDONLY;
}
- ext2_sync_super(sb, es);
+ ext2_sync_super(sb, es, 1);
unlock_kernel();
return 0;
restore_opts:
--
1.6.4.2

Jan Blunck

unread,
Apr 14, 2010, 8:40:02 AM4/14/10
to
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 <jbl...@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 a304c54..f28a7ad 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:
@@ -1238,6 +1239,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, 1);
} else {
__le32 ret = EXT2_HAS_RO_COMPAT_FEATURE(sb,
~EXT2_FEATURE_RO_COMPAT_SUPP);
@@ -1257,8 +1259,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, 1);


unlock_kernel();
return 0;
restore_opts:
--
1.6.4.2

--

Jan Blunck

unread,
Apr 14, 2010, 8:50:01 AM4/14/10
to
The BKL is still used in ext2_put_super(), ext2_fill_super(), ext2_sync_fs()
ext2_remount() and ext2_write_inode(). From these calls ext2_put_super(),
ext2_fill_super() and ext2_remount() are protected against each other by
the struct super_block s_umount rw semaphore. The call in ext2_write_inode()
could only protect the modification of the ext2_sb_info through
ext2_update_dynamic_rev() against concurrent ext2_sync_fs() or ext2_remount().
ext2_fill_super() and ext2_put_super() can be left out because you need a
valid filesystem reference in all three cases, which you do not have when
you are one of these functions.

If the BKL is only protecting the modification of the ext2_sb_info it can
safely be removed since this is protected by the struct ext2_sb_info s_lock.

Signed-off-by: Jan Blunck <jbl...@suse.de>
Cc: Jan Kara <ja...@suse.cz>
---
fs/ext2/inode.c | 3 ---
fs/ext2/super.c | 13 -------------
2 files changed, 0 insertions(+), 16 deletions(-)

diff --git a/fs/ext2/inode.c b/fs/ext2/inode.c
index 5d15442..b90c3bf 100644
--- a/fs/ext2/inode.c
+++ b/fs/ext2/inode.c
@@ -22,7 +22,6 @@
* Assorted race fixes, rewrite of ext2_get_block() by Al Viro, 2000
*/

-#include <linux/smp_lock.h>
#include <linux/time.h>
#include <linux/highuid.h>
#include <linux/pagemap.h>
@@ -1406,13 +1405,11 @@ static int __ext2_write_inode(struct inode *inode, int do_sync)
/* If this is the first large file
* 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 28f6560..71e9eb1 100644
--- a/fs/ext2/super.c
+++ b/fs/ext2/super.c
@@ -26,7 +26,6 @@
#include <linux/random.h>
#include <linux/buffer_head.h>
#include <linux/exportfs.h>
-#include <linux/smp_lock.h>
#include <linux/vfs.h>
#include <linux/seq_file.h>
#include <linux/mount.h>
@@ -120,8 +119,6 @@ static void ext2_put_super (struct super_block * sb)
int i;
struct ext2_sb_info *sbi = EXT2_SB(sb);

- lock_kernel();
-
if (sb->s_dirt)
ext2_write_super(sb);

@@ -147,8 +144,6 @@ static void ext2_put_super (struct super_block * sb)
sb->s_fs_info = NULL;
kfree(sbi->s_blockgroup_lock);
kfree(sbi);
-
- unlock_kernel();
}

static struct kmem_cache * ext2_inode_cachep;
@@ -1170,7 +1165,6 @@ static int ext2_sync_fs(struct super_block *sb, int wait)
struct ext2_sb_info *sbi = EXT2_SB(sb);


struct ext2_super_block *es = EXT2_SB(sb)->s_es;

- lock_kernel();
spin_lock(&sbi->s_lock);


if (es->s_state & cpu_to_le16(EXT2_VALID_FS)) {
ext2_debug("setting valid to 0\n");

@@ -1178,8 +1172,6 @@ static int ext2_sync_fs(struct super_block *sb, int wait)
}
spin_unlock(&sbi->s_lock);
ext2_sync_super(sb, es, wait);
- unlock_kernel();
-
return 0;
}

@@ -1201,7 +1193,6 @@ static int ext2_remount (struct super_block * sb, int * flags, char * data)
unsigned long old_sb_flags;
int err;

- lock_kernel();
spin_lock(&sbi->s_lock);

/* Store the old options */
@@ -1242,14 +1233,12 @@ static int ext2_remount (struct super_block * sb, int * flags, char * data)
}
if ((*flags & MS_RDONLY) == (sb->s_flags & MS_RDONLY)) {
spin_unlock(&sbi->s_lock);
- unlock_kernel();
return 0;
}
if (*flags & MS_RDONLY) {
if (le16_to_cpu(es->s_state) & EXT2_VALID_FS ||
!(sbi->s_mount_state & EXT2_VALID_FS)) {
spin_unlock(&sbi->s_lock);
- unlock_kernel();
return 0;
}
/*
@@ -1282,7 +1271,6 @@ static int ext2_remount (struct super_block * sb, int * flags, char * data)
spin_unlock(&sbi->s_lock);
ext2_write_super(sb);
}
- unlock_kernel();
return 0;
restore_opts:
sbi->s_mount_opt = old_opts.s_mount_opt;
@@ -1290,7 +1278,6 @@ restore_opts:
sbi->s_resgid = old_opts.s_resgid;
sb->s_flags = old_sb_flags;
spin_unlock(&sbi->s_lock);
- unlock_kernel();
return err;
}

--
1.6.4.2

Jan Blunck

unread,
Apr 14, 2010, 8:50:02 AM4/14/10
to
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. The spinlock is now used in
ext2_xattr_update_super_block() which was setting the
EXT2_FEATURE_COMPAT_EXT_ATTR flag on the superblock without protection
before. Likewise the spinlock is used in ext2_show_options() to have a
consistent view of the mount options.

This is a preparation patch for removing the BKL from ext2 in the next
patch.

Signed-off-by: Jan Blunck <jbl...@suse.de>
Cc: Andi Kleen <an...@firstfloor.org>
Cc: Jan Kara <ja...@suse.cz>
Cc: OGAWA Hirofumi <hiro...@mail.parknet.co.jp>
---
fs/ext2/inode.c | 2 ++
fs/ext2/super.c | 27 ++++++++++++++++++++++++++-
fs/ext2/xattr.c | 2 ++
include/linux/ext2_fs_sb.h | 9 +++++++++
4 files changed, 39 insertions(+), 1 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 f28a7ad..28f6560 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, 1);
}

@@ -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)) {

struct ext2_super_block *es = sbi->s_es;

+ spin_lock(&sbi->s_lock);


es->s_state = cpu_to_le16(sbi->s_mount_state);

+ spin_unlock(&sbi->s_lock);


ext2_sync_super(sb, es, 1);
}
db_count = sbi->s_gdb_count;

@@ -209,6 +216,7 @@ static int ext2_show_options(struct seq_file *seq, struct vfsmount *vfs)


struct ext2_super_block *es = sbi->s_es;

unsigned long def_mount_opts;

+ spin_lock(&sbi->s_lock);
def_mount_opts = le32_to_cpu(es->s_default_mount_opts);

if (sbi->s_sb_block != 1)
@@ -281,6 +289,7 @@ static int ext2_show_options(struct seq_file *seq, struct vfsmount *vfs)
if (!test_opt(sb, RESERVATION))
seq_puts(seq, ",noreservation");

+ spin_unlock(&sbi->s_lock);
return 0;
}

@@ -766,6 +775,8 @@ static int ext2_fill_super(struct super_block *sb, void *data, int silent)
sb->s_fs_info = sbi;
sbi->s_sb_block = sb_block;

+ spin_lock_init(&sbi->s_lock);
+
/*
* See what the current blocksize for the device is, and
* use that as the blocksize. Otherwise (or if the blocksize
@@ -1132,9 +1143,12 @@ static void ext2_sync_super(struct super_block *sb, struct ext2_super_block *es,
int wait)
{
ext2_clear_super_error(sb);
+ spin_lock(&EXT2_SB(sb)->s_lock);


es->s_free_blocks_count = cpu_to_le32(ext2_count_free_blocks(sb));
es->s_free_inodes_count = cpu_to_le32(ext2_count_free_inodes(sb));
es->s_wtime = cpu_to_le32(get_seconds());

+ /* unlock before we do IO */
+ spin_unlock(&EXT2_SB(sb)->s_lock);
mark_buffer_dirty(EXT2_SB(sb)->s_sbh);
if (wait)
sync_dirty_buffer(EXT2_SB(sb)->s_sbh);
@@ -1151,16 +1165,18 @@ static void ext2_sync_super(struct super_block *sb, struct ext2_super_block *es,
* may have been checked while mounted and e2fsck may have
* set s_state to EXT2_VALID_FS after some corrections.
*/
-


static int ext2_sync_fs(struct super_block *sb, int wait)

{
+ struct ext2_sb_info *sbi = EXT2_SB(sb);


struct ext2_super_block *es = EXT2_SB(sb)->s_es;

lock_kernel();
+ spin_lock(&sbi->s_lock);


if (es->s_state & cpu_to_le16(EXT2_VALID_FS)) {
ext2_debug("setting valid to 0\n");

es->s_state &= cpu_to_le16(~EXT2_VALID_FS);
}
+ spin_unlock(&sbi->s_lock);
ext2_sync_super(sb, es, wait);
unlock_kernel();

@@ -1186,6 +1202,7 @@ static int ext2_remount (struct super_block * sb, int * flags, char * data)
int err;

lock_kernel();
+ spin_lock(&sbi->s_lock);



/* Store the old options */

old_sb_flags = sb->s_flags;
@@ -1224,12 +1241,14 @@ static int ext2_remount (struct super_block * sb, int * flags, char * data)
sbi->s_mount_opt |= old_mount_opt & EXT2_MOUNT_XIP;


}
if ((*flags & MS_RDONLY) == (sb->s_flags & MS_RDONLY)) {

+ spin_unlock(&sbi->s_lock);


unlock_kernel();
return 0;
}
if (*flags & MS_RDONLY) {
if (le16_to_cpu(es->s_state) & EXT2_VALID_FS ||
!(sbi->s_mount_state & EXT2_VALID_FS)) {

+ spin_unlock(&sbi->s_lock);
unlock_kernel();
return 0;
}
@@ -1239,6 +1258,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());

+ spin_unlock(&sbi->s_lock);


ext2_sync_super(sb, es, 1);
} else {
__le32 ret = EXT2_HAS_RO_COMPAT_FEATURE(sb,

@@ -1259,6 +1279,7 @@ 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;

+ spin_unlock(&sbi->s_lock);
ext2_write_super(sb);
}
unlock_kernel();
@@ -1268,6 +1289,7 @@ restore_opts:
sbi->s_resuid = old_opts.s_resuid;


sbi->s_resgid = old_opts.s_resgid;
sb->s_flags = old_sb_flags;

+ spin_unlock(&sbi->s_lock);
unlock_kernel();
return err;
}
@@ -1279,6 +1301,8 @@ static int ext2_statfs (struct dentry * dentry, struct kstatfs * buf)


struct ext2_super_block *es = sbi->s_es;

u64 fsid;

+ spin_lock(&sbi->s_lock);
+
if (test_opt (sb, MINIX_DF))
sbi->s_overhead_last = 0;
else if (sbi->s_blocks_last != le32_to_cpu(es->s_blocks_count)) {
@@ -1333,6 +1357,7 @@ static int ext2_statfs (struct dentry * dentry, struct kstatfs * buf)
le64_to_cpup((void *)es->s_uuid + sizeof(u64));
buf->f_fsid.val[0] = fsid & 0xFFFFFFFFUL;
buf->f_fsid.val[1] = (fsid >> 32) & 0xFFFFFFFFUL;
+ spin_unlock(&sbi->s_lock);
return 0;
}

diff --git a/fs/ext2/xattr.c b/fs/ext2/xattr.c
index e44dc92..3b96045 100644
--- a/fs/ext2/xattr.c
+++ b/fs/ext2/xattr.c
@@ -345,7 +345,9 @@ static void ext2_xattr_update_super_block(struct super_block *sb)
if (EXT2_HAS_COMPAT_FEATURE(sb, EXT2_FEATURE_COMPAT_EXT_ATTR))
return;

+ spin_lock(&EXT2_SB(sb)->s_lock);
EXT2_SET_COMPAT_FEATURE(sb, EXT2_FEATURE_COMPAT_EXT_ATTR);
+ spin_unlock(&EXT2_SB(sb)->s_lock);
sb->s_dirt = 1;
mark_buffer_dirty(EXT2_SB(sb)->s_sbh);
}
diff --git a/include/linux/ext2_fs_sb.h b/include/linux/ext2_fs_sb.h
index 1cdb663..db4d9f5 100644
--- a/include/linux/ext2_fs_sb.h
+++ b/include/linux/ext2_fs_sb.h
@@ -106,6 +106,15 @@ struct ext2_sb_info {
spinlock_t s_rsv_window_lock;
struct rb_root s_rsv_window_root;
struct ext2_reserve_window_node s_rsv_window_head;
+ /*
+ * s_lock protects against concurrent modifications of s_mount_state,
+ * s_blocks_last, s_overhead_last and the content of superblock's
+ * buffer pointed to by sbi->s_es.
+ *
+ * Note: It is used in ext2_show_options() to provide a consistent view
+ * of the mount options.
+ */
+ spinlock_t s_lock;
};

static inline spinlock_t *
--
1.6.4.2

Jan Blunck

unread,
Apr 14, 2010, 8:50:02 AM4/14/10
to
ext2_sync_fs() used to duplicate the code from ext2_clear_super_error().

Signed-off-by: Jan Blunck <jbl...@suse.de>
---

fs/ext2/super.c | 20 +++-----------------
1 files changed, 3 insertions(+), 17 deletions(-)

diff --git a/fs/ext2/super.c b/fs/ext2/super.c
index 42e4a30..8e8b675 100644
--- a/fs/ext2/super.c
+++ b/fs/ext2/super.c
@@ -1120,8 +1120,8 @@ static void ext2_clear_super_error(struct super_block *sb)
* be remapped. Nothing we can do but to retry the
* write and hope for the best.
*/
- printk(KERN_ERR "EXT2-fs: %s previous I/O error to "
- "superblock detected", sb->s_id);
+ ext2_msg(sb, KERN_ERR,
+ "previous I/O error to superblock detected\n");
clear_buffer_write_io_error(sbh);
set_buffer_uptodate(sbh);
}
@@ -1161,23 +1161,9 @@ static void ext2_sync_super(struct super_block *sb, struct ext2_super_block *es)


static int ext2_sync_fs(struct super_block *sb, int wait)
{

struct ext2_super_block *es = EXT2_SB(sb)->s_es;

- struct buffer_head *sbh = EXT2_SB(sb)->s_sbh;

lock_kernel();
- if (buffer_write_io_error(sbh)) {
- /*
- * Oh, dear. A previous attempt to write the
- * superblock failed. This could happen because the
- * USB device was yanked out. Or it could happen to
- * be a transient write error and maybe the block will
- * be remapped. Nothing we can do but to retry the
- * write and hope for the best.
- */
- ext2_msg(sb, KERN_ERR,
- "previous I/O error to superblock detected\n");
- clear_buffer_write_io_error(sbh);
- set_buffer_uptodate(sbh);
- }
+ ext2_clear_super_error(sb);



if (es->s_state & cpu_to_le16(EXT2_VALID_FS)) {
ext2_debug("setting valid to 0\n");

--
1.6.4.2

Jan Kara

unread,
Apr 14, 2010, 10:00:02 AM4/14/10
to
On Wed 14-04-10 14:38:32, Jan Blunck wrote:
> This is a series of patches in preparation to the removal of the big kernel
> lock from ext2. It consists mostly of cleanup patches to later introduce a
> spinlock to protect some of the superblock's fields against concurrent access.
> With the spinlock in place we don't need to use the big kernel lock anymore.
>
> This series has been part of the BKL removal patches that I have posted in
> November 2009 already. I've addressed the feedback kindly provided by Ogawa-san
> by moving the ext2_write_super() out of ext2_setup_super().
>
> Changes since v1:
> - fix typo when setting MS_RDONLY
> - take s_lock when setting EXT2_FEATURE_COMPAT_EXT_ATTR
> - fold ext2_commit_super() into ext2_sync_super()
> - include BKL removal patch
OK, I've merged all the patches into my for_testing branch. When it
survives some testing, I'll push them to for_next branch to be pushed
for 2.6.35.

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

0 new messages