linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PTCH] push down lock_super and BKL into ->put_super
@ 2009-05-05 13:40 Christoph Hellwig
  2009-05-06  2:09 ` Al Viro
  0 siblings, 1 reply; 5+ messages in thread
From: Christoph Hellwig @ 2009-05-05 13:40 UTC (permalink / raw)
  To: viro; +Cc: linux-fsdevel

From: Christoph Hellwig <hch@lst.de>

Move s_lock and BKL into ->put_super from the only caller.  A couple of
filesystems had trivial enough ->put_super (only kfree and NULLing of
s_fs_info + stuff in there) to not get any locking: code, cramfs, efs,
hugetlbfs, omfs, qnx4, shmem, all others got the full treatment.  Most
of them probably don't need it, but I'd rather sort that out individually.
Preferably after all the other BKL and lock_super pushdowns in that area.


Signed-off-by: Christoph Hellwig <hch@lst.de>

Index: vfs-2.6/fs/adfs/super.c
===================================================================
--- vfs-2.6.orig/fs/adfs/super.c	2009-05-05 14:57:45.469659341 +0200
+++ vfs-2.6/fs/adfs/super.c	2009-05-05 15:04:32.612661504 +0200
@@ -132,11 +132,17 @@ static void adfs_put_super(struct super_
 	int i;
 	struct adfs_sb_info *asb = ADFS_SB(sb);
 
+	lock_super(sb);
+	lock_kernel();
+
 	for (i = 0; i < asb->s_map_size; i++)
 		brelse(asb->s_map[i].dm_bh);
 	kfree(asb->s_map);
 	kfree(asb);
 	sb->s_fs_info = NULL;
+
+	unlock_kernel();
+	unlock_super(sb);
 }
 
 static int adfs_show_options(struct seq_file *seq, struct vfsmount *mnt)
Index: vfs-2.6/fs/affs/super.c
===================================================================
--- vfs-2.6.orig/fs/affs/super.c	2009-05-05 14:57:45.529784383 +0200
+++ vfs-2.6/fs/affs/super.c	2009-05-05 15:04:42.143661606 +0200
@@ -29,6 +29,9 @@ affs_put_super(struct super_block *sb)
 	struct affs_sb_info *sbi = AFFS_SB(sb);
 	pr_debug("AFFS: put_super()\n");
 
+	lock_super(sb);
+	lock_kernel();
+
 	if (!(sb->s_flags & MS_RDONLY)) {
 		AFFS_ROOT_TAIL(sb, sbi->s_root_bh)->bm_flag = cpu_to_be32(1);
 		secs_to_datestamp(get_seconds(),
@@ -42,7 +45,9 @@ affs_put_super(struct super_block *sb)
 	affs_brelse(sbi->s_root_bh);
 	kfree(sbi);
 	sb->s_fs_info = NULL;
-	return;
+
+	unlock_kernel();
+	unlock_super(sb);
 }
 
 static void
Index: vfs-2.6/fs/afs/super.c
===================================================================
--- vfs-2.6.orig/fs/afs/super.c	2009-05-05 14:57:45.552688905 +0200
+++ vfs-2.6/fs/afs/super.c	2009-05-05 15:04:45.961661517 +0200
@@ -441,8 +441,14 @@ static void afs_put_super(struct super_b
 
 	_enter("");
 
+	lock_super(sb);
+	lock_kernel();
+
 	afs_put_volume(as->volume);
 
+	unlock_kernel();
+	unlock_super(sb);
+
 	_leave("");
 }
 
Index: vfs-2.6/fs/befs/linuxvfs.c
===================================================================
--- vfs-2.6.orig/fs/befs/linuxvfs.c	2009-05-05 14:57:45.602783758 +0200
+++ vfs-2.6/fs/befs/linuxvfs.c	2009-05-05 15:04:55.222786299 +0200
@@ -737,6 +737,9 @@ parse_options(char *options, befs_mount_
 static void
 befs_put_super(struct super_block *sb)
 {
+	lock_super(sb);
+	lock_kernel();
+
 	kfree(BEFS_SB(sb)->mount_opts.iocharset);
 	BEFS_SB(sb)->mount_opts.iocharset = NULL;
 
@@ -747,7 +750,9 @@ befs_put_super(struct super_block *sb)
 
 	kfree(sb->s_fs_info);
 	sb->s_fs_info = NULL;
-	return;
+
+	unlock_kernel();
+	unlock_super(sb);
 }
 
 /* Allocate private field of the superblock, fill it.
Index: vfs-2.6/fs/bfs/inode.c
===================================================================
--- vfs-2.6.orig/fs/bfs/inode.c	2009-05-05 14:57:45.622814869 +0200
+++ vfs-2.6/fs/bfs/inode.c	2009-05-05 15:09:37.327788466 +0200
@@ -217,6 +217,9 @@ static void bfs_put_super(struct super_b
 	if (!info)
 		return;
 
+	lock_super(s);
+	lock_kernel();
+
 	if (s->s_dirt)
 		bfs_write_super(s);
 
@@ -225,6 +228,9 @@ static void bfs_put_super(struct super_b
 	kfree(info->si_imap);
 	kfree(info);
 	s->s_fs_info = NULL;
+
+	unlock_kernel();
+	unlock_super(s);
 }
 
 static int bfs_statfs(struct dentry *dentry, struct kstatfs *buf)
Index: vfs-2.6/fs/btrfs/super.c
===================================================================
--- vfs-2.6.orig/fs/btrfs/super.c	2009-05-05 14:57:45.662783435 +0200
+++ vfs-2.6/fs/btrfs/super.c	2009-05-05 15:05:05.946661835 +0200
@@ -60,8 +60,14 @@ static void btrfs_put_super(struct super
 	struct btrfs_root *root = btrfs_sb(sb);
 	int ret;
 
+	lock_super(sb);
+	lock_kernel();
+
 	ret = close_ctree(root);
 	sb->s_fs_info = NULL;
+
+	unlock_kernel();
+	unlock_super(sb);
 }
 
 enum {
Index: vfs-2.6/fs/cifs/cifsfs.c
===================================================================
--- vfs-2.6.orig/fs/cifs/cifsfs.c	2009-05-05 14:57:45.701784489 +0200
+++ vfs-2.6/fs/cifs/cifsfs.c	2009-05-05 15:05:42.438661785 +0200
@@ -204,6 +204,10 @@ cifs_put_super(struct super_block *sb)
 		cFYI(1, ("Empty cifs superblock info passed to unmount"));
 		return;
 	}
+
+	lock_super(sb);
+	lock_kernel();
+
 	rc = cifs_umount(sb, cifs_sb);
 	if (rc)
 		cERROR(1, ("cifs_umount failed with return code %d", rc));
@@ -216,7 +220,9 @@ cifs_put_super(struct super_block *sb)
 
 	unload_nls(cifs_sb->local_nls);
 	kfree(cifs_sb);
-	return;
+
+	unlock_kernel();
+	unlock_super(sb);
 }
 
 static int
Index: vfs-2.6/fs/ecryptfs/super.c
===================================================================
--- vfs-2.6.orig/fs/ecryptfs/super.c	2009-05-05 14:57:45.793784468 +0200
+++ vfs-2.6/fs/ecryptfs/super.c	2009-05-05 15:12:21.327659608 +0200
@@ -27,6 +27,7 @@
 #include <linux/mount.h>
 #include <linux/key.h>
 #include <linux/seq_file.h>
+#include <linux/smp_lock.h>
 #include <linux/file.h>
 #include <linux/crypto.h>
 #include "ecryptfs_kernel.h"
@@ -120,9 +121,15 @@ static void ecryptfs_put_super(struct su
 {
 	struct ecryptfs_sb_info *sb_info = ecryptfs_superblock_to_private(sb);
 
+	lock_super(sb);
+	lock_kernel();
+
 	ecryptfs_destroy_mount_crypt_stat(&sb_info->mount_crypt_stat);
 	kmem_cache_free(ecryptfs_sb_info_cache, sb_info);
 	ecryptfs_set_superblock_private(sb, NULL);
+
+	unlock_kernel();
+	unlock_super(sb);
 }
 
 /**
Index: vfs-2.6/fs/exofs/super.c
===================================================================
--- vfs-2.6.orig/fs/exofs/super.c	2009-05-05 14:57:45.956784545 +0200
+++ vfs-2.6/fs/exofs/super.c	2009-05-05 15:05:54.103661232 +0200
@@ -258,6 +258,9 @@ static void exofs_put_super(struct super
 	int num_pend;
 	struct exofs_sb_info *sbi = sb->s_fs_info;
 
+	lock_super(sb);
+	lock_kernel();
+
 	if (sb->s_dirt)
 		exofs_write_super(sb);
 
@@ -274,6 +277,9 @@ static void exofs_put_super(struct super
 	osduld_put_device(sbi->s_dev);
 	kfree(sb->s_fs_info);
 	sb->s_fs_info = NULL;
+
+	unlock_kernel();
+	unlock_super(sb);
 }
 
 /*
Index: vfs-2.6/fs/ext2/super.c
===================================================================
--- vfs-2.6.orig/fs/ext2/super.c	2009-05-05 14:57:45.993784785 +0200
+++ vfs-2.6/fs/ext2/super.c	2009-05-05 15:06:02.335661775 +0200
@@ -114,6 +114,9 @@ static void ext2_put_super (struct super
 	int i;
 	struct ext2_sb_info *sbi = EXT2_SB(sb);
 
+	lock_super(sb);
+	lock_kernel();
+
 	if (sb->s_dirt)
 		ext2_write_super(sb);
 
@@ -138,7 +141,8 @@ static void ext2_put_super (struct super
 	kfree(sbi->s_blockgroup_lock);
 	kfree(sbi);
 
-	return;
+	unlock_kernel();
+	unlock_super(sb);
 }
 
 static struct kmem_cache * ext2_inode_cachep;
Index: vfs-2.6/fs/ext3/super.c
===================================================================
--- vfs-2.6.orig/fs/ext3/super.c	2009-05-05 14:57:46.043784306 +0200
+++ vfs-2.6/fs/ext3/super.c	2009-05-05 15:06:10.641661592 +0200
@@ -398,6 +398,9 @@ static void ext3_put_super (struct super
 	struct ext3_super_block *es = sbi->s_es;
 	int i, err;
 
+	lock_super(sb);
+	lock_kernel();
+
 	ext3_xattr_put_super(sb);
 	err = journal_destroy(sbi->s_journal);
 	sbi->s_journal = NULL;
@@ -446,7 +449,9 @@ static void ext3_put_super (struct super
 	sb->s_fs_info = NULL;
 	kfree(sbi->s_blockgroup_lock);
 	kfree(sbi);
-	return;
+
+	unlock_kernel();
+	unlock_super(sb);
 }
 
 static struct kmem_cache *ext3_inode_cachep;
Index: vfs-2.6/fs/ext4/super.c
===================================================================
--- vfs-2.6.orig/fs/ext4/super.c	2009-05-05 14:57:46.075784259 +0200
+++ vfs-2.6/fs/ext4/super.c	2009-05-05 15:06:21.525785642 +0200
@@ -563,6 +563,9 @@ static void ext4_put_super(struct super_
 	struct ext4_super_block *es = sbi->s_es;
 	int i, err;
 
+	lock_super(sb);
+	lock_kernel();
+
 	if (sb->s_dirt)
 		ext4_write_super(sb);
 
@@ -632,7 +635,9 @@ static void ext4_put_super(struct super_
 	lock_kernel();
 	kfree(sbi->s_blockgroup_lock);
 	kfree(sbi);
-	return;
+
+	unlock_kernel();
+	unlock_super(sb);
 }
 
 static struct kmem_cache *ext4_inode_cachep;
Index: vfs-2.6/fs/fat/inode.c
===================================================================
--- vfs-2.6.orig/fs/fat/inode.c	2009-05-05 14:57:46.124784210 +0200
+++ vfs-2.6/fs/fat/inode.c	2009-05-05 15:06:31.523799672 +0200
@@ -451,6 +451,9 @@ static void fat_put_super(struct super_b
 {
 	struct msdos_sb_info *sbi = MSDOS_SB(sb);
 
+	lock_super(sb);
+	lock_kernel();
+
 	if (sb->s_dirt)
 		fat_write_super(sb);
 
@@ -470,6 +473,9 @@ static void fat_put_super(struct super_b
 
 	sb->s_fs_info = NULL;
 	kfree(sbi);
+
+	unlock_kernel();
+	unlock_super(sb);
 }
 
 static struct kmem_cache *fat_inode_cachep;
Index: vfs-2.6/fs/freevxfs/vxfs_super.c
===================================================================
--- vfs-2.6.orig/fs/freevxfs/vxfs_super.c	2009-05-05 14:57:46.203783861 +0200
+++ vfs-2.6/fs/freevxfs/vxfs_super.c	2009-05-05 15:10:18.654661548 +0200
@@ -80,12 +80,18 @@ vxfs_put_super(struct super_block *sbp)
 {
 	struct vxfs_sb_info	*infp = VXFS_SBI(sbp);
 
+	lock_super(sbp);
+	lock_kernel();
+
 	vxfs_put_fake_inode(infp->vsi_fship);
 	vxfs_put_fake_inode(infp->vsi_ilist);
 	vxfs_put_fake_inode(infp->vsi_stilist);
 
 	brelse(infp->vsi_bp);
 	kfree(infp);
+
+	unlock_kernel();
+	unlock_super(sbp);
 }
 
 /**
Index: vfs-2.6/fs/fuse/inode.c
===================================================================
--- vfs-2.6.orig/fs/fuse/inode.c	2009-05-05 14:57:46.216784469 +0200
+++ vfs-2.6/fs/fuse/inode.c	2009-05-05 15:06:39.628661417 +0200
@@ -281,6 +281,9 @@ static void fuse_put_super(struct super_
 {
 	struct fuse_conn *fc = get_fuse_conn_super(sb);
 
+	lock_super(sb);
+	lock_kernel();
+
 	fuse_send_destroy(fc);
 	spin_lock(&fc->lock);
 	fc->connected = 0;
@@ -297,6 +300,9 @@ static void fuse_put_super(struct super_
 	mutex_unlock(&fuse_mutex);
 	bdi_destroy(&fc->bdi);
 	fuse_conn_put(fc);
+
+	unlock_kernel();
+	unlock_super(sb);
 }
 
 static void convert_fuse_statfs(struct kstatfs *stbuf, struct fuse_kstatfs *attr)
Index: vfs-2.6/fs/gfs2/ops_super.c
===================================================================
--- vfs-2.6.orig/fs/gfs2/ops_super.c	2009-05-05 14:57:46.303784439 +0200
+++ vfs-2.6/fs/gfs2/ops_super.c	2009-05-05 15:06:45.139786894 +0200
@@ -132,6 +132,9 @@ static void gfs2_put_super(struct super_
 	struct gfs2_sbd *sdp = sb->s_fs_info;
 	int error;
 
+	lock_super(sb);
+	lock_kernel();
+
 	/*  Unfreeze the filesystem, if we need to  */
 
 	mutex_lock(&sdp->sd_freeze_lock);
@@ -182,6 +185,9 @@ static void gfs2_put_super(struct super_
 
 	/*  At this point, we're through participating in the lockspace  */
 	gfs2_sys_fs_del(sdp);
+
+	unlock_kernel();
+	unlock_super(sb);
 }
 
 /**
Index: vfs-2.6/fs/hfs/super.c
===================================================================
--- vfs-2.6.orig/fs/hfs/super.c	2009-05-05 14:57:46.317783986 +0200
+++ vfs-2.6/fs/hfs/super.c	2009-05-05 15:06:48.366786256 +0200
@@ -65,11 +65,17 @@ static void hfs_write_super(struct super
  */
 static void hfs_put_super(struct super_block *sb)
 {
+	lock_super(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();
+	unlock_super(sb);
 }
 
 /*
Index: vfs-2.6/fs/hfsplus/super.c
===================================================================
--- vfs-2.6.orig/fs/hfsplus/super.c	2009-05-05 14:57:46.402784470 +0200
+++ vfs-2.6/fs/hfsplus/super.c	2009-05-05 15:06:52.071662072 +0200
@@ -199,6 +199,10 @@ static void hfsplus_put_super(struct sup
 	dprint(DBG_SUPER, "hfsplus_put_super\n");
 	if (!sb->s_fs_info)
 		return;
+
+	lock_super(sb);
+	lock_kernel();
+
 	if (sb->s_dirt)
 		hfsplus_write_super(sb);
 	if (!(sb->s_flags & MS_RDONLY) && HFSPLUS_SB(sb).s_vhdr) {
@@ -220,6 +224,9 @@ static void hfsplus_put_super(struct sup
 		unload_nls(HFSPLUS_SB(sb).nls);
 	kfree(sb->s_fs_info);
 	sb->s_fs_info = NULL;
+
+	unlock_kernel();
+	unlock_super(sb);
 }
 
 static int hfsplus_statfs(struct dentry *dentry, struct kstatfs *buf)
Index: vfs-2.6/fs/hpfs/super.c
===================================================================
--- vfs-2.6.orig/fs/hpfs/super.c	2009-05-05 14:57:46.444784439 +0200
+++ vfs-2.6/fs/hpfs/super.c	2009-05-05 15:10:44.132661535 +0200
@@ -99,11 +99,18 @@ int hpfs_stop_cycles(struct super_block 
 static void hpfs_put_super(struct super_block *s)
 {
 	struct hpfs_sb_info *sbi = hpfs_sb(s);
+
+	lock_super(s);
+	lock_kernel();
+
 	kfree(sbi->sb_cp_table);
 	kfree(sbi->sb_bmp_dir);
 	unmark_dirty(s);
 	s->s_fs_info = NULL;
 	kfree(sbi);
+
+	unlock_kernel();
+	unlock_super(s);
 }
 
 unsigned hpfs_count_one_bitmap(struct super_block *s, secno secno)
Index: vfs-2.6/fs/isofs/inode.c
===================================================================
--- vfs-2.6.orig/fs/isofs/inode.c	2009-05-05 14:57:46.545784026 +0200
+++ vfs-2.6/fs/isofs/inode.c	2009-05-05 15:07:01.595661547 +0200
@@ -42,11 +42,18 @@ static int isofs_dentry_cmp_ms(struct de
 static void isofs_put_super(struct super_block *sb)
 {
 	struct isofs_sb_info *sbi = ISOFS_SB(sb);
+
 #ifdef CONFIG_JOLIET
+	lock_super(sb);
+	lock_kernel();
+
 	if (sbi->s_nls_iocharset) {
 		unload_nls(sbi->s_nls_iocharset);
 		sbi->s_nls_iocharset = NULL;
 	}
+
+	unlock_kernel();
+	unlock_super(sb);
 #endif
 
 	kfree(sbi);
Index: vfs-2.6/fs/jffs2/super.c
===================================================================
--- vfs-2.6.orig/fs/jffs2/super.c	2009-05-05 14:57:46.558783935 +0200
+++ vfs-2.6/fs/jffs2/super.c	2009-05-05 15:07:07.168661994 +0200
@@ -174,6 +174,9 @@ static void jffs2_put_super (struct supe
 
 	D2(printk(KERN_DEBUG "jffs2: jffs2_put_super()\n"));
 
+	lock_super(sb);
+	lock_kernel();
+
 	if (sb->s_dirt)
 		jffs2_write_super(sb);
 
@@ -195,6 +198,9 @@ static void jffs2_put_super (struct supe
 	if (c->mtd->sync)
 		c->mtd->sync(c->mtd);
 
+	unlock_kernel();
+	unlock_super(sb);
+
 	D1(printk(KERN_DEBUG "jffs2_put_super returning\n"));
 }
 
Index: vfs-2.6/fs/jfs/super.c
===================================================================
--- vfs-2.6.orig/fs/jfs/super.c	2009-05-05 14:57:46.572784320 +0200
+++ vfs-2.6/fs/jfs/super.c	2009-05-05 15:07:11.874661578 +0200
@@ -183,6 +183,10 @@ static void jfs_put_super(struct super_b
 	int rc;
 
 	jfs_info("In jfs_put_super");
+
+	lock_super(sb);
+	lock_kernel();
+
 	rc = jfs_umount(sb);
 	if (rc)
 		jfs_err("jfs_umount failed with return code %d", rc);
@@ -195,6 +199,9 @@ static void jfs_put_super(struct super_b
 	sbi->direct_inode = NULL;
 
 	kfree(sbi);
+
+	unlock_kernel();
+	unlock_super(sb);
 }
 
 enum {
Index: vfs-2.6/fs/minix/inode.c
===================================================================
--- vfs-2.6.orig/fs/minix/inode.c	2009-05-05 14:57:46.627784059 +0200
+++ vfs-2.6/fs/minix/inode.c	2009-05-05 15:07:19.251661566 +0200
@@ -35,6 +35,9 @@ static void minix_put_super(struct super
 	int i;
 	struct minix_sb_info *sbi = minix_sb(sb);
 
+	lock_super(sb);
+	lock_kernel();
+
 	if (!(sb->s_flags & MS_RDONLY)) {
 		if (sbi->s_version != MINIX_V3)	 /* s_state is now out from V3 sb */
 			sbi->s_ms->s_state = sbi->s_mount_state;
@@ -49,7 +52,8 @@ static void minix_put_super(struct super
 	sb->s_fs_info = NULL;
 	kfree(sbi);
 
-	return;
+	unlock_kernel();
+	unlock_super(sb);
 }
 
 static struct kmem_cache * minix_inode_cachep;
Index: vfs-2.6/fs/ncpfs/inode.c
===================================================================
--- vfs-2.6.orig/fs/ncpfs/inode.c	2009-05-05 14:57:46.698784577 +0200
+++ vfs-2.6/fs/ncpfs/inode.c	2009-05-05 15:07:23.745661759 +0200
@@ -736,6 +736,9 @@ static void ncp_put_super(struct super_b
 {
 	struct ncp_server *server = NCP_SBP(sb);
 
+	lock_super(sb);
+	lock_kernel();
+
 	ncp_lock_server(server);
 	ncp_disconnect(server);
 	ncp_unlock_server(server);
@@ -769,6 +772,9 @@ static void ncp_put_super(struct super_b
 	vfree(server->packet);
 	sb->s_fs_info = NULL;
 	kfree(server);
+
+	unlock_kernel();
+	unlock_super(sb);
 }
 
 static int ncp_statfs(struct dentry *dentry, struct kstatfs *buf)
Index: vfs-2.6/fs/nilfs2/super.c
===================================================================
--- vfs-2.6.orig/fs/nilfs2/super.c	2009-05-05 14:57:46.765784725 +0200
+++ vfs-2.6/fs/nilfs2/super.c	2009-05-05 15:07:27.718661439 +0200
@@ -316,6 +316,9 @@ static void nilfs_put_super(struct super
 	struct nilfs_sb_info *sbi = NILFS_SB(sb);
 	struct the_nilfs *nilfs = sbi->s_nilfs;
 
+	lock_super(sb);
+	lock_kernel();
+
 	if (sb->s_dirt)
 		nilfs_write_super(sb);
 
@@ -333,6 +336,9 @@ static void nilfs_put_super(struct super
 	sbi->s_super = NULL;
 	sb->s_fs_info = NULL;
 	kfree(sbi);
+
+	unlock_kernel();
+	unlock_super(sb);
 }
 
 /**
Index: vfs-2.6/fs/ntfs/super.c
===================================================================
--- vfs-2.6.orig/fs/ntfs/super.c	2009-05-05 14:57:46.780784470 +0200
+++ vfs-2.6/fs/ntfs/super.c	2009-05-05 15:07:41.000822082 +0200
@@ -2246,6 +2246,10 @@ static void ntfs_put_super(struct super_
 	ntfs_volume *vol = NTFS_SB(sb);
 
 	ntfs_debug("Entering.");
+
+	lock_super(sb);
+	lock_kernel();
+
 #ifdef NTFS_RW
 	/*
 	 * Commit all inodes while they are still open in case some of them
@@ -2444,7 +2448,9 @@ static void ntfs_put_super(struct super_
 	}
 	sb->s_fs_info = NULL;
 	kfree(vol);
-	return;
+
+	unlock_kernel();
+	unlock_super(sb);
 }
 
 /**
Index: vfs-2.6/fs/ocfs2/super.c
===================================================================
--- vfs-2.6.orig/fs/ocfs2/super.c	2009-05-05 14:57:46.831784537 +0200
+++ vfs-2.6/fs/ocfs2/super.c	2009-05-05 15:07:44.333661483 +0200
@@ -1536,9 +1536,15 @@ static void ocfs2_put_super(struct super
 {
 	mlog_entry("(0x%p)\n", sb);
 
+	lock_super(sb);
+	lock_kernel();
+
 	ocfs2_sync_blockdev(sb);
 	ocfs2_dismount_volume(sb, 0);
 
+	unlock_kernel();
+	unlock_super(sb);
+
 	mlog_exit_void();
 }
 
Index: vfs-2.6/fs/reiserfs/super.c
===================================================================
--- vfs-2.6.orig/fs/reiserfs/super.c	2009-05-05 14:57:47.025784439 +0200
+++ vfs-2.6/fs/reiserfs/super.c	2009-05-05 15:09:52.760659069 +0200
@@ -470,6 +470,9 @@ static void reiserfs_put_super(struct su
 	struct reiserfs_transaction_handle th;
 	th.t_trans_id = 0;
 
+	lock_super(s);
+	lock_kernel();
+
 	if (s->s_dirt)
 		reiserfs_write_super(s);
 
@@ -505,7 +508,8 @@ static void reiserfs_put_super(struct su
 	kfree(s->s_fs_info);
 	s->s_fs_info = NULL;
 
-	return;
+	unlock_kernel();
+	unlock_super(s);
 }
 
 static struct kmem_cache *reiserfs_inode_cachep;
Index: vfs-2.6/fs/smbfs/inode.c
===================================================================
--- vfs-2.6.orig/fs/smbfs/inode.c	2009-05-05 14:57:47.091783900 +0200
+++ vfs-2.6/fs/smbfs/inode.c	2009-05-05 15:08:00.143666563 +0200
@@ -474,6 +474,9 @@ smb_put_super(struct super_block *sb)
 {
 	struct smb_sb_info *server = SMB_SB(sb);
 
+	lock_super(sb);
+	lock_kernel();
+
 	smb_lock_server(server);
 	server->state = CONN_INVALID;
 	smbiod_unregister_server(server);
@@ -489,6 +492,9 @@ smb_put_super(struct super_block *sb)
 	smb_unlock_server(server);
 	put_pid(server->conn_pid);
 	kfree(server);
+
+	unlock_kernel();
+	unlock_super(sb);
 }
 
 static int smb_fill_super(struct super_block *sb, void *raw_data, int silent)
Index: vfs-2.6/fs/squashfs/super.c
===================================================================
--- vfs-2.6.orig/fs/squashfs/super.c	2009-05-05 14:57:47.154784307 +0200
+++ vfs-2.6/fs/squashfs/super.c	2009-05-05 15:08:05.378786756 +0200
@@ -328,6 +328,9 @@ static int squashfs_remount(struct super
 
 static void squashfs_put_super(struct super_block *sb)
 {
+	lock_super(sb);
+	lock_kernel();
+
 	if (sb->s_fs_info) {
 		struct squashfs_sb_info *sbi = sb->s_fs_info;
 		squashfs_cache_delete(sbi->block_cache);
@@ -340,6 +343,9 @@ static void squashfs_put_super(struct su
 		kfree(sb->s_fs_info);
 		sb->s_fs_info = NULL;
 	}
+
+	unlock_kernel();
+	unlock_super(sb);
 }
 
 
Index: vfs-2.6/fs/super.c
===================================================================
--- vfs-2.6.orig/fs/super.c	2009-05-05 14:56:02.183659132 +0200
+++ vfs-2.6/fs/super.c	2009-05-05 14:56:39.732661447 +0200
@@ -276,12 +276,10 @@ void generic_shutdown_super(struct super
 	if (sb->s_root) {
 		shrink_dcache_for_umount(sb);
 		sync_filesystem(sb);
-		lock_super(sb);
 		sb->s_flags &= ~MS_ACTIVE;
 
 		/* bad name - it should be evict_inodes() */
 		invalidate_inodes(sb);
-		lock_kernel();
 
 		if (sop->put_super)
 			sop->put_super(sb);
@@ -292,9 +290,6 @@ void generic_shutdown_super(struct super
 			   "Self-destruct in 5 seconds.  Have a nice day...\n",
 			   sb->s_id);
 		}
-
-		unlock_kernel();
-		unlock_super(sb);
 	}
 	spin_lock(&sb_lock);
 	/* should be initialized for __put_super_and_need_restart() */
Index: vfs-2.6/fs/sysv/inode.c
===================================================================
--- vfs-2.6.orig/fs/sysv/inode.c	2009-05-05 14:57:47.167784425 +0200
+++ vfs-2.6/fs/sysv/inode.c	2009-05-05 15:08:56.901785798 +0200
@@ -72,6 +72,9 @@ static void sysv_put_super(struct super_
 {
 	struct sysv_sb_info *sbi = SYSV_SB(sb);
 
+	lock_super(sb);
+	lock_kernel();
+
 	if (sb->s_dirt)
 		sysv_write_super(sb);
 
@@ -87,6 +90,9 @@ static void sysv_put_super(struct super_
 		brelse(sbi->s_bh2);
 
 	kfree(sbi);
+
+	unlock_kernel();
+	unlock_super(sb);
 }
 
 static int sysv_statfs(struct dentry *dentry, struct kstatfs *buf)
Index: vfs-2.6/fs/ubifs/super.c
===================================================================
--- vfs-2.6.orig/fs/ubifs/super.c	2009-05-05 14:57:47.180784265 +0200
+++ vfs-2.6/fs/ubifs/super.c	2009-05-05 15:08:17.902786750 +0200
@@ -1687,6 +1687,10 @@ static void ubifs_put_super(struct super
 
 	ubifs_msg("un-mount UBI device %d, volume %d", c->vi.ubi_num,
 		  c->vi.vol_id);
+
+	lock_super(sb);
+	lock_kernel();
+
 	/*
 	 * The following asserts are only valid if there has not been a failure
 	 * of the media. For example, there will be dirty inodes if we failed
@@ -1753,6 +1757,9 @@ static void ubifs_put_super(struct super
 	ubi_close_volume(c->ubi);
 	mutex_unlock(&c->umount_mutex);
 	kfree(c);
+
+	unlock_kernel();
+	unlock_super(sb);
 }
 
 static int ubifs_remount_fs(struct super_block *sb, int *flags, char *data)
Index: vfs-2.6/fs/udf/super.c
===================================================================
--- vfs-2.6.orig/fs/udf/super.c	2009-05-05 14:57:47.221784665 +0200
+++ vfs-2.6/fs/udf/super.c	2009-05-05 15:08:22.650694994 +0200
@@ -2062,6 +2062,10 @@ static void udf_put_super(struct super_b
 	struct udf_sb_info *sbi;
 
 	sbi = UDF_SB(sb);
+
+	lock_super(sb);
+	lock_kernel();
+
 	if (sbi->s_vat_inode)
 		iput(sbi->s_vat_inode);
 	if (sbi->s_partitions)
@@ -2077,6 +2081,9 @@ static void udf_put_super(struct super_b
 	kfree(sbi->s_partmaps);
 	kfree(sb->s_fs_info);
 	sb->s_fs_info = NULL;
+
+	unlock_kernel();
+	unlock_super(sb);
 }
 
 static int udf_sync_fs(struct super_block *sb, int wait)
Index: vfs-2.6/fs/ufs/super.c
===================================================================
--- vfs-2.6.orig/fs/ufs/super.c	2009-05-05 14:57:47.286784418 +0200
+++ vfs-2.6/fs/ufs/super.c	2009-05-05 15:08:45.443661872 +0200
@@ -594,6 +594,10 @@ static void ufs_put_super_internal(struc
 
 	
 	UFSD("ENTER\n");
+
+	lock_super(sb);
+	lock_kernel();
+
 	ufs_put_cstotal(sb);
 	size = uspi->s_cssize;
 	blks = (size + uspi->s_fsize - 1) >> uspi->s_fshift;
@@ -621,6 +625,10 @@ static void ufs_put_super_internal(struc
 		brelse (sbi->s_ucg[i]);
 	kfree (sbi->s_ucg);
 	kfree (base);
+
+	unlock_kernel();
+	unlock_super(sb);
+
 	UFSD("EXIT\n");
 }
 
Index: vfs-2.6/fs/xfs/linux-2.6/xfs_super.c
===================================================================
--- vfs-2.6.orig/fs/xfs/linux-2.6/xfs_super.c	2009-05-05 14:57:47.350784743 +0200
+++ vfs-2.6/fs/xfs/linux-2.6/xfs_super.c	2009-05-05 15:08:37.047661747 +0200
@@ -1062,6 +1062,9 @@ xfs_fs_put_super(
 	struct xfs_inode	*rip = mp->m_rootip;
 	int			unmount_event_flags = 0;
 
+	lock_super(sb);
+	lock_kernel();
+
 	xfs_syncd_stop(mp);
 	xfs_sync_inodes(mp, SYNC_ATTR|SYNC_DELWRI);
 
@@ -1102,6 +1105,9 @@ xfs_fs_put_super(
 	xfs_dmops_put(mp);
 	xfs_free_fsname(mp);
 	kfree(mp);
+
+	unlock_kernel();
+	unlock_super(sb);
 }
 
 STATIC void

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PTCH] push down lock_super and BKL into ->put_super
  2009-05-05 13:40 [PTCH] push down lock_super and BKL into ->put_super Christoph Hellwig
@ 2009-05-06  2:09 ` Al Viro
  2009-05-06  6:23   ` Christoph Hellwig
  0 siblings, 1 reply; 5+ messages in thread
From: Al Viro @ 2009-05-06  2:09 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-fsdevel

On Tue, May 05, 2009 at 03:40:36PM +0200, Christoph Hellwig wrote:
> From: Christoph Hellwig <hch@lst.de>
> 
> Move s_lock and BKL into ->put_super from the only caller.  A couple of
> filesystems had trivial enough ->put_super (only kfree and NULLing of
> s_fs_info + stuff in there) to not get any locking: code, cramfs, efs,
> hugetlbfs, omfs, qnx4, shmem, all others got the full treatment.  Most
> of them probably don't need it, but I'd rather sort that out individually.
> Preferably after all the other BKL and lock_super pushdowns in that area.

NAK.  Getting BKL down there is fine, but lock_super() in that place should
simply die now.

Look: lock_super() does two things - exclusion on s_lock and marking the
task as "holds excl fs resource, other tasks are likely to be blocked by
it, try to push its IO at higher priority" for CFQ.

Let's consider the exclusion first.  We *can't* get contention on s_lock
at that point.  The only thing that takes s_lock is lock_super() itself.
Callers:
	generic_shutdown_super (this one, s_umount held exclusive since
it's always called from ->kill_sb) [1]
	write_super() from sync_supers() - s_umount held shared
	do_remount_sb() - s_umount held [2]
	file_fsync() - we shouldn't ever be in generic_shutdown_super()
while there are files opened on that superblock, TYVM
	__sync_filesystem() - called from sync_filesystems() (s_umount shared)
and from sync_filesystem().
Callers of sync_filesystem():
	fsync_bdev(), freeze_bdev() - s_umount shared.
	fs/cachefiles - there we have a vfsmount with ->mnt_sb pointing to
that superblock, so generic_shutdown_super() shouldn't be possible (active
reference to superblock)
	and generic_shutdown_super() itself.  Again, s_umount exclusive and
in any case, there's only one call of that sucker in the entire lifetime of
given struct super_block.

So we can't have contention on s_lock in that area - any such contention would
either happen on s_umount or would be a result of severely buggered refcounting
on struct super_block.

That leaves us with fs_excl alone.  And fair enough, this thing *does* hold
exclusive fs resources.  Only one, in fact - s_umount.  Nothing is going to
wait for it to finish in order to do something with fs itself.  So if we
are going to bump fs_excl, we ought to do it in deactivate_super(), not
here.  After all, ->put_super() is not where the majority of IO on final
umount will be happening...

As for this patch: we need to
	* replace lock_super/unlock_super() with get_fs_excl()/put_fs_excl()
in the same places.
	* don't do lock_super() inside ->put_super() at all.
	* take BKL down to ->put_super().

I'm going to put it in as two patches - lock_super part + trimmed down
version of this one, sans lock_super.

[1] and from grepping through the instances... some of the callers are
seriously fishy.  All of them *are* in ->kill_sb() and not called other
that via deactivate_super().

[2] normally exclusive, but there are callers holding it shared; all of them
should switch to excl, but that doesn't matter for our purposes.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PTCH] push down lock_super and BKL into ->put_super
  2009-05-06  2:09 ` Al Viro
@ 2009-05-06  6:23   ` Christoph Hellwig
  2009-05-06  6:46     ` Al Viro
  0 siblings, 1 reply; 5+ messages in thread
From: Christoph Hellwig @ 2009-05-06  6:23 UTC (permalink / raw)
  To: Al Viro; +Cc: Christoph Hellwig, linux-fsdevel

On Wed, May 06, 2009 at 03:09:16AM +0100, Al Viro wrote:
> As for this patch: we need to
> 	* replace lock_super/unlock_super() with get_fs_excl()/put_fs_excl()
> in the same places.

Nope.  See the discussion I had with Jens and other about what it
is good for.  It's for fs-central ressouces that are performance
critical for FS.  FS beeing shut down is per defintion not performance
critical.


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PTCH] push down lock_super and BKL into ->put_super
  2009-05-06  6:23   ` Christoph Hellwig
@ 2009-05-06  6:46     ` Al Viro
  2009-05-06  7:34       ` Theodore Tso
  0 siblings, 1 reply; 5+ messages in thread
From: Al Viro @ 2009-05-06  6:46 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-fsdevel

On Wed, May 06, 2009 at 08:23:00AM +0200, Christoph Hellwig wrote:
> On Wed, May 06, 2009 at 03:09:16AM +0100, Al Viro wrote:
> > As for this patch: we need to
> > 	* replace lock_super/unlock_super() with get_fs_excl()/put_fs_excl()
> > in the same places.
> 
> Nope.  See the discussion I had with Jens and other about what it
> is good for.  It's for fs-central ressouces that are performance
> critical for FS.  FS beeing shut down is per defintion not performance
> critical.

FS blocking writeback may very well be, though.

In any case, the point is that we should separate get_fs_excl() from
exclusion there and kill exclusion part for everything except ext4.

_Then_ we can deal with fs_excl separately.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PTCH] push down lock_super and BKL into ->put_super
  2009-05-06  6:46     ` Al Viro
@ 2009-05-06  7:34       ` Theodore Tso
  0 siblings, 0 replies; 5+ messages in thread
From: Theodore Tso @ 2009-05-06  7:34 UTC (permalink / raw)
  To: Al Viro; +Cc: Christoph Hellwig, linux-fsdevel

On Wed, May 06, 2009 at 07:46:22AM +0100, Al Viro wrote:
> 
> FS blocking writeback may very well be, though.
> 
> In any case, the point is that we should separate get_fs_excl() from
> exclusion there and kill exclusion part for everything except ext4.

Note that I already have patches queued for ext4 that removes the use
of lock_super() for everything other than write_super() exclusion.
(i.e., we were using it to protect oneline resize and the orphan
list.)  Similar patches are needed for ext3, which I'll backport
before the next merge window.

BTW, I'm *not* at all convinced that get_fs_excl() is the right
interface for boosting I/O priority, since it only boosts priority for
idle processes.  So high priority processes will still get screwed by
normal I/O.  Hence, in the common case, where there are no I/O class
nince processes, get_fs_excl() is a no-op anyway.  And the case which
I'm personally most interested in, which is real-time processes that
want to do I/O and so have an elevated I/O priority, get_fs_excl()
does nothing for them.

						- Ted

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2009-05-06  7:34 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-05-05 13:40 [PTCH] push down lock_super and BKL into ->put_super Christoph Hellwig
2009-05-06  2:09 ` Al Viro
2009-05-06  6:23   ` Christoph Hellwig
2009-05-06  6:46     ` Al Viro
2009-05-06  7:34       ` Theodore Tso

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).