[patch 4/5] mtd: Remove BKL and convert to unlocked_ioctl

[patch 4/5] mtd: Remove BKL and convert to unlocked_ioctl

[Thomas Gleixner]

[-- Attachment #1: mtd-remove-bkl-and-convert-to-unlocked-ioctl.patch --]
[-- Type: text/plain, Size: 3113 bytes --]

mtd_open() got lock/unlock kernel from the big BKL push down, but it
never relied on the BKL serialization as get_mtd_device() takes care
of serialization vs. device init/teardown.

mtd_ioctl() is safe w/o the BKL as well. The data which is copied from
the mtd data structure is either set up during device initialization
or statistics which have never been protected by the BKL against
concurrent modification. The mtd functions which are called from
various ioctl commands are safe as well.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: linux-mtd@lists.infradead.org
---
 drivers/mtd/mtdchar.c |   30 ++++++++----------------------
 1 file changed, 8 insertions(+), 22 deletions(-)

Index: linux-2.6-tip/drivers/mtd/mtdchar.c
===================================================================
--- linux-2.6-tip.orig/drivers/mtd/mtdchar.c
+++ linux-2.6-tip/drivers/mtd/mtdchar.c
@@ -12,7 +12,6 @@
 #include <linux/module.h>
 #include <linux/slab.h>
 #include <linux/sched.h>
-#include <linux/smp_lock.h>
 #include <linux/backing-dev.h>
 #include <linux/compat.h>
 
@@ -74,18 +73,14 @@ static int mtd_open(struct inode *inode,
 	if ((file->f_mode & FMODE_WRITE) && (minor & 1))
 		return -EACCES;
 
-	lock_kernel();
 	mtd = get_mtd_device(NULL, devnum);
 
-	if (IS_ERR(mtd)) {
-		ret = PTR_ERR(mtd);
-		goto out;
-	}
+	if (IS_ERR(mtd))
+		return PTR_ERR(mtd);
 
 	if (mtd->type == MTD_ABSENT) {
 		put_mtd_device(mtd);
-		ret = -ENODEV;
-		goto out;
+		return -ENODEV;
 	}
 
 	if (mtd->backing_dev_info)
@@ -94,21 +89,17 @@ static int mtd_open(struct inode *inode,
 	/* You can't open it RW if it's not a writeable device */
 	if ((file->f_mode & FMODE_WRITE) && !(mtd->flags & MTD_WRITEABLE)) {
 		put_mtd_device(mtd);
-		ret = -EACCES;
-		goto out;
+		return -EACCES;
 	}
 
 	mfi = kzalloc(sizeof(*mfi), GFP_KERNEL);
 	if (!mfi) {
 		put_mtd_device(mtd);
-		ret = -ENOMEM;
-		goto out;
+		return -ENOMEM;
 	}
 	mfi->mtd = mtd;
 	file->private_data = mfi;
 
-out:
-	unlock_kernel();
 	return ret;
 } /* mtd_open */
 
@@ -450,13 +441,12 @@ static int mtd_do_readoob(struct mtd_inf
 	return ret;
 }
 
-static int mtd_ioctl(struct inode *inode, struct file *file,
-		     u_int cmd, u_long arg)
+static long mtd_ioctl(struct file *file, u_int cmd, u_long arg)
 {
 	struct mtd_file_info *mfi = file->private_data;
 	struct mtd_info *mtd = mfi->mtd;
 	void __user *argp = (void __user *)arg;
-	int ret = 0;
+	long ret = 0;
 	u_long size;
 	struct mtd_info_user info;
 
@@ -842,8 +832,6 @@ static long mtd_compat_ioctl(struct file
 	void __user *argp = compat_ptr(arg);
 	int ret = 0;
 
-	lock_kernel();
-
 	switch (cmd) {
 	case MEMWRITEOOB32:
 	{
@@ -877,8 +865,6 @@ static long mtd_compat_ioctl(struct file
 		ret = mtd_ioctl(inode, file, cmd, (unsigned long)argp);
 	}
 
-	unlock_kernel();
-
 	return ret;
 }
 
@@ -942,7 +928,7 @@ static const struct file_operations mtd_
 	.llseek		= mtd_lseek,
 	.read		= mtd_read,
 	.write		= mtd_write,
-	.ioctl		= mtd_ioctl,
+	.unlocked_ioctl	= mtd_ioctl,
 #ifdef CONFIG_COMPAT
 	.compat_ioctl	= mtd_compat_ioctl,
 #endif

Re: [patch 4/5] mtd: Remove BKL and convert to unlocked_ioctl

[Artem Bityutskiy]

On Thu, 2009-10-15 at 20:28 +0000, Thomas Gleixner wrote:
> plain text document attachment
> (mtd-remove-bkl-and-convert-to-unlocked-ioctl.patch)
> mtd_open() got lock/unlock kernel from the big BKL push down, but it
> never relied on the BKL serialization as get_mtd_device() takes care
> of serialization vs. device init/teardown.
> 
> mtd_ioctl() is safe w/o the BKL as well. The data which is copied from
> the mtd data structure is either set up during device initialization
> or statistics which have never been protected by the BKL against
> concurrent modification. The mtd functions which are called from
> various ioctl commands are safe as well.
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Cc: David Woodhouse <dwmw2@infradead.org>
> Cc: linux-mtd@lists.infradead.org
> ---
>  drivers/mtd/mtdchar.c |   30 ++++++++----------------------
>  1 file changed, 8 insertions(+), 22 deletions(-)

[dedekind@eru l2-mtd-2.6.git]$ make -j8
  CHK     include/linux/version.h
  CHK     include/linux/utsrelease.h
  SYMLINK include/asm -> include/asm-x86
  CALL    scripts/checksyscalls.sh
  CHK     include/linux/compile.h
  CC [M]  drivers/mtd/mtdchar.o
drivers/mtd/mtdchar.c: In function ‘mtd_compat_ioctl’:
drivers/mtd/mtdchar.c:865: warning: passing argument 1 of ‘mtd_ioctl’ from incompatible pointer type
drivers/mtd/mtdchar.c:444: note: expected ‘struct file *’ but argument is of type ‘struct inode *’
drivers/mtd/mtdchar.c:865: warning: passing argument 2 of ‘mtd_ioctl’ makes integer from pointer without a cast
drivers/mtd/mtdchar.c:444: note: expected ‘u_int’ but argument is of type ‘struct file *’
drivers/mtd/mtdchar.c:865: error: too many arguments to function ‘mtd_ioctl’

-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)

Re: [patch 4/5] mtd: Remove BKL and convert to unlocked_ioctl

[Thomas Gleixner]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1894 bytes --]

On Fri, 16 Oct 2009, Artem Bityutskiy wrote:
> On Thu, 2009-10-15 at 20:28 +0000, Thomas Gleixner wrote:
> > plain text document attachment
> > (mtd-remove-bkl-and-convert-to-unlocked-ioctl.patch)
> > mtd_open() got lock/unlock kernel from the big BKL push down, but it
> > never relied on the BKL serialization as get_mtd_device() takes care
> > of serialization vs. device init/teardown.
> > 
> > mtd_ioctl() is safe w/o the BKL as well. The data which is copied from
> > the mtd data structure is either set up during device initialization
> > or statistics which have never been protected by the BKL against
> > concurrent modification. The mtd functions which are called from
> > various ioctl commands are safe as well.
> > 
> > Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> > Cc: David Woodhouse <dwmw2@infradead.org>
> > Cc: linux-mtd@lists.infradead.org
> > ---
> >  drivers/mtd/mtdchar.c |   30 ++++++++----------------------
> >  1 file changed, 8 insertions(+), 22 deletions(-)
> 
> [dedekind@eru l2-mtd-2.6.git]$ make -j8
>   CHK     include/linux/version.h
>   CHK     include/linux/utsrelease.h
>   SYMLINK include/asm -> include/asm-x86
>   CALL    scripts/checksyscalls.sh
>   CHK     include/linux/compile.h
>   CC [M]  drivers/mtd/mtdchar.o
> drivers/mtd/mtdchar.c: In function ‘mtd_compat_ioctl’:
> drivers/mtd/mtdchar.c:865: warning: passing argument 1 of ‘mtd_ioctl’ from incompatible pointer type
> drivers/mtd/mtdchar.c:444: note: expected ‘struct file *’ but argument is of type ‘struct inode *’
> drivers/mtd/mtdchar.c:865: warning: passing argument 2 of ‘mtd_ioctl’ makes integer from pointer without a cast
> drivers/mtd/mtdchar.c:444: note: expected ‘u_int’ but argument is of type ‘struct file *’
> drivers/mtd/mtdchar.c:865: error: too many arguments to function ‘mtd_ioctl’

Yeah, found and fixed that already.

      tglx