deadlocks in using autopm interfaces

Hi Alan, list,

I am having trouble using the usb_autopm_get/put_interface.
To wake a device on demand I need to use usb_autopm_get_interface().
However the driver needs to hold a lock while calling
usb_autopm_put_interface(). The driver's suspend method in turn
needs to take a lock to wait for ongoing IO. As the actual autosuspend
takes places in a workqueue, there's a classic AB-BA deadlock involving
the driver's lock and pm_mutex.

I think the cleanest solution is to provide lockless versions and let
the caller take pm_mutex.

	Regards
		Oliver

-- 
SUSE LINUX Products GmbH, GF: Markus Rex, HRB 16746 (AG Nürnberg)
This signature is a legal requirement

----

--- a/include/linux/usb.h	2007-02-06 14:14:57.000000000 +0100
+++ b/include/linux/usb.h	2007-02-12 14:35:50.000000000 +0100
@@ -419,6 +419,8 @@
 extern int usb_autopm_set_interface(struct usb_interface *intf);
 extern int usb_autopm_get_interface(struct usb_interface *intf);
 extern void usb_autopm_put_interface(struct usb_interface *intf);
+extern int __usb_autopm_get_interface(struct usb_interface *intf);
+extern void __usb_autopm_put_interface(struct usb_interface *intf);
 
 static inline void usb_autopm_enable(struct usb_interface *intf)
 {

--- a/drivers/usb/core/driver.c	2007-02-06 14:14:48.000000000 +0100
+++ b/drivers/usb/core/driver.c	2007-02-12 15:14:33.000000000 +0100
@@ -1161,7 +1161,6 @@
 {
 	int	status = 0;
 
-	usb_pm_lock(udev);
 	udev->pm_usage_cnt += inc_usage_cnt;
 	WARN_ON(udev->pm_usage_cnt < 0);
 	if (inc_usage_cnt >= 0 && udev->pm_usage_cnt > 0) {
@@ -1172,7 +1171,7 @@
 	} else if (inc_usage_cnt <= 0 && autosuspend_check(udev) == 0)
 		queue_delayed_work(ksuspend_usb_wq, &udev->autosuspend,
 				USB_AUTOSUSPEND_DELAY);
-	usb_pm_unlock(udev);
+
 	return status;
 }
 
@@ -1200,7 +1199,9 @@
 {
 	int	status;
 
+	usb_pm_lock(udev);
 	status = usb_autopm_do_device(udev, -1);
+	usb_pm_unlock(udev);
 	// dev_dbg(&udev->dev, "%s: cnt %d\n",
 	//		__FUNCTION__, udev->pm_usage_cnt);
 }
@@ -1228,7 +1229,9 @@
 {
 	int	status;
 
+	usb_pm_lock(udev);
 	status = usb_autopm_do_device(udev, 1);
+	usb_pm_unlock(udev);
 	// dev_dbg(&udev->dev, "%s: status %d cnt %d\n",
 	//		__FUNCTION__, status, udev->pm_usage_cnt);
 	return status;
@@ -1243,7 +1246,6 @@
 	struct usb_device	*udev = interface_to_usbdev(intf);
 	int			status = 0;
 
-	usb_pm_lock(udev);
 	if (intf->condition == USB_INTERFACE_UNBOUND)
 		status = -ENODEV;
 	else {
@@ -1257,7 +1259,7 @@
 			queue_delayed_work(ksuspend_usb_wq, &udev->autosuspend,
 					USB_AUTOSUSPEND_DELAY);
 	}
-	usb_pm_unlock(udev);
+
 	return status;
 }
 
@@ -1296,12 +1298,25 @@
 {
 	int	status;
 
+	usb_pm_lock(interface_to_usbdev(intf));
 	status = usb_autopm_do_interface(intf, -1);
+	usb_pm_unlock(interface_to_usbdev(intf));
 	// dev_dbg(&intf->dev, "%s: status %d cnt %d\n",
 	//		__FUNCTION__, status, intf->pm_usage_cnt);
 }
 EXPORT_SYMBOL_GPL(usb_autopm_put_interface);
 
+void __usb_autopm_put_interface(struct usb_interface *intf)
+{
+	int	status;
+
+	status = usb_autopm_do_interface(intf, -1);
+
+	// dev_dbg(&intf->dev, "%s: status %d cnt %d\n",
+	//		__FUNCTION__, status, intf->pm_usage_cnt);
+}
+EXPORT_SYMBOL_GPL(__usb_autopm_put_interface);
+
 /**
  * usb_autopm_get_interface - increment a USB interface's PM-usage counter
  * @intf: the usb_interface whose counter should be incremented
@@ -1337,13 +1352,25 @@
 {
 	int	status;
 
+	usb_pm_lock(interface_to_usbdev(intf));
 	status = usb_autopm_do_interface(intf, 1);
+	usb_pm_unlock(interface_to_usbdev(intf));
 	// dev_dbg(&intf->dev, "%s: status %d cnt %d\n",
 	//		__FUNCTION__, status, intf->pm_usage_cnt);
 	return status;
 }
 EXPORT_SYMBOL_GPL(usb_autopm_get_interface);
 
+int __usb_autopm_get_interface(struct usb_interface *intf)
+{
+	int	status;
+
+	status = usb_autopm_do_interface(intf, 1);
+	// dev_dbg(&intf->dev, "%s: status %d cnt %d\n",
+	//		__FUNCTION__, status, intf->pm_usage_cnt);
+	return status;
+}
+EXPORT_SYMBOL_GPL(__usb_autopm_get_interface);
 /**
  * usb_autopm_set_interface - set a USB interface's autosuspend state
  * @intf: the usb_interface whose state should be set
@@ -1359,7 +1386,9 @@
 {
 	int	status;
 
+	usb_pm_lock(interface_to_usbdev(intf));
 	status = usb_autopm_do_interface(intf, 0);
+	usb_pm_unlock(interface_to_usbdev(intf));
 	// dev_dbg(&intf->dev, "%s: status %d cnt %d\n",
 	//		__FUNCTION__, status, intf->pm_usage_cnt);
 	return status;

-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier.
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
_______________________________________________
[email protected]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel