We're holding the global wdm_mutex through most of wdm_open, including
when it calls usb_autopm_get_interface. This call can block waiting for
a resume, which will need to look up the device list. So we deadlock.
Fix by using a dedicated lock for the device list.
Signed-off-by: Bjørn Mork <bjorn@xxxxxxx>
---
OK, I'm slowly beginning to understand the value of the "does
autosuspend work" question :-)
Hit a nice deadlock when wdm_open required resuming the device. Easily
fixed by using a dedicated lock for the list, and that makes the
locking somewhat cleaner and easily followed anyway. And locks are
cheap.
I'll be the first to admit that this code probably isn't bug free, but
at least I'm hitting them myself first. That must count for something,
or?
Still hoping you will consider including this patch series.
Bjørn
drivers/usb/class/cdc-wdm.c | 14 +++++++++-----
1 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/class/cdc-wdm.c b/drivers/usb/class/cdc-wdm.c
index 9f2cfb5..7ddc3fd 100644
--- a/drivers/usb/class/cdc-wdm.c
+++ b/drivers/usb/class/cdc-wdm.c
@@ -78,6 +78,7 @@ MODULE_DEVICE_TABLE (usb, wdm_ids);
#define WDM_DEFAULT_BUFSIZE 256
static DEFINE_MUTEX(wdm_mutex);
+static DEFINE_MUTEX(wdm_device_list_lock);
static LIST_HEAD(wdm_device_list);
/* --- method tables --- */
@@ -124,23 +125,24 @@ static struct wdm_device *wdm_find_device(struct usb_interface *intf)
{
struct wdm_device *desc = NULL;
- mutex_lock(&wdm_mutex);
+ mutex_lock(&wdm_device_list_lock);
list_for_each_entry(desc, &wdm_device_list, device_list)
if (desc->intf == intf)
break;
- mutex_unlock(&wdm_mutex);
+ mutex_unlock(&wdm_device_list_lock);
return desc;
}
-/* caller holds wdm_mutex */
static struct wdm_device *wdm_find_device_by_minor(int minor)
{
struct wdm_device *desc = NULL;
+ mutex_lock(&wdm_device_list_lock);
list_for_each_entry(desc, &wdm_device_list, device_list)
if (desc->intf->minor == minor)
break;
+ mutex_unlock(&wdm_device_list_lock);
return desc;
}
@@ -304,7 +306,9 @@ static void free_urbs(struct wdm_device *desc)
static void cleanup(struct wdm_device *desc)
{
+ mutex_lock(&wdm_device_list_lock);
list_del(&desc->device_list);
+ mutex_unlock(&wdm_device_list_lock);
kfree(desc->sbuf);
kfree(desc->inbuf);
kfree(desc->orq);
@@ -733,9 +737,9 @@ static int wdm_create(struct usb_interface *intf, struct usb_endpoint_descriptor
desc
);
- mutex_lock(&wdm_mutex);
+ mutex_lock(&wdm_device_list_lock);
list_add(&desc->device_list, &wdm_device_list);
- mutex_unlock(&wdm_mutex);
+ mutex_unlock(&wdm_device_list_lock);
rv = usb_register_dev(intf, &wdm_class);
if (rv < 0)
--
1.7.8.3
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
