BKL: Remove BKL from isofs

As in other file systems, we can replace the big kernel lock with a private mutex in isofs. This means we can now access multiple file systems concurrently, but it also means that we serialize readdir and lookup across sleeping operations which previously released the big kernel lock. This should not matter though, as these operations are in practice serialized through the hardware access. The isofs_get_blocks functions now does not take any lock any more, it used to recursively get the BKL. After looking at the code for hours, I convinced myself that it was never needed here anyway, because it only reads constant fields of the inode and writes to a buffer head array that is at this time only visible to the caller. The get_sb and fill_super operations do not need the locking at all because they operate on a file system that is either about to be created or to be destroyed but in either case is not visible to other threads. Signed-off-by: Arnd Bergmann <arnd@arndb.de>

BKL: Remove BKL from isofs
As in other file systems, we can replace the big kernel lock with a private mutex in isofs. This means we can now access multiple file systems concurrently, but it also means that we serialize readdir and lookup across sleeping operations which previously released the big kernel lock. This should not matter though, as these operations are in practice serialized through the hardware access. The isofs_get_blocks functions now does not take any lock any more, it used to recursively get the BKL. After looking at the code for hours, I convinced myself that it was never needed here anyway, because it only reads constant fields of the inode and writes to a buffer head array that is at this time only visible to the caller. The get_sb and fill_super operations do not need the locking at all because they operate on a file system that is either about to be created or to be destroyed but in either case is not visible to other threads. Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Arnd Bergmann
1 parent 3768744cfe
Showing 5 changed files with 15 additions and 27 deletions Side-by-side Diff
fs/isofs/dir.c
fs/isofs/inode.c
fs/isofs/isofs.h
fs/isofs/namei.c
fs/isofs/rock.c
@@ -10,7 +10,6 @@
  *
  *  isofs directory handling functions
  */
-#include <linux/smp_lock.h>
 #include <linux/gfp.h>
 #include "isofs.h"
  
  
  
@@ -255,18 +254,19 @@
 	char *tmpname;
 	struct iso_directory_record *tmpde;
 	struct inode *inode = filp->f_path.dentry->d_inode;
+	struct isofs_sb_info *sbi = ISOFS_SB(inode->i_sb);
  
 	tmpname = (char *)__get_free_page(GFP_KERNEL);
 	if (tmpname == NULL)
 		return -ENOMEM;
  
-	lock_kernel();
+	mutex_lock(&sbi->s_mutex);
 	tmpde = (struct iso_directory_record *) (tmpname+1024);
  
 	result = do_isofs_readdir(inode, filp, dirent, filldir, tmpname, tmpde);
  
 	free_page((unsigned long) tmpname);
-	unlock_kernel();
+	mutex_unlock(&sbi->s_mutex);
 	return result;
 }
  
@@ -17,7 +17,6 @@
 #include <linux/slab.h>
 #include <linux/nls.h>
 #include <linux/ctype.h>
-#include <linux/smp_lock.h>
 #include <linux/statfs.h>
 #include <linux/cdrom.h>
 #include <linux/parser.h>
  
@@ -44,11 +43,7 @@
 	struct isofs_sb_info *sbi = ISOFS_SB(sb);
  
 #ifdef CONFIG_JOLIET
-	lock_kernel();
-
 	unload_nls(sbi->s_nls_iocharset);
-
-	unlock_kernel();
 #endif
  
 	kfree(sbi);
  
  
@@ -571,15 +566,11 @@
 	int table, error = -EINVAL;
 	unsigned int vol_desc_start;
  
-	lock_kernel();
-
 	save_mount_options(s, data);
  
 	sbi = kzalloc(sizeof(*sbi), GFP_KERNEL);
-	if (!sbi) {
-		unlock_kernel();
+	if (!sbi)
 		return -ENOMEM;
-	}
 	s->s_fs_info = sbi;
  
 	if (!parse_options((char *)data, &opt))
@@ -827,6 +818,7 @@
 	sbi->s_utf8 = opt.utf8;
 	sbi->s_nocompress = opt.nocompress;
 	sbi->s_overriderockperm = opt.overriderockperm;
+	mutex_init(&sbi->s_mutex);
 	/*
 	 * It would be incredibly stupid to allow people to mark every file
 	 * on the disk as suid, so we merely allow them to set the default
@@ -904,7 +896,6 @@
  
 	kfree(opt.iocharset);
  
-	unlock_kernel();
 	return 0;
  
 	/*
@@ -944,7 +935,6 @@
 	kfree(opt.iocharset);
 	kfree(sbi);
 	s->s_fs_info = NULL;
-	unlock_kernel();
 	return error;
 }
  
@@ -983,8 +973,6 @@
 	int section, rv, error;
 	struct iso_inode_info *ei = ISOFS_I(inode);
  
-	lock_kernel();
-
 	error = -EIO;
 	rv = 0;
 	if (iblock < 0 || iblock != iblock_s) {
@@ -1060,7 +1048,6 @@
  
 	error = 0;
 abort:
-	unlock_kernel();
 	return rv != 0 ? rv : error;
 }
  
@@ -55,6 +55,7 @@
 	gid_t s_gid;
 	uid_t s_uid;
 	struct nls_table *s_nls_iocharset; /* Native language support table */
+	struct mutex s_mutex; /* replaces BKL, please remove if possible */
 };
  
 #define ISOFS_INVALID_MODE ((mode_t) -1)
@@ -6,7 +6,6 @@
  *  (C) 1991  Linus Torvalds - minix filesystem
  */
  
-#include <linux/smp_lock.h>
 #include <linux/gfp.h>
 #include "isofs.h"
  
@@ -168,6 +167,7 @@
 	int found;
 	unsigned long uninitialized_var(block);
 	unsigned long uninitialized_var(offset);
+	struct isofs_sb_info *sbi = ISOFS_SB(dir->i_sb);
 	struct inode *inode;
 	struct page *page;
  
@@ -177,7 +177,7 @@
 	if (!page)
 		return ERR_PTR(-ENOMEM);
  
-	lock_kernel();
+	mutex_lock(&sbi->s_mutex);
 	found = isofs_find_entry(dir, dentry,
 				&block, &offset,
 				page_address(page),
  
@@ -188,11 +188,11 @@
 	if (found) {
 		inode = isofs_iget(dir->i_sb, block, offset);
 		if (IS_ERR(inode)) {
-			unlock_kernel();
+			mutex_unlock(&sbi->s_mutex);
 			return ERR_CAST(inode);
 		}
 	}
-	unlock_kernel();
+	mutex_unlock(&sbi->s_mutex);
 	return d_splice_alias(inode, dentry);
 }
@@ -8,7 +8,6 @@
  
 #include <linux/slab.h>
 #include <linux/pagemap.h>
-#include <linux/smp_lock.h>
  
 #include "isofs.h"
 #include "rock.h"
@@ -661,6 +660,7 @@
 {
 	struct inode *inode = page->mapping->host;
 	struct iso_inode_info *ei = ISOFS_I(inode);
+	struct isofs_sb_info *sbi = ISOFS_SB(inode->i_sb);
 	char *link = kmap(page);
 	unsigned long bufsize = ISOFS_BUFFER_SIZE(inode);
 	struct buffer_head *bh;
  
@@ -673,12 +673,12 @@
 	struct rock_state rs;
 	int ret;
  
-	if (!ISOFS_SB(inode->i_sb)->s_rock)
+	if (!sbi->s_rock)
 		goto error;
  
 	init_rock_state(&rs, inode);
 	block = ei->i_iget5_block;
-	lock_kernel();
+	mutex_lock(&sbi->s_mutex);
 	bh = sb_bread(inode->i_sb, block);
 	if (!bh)
 		goto out_noread;
@@ -748,7 +748,7 @@
 		goto fail;
 	brelse(bh);
 	*rpnt = '\0';
-	unlock_kernel();
+	mutex_unlock(&sbi->s_mutex);
 	SetPageUptodate(page);
 	kunmap(page);
 	unlock_page(page);
@@ -765,7 +765,7 @@
 	printk("symlink spans iso9660 blocks\n");
 fail:
 	brelse(bh);
-	unlock_kernel();
+	mutex_unlock(&sbi->s_mutex);
 error:
 	SetPageError(page);
 	kunmap(page);
...	...	@@ -10,7 +10,6 @@
10	10	*
11	11	* isofs directory handling functions
12	12	*/
13		-#include <linux/smp_lock.h>
14	13	#include <linux/gfp.h>
15	14	#include "isofs.h"
16	15
17	16
18	17
...	...	@@ -255,18 +254,19 @@
255	254	char *tmpname;
256	255	struct iso_directory_record *tmpde;
257	256	struct inode *inode = filp->f_path.dentry->d_inode;
	257	+ struct isofs_sb_info *sbi = ISOFS_SB(inode->i_sb);
258	258
259	259	tmpname = (char *)__get_free_page(GFP_KERNEL);
260	260	if (tmpname == NULL)
261	261	return -ENOMEM;
262	262
263		- lock_kernel();
	263	+ mutex_lock(&sbi->s_mutex);
264	264	tmpde = (struct iso_directory_record *) (tmpname+1024);
265	265
266	266	result = do_isofs_readdir(inode, filp, dirent, filldir, tmpname, tmpde);
267	267
268	268	free_page((unsigned long) tmpname);
269		- unlock_kernel();
	269	+ mutex_unlock(&sbi->s_mutex);
270	270	return result;
271	271	}
272	272
...	...	@@ -17,7 +17,6 @@
17	17	#include <linux/slab.h>
18	18	#include <linux/nls.h>
19	19	#include <linux/ctype.h>
20		-#include <linux/smp_lock.h>
21	20	#include <linux/statfs.h>
22	21	#include <linux/cdrom.h>
23	22	#include <linux/parser.h>
24	23
...	...	@@ -44,11 +43,7 @@
44	43	struct isofs_sb_info *sbi = ISOFS_SB(sb);
45	44
46	45	#ifdef CONFIG_JOLIET
47		- lock_kernel();
48		-
49	46	unload_nls(sbi->s_nls_iocharset);
50		-
51		- unlock_kernel();
52	47	#endif
53	48
54	49	kfree(sbi);
55	50
56	51
...	...	@@ -571,15 +566,11 @@
571	566	int table, error = -EINVAL;
572	567	unsigned int vol_desc_start;
573	568
574		- lock_kernel();
575		-
576	569	save_mount_options(s, data);
577	570
578	571	sbi = kzalloc(sizeof(*sbi), GFP_KERNEL);
579		- if (!sbi) {
580		- unlock_kernel();
	572	+ if (!sbi)
581	573	return -ENOMEM;
582		- }
583	574	s->s_fs_info = sbi;
584	575
585	576	if (!parse_options((char *)data, &opt))
...	...	@@ -827,6 +818,7 @@
827	818	sbi->s_utf8 = opt.utf8;
828	819	sbi->s_nocompress = opt.nocompress;
829	820	sbi->s_overriderockperm = opt.overriderockperm;
	821	+ mutex_init(&sbi->s_mutex);
830	822	/*
831	823	* It would be incredibly stupid to allow people to mark every file
832	824	* on the disk as suid, so we merely allow them to set the default
...	...	@@ -904,7 +896,6 @@
904	896
905	897	kfree(opt.iocharset);
906	898
907		- unlock_kernel();
908	899	return 0;
909	900
910	901	/*
...	...	@@ -944,7 +935,6 @@
944	935	kfree(opt.iocharset);
945	936	kfree(sbi);
946	937	s->s_fs_info = NULL;
947		- unlock_kernel();
948	938	return error;
949	939	}
950	940
...	...	@@ -983,8 +973,6 @@
983	973	int section, rv, error;
984	974	struct iso_inode_info *ei = ISOFS_I(inode);
985	975
986		- lock_kernel();
987		-
988	976	error = -EIO;
989	977	rv = 0;
990	978	if (iblock < 0 \|\| iblock != iblock_s) {
...	...	@@ -1060,7 +1048,6 @@
1060	1048
1061	1049	error = 0;
1062	1050	abort:
1063		- unlock_kernel();
1064	1051	return rv != 0 ? rv : error;
1065	1052	}
1066	1053
...	...	@@ -55,6 +55,7 @@
55	55	gid_t s_gid;
56	56	uid_t s_uid;
57	57	struct nls_table s_nls_iocharset; / Native language support table */
	58	+ struct mutex s_mutex; /* replaces BKL, please remove if possible */
58	59	};
59	60
60	61	#define ISOFS_INVALID_MODE ((mode_t) -1)
...	...	@@ -6,7 +6,6 @@
6	6	* (C) 1991 Linus Torvalds - minix filesystem
7	7	*/
8	8
9		-#include <linux/smp_lock.h>
10	9	#include <linux/gfp.h>
11	10	#include "isofs.h"
12	11
...	...	@@ -168,6 +167,7 @@
168	167	int found;
169	168	unsigned long uninitialized_var(block);
170	169	unsigned long uninitialized_var(offset);
	170	+ struct isofs_sb_info *sbi = ISOFS_SB(dir->i_sb);
171	171	struct inode *inode;
172	172	struct page *page;
173	173
...	...	@@ -177,7 +177,7 @@
177	177	if (!page)
178	178	return ERR_PTR(-ENOMEM);
179	179
180		- lock_kernel();
	180	+ mutex_lock(&sbi->s_mutex);
181	181	found = isofs_find_entry(dir, dentry,
182	182	&block, &offset,
183	183	page_address(page),
184	184
...	...	@@ -188,11 +188,11 @@
188	188	if (found) {
189	189	inode = isofs_iget(dir->i_sb, block, offset);
190	190	if (IS_ERR(inode)) {
191		- unlock_kernel();
	191	+ mutex_unlock(&sbi->s_mutex);
192	192	return ERR_CAST(inode);
193	193	}
194	194	}
195		- unlock_kernel();
	195	+ mutex_unlock(&sbi->s_mutex);
196	196	return d_splice_alias(inode, dentry);
197	197	}
...	...	@@ -8,7 +8,6 @@
8	8
9	9	#include <linux/slab.h>
10	10	#include <linux/pagemap.h>
11		-#include <linux/smp_lock.h>
12	11
13	12	#include "isofs.h"
14	13	#include "rock.h"
...	...	@@ -661,6 +660,7 @@
661	660	{
662	661	struct inode *inode = page->mapping->host;
663	662	struct iso_inode_info *ei = ISOFS_I(inode);
	663	+ struct isofs_sb_info *sbi = ISOFS_SB(inode->i_sb);
664	664	char *link = kmap(page);
665	665	unsigned long bufsize = ISOFS_BUFFER_SIZE(inode);
666	666	struct buffer_head *bh;
667	667
...	...	@@ -673,12 +673,12 @@
673	673	struct rock_state rs;
674	674	int ret;
675	675
676		- if (!ISOFS_SB(inode->i_sb)->s_rock)
	676	+ if (!sbi->s_rock)
677	677	goto error;
678	678
679	679	init_rock_state(&rs, inode);
680	680	block = ei->i_iget5_block;
681		- lock_kernel();
	681	+ mutex_lock(&sbi->s_mutex);
682	682	bh = sb_bread(inode->i_sb, block);
683	683	if (!bh)
684	684	goto out_noread;
...	...	@@ -748,7 +748,7 @@
748	748	goto fail;
749	749	brelse(bh);
750	750	*rpnt = '\0';
751		- unlock_kernel();
	751	+ mutex_unlock(&sbi->s_mutex);
752	752	SetPageUptodate(page);
753	753	kunmap(page);
754	754	unlock_page(page);
...	...	@@ -765,7 +765,7 @@
765	765	printk("symlink spans iso9660 blocks\n");
766	766	fail:
767	767	brelse(bh);
768		- unlock_kernel();
	768	+ mutex_unlock(&sbi->s_mutex);
769	769	error:
770	770	SetPageError(page);
771	771	kunmap(page);