From c532b29a6f6d31e84a7c88f995eebdc75ebd4248 Mon Sep 17 00:00:00 2001 From: Andi Kleen Date: Tue, 1 Jun 2010 23:04:41 +0200 Subject: USB-BKL: Convert usb_driver ioctl to unlocked_ioctl 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 Cc: Arnd Bergmann From: Andi Kleen --- drivers/usb/misc/usbtest.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'drivers/usb/misc') 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, -- cgit v1.1 From 925ce689bb31960c839804c19ef38d676f1939b9 Mon Sep 17 00:00:00 2001 From: Arnd Bergmann Date: Sun, 11 Jul 2010 23:18:56 +0200 Subject: USB: autoconvert trivial BKL users to private mutex All these files use the big kernel lock in a trivial way to serialize their private file operations, typically resulting from an earlier semi-automatic pushdown from VFS. None of these drivers appears to want to lock against other code, and they all use the BKL as the top-level lock in their file operations, meaning that there is no lock-order inversion problem. Consequently, we can remove the BKL completely, replacing it with a per-file mutex in every case. Using a scripted approach means we can avoid typos. file=$1 name=$2 if grep -q lock_kernel ${file} ; then if grep -q 'include.*linux.mutex.h' ${file} ; then sed -i '/include.*/d' ${file} else sed -i 's/include.*.*$/include /g' ${file} fi sed -i ${file} \ -e "/^#include.*linux.mutex.h/,$ { 1,/^\(static\|int\|long\)/ { /^\(static\|int\|long\)/istatic DEFINE_MUTEX(${name}_mutex); } }" \ -e "s/\(un\)*lock_kernel\>[ ]*()/mutex_\1lock(\&${name}_mutex)/g" \ -e '/[ ]*cycle_kernel_lock();/d' else sed -i -e '/include.*\/d' ${file} \ -e '/cycle_kernel_lock()/d' fi Signed-off-by: Arnd Bergmann Signed-off-by: Greg Kroah-Hartman --- drivers/usb/misc/iowarrior.c | 15 ++++++++------- drivers/usb/misc/rio500.c | 15 ++++++++------- drivers/usb/misc/usblcd.c | 16 ++++++++-------- 3 files changed, 24 insertions(+), 22 deletions(-) (limited to 'drivers/usb/misc') diff --git a/drivers/usb/misc/iowarrior.c b/drivers/usb/misc/iowarrior.c index 7dc9d3c..8296645 100644 --- a/drivers/usb/misc/iowarrior.c +++ b/drivers/usb/misc/iowarrior.c @@ -18,7 +18,7 @@ #include #include #include -#include +#include #include #include @@ -61,6 +61,7 @@ MODULE_DESCRIPTION(DRIVER_DESC); MODULE_LICENSE("GPL"); /* Module parameters */ +static DEFINE_MUTEX(iowarrior_mutex); static int debug = 0; module_param(debug, bool, 0644); MODULE_PARM_DESC(debug, "debug=1 enables debugging messages"); @@ -493,7 +494,7 @@ static long iowarrior_ioctl(struct file *file, unsigned int cmd, return -ENOMEM; /* lock this object */ - lock_kernel(); + mutex_lock(&iowarrior_mutex); mutex_lock(&dev->mutex); /* verify that the device wasn't unplugged */ @@ -585,7 +586,7 @@ static long iowarrior_ioctl(struct file *file, unsigned int cmd, error_out: /* unlock the device */ mutex_unlock(&dev->mutex); - unlock_kernel(); + mutex_unlock(&iowarrior_mutex); kfree(buffer); return retval; } @@ -602,12 +603,12 @@ static int iowarrior_open(struct inode *inode, struct file *file) dbg("%s", __func__); - lock_kernel(); + mutex_lock(&iowarrior_mutex); subminor = iminor(inode); interface = usb_find_interface(&iowarrior_driver, subminor); if (!interface) { - unlock_kernel(); + mutex_unlock(&iowarrior_mutex); err("%s - error, can't find device for minor %d", __func__, subminor); return -ENODEV; @@ -617,7 +618,7 @@ static int iowarrior_open(struct inode *inode, struct file *file) dev = usb_get_intfdata(interface); if (!dev) { mutex_unlock(&iowarrior_open_disc_lock); - unlock_kernel(); + mutex_unlock(&iowarrior_mutex); return -ENODEV; } @@ -644,7 +645,7 @@ static int iowarrior_open(struct inode *inode, struct file *file) out: mutex_unlock(&dev->mutex); - unlock_kernel(); + mutex_unlock(&iowarrior_mutex); return retval; } diff --git a/drivers/usb/misc/rio500.c b/drivers/usb/misc/rio500.c index a85771b..cc13ae6 100644 --- a/drivers/usb/misc/rio500.c +++ b/drivers/usb/misc/rio500.c @@ -32,7 +32,7 @@ #include #include #include -#include +#include #include #include #include @@ -72,6 +72,7 @@ struct rio_usb_data { struct mutex lock; /* general race avoidance */ }; +static DEFINE_MUTEX(rio500_mutex); static struct rio_usb_data rio_instance; static int open_rio(struct inode *inode, struct file *file) @@ -79,12 +80,12 @@ static int open_rio(struct inode *inode, struct file *file) struct rio_usb_data *rio = &rio_instance; /* against disconnect() */ - lock_kernel(); + mutex_lock(&rio500_mutex); mutex_lock(&(rio->lock)); if (rio->isopen || !rio->present) { mutex_unlock(&(rio->lock)); - unlock_kernel(); + mutex_unlock(&rio500_mutex); return -EBUSY; } rio->isopen = 1; @@ -94,7 +95,7 @@ static int open_rio(struct inode *inode, struct file *file) mutex_unlock(&(rio->lock)); dev_info(&rio->rio_dev->dev, "Rio opened.\n"); - unlock_kernel(); + mutex_unlock(&rio500_mutex); return 0; } @@ -491,7 +492,7 @@ static void disconnect_rio(struct usb_interface *intf) struct rio_usb_data *rio = usb_get_intfdata (intf); usb_set_intfdata (intf, NULL); - lock_kernel(); + mutex_lock(&rio500_mutex); if (rio) { usb_deregister_dev(intf, &usb_rio_class); @@ -501,7 +502,7 @@ static void disconnect_rio(struct usb_interface *intf) /* better let it finish - the release will do whats needed */ rio->rio_dev = NULL; mutex_unlock(&(rio->lock)); - unlock_kernel(); + mutex_unlock(&rio500_mutex); return; } kfree(rio->ibuf); @@ -512,7 +513,7 @@ static void disconnect_rio(struct usb_interface *intf) rio->present = 0; mutex_unlock(&(rio->lock)); } - unlock_kernel(); + mutex_unlock(&rio500_mutex); } static const struct usb_device_id rio_table[] = { diff --git a/drivers/usb/misc/usblcd.c b/drivers/usb/misc/usblcd.c index 7828c76..6ae39e3 100644 --- a/drivers/usb/misc/usblcd.c +++ b/drivers/usb/misc/usblcd.c @@ -16,7 +16,6 @@ #include #include #include -#include #include #include #include @@ -30,6 +29,7 @@ #define IOCTL_GET_DRV_VERSION 2 +static DEFINE_MUTEX(lcd_mutex); static const struct usb_device_id id_table[] = { { .idVendor = 0x10D2, .match_flags = USB_DEVICE_ID_MATCH_VENDOR, }, { }, @@ -74,12 +74,12 @@ static int lcd_open(struct inode *inode, struct file *file) struct usb_interface *interface; int subminor, r; - lock_kernel(); + mutex_lock(&lcd_mutex); subminor = iminor(inode); interface = usb_find_interface(&lcd_driver, subminor); if (!interface) { - unlock_kernel(); + mutex_unlock(&lcd_mutex); err ("USBLCD: %s - error, can't find device for minor %d", __func__, subminor); return -ENODEV; @@ -89,7 +89,7 @@ static int lcd_open(struct inode *inode, struct file *file) dev = usb_get_intfdata(interface); if (!dev) { mutex_unlock(&open_disc_mutex); - unlock_kernel(); + mutex_unlock(&lcd_mutex); return -ENODEV; } @@ -101,13 +101,13 @@ static int lcd_open(struct inode *inode, struct file *file) r = usb_autopm_get_interface(interface); if (r < 0) { kref_put(&dev->kref, lcd_delete); - unlock_kernel(); + mutex_unlock(&lcd_mutex); return r; } /* save our object in the file's private structure */ file->private_data = dev; - unlock_kernel(); + mutex_unlock(&lcd_mutex); return 0; } @@ -164,14 +164,14 @@ static long lcd_ioctl(struct file *file, unsigned int cmd, unsigned long arg) switch (cmd) { case IOCTL_GET_HARD_VERSION: - lock_kernel(); + mutex_lock(&lcd_mutex); bcdDevice = le16_to_cpu((dev->udev)->descriptor.bcdDevice); sprintf(buf,"%1d%1d.%1d%1d", (bcdDevice & 0xF000)>>12, (bcdDevice & 0xF00)>>8, (bcdDevice & 0xF0)>>4, (bcdDevice & 0xF)); - unlock_kernel(); + mutex_unlock(&lcd_mutex); if (copy_to_user((void __user *)arg,buf,strlen(buf))!=0) return -EFAULT; break; -- cgit v1.1 From 5bd6e8b3fb787b7337b681aaa601e5c7bdc67c55 Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Mon, 12 Jul 2010 13:50:12 -0700 Subject: USB: misc: Remove unnecessary casts of private_data Signed-off-by: Joe Perches Signed-off-by: Greg Kroah-Hartman --- drivers/usb/misc/ftdi-elan.c | 4 ++-- drivers/usb/misc/iowarrior.c | 8 ++++---- drivers/usb/misc/legousbtower.c | 6 +++--- drivers/usb/misc/sisusbvga/sisusb.c | 10 +++++----- drivers/usb/misc/usblcd.c | 8 ++++---- 5 files changed, 18 insertions(+), 18 deletions(-) (limited to 'drivers/usb/misc') diff --git a/drivers/usb/misc/ftdi-elan.c b/drivers/usb/misc/ftdi-elan.c index 82e1663..aecf380 100644 --- a/drivers/usb/misc/ftdi-elan.c +++ b/drivers/usb/misc/ftdi-elan.c @@ -650,7 +650,7 @@ static int ftdi_elan_open(struct inode *inode, struct file *file) static int ftdi_elan_release(struct inode *inode, struct file *file) { - struct usb_ftdi *ftdi = (struct usb_ftdi *)file->private_data; + struct usb_ftdi *ftdi = file->private_data; if (ftdi == NULL) return -ENODEV; up(&ftdi->sw_lock); /* decrement the count on our device */ @@ -673,7 +673,7 @@ static ssize_t ftdi_elan_read(struct file *file, char __user *buffer, int bytes_read = 0; int retry_on_empty = 10; int retry_on_timeout = 5; - struct usb_ftdi *ftdi = (struct usb_ftdi *)file->private_data; + struct usb_ftdi *ftdi = file->private_data; if (ftdi->disconnected > 0) { return -ENODEV; } diff --git a/drivers/usb/misc/iowarrior.c b/drivers/usb/misc/iowarrior.c index 8296645..2de49c8 100644 --- a/drivers/usb/misc/iowarrior.c +++ b/drivers/usb/misc/iowarrior.c @@ -283,7 +283,7 @@ static ssize_t iowarrior_read(struct file *file, char __user *buffer, int read_idx; int offset; - dev = (struct iowarrior *)file->private_data; + dev = file->private_data; /* verify that the device wasn't unplugged */ if (dev == NULL || !dev->present) @@ -349,7 +349,7 @@ static ssize_t iowarrior_write(struct file *file, char *buf = NULL; /* for IOW24 and IOW56 we need a buffer */ struct urb *int_out_urb = NULL; - dev = (struct iowarrior *)file->private_data; + dev = file->private_data; mutex_lock(&dev->mutex); /* verify that the device wasn't unplugged */ @@ -484,7 +484,7 @@ static long iowarrior_ioctl(struct file *file, unsigned int cmd, int retval; int io_res; /* checks for bytes read/written and copy_to/from_user results */ - dev = (struct iowarrior *)file->private_data; + dev = file->private_data; if (dev == NULL) { return -ENODEV; } @@ -657,7 +657,7 @@ static int iowarrior_release(struct inode *inode, struct file *file) struct iowarrior *dev; int retval = 0; - dev = (struct iowarrior *)file->private_data; + dev = file->private_data; if (dev == NULL) { return -ENODEV; } diff --git a/drivers/usb/misc/legousbtower.c b/drivers/usb/misc/legousbtower.c index 8547bf9..6482c6e 100644 --- a/drivers/usb/misc/legousbtower.c +++ b/drivers/usb/misc/legousbtower.c @@ -448,7 +448,7 @@ static int tower_release (struct inode *inode, struct file *file) dbg(2, "%s: enter", __func__); - dev = (struct lego_usb_tower *)file->private_data; + dev = file->private_data; if (dev == NULL) { dbg(1, "%s: object is NULL", __func__); @@ -597,7 +597,7 @@ static ssize_t tower_read (struct file *file, char __user *buffer, size_t count, dbg(2, "%s: enter, count = %Zd", __func__, count); - dev = (struct lego_usb_tower *)file->private_data; + dev = file->private_data; /* lock this object */ if (mutex_lock_interruptible(&dev->lock)) { @@ -686,7 +686,7 @@ static ssize_t tower_write (struct file *file, const char __user *buffer, size_t dbg(2, "%s: enter, count = %Zd", __func__, count); - dev = (struct lego_usb_tower *)file->private_data; + dev = file->private_data; /* lock this object */ if (mutex_lock_interruptible(&dev->lock)) { diff --git a/drivers/usb/misc/sisusbvga/sisusb.c b/drivers/usb/misc/sisusbvga/sisusb.c index d25814c..70d00e9 100644 --- a/drivers/usb/misc/sisusbvga/sisusb.c +++ b/drivers/usb/misc/sisusbvga/sisusb.c @@ -2487,7 +2487,7 @@ sisusb_release(struct inode *inode, struct file *file) { struct sisusb_usb_data *sisusb; - if (!(sisusb = (struct sisusb_usb_data *)file->private_data)) + if (!(sisusb = file->private_data)) return -ENODEV; mutex_lock(&sisusb->lock); @@ -2519,7 +2519,7 @@ sisusb_read(struct file *file, char __user *buffer, size_t count, loff_t *ppos) u16 buf16; u32 buf32, address; - if (!(sisusb = (struct sisusb_usb_data *)file->private_data)) + if (!(sisusb = file->private_data)) return -ENODEV; mutex_lock(&sisusb->lock); @@ -2661,7 +2661,7 @@ sisusb_write(struct file *file, const char __user *buffer, size_t count, u16 buf16; u32 buf32, address; - if (!(sisusb = (struct sisusb_usb_data *)file->private_data)) + if (!(sisusb = file->private_data)) return -ENODEV; mutex_lock(&sisusb->lock); @@ -2804,7 +2804,7 @@ sisusb_lseek(struct file *file, loff_t offset, int orig) struct sisusb_usb_data *sisusb; loff_t ret; - if (!(sisusb = (struct sisusb_usb_data *)file->private_data)) + if (!(sisusb = file->private_data)) return -ENODEV; mutex_lock(&sisusb->lock); @@ -2969,7 +2969,7 @@ sisusb_ioctl(struct file *file, unsigned int cmd, unsigned long arg) long retval = 0; u32 __user *argp = (u32 __user *)arg; - if (!(sisusb = (struct sisusb_usb_data *)file->private_data)) + if (!(sisusb = file->private_data)) return -ENODEV; mutex_lock(&sisusb->lock); diff --git a/drivers/usb/misc/usblcd.c b/drivers/usb/misc/usblcd.c index 6ae39e3..d00dde1 100644 --- a/drivers/usb/misc/usblcd.c +++ b/drivers/usb/misc/usblcd.c @@ -116,7 +116,7 @@ static int lcd_release(struct inode *inode, struct file *file) { struct usb_lcd *dev; - dev = (struct usb_lcd *)file->private_data; + dev = file->private_data; if (dev == NULL) return -ENODEV; @@ -132,7 +132,7 @@ static ssize_t lcd_read(struct file *file, char __user * buffer, size_t count, l int retval = 0; int bytes_read; - dev = (struct usb_lcd *)file->private_data; + dev = file->private_data; /* do a blocking bulk read to get data from the device */ retval = usb_bulk_msg(dev->udev, @@ -158,7 +158,7 @@ static long lcd_ioctl(struct file *file, unsigned int cmd, unsigned long arg) u16 bcdDevice; char buf[30]; - dev = (struct usb_lcd *)file->private_data; + dev = file->private_data; if (dev == NULL) return -ENODEV; @@ -217,7 +217,7 @@ static ssize_t lcd_write(struct file *file, const char __user * user_buffer, siz struct urb *urb = NULL; char *buf = NULL; - dev = (struct usb_lcd *)file->private_data; + dev = file->private_data; /* verify that we actually have some data to write */ if (count == 0) -- cgit v1.1 From e10e1bec8e6654de4591ef45ddd6a6d1e5b2591c Mon Sep 17 00:00:00 2001 From: Ming Lei Date: Mon, 2 Aug 2010 22:09:01 +0800 Subject: USB: usbtest: avoid to free coherent buffer in atomic context This patch fixes the warning below: [30753.755998] ------------[ cut here ]------------ [30753.755998] WARNING: at /home/tom/git/linux-2.6/linux-2.6-next/arch/x86/include/asm/dma-mapping.h:155 hcd_buffer_free+0xb1/0xd4 [usbcore]() [30753.755998] Hardware name: 6475EK2 [30753.755998] Modules linked in: uvcvideo ehci_hcd usbtest cdc_ether usbnet vfat fat usb_storage nfsd lockd nfs_acl auth_rpcgss exportfs mii tun videodev v4l1_compat v4l2_compat_ioctl32 fuse bridge stp llc sunrpc ipv6 cpufreq_ondemand acpi_cpufreq freq_table mperf kvm_intel kvm arc4 ecb ath5k usbhid mac80211 snd_hda_codec_conexant ch341 usbserial ath cfg80211 thinkpad_acpi snd_hda_intel pcspkr wmi hwmon yenta_socket iTCO_wdt iTCO_vendor_support i2c_i801 e1000e snd_hda_codec snd_hwdep snd_pcm snd_timer snd soundcore snd_page_alloc pata_acpi uhci_hcd ohci_hcd usbcore i915 drm_kms_helper drm i2c_algo_bit i2c_core video output [last unloaded: uvcvideo] [30753.755998] Pid: 0, comm: swapper Tainted: G W 2.6.35-rc6-gkh-wl+ #49 [30753.755998] Call Trace: [30753.755998] [] warn_slowpath_common+0x80/0x98 [30753.755998] [] warn_slowpath_null+0x15/0x17 [30753.755998] [] hcd_buffer_free+0xb1/0xd4 [usbcore] [30753.755998] [] usb_free_coherent+0x1c/0x1e [usbcore] [30753.755998] [] simple_free_urb+0x23/0x2f [usbtest] [30753.755998] [] iso_callback+0xbb/0x10f [usbtest] [30753.755998] [] usb_hcd_giveback_urb+0x8c/0xc0 [usbcore] [30753.755998] [] ehci_urb_done+0x84/0x95 [ehci_hcd] [30753.755998] [] ehci_work+0x41a/0x7dd [ehci_hcd] [30753.755998] [] ehci_irq+0x33b/0x370 [ehci_hcd] [30753.755998] [] ? sched_clock+0x9/0xd [30753.755998] [] ? sched_clock_local+0x1c/0x82 [30753.755998] [] ? sched_clock_cpu+0xc3/0xce [30753.755998] [] ? trace_hardirqs_off+0xd/0xf [30753.755998] [] ? cpu_clock+0x43/0x5e [30753.755998] [] usb_hcd_irq+0x45/0xa1 [usbcore] [30753.755998] [] handle_IRQ_event+0x20/0xa5 [30753.755998] [] handle_fasteoi_irq+0x92/0xd2 [30753.755998] [] handle_irq+0x1f/0x2a [30753.755998] [] do_IRQ+0x57/0xbe [30753.755998] [] ret_from_intr+0x0/0x16 [30753.755998] [] ? acpi_idle_enter_bm+0x231/0x269 [30753.755998] [] ? acpi_idle_enter_bm+0x22a/0x269 [30753.755998] [] cpuidle_idle_call+0x99/0xce [30753.755998] [] cpu_idle+0x61/0xaa [30753.755998] [] start_secondary+0x1c2/0x1c6 [30753.755998] ---[ end trace 904cfaf7ab4cb1a2 ]--- Signed-off-by: Ming Lei Cc: stable Signed-off-by: Greg Kroah-Hartman --- drivers/usb/misc/usbtest.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'drivers/usb/misc') diff --git a/drivers/usb/misc/usbtest.c b/drivers/usb/misc/usbtest.c index 0cfbd78..d92b7ec 100644 --- a/drivers/usb/misc/usbtest.c +++ b/drivers/usb/misc/usbtest.c @@ -1378,7 +1378,6 @@ static void iso_callback (struct urb *urb) break; } } - simple_free_urb (urb); ctx->pending--; if (ctx->pending == 0) { @@ -1495,6 +1494,7 @@ test_iso_queue (struct usbtest_dev *dev, struct usbtest_param *param, } simple_free_urb (urbs [i]); + urbs[i] = NULL; context.pending--; context.submit_error = 1; break; @@ -1504,6 +1504,10 @@ test_iso_queue (struct usbtest_dev *dev, struct usbtest_param *param, wait_for_completion (&context.done); + for (i = 0; i < param->sglen; i++) { + if (urbs[i]) + simple_free_urb(urbs[i]); + } /* * Isochronous transfers are expected to fail sometimes. As an * arbitrary limit, we will report an error if any submissions -- cgit v1.1 From 951fd8ee6a9493f80bc5bccf2c8cfa1535c6ce15 Mon Sep 17 00:00:00 2001 From: Ming Lei Date: Mon, 2 Aug 2010 22:09:17 +0800 Subject: USB: usbtest: support test device with only one iso-in or iso-out endpoint It is very common that one altsetting may include only one iso-in or iso-out single endpoint, especially for high bandwidth endpoint, so support it. Signed-off-by: Ming Lei Signed-off-by: Greg Kroah-Hartman --- drivers/usb/misc/usbtest.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'drivers/usb/misc') diff --git a/drivers/usb/misc/usbtest.c b/drivers/usb/misc/usbtest.c index d92b7ec..eef370e 100644 --- a/drivers/usb/misc/usbtest.c +++ b/drivers/usb/misc/usbtest.c @@ -136,7 +136,7 @@ try_iso: iso_out = e; } } - if ((in && out) || (iso_in && iso_out)) + if ((in && out) || iso_in || iso_out) goto found; } return -EINVAL; @@ -162,6 +162,9 @@ found: dev->in_iso_pipe = usb_rcvisocpipe (udev, iso_in->desc.bEndpointAddress & USB_ENDPOINT_NUMBER_MASK); + } + + if (iso_out) { dev->iso_out = &iso_out->desc; dev->out_iso_pipe = usb_sndisocpipe (udev, iso_out->desc.bEndpointAddress -- cgit v1.1