ext3: remove the BKL in ext3/ioctl.c

Reformat ext3/ioctl.c to make it look more like ext4/ioctl.c and remove the BKL around ext3_ioctl(). Signed-off-by: Cyrus Massoumi <cyrusm@gmx.net> Cc: <linux-ext4@vger.kernel.org> Acked-by: Jan Kara <jack@ucw.cz> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

ext3: remove the BKL in ext3/ioctl.c
Reformat ext3/ioctl.c to make it look more like ext4/ioctl.c and remove the BKL around ext3_ioctl(). Signed-off-by: Cyrus Massoumi <cyrusm@gmx.net> Cc: <linux-ext4@vger.kernel.org> Acked-by: Jan Kara <jack@ucw.cz> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Cyrus Massoumi · Linus Torvalds
1 parent b277c884f7
Showing 4 changed files with 24 additions and 44 deletions Side-by-side Diff
fs/ext3/dir.c
fs/ext3/file.c
fs/ext3/ioctl.c
include/linux/ext3_fs.h
@@ -42,7 +42,7 @@
 	.llseek		= generic_file_llseek,
 	.read		= generic_read_dir,
 	.readdir	= ext3_readdir,		/* we take BKL. needed?*/
-	.ioctl		= ext3_ioctl,		/* BKL held */
+	.unlocked_ioctl	= ext3_ioctl,
 #ifdef CONFIG_COMPAT
 	.compat_ioctl	= ext3_compat_ioctl,
 #endif
@@ -112,7 +112,7 @@
 	.write		= do_sync_write,
 	.aio_read	= generic_file_aio_read,
 	.aio_write	= ext3_file_write,
-	.ioctl		= ext3_ioctl,
+	.unlocked_ioctl	= ext3_ioctl,
 #ifdef CONFIG_COMPAT
 	.compat_ioctl	= ext3_compat_ioctl,
 #endif
@@ -15,12 +15,11 @@
 #include <linux/mount.h>
 #include <linux/time.h>
 #include <linux/compat.h>
-#include <linux/smp_lock.h>
 #include <asm/uaccess.h>
  
-int ext3_ioctl (struct inode * inode, struct file * filp, unsigned int cmd,
-		unsigned long arg)
+long ext3_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 {
+	struct inode *inode = filp->f_dentry->d_inode;
 	struct ext3_inode_info *ei = EXT3_I(inode);
 	unsigned int flags;
 	unsigned short rsv_window_size;
  
  
  
  
@@ -39,29 +38,25 @@
 		unsigned int oldflags;
 		unsigned int jflag;
  
+		if (!is_owner_or_cap(inode))
+			return -EACCES;
+
+		if (get_user(flags, (int __user *) arg))
+			return -EFAULT;
+
 		err = mnt_want_write(filp->f_path.mnt);
 		if (err)
 			return err;
  
-		if (!is_owner_or_cap(inode)) {
-			err = -EACCES;
-			goto flags_out;
-		}
-
-		if (get_user(flags, (int __user *) arg)) {
-			err = -EFAULT;
-			goto flags_out;
-		}
-
 		flags = ext3_mask_flags(inode->i_mode, flags);
  
 		mutex_lock(&inode->i_mutex);
+
 		/* Is it quota file? Do not allow user to mess with it */
-		if (IS_NOQUOTA(inode)) {
-			mutex_unlock(&inode->i_mutex);
-			err = -EPERM;
+		err = -EPERM;
+		if (IS_NOQUOTA(inode))
 			goto flags_out;
-		}
+
 		oldflags = ei->i_flags;
  
 		/* The JOURNAL_DATA flag is modifiable only by root */
  
@@ -74,11 +69,8 @@
 		 * This test looks nicer. Thanks to Pauline Middelink
 		 */
 		if ((flags ^ oldflags) & (EXT3_APPEND_FL | EXT3_IMMUTABLE_FL)) {
-			if (!capable(CAP_LINUX_IMMUTABLE)) {
-				mutex_unlock(&inode->i_mutex);
-				err = -EPERM;
+			if (!capable(CAP_LINUX_IMMUTABLE))
 				goto flags_out;
-			}
 		}
  
 		/*
  
  
  
@@ -86,17 +78,12 @@
 		 * the relevant capability.
 		 */
 		if ((jflag ^ oldflags) & (EXT3_JOURNAL_DATA_FL)) {
-			if (!capable(CAP_SYS_RESOURCE)) {
-				mutex_unlock(&inode->i_mutex);
-				err = -EPERM;
+			if (!capable(CAP_SYS_RESOURCE))
 				goto flags_out;
-			}
 		}
  
-
 		handle = ext3_journal_start(inode, 1);
 		if (IS_ERR(handle)) {
-			mutex_unlock(&inode->i_mutex);
 			err = PTR_ERR(handle);
 			goto flags_out;
 		}
  
  
@@ -116,15 +103,13 @@
 		err = ext3_mark_iloc_dirty(handle, inode, &iloc);
 flags_err:
 		ext3_journal_stop(handle);
-		if (err) {
-			mutex_unlock(&inode->i_mutex);
-			return err;
-		}
+		if (err)
+			goto flags_out;
  
 		if ((jflag ^ oldflags) & (EXT3_JOURNAL_DATA_FL))
 			err = ext3_change_inode_journal_flag(inode, jflag);
-		mutex_unlock(&inode->i_mutex);
 flags_out:
+		mutex_unlock(&inode->i_mutex);
 		mnt_drop_write(filp->f_path.mnt);
 		return err;
 	}
@@ -140,6 +125,7 @@
  
 		if (!is_owner_or_cap(inode))
 			return -EPERM;
+
 		err = mnt_want_write(filp->f_path.mnt);
 		if (err)
 			return err;
@@ -147,6 +133,7 @@
 			err = -EFAULT;
 			goto setversion_out;
 		}
+
 		handle = ext3_journal_start(inode, 1);
 		if (IS_ERR(handle)) {
 			err = PTR_ERR(handle);
@@ -299,9 +286,6 @@
 #ifdef CONFIG_COMPAT
 long ext3_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 {
-	struct inode *inode = file->f_path.dentry->d_inode;
-	int ret;
-
 	/* These are just misnamed, they actually get/put from/to user an int */
 	switch (cmd) {
 	case EXT3_IOC32_GETFLAGS:
@@ -341,10 +325,7 @@
 	default:
 		return -ENOIOCTLCMD;
 	}
-	lock_kernel();
-	ret = ext3_ioctl(inode, file, cmd, (unsigned long) compat_ptr(arg));
-	unlock_kernel();
-	return ret;
+	return ext3_ioctl(file, cmd, (unsigned long) compat_ptr(arg));
 }
 #endif
@@ -893,9 +893,8 @@
 		       u64 start, u64 len);
  
 /* ioctl.c */
-extern int ext3_ioctl (struct inode *, struct file *, unsigned int,
-		       unsigned long);
-extern long ext3_compat_ioctl (struct file *, unsigned int, unsigned long);
+extern long ext3_ioctl(struct file *, unsigned int, unsigned long);
+extern long ext3_compat_ioctl(struct file *, unsigned int, unsigned long);
  
 /* namei.c */
 extern int ext3_orphan_add(handle_t *, struct inode *);
...	...	@@ -42,7 +42,7 @@
42	42	.llseek = generic_file_llseek,
43	43	.read = generic_read_dir,
44	44	.readdir = ext3_readdir, /* we take BKL. needed?*/
45		- .ioctl = ext3_ioctl, /* BKL held */
	45	+ .unlocked_ioctl = ext3_ioctl,
46	46	#ifdef CONFIG_COMPAT
47	47	.compat_ioctl = ext3_compat_ioctl,
48	48	#endif
...	...	@@ -112,7 +112,7 @@
112	112	.write = do_sync_write,
113	113	.aio_read = generic_file_aio_read,
114	114	.aio_write = ext3_file_write,
115		- .ioctl = ext3_ioctl,
	115	+ .unlocked_ioctl = ext3_ioctl,
116	116	#ifdef CONFIG_COMPAT
117	117	.compat_ioctl = ext3_compat_ioctl,
118	118	#endif
...	...	@@ -15,12 +15,11 @@
15	15	#include <linux/mount.h>
16	16	#include <linux/time.h>
17	17	#include <linux/compat.h>
18		-#include <linux/smp_lock.h>
19	18	#include <asm/uaccess.h>
20	19
21		-int ext3_ioctl (struct inode * inode, struct file * filp, unsigned int cmd,
22		- unsigned long arg)
	20	+long ext3_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
23	21	{
	22	+ struct inode *inode = filp->f_dentry->d_inode;
24	23	struct ext3_inode_info *ei = EXT3_I(inode);
25	24	unsigned int flags;
26	25	unsigned short rsv_window_size;
27	26
28	27
29	28
30	29
...	...	@@ -39,29 +38,25 @@
39	38	unsigned int oldflags;
40	39	unsigned int jflag;
41	40
	41	+ if (!is_owner_or_cap(inode))
	42	+ return -EACCES;
	43	+
	44	+ if (get_user(flags, (int __user *) arg))
	45	+ return -EFAULT;
	46	+
42	47	err = mnt_want_write(filp->f_path.mnt);
43	48	if (err)
44	49	return err;
45	50
46		- if (!is_owner_or_cap(inode)) {
47		- err = -EACCES;
48		- goto flags_out;
49		- }
50		-
51		- if (get_user(flags, (int __user *) arg)) {
52		- err = -EFAULT;
53		- goto flags_out;
54		- }
55		-
56	51	flags = ext3_mask_flags(inode->i_mode, flags);
57	52
58	53	mutex_lock(&inode->i_mutex);
	54	+
59	55	/* Is it quota file? Do not allow user to mess with it */
60		- if (IS_NOQUOTA(inode)) {
61		- mutex_unlock(&inode->i_mutex);
62		- err = -EPERM;
	56	+ err = -EPERM;
	57	+ if (IS_NOQUOTA(inode))
63	58	goto flags_out;
64		- }
	59	+
65	60	oldflags = ei->i_flags;
66	61
67	62	/* The JOURNAL_DATA flag is modifiable only by root */
68	63
...	...	@@ -74,11 +69,8 @@
74	69	* This test looks nicer. Thanks to Pauline Middelink
75	70	*/
76	71	if ((flags ^ oldflags) & (EXT3_APPEND_FL \| EXT3_IMMUTABLE_FL)) {
77		- if (!capable(CAP_LINUX_IMMUTABLE)) {
78		- mutex_unlock(&inode->i_mutex);
79		- err = -EPERM;
	72	+ if (!capable(CAP_LINUX_IMMUTABLE))
80	73	goto flags_out;
81		- }
82	74	}
83	75
84	76	/*
85	77
86	78
87	79
...	...	@@ -86,17 +78,12 @@
86	78	* the relevant capability.
87	79	*/
88	80	if ((jflag ^ oldflags) & (EXT3_JOURNAL_DATA_FL)) {
89		- if (!capable(CAP_SYS_RESOURCE)) {
90		- mutex_unlock(&inode->i_mutex);
91		- err = -EPERM;
	81	+ if (!capable(CAP_SYS_RESOURCE))
92	82	goto flags_out;
93		- }
94	83	}
95	84
96		-
97	85	handle = ext3_journal_start(inode, 1);
98	86	if (IS_ERR(handle)) {
99		- mutex_unlock(&inode->i_mutex);
100	87	err = PTR_ERR(handle);
101	88	goto flags_out;
102	89	}
103	90
104	91
...	...	@@ -116,15 +103,13 @@
116	103	err = ext3_mark_iloc_dirty(handle, inode, &iloc);
117	104	flags_err:
118	105	ext3_journal_stop(handle);
119		- if (err) {
120		- mutex_unlock(&inode->i_mutex);
121		- return err;
122		- }
	106	+ if (err)
	107	+ goto flags_out;
123	108
124	109	if ((jflag ^ oldflags) & (EXT3_JOURNAL_DATA_FL))
125	110	err = ext3_change_inode_journal_flag(inode, jflag);
126		- mutex_unlock(&inode->i_mutex);
127	111	flags_out:
	112	+ mutex_unlock(&inode->i_mutex);
128	113	mnt_drop_write(filp->f_path.mnt);
129	114	return err;
130	115	}
...	...	@@ -140,6 +125,7 @@
140	125
141	126	if (!is_owner_or_cap(inode))
142	127	return -EPERM;
	128	+
143	129	err = mnt_want_write(filp->f_path.mnt);
144	130	if (err)
145	131	return err;
...	...	@@ -147,6 +133,7 @@
147	133	err = -EFAULT;
148	134	goto setversion_out;
149	135	}
	136	+
150	137	handle = ext3_journal_start(inode, 1);
151	138	if (IS_ERR(handle)) {
152	139	err = PTR_ERR(handle);
...	...	@@ -299,9 +286,6 @@
299	286	#ifdef CONFIG_COMPAT
300	287	long ext3_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
301	288	{
302		- struct inode *inode = file->f_path.dentry->d_inode;
303		- int ret;
304		-
305	289	/* These are just misnamed, they actually get/put from/to user an int */
306	290	switch (cmd) {
307	291	case EXT3_IOC32_GETFLAGS:
...	...	@@ -341,10 +325,7 @@
341	325	default:
342	326	return -ENOIOCTLCMD;
343	327	}
344		- lock_kernel();
345		- ret = ext3_ioctl(inode, file, cmd, (unsigned long) compat_ptr(arg));
346		- unlock_kernel();
347		- return ret;
	328	+ return ext3_ioctl(file, cmd, (unsigned long) compat_ptr(arg));
348	329	}
349	330	#endif
...	...	@@ -893,9 +893,8 @@
893	893	u64 start, u64 len);
894	894
895	895	/* ioctl.c */
896		-extern int ext3_ioctl (struct inode , struct file , unsigned int,
897		- unsigned long);
898		-extern long ext3_compat_ioctl (struct file *, unsigned int, unsigned long);
	896	+extern long ext3_ioctl(struct file *, unsigned int, unsigned long);
	897	+extern long ext3_compat_ioctl(struct file *, unsigned int, unsigned long);
899	898
900	899	/* namei.c */
901	900	extern int ext3_orphan_add(handle_t , struct inode );