During the realtime preemption mini-summit we discussed the entire removal of
the big kernel lock. I've started working on this for some filesystems. My plan
is to push the BKL down to the implementations first and remove it from there
later.
This series is pushing the BKL from do_new_mount() down to the filesystems.
If the filesystem wasn't using the BKL at all before, it gets removed instead.
If removing is trivial (e.g. because it is obviously already protected by an
existing lock) it gets removed by a follow up patch in the series.
Later on in the series the big kernel lock is removed from EXT2. Instead, a new
lock that protects the modification of some of the superblocks fields is
introduced.
Version 2 of the series addressed the feedback from Matthew Wilcox and I tried
to be more verbose about why I think it is safe to remove the BKL. In most
cases there is no shared resource accessed anyway or the superblock s_umount
rwsemaphore is used so it is trivially safe to remove the big kernel lock.
Version 3 of the series addressed the feedback from Christoph Hellwig and Arnd
Bergmann to not push down the BKL to filesystems that don't use it anyway. I've
changed the ext2 s_lock usage in ext2_sync_super() to address Jan Kara's
comments.
If all affected maintainers acknowledge their parts of the patches it should
get merged through one of the bkl branches in the tip tree.
Jan Blunck (20):
JFS: Free sbi memory in error path
AFFS: Free sbi memory in error path
BKL: Explicitly add BKL around get_sb/fill_super
BKL: Remove outdated comment and include
BKL: Remove BKL from Amiga FFS
BKL: Remove BKL from BFS
BKL: Remove BKL from CifsFS
BKL: Remove BKL from ext3 fill_super()
BKL: Remove BKL from ext3_put_super() and ext3_remount()
BKL: Remove BKL from ext4 filesystem
BKL: Remove BKL from exofs
BKL: Remove BKL from HFS
BKL: Remove BKL from HFS+
BKL: Remove BKL from JFS
BKL: Remove BKL from NILFS2
BKL: Remove BKL from NTFS
BKL: Remove BKL from cgroup
BKL: Remove BKL from do_new_mount()
ext2: Add ext2_sb_info s_lock spinlock
BKL: Remove BKL from ext2 filesystem
drivers/isdn/capi/capifs.c | 10 ++++++-
drivers/usb/core/inode.c | 5 +++
drivers/usb/gadget/inode.c | 12 +++++++-
fs/adfs/super.c | 8 +++++-
fs/affs/super.c | 17 ++++--------
fs/afs/super.c | 5 +++
fs/bfs/inode.c | 8 +----
fs/cifs/cifsfs.c | 9 ++----
fs/coda/inode.c | 8 +++++-
fs/ecryptfs/main.c | 4 +++
fs/exofs/super.c | 1 -
fs/ext2/inode.c | 5 +--
fs/ext2/super.c | 60 ++++++++++++++++++++++++++++++--------------
fs/ext3/super.c | 12 ---------
fs/ext4/super.c | 11 --------
fs/fat/namei_msdos.c | 7 ++++-
fs/fat/namei_vfat.c | 7 ++++-
fs/freevxfs/vxfs_super.c | 7 ++++-
fs/hfs/super.c | 6 +---
fs/hfsplus/super.c | 5 ---
fs/hpfs/super.c | 8 +++++-
fs/isofs/inode.c | 8 +++++-
fs/jffs2/super.c | 11 ++++++-
fs/jfs/super.c | 23 ++++------------
fs/namespace.c | 2 -
fs/ncpfs/inode.c | 8 +++++-
fs/nfs/super.c | 24 +++++++++++++++++
fs/nilfs2/ioctl.c | 1 -
fs/nilfs2/super.c | 10 -------
fs/ntfs/super.c | 24 +----------------
fs/ocfs2/dlm/dlmfs.c | 9 ++++++-
fs/ocfs2/super.c | 5 +++
fs/qnx4/inode.c | 8 +++++-
fs/reiserfs/super.c | 4 +++
fs/smbfs/inode.c | 5 +++
fs/squashfs/super.c | 6 ++++
fs/super.c | 3 --
fs/udf/super.c | 8 +++++-
fs/ufs/super.c | 5 +++
include/linux/ext2_fs_sb.h | 6 ++++
kernel/cgroup.c | 4 ---
41 files changed, 234 insertions(+), 155 deletions(-)
I spotted the missing kfree() while removing the BKL.
Signed-off-by: Jan Blunck <[email protected]>
---
fs/jfs/super.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/fs/jfs/super.c b/fs/jfs/super.c
index 2234c73..67f8c3e 100644
--- a/fs/jfs/super.c
+++ b/fs/jfs/super.c
@@ -452,6 +452,7 @@ static int jfs_fill_super(struct super_block *sb, void *data, int silent)
if (newLVSize) {
printk(KERN_ERR "resize option for remount only\n");
+ kfree(sbi);
return -EINVAL;
}
--
1.6.4.2
I spotted the missing kfree() while removing the BKL.
Signed-off-by: Jan Blunck <[email protected]>
---
fs/affs/super.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/fs/affs/super.c b/fs/affs/super.c
index 104fdcb..058c2c7 100644
--- a/fs/affs/super.c
+++ b/fs/affs/super.c
@@ -316,7 +316,7 @@ static int affs_fill_super(struct super_block *sb, void *data, int silent)
&blocksize,&sbi->s_prefix,
sbi->s_volume, &mount_flags)) {
printk(KERN_ERR "AFFS: Error parsing options\n");
- return -EINVAL;
+ goto out_error_free_sbi;
}
/* N.B. after this point s_prefix must be released */
@@ -498,6 +498,7 @@ out_error_noinode:
kfree(sbi->s_bitmap);
affs_brelse(root_bh);
kfree(sbi->s_prefix);
+out_error_free_sbi:
kfree(sbi);
sb->s_fs_info = NULL;
return ret;
--
1.6.4.2
remount_fs does not need lock_kernel() anymore since that was pushed down some
time ago. Since nothing needs smp_lock.h anymore lets remove it.
Signed-off-by: Jan Blunck <[email protected]>
---
fs/super.c | 3 ---
1 files changed, 0 insertions(+), 3 deletions(-)
diff --git a/fs/super.c b/fs/super.c
index 19eb70b..6fe4611 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -23,7 +23,6 @@
#include <linux/module.h>
#include <linux/slab.h>
#include <linux/init.h>
-#include <linux/smp_lock.h>
#include <linux/acct.h>
#include <linux/blkdev.h>
#include <linux/quotaops.h>
@@ -618,8 +617,6 @@ static void do_emergency_remount(struct work_struct *work)
down_write(&sb->s_umount);
if (sb->s_root && sb->s_bdev && !(sb->s_flags & MS_RDONLY)) {
/*
- * ->remount_fs needs lock_kernel().
- *
* What lock protects sb->s_flags??
*/
do_remount_sb(sb, MS_RDONLY, NULL, 1);
--
1.6.4.2
The BKL is only used in put_super, fill_super and remount_fs that are all
three protected by the superblocks s_umount rw_semaphore. Therefore it is
safe to remove the BKL entirely.
Signed-off-by: Jan Blunck <[email protected]>
---
fs/affs/super.c | 22 +++++-----------------
1 files changed, 5 insertions(+), 17 deletions(-)
diff --git a/fs/affs/super.c b/fs/affs/super.c
index 151b930..ede5e44 100644
--- a/fs/affs/super.c
+++ b/fs/affs/super.c
@@ -16,7 +16,6 @@
#include <linux/parser.h>
#include <linux/magic.h>
#include <linux/sched.h>
-#include <linux/smp_lock.h>
#include "affs.h"
extern struct timezone sys_tz;
@@ -43,8 +42,6 @@ affs_put_super(struct super_block *sb)
struct affs_sb_info *sbi = AFFS_SB(sb);
pr_debug("AFFS: put_super()\n");
- lock_kernel();
-
if (!(sb->s_flags & MS_RDONLY))
affs_commit_super(sb, 1);
@@ -53,8 +50,6 @@ affs_put_super(struct super_block *sb)
affs_brelse(sbi->s_root_bh);
kfree(sbi);
sb->s_fs_info = NULL;
-
- unlock_kernel();
}
static void
@@ -298,8 +293,6 @@ static int affs_fill_super(struct super_block *sb, void *data, int silent)
u8 sig[4];
int ret = -EINVAL;
- lock_kernel();
-
save_mount_options(sb, data);
pr_debug("AFFS: read_super(%s)\n",data ? (const char *)data : "no options");
@@ -309,10 +302,9 @@ static int affs_fill_super(struct super_block *sb, void *data, int silent)
sb->s_flags |= MS_NODIRATIME;
sbi = kzalloc(sizeof(struct affs_sb_info), GFP_KERNEL);
- if (!sbi) {
- unlock_kernel();
+ if (!sbi)
return -ENOMEM;
- }
+
sb->s_fs_info = sbi;
mutex_init(&sbi->s_bmlock);
@@ -490,7 +482,6 @@ got_root:
sb->s_root->d_op = &affs_dentry_operations;
pr_debug("AFFS: s_flags=%lX\n",sb->s_flags);
- unlock_kernel();
return 0;
/*
@@ -506,7 +497,6 @@ out_error_noinode:
out_error_free_sbi:
kfree(sbi);
sb->s_fs_info = NULL;
- unlock_kernel();
return ret;
}
@@ -534,7 +524,7 @@ affs_remount(struct super_block *sb, int *flags, char *data)
kfree(new_opts);
return -EINVAL;
}
- lock_kernel();
+
replace_mount_options(sb, new_opts);
sbi->s_flags = mount_flags;
@@ -542,10 +532,9 @@ affs_remount(struct super_block *sb, int *flags, char *data)
sbi->s_uid = uid;
sbi->s_gid = gid;
- if ((*flags & MS_RDONLY) == (sb->s_flags & MS_RDONLY)) {
- unlock_kernel();
+ if ((*flags & MS_RDONLY) == (sb->s_flags & MS_RDONLY))
return 0;
- }
+
if (*flags & MS_RDONLY) {
sb->s_dirt = 1;
while (sb->s_dirt)
@@ -554,7 +543,6 @@ affs_remount(struct super_block *sb, int *flags, char *data)
} else
res = affs_init_bitmap(sb, flags);
- unlock_kernel();
return res;
}
--
1.6.4.2
The BKL is only used in put_super and fill_super that are both protected by
the superblocks s_umount rw_semaphore. Therefore it is safe to remove the BKL
entirely.
Signed-off-by: Jan Blunck <[email protected]>
---
fs/bfs/inode.c | 17 +++--------------
1 files changed, 3 insertions(+), 14 deletions(-)
diff --git a/fs/bfs/inode.c b/fs/bfs/inode.c
index 4bff506..8ed127a 100644
--- a/fs/bfs/inode.c
+++ b/fs/bfs/inode.c
@@ -12,7 +12,6 @@
#include <linux/slab.h>
#include <linux/init.h>
#include <linux/fs.h>
-#include <linux/smp_lock.h>
#include <linux/buffer_head.h>
#include <linux/vfs.h>
#include <asm/uaccess.h>
@@ -243,8 +242,6 @@ static void bfs_put_super(struct super_block *s)
if (!info)
return;
- lock_kernel();
-
if (s->s_dirt)
bfs_write_super(s);
@@ -253,8 +250,6 @@ static void bfs_put_super(struct super_block *s)
kfree(info->si_imap);
kfree(info);
s->s_fs_info = NULL;
-
- unlock_kernel();
}
static int bfs_statfs(struct dentry *dentry, struct kstatfs *buf)
@@ -356,13 +351,10 @@ static int bfs_fill_super(struct super_block *s, void *data, int silent)
long ret = -EINVAL;
unsigned long i_sblock, i_eblock, i_eoff, s_size;
- lock_kernel();
-
info = kzalloc(sizeof(*info), GFP_KERNEL);
- if (!info) {
- unlock_kernel();
+ if (!info)
return -ENOMEM;
- }
+
s->s_fs_info = info;
sb_set_blocksize(s, BFS_BSIZE);
@@ -467,7 +459,6 @@ static int bfs_fill_super(struct super_block *s, void *data, int silent)
kfree(info->si_imap);
kfree(info);
s->s_fs_info = NULL;
- unlock_kernel();
return -EIO;
}
@@ -486,17 +477,15 @@ static int bfs_fill_super(struct super_block *s, void *data, int silent)
if (!(s->s_flags & MS_RDONLY)) {
mark_buffer_dirty(info->si_sbh);
s->s_dirt = 1;
- }
+ }
dump_imap("read_super", s);
mutex_init(&info->bfs_lock);
- unlock_kernel();
return 0;
out:
brelse(bh);
kfree(info);
s->s_fs_info = NULL;
- unlock_kernel();
return ret;
}
--
1.6.4.2
The BKL is only used in put_super and fill_super that are both protected by
the superblocks s_umount rw_semaphore. Therefore it is safe to remove the
BKL entirely.
Signed-off-by: Jan Blunck <[email protected]>
Cc: Steve French <[email protected]>
---
fs/cifs/cifsfs.c | 13 +------------
1 files changed, 1 insertions(+), 12 deletions(-)
diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index 09ccb9d..137d1e8 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -35,7 +35,6 @@
#include <linux/delay.h>
#include <linux/kthread.h>
#include <linux/freezer.h>
-#include <linux/smp_lock.h>
#include "cifsfs.h"
#include "cifspdu.h"
#define DECLARE_GLOBALS_HERE
@@ -201,8 +200,6 @@ cifs_put_super(struct super_block *sb)
return;
}
- lock_kernel();
-
rc = cifs_umount(sb, cifs_sb);
if (rc)
cERROR(1, ("cifs_umount failed with return code %d", rc));
@@ -215,8 +212,6 @@ cifs_put_super(struct super_block *sb)
unload_nls(cifs_sb->local_nls);
kfree(cifs_sb);
-
- unlock_kernel();
}
static int
@@ -598,28 +593,22 @@ cifs_get_sb(struct file_system_type *fs_type,
int rc;
struct super_block *sb;
- lock_kernel();
-
sb = sget(fs_type, NULL, set_anon_super, NULL);
cFYI(1, ("Devname: %s flags: %d ", dev_name, flags));
- if (IS_ERR(sb)) {
- unlock_kernel();
+ if (IS_ERR(sb))
return PTR_ERR(sb);
- }
sb->s_flags = flags;
rc = cifs_read_super(sb, data, dev_name, flags & MS_SILENT ? 1 : 0);
if (rc) {
deactivate_locked_super(sb);
- unlock_kernel();
return rc;
}
sb->s_flags |= MS_ACTIVE;
simple_set_mnt(mnt, sb);
- unlock_kernel();
return 0;
}
--
1.6.4.2
The BKL is protecting nothing than two memory allocations here.
Signed-off-by: Jan Blunck <[email protected]>
Acked-by: Jan Kara <[email protected]>
---
fs/ext3/super.c | 13 +------------
1 files changed, 1 insertions(+), 12 deletions(-)
diff --git a/fs/ext3/super.c b/fs/ext3/super.c
index 0955c3f..373fc54 100644
--- a/fs/ext3/super.c
+++ b/fs/ext3/super.c
@@ -1570,19 +1570,14 @@ static int ext3_fill_super (struct super_block *sb, void *data, int silent)
__le32 features;
int err;
- lock_kernel();
-
sbi = kzalloc(sizeof(*sbi), GFP_KERNEL);
- if (!sbi) {
- unlock_kernel();
+ if (!sbi)
return -ENOMEM;
- }
sbi->s_blockgroup_lock =
kzalloc(sizeof(struct blockgroup_lock), GFP_KERNEL);
if (!sbi->s_blockgroup_lock) {
kfree(sbi);
- unlock_kernel();
return -ENOMEM;
}
sb->s_fs_info = sbi;
@@ -1591,8 +1586,6 @@ static int ext3_fill_super (struct super_block *sb, void *data, int silent)
sbi->s_resgid = EXT3_DEF_RESGID;
sbi->s_sb_block = sb_block;
- unlock_kernel();
-
blocksize = sb_min_blocksize(sb, EXT3_MIN_BLOCK_SIZE);
if (!blocksize) {
printk(KERN_ERR "EXT3-fs: unable to set blocksize\n");
@@ -1998,8 +1991,6 @@ static int ext3_fill_super (struct super_block *sb, void *data, int silent)
test_opt(sb,DATA_FLAGS) == EXT3_MOUNT_ORDERED_DATA ? "ordered":
"writeback");
- lock_kernel();
- unlock_kernel();
return 0;
cantfind_ext3:
@@ -2029,8 +2020,6 @@ out_fail:
sb->s_fs_info = NULL;
kfree(sbi->s_blockgroup_lock);
kfree(sbi);
- lock_kernel();
- unlock_kernel();
return ret;
}
--
1.6.4.2
The BKL lock is protecting the remounting against a potential call to
ext3_put_super(). This could not happen, since this is protected by the
s_umount rw semaphore of struct super_block.
Therefore I think the BKL is protecting nothing here.
Signed-off-by: Jan Blunck <[email protected]>
Acked-by: Jan Kara <[email protected]>
---
fs/ext3/super.c | 8 --------
1 files changed, 0 insertions(+), 8 deletions(-)
diff --git a/fs/ext3/super.c b/fs/ext3/super.c
index 373fc54..d02fbc6 100644
--- a/fs/ext3/super.c
+++ b/fs/ext3/super.c
@@ -398,8 +398,6 @@ static void ext3_put_super (struct super_block * sb)
struct ext3_super_block *es = sbi->s_es;
int i, err;
- lock_kernel();
-
ext3_xattr_put_super(sb);
err = journal_destroy(sbi->s_journal);
sbi->s_journal = NULL;
@@ -448,8 +446,6 @@ static void ext3_put_super (struct super_block * sb)
sb->s_fs_info = NULL;
kfree(sbi->s_blockgroup_lock);
kfree(sbi);
-
- unlock_kernel();
}
static struct kmem_cache *ext3_inode_cachep;
@@ -2497,8 +2493,6 @@ static int ext3_remount (struct super_block * sb, int * flags, char * data)
int i;
#endif
- lock_kernel();
-
/* Store the original options */
lock_super(sb);
old_sb_flags = sb->s_flags;
@@ -2609,7 +2603,6 @@ static int ext3_remount (struct super_block * sb, int * flags, char * data)
kfree(old_opts.s_qf_names[i]);
#endif
unlock_super(sb);
- unlock_kernel();
return 0;
restore_opts:
sb->s_flags = old_sb_flags;
@@ -2627,7 +2620,6 @@ restore_opts:
}
#endif
unlock_super(sb);
- unlock_kernel();
return err;
}
--
1.6.4.2
The BKL is still used in ext4_put_super(), ext4_fill_super() and
ext4_remount(). All three calles are protected against concurrent calls by
the s_umount rw semaphore of struct super_block.
Therefore the BKL is protecting nothing in this case.
Signed-off-by: Jan Blunck <[email protected]>
Acked-by: "Theodore Ts'o" <[email protected]>
---
fs/ext4/super.c | 16 +---------------
1 files changed, 1 insertions(+), 15 deletions(-)
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index cdfe2a8..86bcab1 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -26,7 +26,6 @@
#include <linux/init.h>
#include <linux/blkdev.h>
#include <linux/parser.h>
-#include <linux/smp_lock.h>
#include <linux/buffer_head.h>
#include <linux/exportfs.h>
#include <linux/vfs.h>
@@ -599,7 +598,6 @@ static void ext4_put_super(struct super_block *sb)
destroy_workqueue(sbi->dio_unwritten_wq);
lock_super(sb);
- lock_kernel();
if (sb->s_dirt)
ext4_commit_super(sb, 1);
@@ -665,7 +663,6 @@ static void ext4_put_super(struct super_block *sb)
* Now that we are completely done shutting down the
* superblock, we need to actually destroy the kobject.
*/
- unlock_kernel();
unlock_super(sb);
kobject_put(&sbi->s_kobj);
wait_for_completion(&sbi->s_kobj_unregister);
@@ -2330,19 +2327,14 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
int err;
unsigned int journal_ioprio = DEFAULT_JOURNAL_IOPRIO;
- lock_kernel();
-
sbi = kzalloc(sizeof(*sbi), GFP_KERNEL);
- if (!sbi) {
- unlock_kernel();
+ if (!sbi)
return -ENOMEM;
- }
sbi->s_blockgroup_lock =
kzalloc(sizeof(struct blockgroup_lock), GFP_KERNEL);
if (!sbi->s_blockgroup_lock) {
kfree(sbi);
- unlock_kernel();
return -ENOMEM;
}
sb->s_fs_info = sbi;
@@ -2354,8 +2346,6 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
sbi->s_sectors_written_start = part_stat_read(sb->s_bdev->bd_part,
sectors[1]);
- unlock_kernel();
-
/* Cleanup superblock name */
for (cp = sb->s_id; (cp = strchr(cp, '/'));)
*cp = '!';
@@ -3464,8 +3454,6 @@ static int ext4_remount(struct super_block *sb, int *flags, char *data)
int i;
#endif
- lock_kernel();
-
/* Store the original options */
lock_super(sb);
old_sb_flags = sb->s_flags;
@@ -3595,7 +3583,6 @@ static int ext4_remount(struct super_block *sb, int *flags, char *data)
kfree(old_opts.s_qf_names[i]);
#endif
unlock_super(sb);
- unlock_kernel();
return 0;
restore_opts:
@@ -3616,7 +3603,6 @@ restore_opts:
}
#endif
unlock_super(sb);
- unlock_kernel();
return err;
}
--
1.6.4.2
We don't need the BKL in exofs.
Signed-off-by: Boaz Harrosh <[email protected]>
Signed-off-by: Jan Blunck <[email protected]>
---
fs/exofs/super.c | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/fs/exofs/super.c b/fs/exofs/super.c
index 9f500de..11e35b1 100644
--- a/fs/exofs/super.c
+++ b/fs/exofs/super.c
@@ -31,7 +31,6 @@
* Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
*/
-#include <linux/smp_lock.h>
#include <linux/string.h>
#include <linux/parser.h>
#include <linux/vfs.h>
--
1.6.4.2
The BKL is only used in put_super and fill_super that are both protected by
the superblocks s_umount rw_semaphore. Therefore it is safe to remove the
BKL entirely.
Signed-off-by: Jan Blunck <[email protected]>
---
fs/hfs/super.c | 14 ++------------
1 files changed, 2 insertions(+), 12 deletions(-)
diff --git a/fs/hfs/super.c b/fs/hfs/super.c
index a2e19ff..6e0ff34 100644
--- a/fs/hfs/super.c
+++ b/fs/hfs/super.c
@@ -19,7 +19,6 @@
#include <linux/nls.h>
#include <linux/parser.h>
#include <linux/seq_file.h>
-#include <linux/smp_lock.h>
#include <linux/vfs.h>
#include "hfs_fs.h"
@@ -78,15 +77,11 @@ static int hfs_sync_fs(struct super_block *sb, int wait)
*/
static void hfs_put_super(struct super_block *sb)
{
- lock_kernel();
-
if (sb->s_dirt)
hfs_write_super(sb);
hfs_mdb_close(sb);
/* release the MDB's resources */
hfs_mdb_put(sb);
-
- unlock_kernel();
}
/*
@@ -381,13 +376,10 @@ static int hfs_fill_super(struct super_block *sb, void *data, int silent)
struct inode *root_inode;
int res;
- lock_kernel();
-
sbi = kzalloc(sizeof(struct hfs_sb_info), GFP_KERNEL);
- if (!sbi) {
- unlock_kernel();
+ if (!sbi)
return -ENOMEM;
- }
+
sb->s_fs_info = sbi;
INIT_HLIST_HEAD(&sbi->rsrc_inodes);
@@ -433,7 +425,6 @@ static int hfs_fill_super(struct super_block *sb, void *data, int silent)
sb->s_root->d_op = &hfs_dentry_operations;
/* everything's okay */
- unlock_kernel();
return 0;
bail_iput:
@@ -442,7 +433,6 @@ bail_no_root:
printk(KERN_ERR "hfs: get root inode failed.\n");
bail:
hfs_mdb_put(sb);
- unlock_kernel();
return res;
}
--
1.6.4.2
The BKL is only used in put_super and fill_super that are both protected by
the superblocks s_umount rw_semaphore. Therefore it is safe to remove the
BKL entirely.
Signed-off-by: Jan Blunck <[email protected]>
---
fs/hfsplus/super.c | 13 +------------
1 files changed, 1 insertions(+), 12 deletions(-)
diff --git a/fs/hfsplus/super.c b/fs/hfsplus/super.c
index 824f57a..8a77d43 100644
--- a/fs/hfsplus/super.c
+++ b/fs/hfsplus/super.c
@@ -12,7 +12,6 @@
#include <linux/pagemap.h>
#include <linux/fs.h>
#include <linux/slab.h>
-#include <linux/smp_lock.h>
#include <linux/vfs.h>
#include <linux/nls.h>
@@ -210,8 +209,6 @@ static void hfsplus_put_super(struct super_block *sb)
if (!sb->s_fs_info)
return;
- lock_kernel();
-
if (sb->s_dirt)
hfsplus_write_super(sb);
if (!(sb->s_flags & MS_RDONLY) && HFSPLUS_SB(sb).s_vhdr) {
@@ -232,8 +229,6 @@ static void hfsplus_put_super(struct super_block *sb)
unload_nls(HFSPLUS_SB(sb).nls);
kfree(sb->s_fs_info);
sb->s_fs_info = NULL;
-
- unlock_kernel();
}
static int hfsplus_statfs(struct dentry *dentry, struct kstatfs *buf)
@@ -312,13 +307,9 @@ static int hfsplus_fill_super(struct super_block *sb, void *data, int silent)
struct nls_table *nls = NULL;
int err = -EINVAL;
- lock_kernel();
-
sbi = kzalloc(sizeof(*sbi), GFP_KERNEL);
- if (!sbi) {
- unlock_kernel();
+ if (!sbi)
return -ENOMEM;
- }
sb->s_fs_info = sbi;
INIT_HLIST_HEAD(&sbi->rsrc_inodes);
@@ -463,13 +454,11 @@ static int hfsplus_fill_super(struct super_block *sb, void *data, int silent)
out:
unload_nls(sbi->nls);
sbi->nls = nls;
- unlock_kernel();
return 0;
cleanup:
hfsplus_put_super(sb);
unload_nls(nls);
- unlock_kernel();
return err;
}
--
1.6.4.2
The BKL is only used in put_super, fill_super and remount_fs that are all
three protected by the superblocks s_umount rw_semaphore. Therefore it is
safe to remove the BKL entirely.
Signed-off-by: Jan Blunck <[email protected]>
---
fs/jfs/super.c | 36 +++++++-----------------------------
1 files changed, 7 insertions(+), 29 deletions(-)
diff --git a/fs/jfs/super.c b/fs/jfs/super.c
index a8e6046..5ccbb32 100644
--- a/fs/jfs/super.c
+++ b/fs/jfs/super.c
@@ -32,7 +32,6 @@
#include <linux/crc32.h>
#include <asm/uaccess.h>
#include <linux/seq_file.h>
-#include <linux/smp_lock.h>
#include "jfs_incore.h"
#include "jfs_filsys.h"
@@ -173,8 +172,6 @@ static void jfs_put_super(struct super_block *sb)
jfs_info("In jfs_put_super");
- lock_kernel();
-
rc = jfs_umount(sb);
if (rc)
jfs_err("jfs_umount failed with return code %d", rc);
@@ -185,8 +182,6 @@ static void jfs_put_super(struct super_block *sb)
iput(sbi->direct_inode);
kfree(sbi);
-
- unlock_kernel();
}
enum {
@@ -366,19 +361,16 @@ static int jfs_remount(struct super_block *sb, int *flags, char *data)
if (!parse_options(data, sb, &newLVSize, &flag)) {
return -EINVAL;
}
- lock_kernel();
+
if (newLVSize) {
if (sb->s_flags & MS_RDONLY) {
printk(KERN_ERR
"JFS: resize requires volume to be mounted read-write\n");
- unlock_kernel();
return -EROFS;
}
rc = jfs_extendfs(sb, newLVSize, 0);
- if (rc) {
- unlock_kernel();
+ if (rc)
return rc;
- }
}
if ((sb->s_flags & MS_RDONLY) && !(*flags & MS_RDONLY)) {
@@ -390,30 +382,25 @@ static int jfs_remount(struct super_block *sb, int *flags, char *data)
JFS_SBI(sb)->flag = flag;
ret = jfs_mount_rw(sb, 1);
- unlock_kernel();
return ret;
}
if ((!(sb->s_flags & MS_RDONLY)) && (*flags & MS_RDONLY)) {
rc = jfs_umount_rw(sb);
JFS_SBI(sb)->flag = flag;
- unlock_kernel();
return rc;
}
if ((JFS_SBI(sb)->flag & JFS_NOINTEGRITY) != (flag & JFS_NOINTEGRITY))
if (!(sb->s_flags & MS_RDONLY)) {
rc = jfs_umount_rw(sb);
- if (rc) {
- unlock_kernel();
+ if (rc)
return rc;
- }
+
JFS_SBI(sb)->flag = flag;
ret = jfs_mount_rw(sb, 1);
- unlock_kernel();
return ret;
}
JFS_SBI(sb)->flag = flag;
- unlock_kernel();
return 0;
}
@@ -425,20 +412,15 @@ static int jfs_fill_super(struct super_block *sb, void *data, int silent)
s64 newLVSize = 0;
int flag, ret = -EINVAL;
- lock_kernel();
-
jfs_info("In jfs_read_super: s_flags=0x%lx", sb->s_flags);
- if (!new_valid_dev(sb->s_bdev->bd_dev)) {
- unlock_kernel();
+ if (!new_valid_dev(sb->s_bdev->bd_dev))
return -EOVERFLOW;
- }
sbi = kzalloc(sizeof (struct jfs_sb_info), GFP_KERNEL);
- if (!sbi) {
- unlock_kernel();
+ if (!sbi)
return -ENOMEM;
- }
+
sb->s_fs_info = sbi;
sbi->sb = sb;
sbi->uid = sbi->gid = sbi->umask = -1;
@@ -448,7 +430,6 @@ static int jfs_fill_super(struct super_block *sb, void *data, int silent)
if (!parse_options((char *) data, sb, &newLVSize, &flag)) {
kfree(sbi);
- unlock_kernel();
return -EINVAL;
}
sbi->flag = flag;
@@ -460,7 +441,6 @@ static int jfs_fill_super(struct super_block *sb, void *data, int silent)
if (newLVSize) {
printk(KERN_ERR "resize option for remount only\n");
kfree(sbi);
- unlock_kernel();
return -EINVAL;
}
@@ -536,7 +516,6 @@ static int jfs_fill_super(struct super_block *sb, void *data, int silent)
sb->s_maxbytes = min(((u64) PAGE_CACHE_SIZE << 32) - 1, sb->s_maxbytes);
#endif
sb->s_time_gran = 1;
- unlock_kernel();
return 0;
out_no_root:
@@ -558,7 +537,6 @@ out_kfree:
if (sbi->nls_tab)
unload_nls(sbi->nls_tab);
kfree(sbi);
- unlock_kernel();
return ret;
}
--
1.6.4.2
The BKL is only used in put_super, fill_super and remount_fs that are all
three protected by the superblocks s_umount rw_semaphore. Therefore it is
safe to remove the BKL entirely.
Signed-off-by: Jan Blunck <[email protected]>
---
fs/nilfs2/ioctl.c | 1 -
fs/nilfs2/super.c | 19 +------------------
2 files changed, 1 insertions(+), 19 deletions(-)
diff --git a/fs/nilfs2/ioctl.c b/fs/nilfs2/ioctl.c
index f6af760..30f7b41 100644
--- a/fs/nilfs2/ioctl.c
+++ b/fs/nilfs2/ioctl.c
@@ -22,7 +22,6 @@
#include <linux/fs.h>
#include <linux/wait.h>
-#include <linux/smp_lock.h> /* lock_kernel(), unlock_kernel() */
#include <linux/capability.h> /* capable() */
#include <linux/uaccess.h> /* copy_from_user(), copy_to_user() */
#include <linux/vmalloc.h>
diff --git a/fs/nilfs2/super.c b/fs/nilfs2/super.c
index 3448ec3..0086dc2 100644
--- a/fs/nilfs2/super.c
+++ b/fs/nilfs2/super.c
@@ -45,7 +45,6 @@
#include <linux/parser.h>
#include <linux/random.h>
#include <linux/crc32.h>
-#include <linux/smp_lock.h>
#include <linux/vfs.h>
#include <linux/writeback.h>
#include <linux/kobject.h>
@@ -310,8 +309,6 @@ static void nilfs_put_super(struct super_block *sb)
struct nilfs_sb_info *sbi = NILFS_SB(sb);
struct the_nilfs *nilfs = sbi->s_nilfs;
- lock_kernel();
-
nilfs_detach_segment_constructor(sbi);
if (!(sb->s_flags & MS_RDONLY)) {
@@ -330,8 +327,6 @@ static void nilfs_put_super(struct super_block *sb)
sbi->s_super = NULL;
sb->s_fs_info = NULL;
nilfs_put_sbinfo(sbi);
-
- unlock_kernel();
}
static int nilfs_sync_fs(struct super_block *sb, int wait)
@@ -893,8 +888,6 @@ static int nilfs_remount(struct super_block *sb, int *flags, char *data)
struct nilfs_mount_options old_opts;
int err;
- lock_kernel();
-
down_write(&nilfs->ns_super_sem);
old_sb_flags = sb->s_flags;
old_opts.mount_opt = sbi->s_mount_opt;
@@ -974,7 +967,6 @@ static int nilfs_remount(struct super_block *sb, int *flags, char *data)
}
out:
up_write(&nilfs->ns_super_sem);
- unlock_kernel();
return 0;
restore_opts:
@@ -982,7 +974,6 @@ static int nilfs_remount(struct super_block *sb, int *flags, char *data)
sbi->s_mount_opt = old_opts.mount_opt;
sbi->s_snapshot_cno = old_opts.snapshot_cno;
up_write(&nilfs->ns_super_sem);
- unlock_kernel();
return err;
}
@@ -1059,13 +1050,9 @@ nilfs_get_sb(struct file_system_type *fs_type, int flags,
struct the_nilfs *nilfs;
int err, need_to_close = 1;
- lock_kernel();
-
sd.bdev = open_bdev_exclusive(dev_name, flags, fs_type);
- if (IS_ERR(sd.bdev)) {
- unlock_kernel();
+ if (IS_ERR(sd.bdev))
return PTR_ERR(sd.bdev);
- }
/*
* To get mount instance using sget() vfs-routine, NILFS needs
@@ -1146,7 +1133,6 @@ nilfs_get_sb(struct file_system_type *fs_type, int flags,
if (need_to_close)
close_bdev_exclusive(sd.bdev, flags);
simple_set_mnt(mnt, s);
- unlock_kernel();
return 0;
failed_unlock:
@@ -1154,8 +1140,6 @@ nilfs_get_sb(struct file_system_type *fs_type, int flags,
put_nilfs(nilfs);
failed:
close_bdev_exclusive(sd.bdev, flags);
-
- unlock_kernel();
return err;
cancel_new:
@@ -1169,7 +1153,6 @@ nilfs_get_sb(struct file_system_type *fs_type, int flags,
* We must finish all post-cleaning before this call;
* put_nilfs() needs the block device.
*/
- unlock_kernel();
return err;
}
--
1.6.4.2
The BKL is only used in put_super, fill_super and remount_fs that are all
three protected by the superblocks s_umount rw_semaphore. Therefore it is
safe to remove the BKL entirely.
Signed-off-by: Jan Blunck <[email protected]>
---
fs/ntfs/super.c | 29 ++---------------------------
1 files changed, 2 insertions(+), 27 deletions(-)
diff --git a/fs/ntfs/super.c b/fs/ntfs/super.c
index ab09c02..4aa55d7 100644
--- a/fs/ntfs/super.c
+++ b/fs/ntfs/super.c
@@ -30,7 +30,6 @@
#include <linux/buffer_head.h>
#include <linux/vfs.h>
#include <linux/moduleparam.h>
-#include <linux/smp_lock.h>
#include "sysctl.h"
#include "logfile.h"
@@ -443,7 +442,6 @@ static int ntfs_remount(struct super_block *sb, int *flags, char *opt)
ntfs_debug("Entering with remount options string: %s", opt);
- lock_kernel();
#ifndef NTFS_RW
/* For read-only compiled driver, enforce read-only flag. */
*flags |= MS_RDONLY;
@@ -467,18 +465,15 @@ static int ntfs_remount(struct super_block *sb, int *flags, char *opt)
if (NVolErrors(vol)) {
ntfs_error(sb, "Volume has errors and is read-only%s",
es);
- unlock_kernel();
return -EROFS;
}
if (vol->vol_flags & VOLUME_IS_DIRTY) {
ntfs_error(sb, "Volume is dirty and read-only%s", es);
- unlock_kernel();
return -EROFS;
}
if (vol->vol_flags & VOLUME_MODIFIED_BY_CHKDSK) {
ntfs_error(sb, "Volume has been modified by chkdsk "
"and is read-only%s", es);
- unlock_kernel();
return -EROFS;
}
if (vol->vol_flags & VOLUME_MUST_MOUNT_RO_MASK) {
@@ -486,13 +481,11 @@ static int ntfs_remount(struct super_block *sb, int *flags, char *opt)
"(0x%x) and is read-only%s",
(unsigned)le16_to_cpu(vol->vol_flags),
es);
- unlock_kernel();
return -EROFS;
}
if (ntfs_set_volume_flags(vol, VOLUME_IS_DIRTY)) {
ntfs_error(sb, "Failed to set dirty bit in volume "
"information flags%s", es);
- unlock_kernel();
return -EROFS;
}
#if 0
@@ -512,21 +505,18 @@ static int ntfs_remount(struct super_block *sb, int *flags, char *opt)
ntfs_error(sb, "Failed to empty journal $LogFile%s",
es);
NVolSetErrors(vol);
- unlock_kernel();
return -EROFS;
}
if (!ntfs_mark_quotas_out_of_date(vol)) {
ntfs_error(sb, "Failed to mark quotas out of date%s",
es);
NVolSetErrors(vol);
- unlock_kernel();
return -EROFS;
}
if (!ntfs_stamp_usnjrnl(vol)) {
ntfs_error(sb, "Failed to stamp transation log "
"($UsnJrnl)%s", es);
NVolSetErrors(vol);
- unlock_kernel();
return -EROFS;
}
} else if (!(sb->s_flags & MS_RDONLY) && (*flags & MS_RDONLY)) {
@@ -542,11 +532,9 @@ static int ntfs_remount(struct super_block *sb, int *flags, char *opt)
// TODO: Deal with *flags.
- if (!parse_options(vol, opt)) {
- unlock_kernel();
+ if (!parse_options(vol, opt))
return -EINVAL;
- }
- unlock_kernel();
+
ntfs_debug("Done.");
return 0;
}
@@ -2259,8 +2247,6 @@ static void ntfs_put_super(struct super_block *sb)
ntfs_debug("Entering.");
- lock_kernel();
-
#ifdef NTFS_RW
/*
* Commit all inodes while they are still open in case some of them
@@ -2431,8 +2417,6 @@ static void ntfs_put_super(struct super_block *sb)
sb->s_fs_info = NULL;
kfree(vol);
-
- unlock_kernel();
}
/**
@@ -2723,8 +2707,6 @@ static int ntfs_fill_super(struct super_block *sb, void *opt, const int silent)
struct inode *tmp_ino;
int blocksize, result;
- lock_kernel();
-
/*
* We do a pretty difficult piece of bootstrap by reading the
* MFT (and other metadata) from disk into memory. We'll only
@@ -2748,7 +2730,6 @@ static int ntfs_fill_super(struct super_block *sb, void *opt, const int silent)
ntfs_error(sb, "Allocation of NTFS volume structure "
"failed. Aborting mount...");
lockdep_on();
- unlock_kernel();
return -ENOMEM;
}
/* Initialize ntfs_volume structure. */
@@ -2766,8 +2747,6 @@ static int ntfs_fill_super(struct super_block *sb, void *opt, const int silent)
init_rwsem(&vol->mftbmp_lock);
init_rwsem(&vol->lcnbmp_lock);
- unlock_kernel();
-
/* By default, enable sparse support. */
NVolSetSparseEnabled(vol);
@@ -2934,9 +2913,7 @@ static int ntfs_fill_super(struct super_block *sb, void *opt, const int silent)
}
mutex_unlock(&ntfs_lock);
sb->s_export_op = &ntfs_export_ops;
- lock_kernel();
lockdep_on();
- unlock_kernel();
return 0;
}
ntfs_error(sb, "Failed to allocate root directory.");
@@ -3052,12 +3029,10 @@ iput_tmp_ino_err_out_now:
}
/* Errors at this stage are irrelevant. */
err_out_now:
- lock_kernel();
sb->s_fs_info = NULL;
kfree(vol);
ntfs_debug("Failed, returning -EINVAL.");
lockdep_on();
- unlock_kernel();
return -EINVAL;
}
--
1.6.4.2
The BKL is only used in remount_fs and get_sb that are both protected by
the superblocks s_umount rw_semaphore. Therefore it is safe to remove the
BKL entirely.
Signed-off-by: Jan Blunck <[email protected]>
---
kernel/cgroup.c | 8 --------
1 files changed, 0 insertions(+), 8 deletions(-)
diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index b86c378..a8844f3 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -47,7 +47,6 @@
#include <linux/cgroupstats.h>
#include <linux/hash.h>
#include <linux/namei.h>
-#include <linux/smp_lock.h>
#include <linux/pid_namespace.h>
#include <linux/idr.h>
#include <linux/vmalloc.h> /* TODO: replace with more sophisticated array */
@@ -1082,7 +1081,6 @@ static int cgroup_remount(struct super_block *sb, int *flags, char *data)
struct cgroup *cgrp = &root->top_cgroup;
struct cgroup_sb_opts opts;
- lock_kernel();
mutex_lock(&cgrp->dentry->d_inode->i_mutex);
mutex_lock(&cgroup_mutex);
@@ -1117,7 +1115,6 @@ static int cgroup_remount(struct super_block *sb, int *flags, char *data)
kfree(opts.name);
mutex_unlock(&cgroup_mutex);
mutex_unlock(&cgrp->dentry->d_inode->i_mutex);
- unlock_kernel();
return ret;
}
@@ -1290,8 +1287,6 @@ static int cgroup_get_sb(struct file_system_type *fs_type,
struct super_block *sb;
struct cgroupfs_root *new_root;
- lock_kernel();
-
/* First find the desired set of subsystems */
ret = parse_cgroupfs_options(data, &opts);
if (ret)
@@ -1412,7 +1407,6 @@ static int cgroup_get_sb(struct file_system_type *fs_type,
simple_set_mnt(mnt, sb);
kfree(opts.release_agent);
kfree(opts.name);
- unlock_kernel();
return 0;
drop_new_super:
@@ -1420,8 +1414,6 @@ static int cgroup_get_sb(struct file_system_type *fs_type,
out_err:
kfree(opts.release_agent);
kfree(opts.name);
- unlock_kernel();
-
return ret;
}
--
1.6.4.2
After pushing down the BKL to the get_sb/fill_super operations of the
filesystems that still make usage of the BKL it is safe to remove it from
do_new_mount().
I've read through all the code formerly covered by the BKL inside
do_kern_mount() and have satisfied myself that it doesn't need the BKL
any more.
Signed-off-by: Jan Blunck <[email protected]>
Cc: Matthew Wilcox <[email protected]>
---
fs/namespace.c | 2 --
1 files changed, 0 insertions(+), 2 deletions(-)
diff --git a/fs/namespace.c b/fs/namespace.c
index bdc3cb4..3f95497 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -1647,9 +1647,7 @@ static int do_new_mount(struct path *path, char *type, int flags,
if (!capable(CAP_SYS_ADMIN))
return -EPERM;
- lock_kernel();
mnt = do_kern_mount(type, flags, name, data);
- unlock_kernel();
if (IS_ERR(mnt))
return PTR_ERR(mnt);
--
1.6.4.2
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 <[email protected]>
Cc: Andi Kleen <[email protected]>
Cc: Jan Kara <[email protected]>
---
fs/ext2/inode.c | 2 ++
fs/ext2/super.c | 42 +++++++++++++++++++++++++++++++++++++-----
include/linux/ext2_fs_sb.h | 6 ++++++
3 files changed, 45 insertions(+), 5 deletions(-)
diff --git a/fs/ext2/inode.c b/fs/ext2/inode.c
index ade6340..65a0d4b 100644
--- a/fs/ext2/inode.c
+++ b/fs/ext2/inode.c
@@ -1401,9 +1401,11 @@ 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 5af1775..0ca42fe 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(&EXT2_SB(sb)->s_lock);
ext2_sync_super(sb, es);
}
@@ -84,6 +86,9 @@ void ext2_warning (struct super_block * sb, const char * function,
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;
@@ -117,14 +122,16 @@ static void ext2_put_super (struct super_block * sb)
lock_kernel();
- if (sb->s_dirt)
- ext2_write_super(sb);
+ if (sb->s_dirt && !(sb->s_flags & MS_RDONLY))
+ ext2_sync_fs(sb, 1);
ext2_xattr_put_super(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(&EXT2_SB(sb)->s_lock);
ext2_sync_super(sb, es);
}
db_count = sbi->s_gdb_count;
@@ -207,6 +214,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)
@@ -279,6 +287,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;
}
@@ -762,6 +771,12 @@ static int ext2_fill_super(struct super_block *sb, void *data, int silent)
sbi->s_sb_block = sb_block;
/*
+ * mutex for protection of modifications of the superblock while being
+ * write out by ext2_write_super() or ext2_sync_fs().
+ */
+ 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
* is smaller than the default) use the default.
@@ -1103,11 +1118,15 @@ static void ext2_commit_super (struct super_block * sb,
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)
{
+ 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);
sync_dirty_buffer(EXT2_SB(sb)->s_sbh);
sb->s_dirt = 0;
@@ -1122,13 +1141,16 @@ static void ext2_sync_super(struct super_block *sb, struct ext2_super_block *es)
* flags to 0. We need to set this flag to 0 since the fs
* may have been checked while mounted and e2fsck may have
* set s_state to EXT2_VALID_FS after some corrections.
+ *
+ * This must be called with sbi->s_lock held.
*/
-
static int ext2_sync_fs(struct super_block *sb, int wait)
{
- struct ext2_super_block *es = EXT2_SB(sb)->s_es;
+ struct ext2_sb_info *sbi = EXT2_SB(sb);
+ struct ext2_super_block *es = sbi->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);
@@ -1137,9 +1159,11 @@ static int ext2_sync_fs(struct super_block *sb, int wait)
es->s_free_inodes_count =
cpu_to_le32(ext2_count_free_inodes(sb));
es->s_mtime = cpu_to_le32(get_seconds());
+ spin_unlock(&EXT2_SB(sb)->s_lock);
ext2_sync_super(sb, es);
} else {
ext2_commit_super(sb, es);
+ spin_unlock(&sbi->s_lock);
}
sb->s_dirt = 0;
unlock_kernel();
@@ -1166,6 +1190,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;
@@ -1203,12 +1228,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;
}
@@ -1237,6 +1264,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;
}
+ spin_unlock(&EXT2_SB(sb)->s_lock);
ext2_sync_super(sb, es);
unlock_kernel();
return 0;
@@ -1245,6 +1273,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;
}
@@ -1256,6 +1285,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)) {
@@ -1310,6 +1341,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/include/linux/ext2_fs_sb.h b/include/linux/ext2_fs_sb.h
index 1cdb663..dbc5934 100644
--- a/include/linux/ext2_fs_sb.h
+++ b/include/linux/ext2_fs_sb.h
@@ -106,6 +106,12 @@ 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;
+ /*
+ * 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.
+ */
+ spinlock_t s_lock;
};
static inline spinlock_t *
--
1.6.4.2
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 <[email protected]>
Cc: Jan Kara <[email protected]>
---
fs/ext2/inode.c | 3 ---
fs/ext2/super.c | 16 ----------------
2 files changed, 0 insertions(+), 19 deletions(-)
diff --git a/fs/ext2/inode.c b/fs/ext2/inode.c
index 65a0d4b..5afb161 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>
@@ -1400,13 +1399,11 @@ 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 0ca42fe..a764002 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 && !(sb->s_flags & MS_RDONLY))
ext2_sync_fs(sb, 1);
@@ -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;
@@ -754,8 +749,6 @@ static int ext2_fill_super(struct super_block *sb, void *data, int silent)
__le32 features;
int err;
- lock_kernel();
-
err = -ENOMEM;
sbi = kzalloc(sizeof(*sbi), GFP_KERNEL);
if (!sbi)
@@ -1081,7 +1074,6 @@ static int ext2_fill_super(struct super_block *sb, void *data, int silent)
ext2_warning(sb, __func__,
"mounting ext3 filesystem as ext2");
ext2_setup_super (sb, es, sb->s_flags & MS_RDONLY);
- unlock_kernel();
return 0;
cantfind_ext2:
@@ -1106,7 +1098,6 @@ failed_sbi:
kfree(sbi->s_blockgroup_lock);
kfree(sbi);
failed_unlock:
- unlock_kernel();
return ret;
}
@@ -1149,7 +1140,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 = sbi->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");
@@ -1166,7 +1156,6 @@ static int ext2_sync_fs(struct super_block *sb, int wait)
spin_unlock(&sbi->s_lock);
}
sb->s_dirt = 0;
- unlock_kernel();
return 0;
}
@@ -1189,7 +1178,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 */
@@ -1229,14 +1217,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;
}
/*
@@ -1266,7 +1252,6 @@ static int ext2_remount (struct super_block * sb, int * flags, char * data)
}
spin_unlock(&EXT2_SB(sb)->s_lock);
ext2_sync_super(sb, es);
- unlock_kernel();
return 0;
restore_opts:
sbi->s_mount_opt = old_opts.s_mount_opt;
@@ -1274,7 +1259,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 <[email protected]> writes:
> +static void ext2_sync_super(struct super_block *sb,
> + struct ext2_super_block *es)
> {
> + 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);
> sync_dirty_buffer(EXT2_SB(sb)->s_sbh);
> sb->s_dirt = 0;
[...]
> static int ext2_sync_fs(struct super_block *sb, int wait)
> {
> - struct ext2_super_block *es = EXT2_SB(sb)->s_es;
> + struct ext2_sb_info *sbi = EXT2_SB(sb);
> + struct ext2_super_block *es = sbi->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);
> @@ -1137,9 +1159,11 @@ static int ext2_sync_fs(struct super_block *sb, int wait)
> es->s_free_inodes_count =
> cpu_to_le32(ext2_count_free_inodes(sb));
> es->s_mtime = cpu_to_le32(get_seconds());
> + spin_unlock(&EXT2_SB(sb)->s_lock);
> ext2_sync_super(sb, es);
> } else {
> ext2_commit_super(sb, es);
> + spin_unlock(&sbi->s_lock);
> }
> sb->s_dirt = 0;
[...]
> @@ -1237,6 +1264,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;
> }
> + spin_unlock(&EXT2_SB(sb)->s_lock);
> ext2_sync_super(sb, es);
> unlock_kernel();
> return 0;
ext2_setup_super() will call ext2_sync_fs(), so ->s_lock seems to be
recursive in here.
Thanks.
--
OGAWA Hirofumi <[email protected]>
On Wed, 18 Nov 2009 10:24:48 +0100, Jan Blunck <[email protected]> wrote:
> The BKL is only used in put_super, fill_super and remount_fs that are all
> three protected by the superblocks s_umount rw_semaphore. Therefore it is
> safe to remove the BKL entirely.
>
> Signed-off-by: Jan Blunck <[email protected]>
All right, thanks.
Acked-by: Ryusuke Konishi <[email protected]>
Ryusuke Konishi
On Wed, Nov 18, 2009 at 1:24 AM, Jan Blunck <[email protected]> wrote:
> The BKL is only used in remount_fs and get_sb that are both protected by
> the superblocks s_umount rw_semaphore. Therefore it is safe to remove the
> BKL entirely.
>
> Signed-off-by: Jan Blunck <[email protected]>
Acked-by: Paul Menage <[email protected]>