Skip to contentSkip to conversation

Shift BKL into ->statfs() (trimmed)

PATCH New Paul Menage 1 participant 1 message Jul 4, 2002

Subsystems (7)
Documentation VFS NFS client UDF filesystem +3 more subsystems
lore
Share & tools
lore

Paul Menage

author
#0 lore raw

PATCH 1/1 Shift BKL into ->statfs() (trimmed)

This patch removes BKL protection from the invocation of the super_operations ->statfs() method, and shifts it into the filesystems where necessary. Any out-of-tree filesystems may need to take the BKL in their statfs() methods if they were relying on it for synchronisation.

Several ->statfs() implementation changes in the previous version of the patch have been dropped following assurances from Oleg Drokin, Anton Altaparmakov and Matthew Wilcox that they were unnecessary.

All ->statfs() implementations have been modified to take the BKL, except for the following:

- those that don't reference any external mutable data:

simple_statfs
isofs_statfs
ncp_statfs
cramfs_statfs
romfs_statfs
efs_statfs
vxfs_statfs
ntfs_statfs
adfs_statfs

- those that already have their own locking:

ext2_statfs
shmem_statfs
fat_statfs
UMSDOS_statfs
minix_statfs
sysv_statfs
reiserfs_statfs
affs_statfs
bfs_statfs
ext3_statfs
hfs_statfs
jffs2_statfs
jfs_statfs

Additionally, capifs is changed to use simple_statfs rather than its own home-grown version.

The BKL change has been flagged at the end of Documentation/filesystems/porting, along with the recent change to ->permission BKL usage.

Paul

Documentation/filesystems/Locking +1 -1
diff -Naur -X /mnt/elbrus/home/pmenage/dontdiff linux-2.5.24/Documentation/filesystems/Locking linux-2.5.24-statfs2/Documentation/filesystems/Locking
@@ -108,7 +108,7 @@
 clear_inode:	no	
 put_super:	yes	yes	maybe		(see below)
 write_super:	no	yes	maybe		(see below)
-statfs:		yes	no	no
+statfs:		no	no	no
 remount_fs:	yes	yes	maybe		(see below)
 umount_begin:	yes	no	maybe		(see below)
Documentation/filesystems/porting +17 -1
diff -Naur -X /mnt/elbrus/home/pmenage/dontdiff linux-2.5.24/Documentation/filesystems/porting linux-2.5.24-statfs2/Documentation/filesystems/porting
@@ -81,7 +81,7 @@
 [mandatory]
 
 ->lookup(), ->truncate(), ->create(), ->unlink(), ->mknod(), ->mkdir(),
-->rmdir(), ->link(), ->lseek(), ->symlink(), ->rename(), ->permission()
+->rmdir(), ->link(), ->lseek(), ->symlink(), ->rename()
 and ->readdir() are called without BKL now.  Grab it on entry, drop upon return
 - that will guarantee the same locking you used to have.  If your method or its
 parts do not need BKL - better yet, now you can shift lock_kernel() and

@@ -228,3 +228,19 @@
 	Use bdev_read_only(bdev) instead of is_read_only(kdev).  The latter
 is still alive, but only because of the mess in drivers/s390/block/dasd.c.
 As soon as it gets fixed is_read_only() will die.
+
+---
+[mandatory]
+
+->permission() is called without BKL now. Grab it on entry, drop upon
+return - that will guarantee the same locking you used to have.  If
+your method or its parts do not need BKL - better yet, now you can
+shift lock_kernel() and unlock_kernel() so that they would protect
+exactly what needs to be protected.
+
+---
+[mandatory]
+
+->statfs() is now called without BKL held.  BKL should have been
+shifted into individual fs sb_op functions where it's not clear that
+it's safe to remove it.  If you don't need it, remove it.
drivers/isdn/capi/capifs.c +1 -16
diff -Naur -X /mnt/elbrus/home/pmenage/dontdiff linux-2.5.24/drivers/isdn/capi/capifs.c linux-2.5.24-statfs2/drivers/isdn/capi/capifs.c
@@ -221,11 +221,9 @@
 	kfree(sbi);
 }
 
-static int capifs_statfs(struct super_block *sb, struct statfs *buf);
-
 static struct super_operations capifs_sops = {
 	put_super:	capifs_put_super,
-	statfs:		capifs_statfs,
+	statfs:		simple_statfs,
 };
 
 static int capifs_parse_options(char *options, struct capifs_sb_info *sbi)

@@ -354,19 +352,6 @@
 	return -EINVAL;
 }
 
-static int capifs_statfs(struct super_block *sb, struct statfs *buf)
-{
-	buf->f_type = CAPIFS_SUPER_MAGIC;
-	buf->f_bsize = 1024;
-	buf->f_blocks = 0;
-	buf->f_bfree = 0;
-	buf->f_bavail = 0;
-	buf->f_files = 0;
-	buf->f_ffree = 0;
-	buf->f_namelen = NAME_MAX;
-	return 0;
-}
-
 static struct inode *capifs_new_inode(struct super_block *sb)
 {
 	struct inode *inode = new_inode(sb);
fs/coda/inode.c +4 -0
diff -Naur -X /mnt/elbrus/home/pmenage/dontdiff linux-2.5.24/fs/coda/inode.c linux-2.5.24-statfs2/fs/coda/inode.c
@@ -281,9 +281,13 @@
 static int coda_statfs(struct super_block *sb, struct statfs *buf)
 {
 	int error;
+	
+	lock_kernel();
 
 	error = venus_statfs(sb, buf);
 
+	unlock_kernel();
+
 	if (error) {
 		/* fake something like AFS does */
 		buf->f_blocks = 9000000;
fs/ext3/super.c +1 -1
diff -Naur -X /mnt/elbrus/home/pmenage/dontdiff linux-2.5.24/fs/ext3/super.c linux-2.5.24-statfs2/fs/ext3/super.c
@@ -504,7 +504,7 @@
 	write_super:	ext3_write_super,	/* BKL not held. We take it. Needed? */
 	write_super_lockfs: ext3_write_super_lockfs, /* BKL not held. Take it */
 	unlockfs:	ext3_unlockfs,		/* BKL not held.  We take it */
-	statfs:		ext3_statfs,		/* BKL held */
+	statfs:		ext3_statfs,		/* BKL not held. */
 	remount_fs:	ext3_remount,		/* BKL held */
 };
fs/freevxfs/vxfs_super.c +1 -1
diff -Naur -X /mnt/elbrus/home/pmenage/dontdiff linux-2.5.24/fs/freevxfs/vxfs_super.c linux-2.5.24-statfs2/fs/freevxfs/vxfs_super.c
@@ -98,7 +98,7 @@
  *   Zero.
  *
  * Locking:
- *   We are under bkl and @sbp->s_lock.
+ *   No locks held.
  *
  * Notes:
  *   This is everything but complete...
fs/hpfs/super.c +5 -0
diff -Naur -X /mnt/elbrus/home/pmenage/dontdiff linux-2.5.24/fs/hpfs/super.c linux-2.5.24-statfs2/fs/hpfs/super.c
@@ -134,6 +134,8 @@
 
 int hpfs_statfs(struct super_block *s, struct statfs *buf)
 {
+	lock_kernel();
+
 	/*if (s->s_hpfs_n_free == -1) {*/
 		s->s_hpfs_n_free = count_bitmaps(s);
 		s->s_hpfs_n_free_dnodes = hpfs_count_one_bitmap(s, s->s_hpfs_dmap);

@@ -146,6 +148,9 @@
 	buf->f_files = s->s_hpfs_dirband_size / 4;
 	buf->f_ffree = s->s_hpfs_n_free_dnodes;
 	buf->f_namelen = 254;
+
+	unlock_kernel();
+
 	return 0;
 }
fs/jffs/inode-v23.c +8 -1
diff -Naur -X /mnt/elbrus/home/pmenage/dontdiff linux-2.5.24/fs/jffs/inode-v23.c linux-2.5.24-statfs2/fs/jffs/inode-v23.c
@@ -383,7 +383,11 @@
 jffs_statfs(struct super_block *sb, struct statfs *buf)
 {
 	struct jffs_control *c = (struct jffs_control *) sb->u.generic_sbp;
-	struct jffs_fmcontrol *fmc = c->fmc;
+	struct jffs_fmcontrol *fmc;
+
+	lock_kernel();
+
+	fmc = c->fmc;
 
 	D2(printk("jffs_statfs()\n"));
 

@@ -401,6 +405,9 @@
 	buf->f_ffree = buf->f_bfree;
 	/* buf->f_fsid = 0; */
 	buf->f_namelen = JFFS_MAX_NAME_LEN;
+
+	unlock_kernel();
+
 	return 0;
 }
fs/nfs/inode.c +10 -2
diff -Naur -X /mnt/elbrus/home/pmenage/dontdiff linux-2.5.24/fs/nfs/inode.c linux-2.5.24-statfs2/fs/nfs/inode.c
@@ -474,6 +474,8 @@
 	struct nfs_fsinfo res;
 	int error;
 
+	lock_kernel();
+
 	error = server->rpc_ops->statfs(server, NFS_FH(sb->s_root->d_inode), &res);
 	buf->f_type = NFS_SUPER_MAGIC;
 	if (error < 0)

@@ -491,11 +493,17 @@
 	if (res.namelen == 0 || res.namelen > server->namelen)
 		res.namelen = server->namelen;
 	buf->f_namelen = res.namelen;
+
+ out:
+	unlock_kernel();
+
 	return 0;
+
  out_err:
-	printk("nfs_statfs: statfs error = %d\n", -error);
+	printk(KERN_WARNING "nfs_statfs: statfs error = %d\n", -error);
 	buf->f_bsize = buf->f_blocks = buf->f_bfree = buf->f_bavail = -1;
-	return 0;
+	goto out;
+
 }
 
 static int nfs_show_options(struct seq_file *m, struct vfsmount *mnt)
fs/open.c +0 -2
diff -Naur -X /mnt/elbrus/home/pmenage/dontdiff linux-2.5.24/fs/open.c linux-2.5.24-statfs2/fs/open.c
@@ -30,9 +30,7 @@
 		retval = -ENOSYS;
 		if (sb->s_op && sb->s_op->statfs) {
 			memset(buf, 0, sizeof(struct statfs));
-			lock_kernel();
 			retval = sb->s_op->statfs(sb, buf);
-			unlock_kernel();
 		}
 	}
 	return retval;
fs/qnx4/inode.c +4 -0
diff -Naur -X /mnt/elbrus/home/pmenage/dontdiff linux-2.5.24/fs/qnx4/inode.c linux-2.5.24-statfs2/fs/qnx4/inode.c
@@ -281,6 +281,8 @@
 
 static int qnx4_statfs(struct super_block *sb, struct statfs *buf)
 {
+	lock_kernel();
+
 	buf->f_type    = sb->s_magic;
 	buf->f_bsize   = sb->s_blocksize;
 	buf->f_blocks  = le32_to_cpu(qnx4_sb(sb)->BitMap->di_size) * 8;

@@ -288,6 +290,8 @@
 	buf->f_bavail  = buf->f_bfree;
 	buf->f_namelen = QNX4_NAME_MAX;
 
+	unlock_kernel();
+
 	return 0;
 }
fs/smbfs/inode.c +7 -1
diff -Naur -X /mnt/elbrus/home/pmenage/dontdiff linux-2.5.24/fs/smbfs/inode.c linux-2.5.24-statfs2/fs/smbfs/inode.c
@@ -604,7 +604,13 @@
 static int
 smb_statfs(struct super_block *sb, struct statfs *buf)
 {
-	int result = smb_proc_dskattr(sb, buf);
+	int result;
+	
+	lock_kernel();
+
+	result = smb_proc_dskattr(sb, buf);
+
+	unlock_kernel();
 
 	buf->f_type = SMB_SUPER_MAGIC;
 	buf->f_namelen = SMB_MAXPATHLEN;
fs/udf/super.c +4 -0
diff -Naur -X /mnt/elbrus/home/pmenage/dontdiff linux-2.5.24/fs/udf/super.c linux-2.5.24-statfs2/fs/udf/super.c
@@ -1726,6 +1726,8 @@
 static int
 udf_statfs(struct super_block *sb, struct statfs *buf)
 {
+	lock_kernel();
+
 	buf->f_type = UDF_SUPER_MAGIC;
 	buf->f_bsize = sb->s_blocksize;
 	buf->f_blocks = UDF_SB_PARTLEN(sb, UDF_SB_PARTITION(sb));

@@ -1738,6 +1740,8 @@
 	/* __kernel_fsid_t f_fsid */
 	buf->f_namelen = UDF_NAME_LEN;
 
+	unlock_kernel();
+
 	return 0;
 }
fs/ufs/super.c +5 -0
diff -Naur -X /mnt/elbrus/home/pmenage/dontdiff linux-2.5.24/fs/ufs/super.c linux-2.5.24-statfs2/fs/ufs/super.c
@@ -959,6 +959,8 @@
 	struct ufs_sb_private_info * uspi;
 	struct ufs_super_block_first * usb1;
 
+	lock_kernel();
+
 	uspi = sb->u.ufs_sb.s_uspi;
 	usb1 = ubh_get_usb_first (USPI_UBH);
 	

@@ -972,6 +974,9 @@
 	buf->f_files = uspi->s_ncg * uspi->s_ipg;
 	buf->f_ffree = fs32_to_cpu(sb, usb1->fs_cstotal.cs_nifree);
 	buf->f_namelen = UFS_MAXNAMLEN;
+
+	unlock_kernel();
+
 	return 0;
 }