[PATCH 0/6] USB: BKL removal

[PATCH 0/6] USB: BKL removal

[Arnd Bergmann]

Hi Greg,

Here goes another series of BKL removal patches, mostly
from Andi. There are some minor drivers left in the
USB subsystem that someone should take care of, but
this series at least gets to the point where USB
works with the BKL disabled.

Please apply to your USB next tree.

	Arnd

Andi Kleen (4):
  USB-BKL: Remove lock_kernel in usbfs update_sb()
  USB-BKL: Convert usb_driver ioctl to unlocked_ioctl
  USB-BKL: Remove BKL use for usb serial driver probing
  USB-BKL: Remove BKL use in uhci-debug

Arnd Bergmann (2):
  usb/gadget: Do not take BKL for gadget->ops->ioctl
  usb/mon: kill BKL usage

 drivers/usb/core/devio.c        |    7 ++-----
 drivers/usb/core/hub.c          |    3 ++-
 drivers/usb/core/inode.c        |    4 ----
 drivers/usb/gadget/f_fs.c       |    2 --
 drivers/usb/gadget/inode.c      |    6 ++----
 drivers/usb/host/uhci-debug.c   |   17 ++++++-----------
 drivers/usb/misc/usbtest.c      |    3 ++-
 drivers/usb/mon/mon_bin.c       |   22 ++--------------------
 drivers/usb/serial/usb-serial.c |   30 ++++++++++++++----------------
 include/linux/usb.h             |    2 +-
 10 files changed, 31 insertions(+), 65 deletions(-)

[PATCH 1/6] USB-BKL: Remove lock_kernel in usbfs update_sb()

[Arnd Bergmann]

From: Andi Kleen <ak@linux.intel.com>

The code this is attempting to lock against does not use the BKL,
so it's not needed.

Most likely this code is still broken/racy (Al Viro also thinks so),
but removing the BKL should not make it worse than before.

Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 drivers/usb/core/inode.c |    4 ----
 1 files changed, 0 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/core/inode.c b/drivers/usb/core/inode.c
index 1a27618..095fa53 100644
--- a/drivers/usb/core/inode.c
+++ b/drivers/usb/core/inode.c
@@ -265,13 +265,9 @@ static int remount(struct super_block *sb, int *flags, char *data)
 		return -EINVAL;
 	}
 
-	lock_kernel();
-
 	if (usbfs_mount && usbfs_mount->mnt_sb)
 		update_sb(usbfs_mount->mnt_sb);
 
-	unlock_kernel();
-
 	return 0;
 }
 
-- 
1.7.0.4

[PATCH 2/6] USB-BKL: Convert usb_driver ioctl to unlocked_ioctl

[Arnd Bergmann]

From: Andi Kleen <ak@linux.intel.com>

And audit all the users. None needed the BKL.  That was easy
because there was only very few around.

Tested with allmodconfig build on x86-64

Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 drivers/usb/core/devio.c   |    7 ++-----
 drivers/usb/core/hub.c     |    3 ++-
 drivers/usb/misc/usbtest.c |    3 ++-
 include/linux/usb.h        |    2 +-
 4 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
index c2f62a3..f1aaff6 100644
--- a/drivers/usb/core/devio.c
+++ b/drivers/usb/core/devio.c
@@ -1668,13 +1668,10 @@ static int proc_ioctl(struct dev_state *ps, struct usbdevfs_ioctl *ctl)
 	default:
 		if (intf->dev.driver)
 			driver = to_usb_driver(intf->dev.driver);
-		if (driver == NULL || driver->ioctl == NULL) {
+		if (driver == NULL || driver->unlocked_ioctl == NULL) {
 			retval = -ENOTTY;
 		} else {
-			/* keep API that guarantees BKL */
-			lock_kernel();
-			retval = driver->ioctl(intf, ctl->ioctl_code, buf);
-			unlock_kernel();
+			retval = driver->unlocked_ioctl(intf, ctl->ioctl_code, buf);
 			if (retval == -ENOIOCTLCMD)
 				retval = -ENOTTY;
 		}
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 83e7bbb..d0e77d4 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -1294,6 +1294,7 @@ descriptor_error:
 	return -ENODEV;
 }
 
+/* No BKL needed */
 static int
 hub_ioctl(struct usb_interface *intf, unsigned int code, void *user_data)
 {
@@ -3461,7 +3462,7 @@ static struct usb_driver hub_driver = {
 	.reset_resume =	hub_reset_resume,
 	.pre_reset =	hub_pre_reset,
 	.post_reset =	hub_post_reset,
-	.ioctl =	hub_ioctl,
+	.unlocked_ioctl = hub_ioctl,
 	.id_table =	hub_id_table,
 	.supports_autosuspend =	1,
 };
diff --git a/drivers/usb/misc/usbtest.c b/drivers/usb/misc/usbtest.c
index 16dffe9..0cfbd78 100644
--- a/drivers/usb/misc/usbtest.c
+++ b/drivers/usb/misc/usbtest.c
@@ -1548,6 +1548,7 @@ fail:
  * off just killing the userspace task and waiting for it to exit.
  */
 
+/* No BKL needed */
 static int
 usbtest_ioctl (struct usb_interface *intf, unsigned int code, void *buf)
 {
@@ -2170,7 +2171,7 @@ static struct usb_driver usbtest_driver = {
 	.name =		"usbtest",
 	.id_table =	id_table,
 	.probe =	usbtest_probe,
-	.ioctl =	usbtest_ioctl,
+	.unlocked_ioctl = usbtest_ioctl,
 	.disconnect =	usbtest_disconnect,
 	.suspend =	usbtest_suspend,
 	.resume =	usbtest_resume,
diff --git a/include/linux/usb.h b/include/linux/usb.h
index d5922a8..e6cbc34 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -843,7 +843,7 @@ struct usb_driver {
 
 	void (*disconnect) (struct usb_interface *intf);
 
-	int (*ioctl) (struct usb_interface *intf, unsigned int code,
+	int (*unlocked_ioctl) (struct usb_interface *intf, unsigned int code,
 			void *buf);
 
 	int (*suspend) (struct usb_interface *intf, pm_message_t message);
-- 
1.7.0.4

[PATCH 3/6] USB-BKL: Remove BKL use for usb serial driver probing

[Arnd Bergmann]

From: Andi Kleen <ak@linux.intel.com>

The usb serial driver initialization tried to use the BKL to stop
driver modules from unloading, but that didn't work anyways.

There was already some code to do proper try_module_get,
but it was conditional on having a new probe interface.
I checked all the low level drivers and they all have proper
.owner = THIS_MODULE, so it's ok to always use.

The other problem was the usb_serial_driver_list needing
protection by a lock. This was broken anyways because unregister
did not necessarily have the BKL.

I extended the extending table_lock mutex to protect this case too.

With these changes the BKL can be removed here.

Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 drivers/usb/serial/usb-serial.c |   30 ++++++++++++++----------------
 1 files changed, 14 insertions(+), 16 deletions(-)

diff --git a/drivers/usb/serial/usb-serial.c b/drivers/usb/serial/usb-serial.c
index 941c2d4..443468e 100644
--- a/drivers/usb/serial/usb-serial.c
+++ b/drivers/usb/serial/usb-serial.c
@@ -653,6 +653,7 @@ exit:
 	return id;
 }
 
+/* Caller must hold table_lock */
 static struct usb_serial_driver *search_serial_device(
 					struct usb_interface *iface)
 {
@@ -718,17 +719,23 @@ int usb_serial_probe(struct usb_interface *interface,
 	int num_ports = 0;
 	int max_endpoints;
 
-	lock_kernel(); /* guard against unloading a serial driver module */
+	mutex_lock(&table_lock);
 	type = search_serial_device(interface);
 	if (!type) {
-		unlock_kernel();
+		mutex_unlock(&table_lock);
 		dbg("none matched");
 		return -ENODEV;
 	}
 
+	if (!try_module_get(type->driver.owner)) {
+		mutex_unlock(&table_lock);
+		dev_err(&interface->dev, "module get failed, exiting\n");
+		return -EIO;
+	}
+	mutex_unlock(&table_lock);
+
 	serial = create_serial(dev, interface, type);
 	if (!serial) {
-		unlock_kernel();
 		dev_err(&interface->dev, "%s - out of memory\n", __func__);
 		return -ENOMEM;
 	}
@@ -737,20 +744,11 @@ int usb_serial_probe(struct usb_interface *interface,
 	if (type->probe) {
 		const struct usb_device_id *id;
 
-		if (!try_module_get(type->driver.owner)) {
-			unlock_kernel();
-			dev_err(&interface->dev,
-				"module get failed, exiting\n");
-			kfree(serial);
-			return -EIO;
-		}
-
 		id = get_iface_id(type, interface);
 		retval = type->probe(serial, id);
 		module_put(type->driver.owner);
 
 		if (retval) {
-			unlock_kernel();
 			dbg("sub driver rejected device");
 			kfree(serial);
 			return retval;
@@ -822,7 +820,6 @@ int usb_serial_probe(struct usb_interface *interface,
 		 * properly during a later invocation of usb_serial_probe
 		 */
 		if (num_bulk_in == 0 || num_bulk_out == 0) {
-			unlock_kernel();
 			dev_info(&interface->dev, "PL-2303 hack: descriptors matched but endpoints did not\n");
 			kfree(serial);
 			return -ENODEV;
@@ -835,7 +832,6 @@ int usb_serial_probe(struct usb_interface *interface,
 	if (type == &usb_serial_generic_device) {
 		num_ports = num_bulk_out;
 		if (num_ports == 0) {
-			unlock_kernel();
 			dev_err(&interface->dev,
 			    "Generic device with no bulk out, not allowed.\n");
 			kfree(serial);
@@ -847,7 +843,6 @@ int usb_serial_probe(struct usb_interface *interface,
 		/* if this device type has a calc_num_ports function, call it */
 		if (type->calc_num_ports) {
 			if (!try_module_get(type->driver.owner)) {
-				unlock_kernel();
 				dev_err(&interface->dev,
 					"module get failed, exiting\n");
 				kfree(serial);
@@ -878,7 +873,6 @@ int usb_serial_probe(struct usb_interface *interface,
 	max_endpoints = max(max_endpoints, num_interrupt_out);
 	max_endpoints = max(max_endpoints, (int)serial->num_ports);
 	serial->num_port_pointers = max_endpoints;
-	unlock_kernel();
 
 	dbg("%s - setting up %d port structures for this device",
 						__func__, max_endpoints);
@@ -1349,6 +1343,7 @@ int usb_serial_register(struct usb_serial_driver *driver)
 		driver->description = driver->driver.name;
 
 	/* Add this device to our list of devices */
+	mutex_lock(&table_lock);
 	list_add(&driver->driver_list, &usb_serial_driver_list);
 
 	retval = usb_serial_bus_register(driver);
@@ -1360,6 +1355,7 @@ int usb_serial_register(struct usb_serial_driver *driver)
 		printk(KERN_INFO "USB Serial support registered for %s\n",
 						driver->description);
 
+	mutex_unlock(&table_lock);
 	return retval;
 }
 EXPORT_SYMBOL_GPL(usb_serial_register);
@@ -1370,8 +1366,10 @@ void usb_serial_deregister(struct usb_serial_driver *device)
 	/* must be called with BKL held */
 	printk(KERN_INFO "USB Serial deregistering driver %s\n",
 	       device->description);
+	mutex_lock(&table_lock);
 	list_del(&device->driver_list);
 	usb_serial_bus_deregister(device);
+	mutex_unlock(&table_lock);
 }
 EXPORT_SYMBOL_GPL(usb_serial_deregister);
 
-- 
1.7.0.4

[PATCH 4/6] USB-BKL: Remove BKL use in uhci-debug

[Arnd Bergmann]

From: Andi Kleen <ak@linux.intel.com>

BKL was not really needed, just came from earlier push downs.

The only part that's a bit dodgy is the lseek function. Would
need another lock or atomic access to fpos on 32bit?
Better to have a libfs lseek

Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 drivers/usb/host/uhci-debug.c |   17 ++++++-----------
 1 files changed, 6 insertions(+), 11 deletions(-)

diff --git a/drivers/usb/host/uhci-debug.c b/drivers/usb/host/uhci-debug.c
index 98cf0b2..b0cf4f8 100644
--- a/drivers/usb/host/uhci-debug.c
+++ b/drivers/usb/host/uhci-debug.c
@@ -495,18 +495,16 @@ static int uhci_debug_open(struct inode *inode, struct file *file)
 {
 	struct uhci_hcd *uhci = inode->i_private;
 	struct uhci_debug *up;
-	int ret = -ENOMEM;
 	unsigned long flags;
 
-	lock_kernel();
 	up = kmalloc(sizeof(*up), GFP_KERNEL);
 	if (!up)
-		goto out;
+		return -ENOMEM;
 
 	up->data = kmalloc(MAX_OUTPUT, GFP_KERNEL);
 	if (!up->data) {
 		kfree(up);
-		goto out;
+		return -ENOMEM;
 	}
 
 	up->size = 0;
@@ -517,10 +515,7 @@ static int uhci_debug_open(struct inode *inode, struct file *file)
 
 	file->private_data = up;
 
-	ret = 0;
-out:
-	unlock_kernel();
-	return ret;
+	return 0;
 }
 
 static loff_t uhci_debug_lseek(struct file *file, loff_t off, int whence)
@@ -528,9 +523,9 @@ static loff_t uhci_debug_lseek(struct file *file, loff_t off, int whence)
 	struct uhci_debug *up;
 	loff_t new = -1;
 
-	lock_kernel();
 	up = file->private_data;
 
+	/* XXX: atomic 64bit seek access, but that needs to be fixed in the VFS */
 	switch (whence) {
 	case 0:
 		new = off;
@@ -539,11 +534,11 @@ static loff_t uhci_debug_lseek(struct file *file, loff_t off, int whence)
 		new = file->f_pos + off;
 		break;
 	}
+
+	/* XXX: Can size shrink? */
 	if (new < 0 || new > up->size) {
-		unlock_kernel();
 		return -EINVAL;
 	}
-	unlock_kernel();
 	return (file->f_pos = new);
 }
 
-- 
1.7.0.4

Re: [PATCH 4/6] USB-BKL: Remove BKL use in uhci-debug

[Sergei Shtylyov]

Hello.

Arnd Bergmann wrote:

> From: Andi Kleen <ak@linux.intel.com>

> BKL was not really needed, just came from earlier push downs.

> The only part that's a bit dodgy is the lseek function. Would
> need another lock or atomic access to fpos on 32bit?
> Better to have a libfs lseek

> Signed-off-by: Andi Kleen <ak@linux.intel.com>

[...]

> diff --git a/drivers/usb/host/uhci-debug.c b/drivers/usb/host/uhci-debug.c
> index 98cf0b2..b0cf4f8 100644
> --- a/drivers/usb/host/uhci-debug.c
> +++ b/drivers/usb/host/uhci-debug.c

[...]

> @@ -539,11 +534,11 @@ static loff_t uhci_debug_lseek(struct file *file, loff_t off, int whence)
>  		new = file->f_pos + off;
>  		break;
>  	}
> +
> +	/* XXX: Can size shrink? */
>  	if (new < 0 || new > up->size) {
> -		unlock_kernel();
>  		return -EINVAL;
>  	}

    Should have dropped {}...

> -	unlock_kernel();
>  	return (file->f_pos = new);
>  }

WBR, Sergei

Re: [PATCH 4/6] USB-BKL: Remove BKL use in uhci-debug

[Greg KH]

On Wed, Jun 02, 2010 at 02:11:53PM +0400, Sergei Shtylyov wrote:
> Hello.
> 
> Arnd Bergmann wrote:
> 
> >From: Andi Kleen <ak@linux.intel.com>
> 
> >BKL was not really needed, just came from earlier push downs.
> 
> >The only part that's a bit dodgy is the lseek function. Would
> >need another lock or atomic access to fpos on 32bit?
> >Better to have a libfs lseek
> 
> >Signed-off-by: Andi Kleen <ak@linux.intel.com>
> 
> [...]
> 
> >diff --git a/drivers/usb/host/uhci-debug.c b/drivers/usb/host/uhci-debug.c
> >index 98cf0b2..b0cf4f8 100644
> >--- a/drivers/usb/host/uhci-debug.c
> >+++ b/drivers/usb/host/uhci-debug.c
> 
> [...]
> 
> >@@ -539,11 +534,11 @@ static loff_t uhci_debug_lseek(struct file *file, loff_t off, int whence)
> > 		new = file->f_pos + off;
> > 		break;
> > 	}
> >+
> >+	/* XXX: Can size shrink? */
> > 	if (new < 0 || new > up->size) {
> >-		unlock_kernel();
> > 		return -EINVAL;
> > 	}
> 
>    Should have dropped {}...

I've fixed that up in the version that got applied.

thanks,

greg k-h

Re: [PATCH 4/6] USB-BKL: Remove BKL use in uhci-debug

[Alan Stern]

On Tue, 1 Jun 2010, Arnd Bergmann wrote:

> From: Andi Kleen <ak@linux.intel.com>
> 
> BKL was not really needed, just came from earlier push downs.

Yes.

> The only part that's a bit dodgy is the lseek function. Would
> need another lock or atomic access to fpos on 32bit?
> Better to have a libfs lseek

It doesn't matter.  Anyone who tries to do lseeks on this file 
from two different threads, simultaneously, deserves what they get.

> @@ -539,11 +534,11 @@ static loff_t uhci_debug_lseek(struct file *file, loff_t off, int whence)
>  		new = file->f_pos + off;
>  		break;
>  	}
> +
> +	/* XXX: Can size shrink? */
>  	if (new < 0 || new > up->size) {
> -		unlock_kernel();
>  		return -EINVAL;
>  	}
> -	unlock_kernel();
>  	return (file->f_pos = new);
>  }

This comment isn't needed; the size cannot change after the file has 
been opened.

Alan Stern

Re: [PATCH 4/6] USB-BKL: Remove BKL use in uhci-debug

[Greg KH]

On Wed, Jun 02, 2010 at 09:47:46AM -0400, Alan Stern wrote:
> On Tue, 1 Jun 2010, Arnd Bergmann wrote:
> 
> > From: Andi Kleen <ak@linux.intel.com>
> > 
> > BKL was not really needed, just came from earlier push downs.
> 
> Yes.
> 
> > The only part that's a bit dodgy is the lseek function. Would
> > need another lock or atomic access to fpos on 32bit?
> > Better to have a libfs lseek
> 
> It doesn't matter.  Anyone who tries to do lseeks on this file 
> from two different threads, simultaneously, deserves what they get.
> 
> > @@ -539,11 +534,11 @@ static loff_t uhci_debug_lseek(struct file *file, loff_t off, int whence)
> >  		new = file->f_pos + off;
> >  		break;
> >  	}
> > +
> > +	/* XXX: Can size shrink? */
> >  	if (new < 0 || new > up->size) {
> > -		unlock_kernel();
> >  		return -EINVAL;
> >  	}
> > -	unlock_kernel();
> >  	return (file->f_pos = new);
> >  }
> 
> This comment isn't needed; the size cannot change after the file has 
> been opened.

I've removed the comment in the version I just committed.

thanks,

greg k-h

[PATCH 5/6] usb/gadget: Do not take BKL for gadget->ops->ioctl

[Arnd Bergmann]

There is no gadget driver in the tree that
actually implements the ioctl operation, so
obviously it is not necessary to hold the
BKL around the call.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/usb/gadget/f_fs.c  |    2 --
 drivers/usb/gadget/inode.c |    6 ++----
 2 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/gadget/f_fs.c b/drivers/usb/gadget/f_fs.c
index d69eccf..715da23 100644
--- a/drivers/usb/gadget/f_fs.c
+++ b/drivers/usb/gadget/f_fs.c
@@ -714,9 +714,7 @@ static long ffs_ep0_ioctl(struct file *file, unsigned code, unsigned long value)
 		struct ffs_function *func = ffs->func;
 		ret = func ? ffs_func_revmap_intf(func, value) : -ENODEV;
 	} else if (gadget->ops->ioctl) {
-		lock_kernel();
 		ret = gadget->ops->ioctl(gadget, code, value);
-		unlock_kernel();
 	} else {
 		ret = -ENOTTY;
 	}
diff --git a/drivers/usb/gadget/inode.c b/drivers/usb/gadget/inode.c
index de8a838..5308392 100644
--- a/drivers/usb/gadget/inode.c
+++ b/drivers/usb/gadget/inode.c
@@ -1299,11 +1299,9 @@ static long dev_ioctl (struct file *fd, unsigned code, unsigned long value)
 	struct usb_gadget	*gadget = dev->gadget;
 	long ret = -ENOTTY;
 
-	if (gadget->ops->ioctl) {
-		lock_kernel();
+	if (gadget->ops->ioctl)
 		ret = gadget->ops->ioctl (gadget, code, value);
-		unlock_kernel();
-	}
+
 	return ret;
 }
 
-- 
1.7.0.4

Re: [PATCH 5/6] usb/gadget: Do not take BKL for gadget->ops->ioctl

[Michał Nazarewicz]

On Tue, 01 Jun 2010 23:04:44 +0200, Arnd Bergmann <arnd@arndb.de> wrote:
> There is no gadget driver in the tree that
> actually implements the ioctl operation, so
> obviously it is not necessary to hold the
> BKL around the call.

Should the callback in ops be renamed to unlocked_ioctl then as to not
create confusion? Just a thought.

> --- a/drivers/usb/gadget/f_fs.c
> +++ b/drivers/usb/gadget/f_fs.c
> @@ -714,9 +714,7 @@ static long ffs_ep0_ioctl(struct file *file, unsigned code, unsigned long value)
>  		struct ffs_function *func = ffs->func;
>  		ret = func ? ffs_func_revmap_intf(func, value) : -ENODEV;
>  	} else if (gadget->ops->ioctl) {
> -		lock_kernel();
>  		ret = gadget->ops->ioctl(gadget, code, value);
> -		unlock_kernel();
>  	} else {
>  		ret = -ENOTTY;
>  	}

> --- a/drivers/usb/gadget/inode.c
> +++ b/drivers/usb/gadget/inode.c
> @@ -1299,11 +1299,9 @@ static long dev_ioctl (struct file *fd, unsigned code, unsigned long value)
>  	struct usb_gadget	*gadget = dev->gadget;
>  	long ret = -ENOTTY;
>-	if (gadget->ops->ioctl) {
> -		lock_kernel();
> +	if (gadget->ops->ioctl)
>  		ret = gadget->ops->ioctl (gadget, code, value);
> -		unlock_kernel();
> -	}
> +
>  	return ret;
>  }

-- 
Best regards,                                        _     _
| Humble Liege of Serenely Enlightened Majesty of  o' \,=./ `o
| Computer Science,  Michał "mina86" Nazarewicz       (o o)
+----[mina86*mina86.com]---[mina86*jabber.org]----ooO--(_)--Ooo--

Re: [PATCH 5/6] usb/gadget: Do not take BKL for gadget->ops->ioctl

[Arnd Bergmann]

On Friday 18 June 2010, Michał Nazarewicz wrote:
> On Tue, 01 Jun 2010 23:04:44 +0200, Arnd Bergmann <arnd@arndb.de> wrote:
> > There is no gadget driver in the tree that
> > actually implements the ioctl operation, so
> > obviously it is not necessary to hold the
> > BKL around the call.
> 
> Should the callback in ops be renamed to unlocked_ioctl then as to not
> create confusion? Just a thought.

No, we decided that the .unlocked_ioctl naming was a bad idea in the other
places after all and they should eventually get renamed back to .ioctl
once all the locked versions are gone.

	Arnd

Re: [PATCH 5/6] usb/gadget: Do not take BKL for gadget->ops->ioctl

[David Brownell]

> Date: Friday, June 18, 2010, 6:59 AM
> On Tue, 01 Jun 2010 23:04:44 +0200,
> Arnd Bergmann <arnd@arndb.de>
> wrote:
> > There is no gadget driver in the tree that
> > actually implements the ioctl operation, so
> > obviously it is not necessary to hold the
> > BKL around the call.

The original gadgetfs code used it as a
passthrough to controller-specific ops, as
I recall.  Not much used, right.

And yes, I suspect someone just threw
some BKL stuff here without actually
analysing whether it was necessary.

[PATCH 6/6] usb/mon: kill BKL usage

[Arnd Bergmann]

compat_ioctl does not use the BKL, so I assume that
the native function does not need it either.

The open function is already protected by the
driver's mutex, the BKL is probably not needed
here either.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/usb/mon/mon_bin.c |   22 ++--------------------
 1 files changed, 2 insertions(+), 20 deletions(-)

diff --git a/drivers/usb/mon/mon_bin.c b/drivers/usb/mon/mon_bin.c
index 61c76b1..1be0b9f 100644
--- a/drivers/usb/mon/mon_bin.c
+++ b/drivers/usb/mon/mon_bin.c
@@ -646,17 +646,14 @@ static int mon_bin_open(struct inode *inode, struct file *file)
 	size_t size;
 	int rc;
 
-	lock_kernel();
 	mutex_lock(&mon_lock);
 	if ((mbus = mon_bus_lookup(iminor(inode))) == NULL) {
 		mutex_unlock(&mon_lock);
-		unlock_kernel();
 		return -ENODEV;
 	}
 	if (mbus != &mon_bus0 && mbus->u_bus == NULL) {
 		printk(KERN_ERR TAG ": consistency error on open\n");
 		mutex_unlock(&mon_lock);
-		unlock_kernel();
 		return -ENODEV;
 	}
 
@@ -689,7 +686,6 @@ static int mon_bin_open(struct inode *inode, struct file *file)
 
 	file->private_data = rp;
 	mutex_unlock(&mon_lock);
-	unlock_kernel();
 	return 0;
 
 err_allocbuff:
@@ -698,7 +694,6 @@ err_allocvec:
 	kfree(rp);
 err_alloc:
 	mutex_unlock(&mon_lock);
-	unlock_kernel();
 	return rc;
 }
 
@@ -954,7 +949,7 @@ static int mon_bin_queued(struct mon_reader_bin *rp)
 
 /*
  */
-static int mon_bin_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
+static long mon_bin_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 {
 	struct mon_reader_bin *rp = file->private_data;
 	// struct mon_bus* mbus = rp->r.m_bus;
@@ -1094,19 +1089,6 @@ static int mon_bin_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 	return ret;
 }
 
-static long mon_bin_unlocked_ioctl(struct file *file, unsigned int cmd,
-				   unsigned long arg)
-{
-	int ret;
-
-	lock_kernel();
-	ret = mon_bin_ioctl(file, cmd, arg);
-	unlock_kernel();
-
-	return ret;
-}
-
-
 #ifdef CONFIG_COMPAT
 static long mon_bin_compat_ioctl(struct file *file,
     unsigned int cmd, unsigned long arg)
@@ -1250,7 +1232,7 @@ static const struct file_operations mon_fops_binary = {
 	.read =		mon_bin_read,
 	/* .write =	mon_text_write, */
 	.poll =		mon_bin_poll,
-	.unlocked_ioctl = mon_bin_unlocked_ioctl,
+	.unlocked_ioctl = mon_bin_ioctl,
 #ifdef CONFIG_COMPAT
 	.compat_ioctl =	mon_bin_compat_ioctl,
 #endif
-- 
1.7.0.4

Re: [PATCH 0/6] USB: BKL removal

[Greg KH]

On Tue, Jun 01, 2010 at 11:04:39PM +0200, Arnd Bergmann wrote:
> Hi Greg,
> 
> Here goes another series of BKL removal patches, mostly
> from Andi. There are some minor drivers left in the
> USB subsystem that someone should take care of, but
> this series at least gets to the point where USB
> works with the BKL disabled.
> 
> Please apply to your USB next tree.

All applied now, thanks so much for this work.

greg k-h