Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 20 Mar 2014 13:53:24 +0000 (UTC)
From:      Hans Petter Selasky <hselasky@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r263423 - head/sys/dev/usb
Message-ID:  <201403201353.s2KDrOxZ048019@svn.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: hselasky
Date: Thu Mar 20 13:53:24 2014
New Revision: 263423
URL: http://svnweb.freebsd.org/changeset/base/263423

Log:
  Try to resolve a possible deadlock when detaching USB devices which
  create character devices. The deadlock can happen if an application is
  issuing IOCTLs which require USB refcounting, at the same time the USB
  device is detaching.
  
  There is already a counter in place in the USB device structure to
  detect this situation, but it was not always checked ahead of invoking
  functions that might destroy character devices, like detach, set
  configuration, set alternate interface or detach active kernel driver.
  
  Reported by:	Daniel O'Connor <doconnor@gsoft.com.au>
  MFC after:	1 week

Modified:
  head/sys/dev/usb/usb_dev.c
  head/sys/dev/usb/usb_device.c
  head/sys/dev/usb/usb_process.c
  head/sys/dev/usb/usb_process.h

Modified: head/sys/dev/usb/usb_dev.c
==============================================================================
--- head/sys/dev/usb/usb_dev.c	Thu Mar 20 11:09:20 2014	(r263422)
+++ head/sys/dev/usb/usb_dev.c	Thu Mar 20 13:53:24 2014	(r263423)
@@ -214,12 +214,13 @@ usb_ref_device(struct usb_cdev_privdata 
 		DPRINTFN(2, "device is detached\n");
 		goto error;
 	}
-	if (cpd->udev->refcount == USB_DEV_REF_MAX) {
-		DPRINTFN(2, "no dev ref\n");
-		goto error;
-	}
 	if (need_uref) {
 		DPRINTFN(2, "ref udev - needed\n");
+
+		if (cpd->udev->refcount == USB_DEV_REF_MAX) {
+			DPRINTFN(2, "no dev ref\n");
+			goto error;
+		}
 		cpd->udev->refcount++;
 
 		mtx_unlock(&usb_ref_lock);
@@ -293,9 +294,8 @@ error:
 		usbd_enum_unlock(cpd->udev);
 
 	if (crd->is_uref) {
-		if (--(cpd->udev->refcount) == 0) {
-			cv_signal(&cpd->udev->ref_cv);
-		}
+		cpd->udev->refcount--;
+		cv_broadcast(&cpd->udev->ref_cv);
 	}
 	mtx_unlock(&usb_ref_lock);
 	DPRINTFN(2, "fail\n");
@@ -361,10 +361,9 @@ usb_unref_device(struct usb_cdev_privdat
 		crd->is_write = 0;
 	}
 	if (crd->is_uref) {
-		if (--(cpd->udev->refcount) == 0) {
-			cv_signal(&cpd->udev->ref_cv);
-		}
 		crd->is_uref = 0;
+		cpd->udev->refcount--;
+		cv_broadcast(&cpd->udev->ref_cv);
 	}
 	mtx_unlock(&usb_ref_lock);
 }

Modified: head/sys/dev/usb/usb_device.c
==============================================================================
--- head/sys/dev/usb/usb_device.c	Thu Mar 20 11:09:20 2014	(r263422)
+++ head/sys/dev/usb/usb_device.c	Thu Mar 20 13:53:24 2014	(r263423)
@@ -452,6 +452,65 @@ usb_endpoint_foreach(struct usb_device *
 }
 
 /*------------------------------------------------------------------------*
+ *	usb_wait_pending_ref_locked
+ *
+ * This function will wait for any USB references to go away before
+ * returning and disable further USB device refcounting on the
+ * specified USB device. This function is used when detaching a USB
+ * device.
+ *------------------------------------------------------------------------*/
+static void
+usb_wait_pending_ref_locked(struct usb_device *udev)
+{
+#if USB_HAVE_UGEN
+	const uint16_t refcount =
+	    usb_proc_is_called_from(
+	    USB_BUS_EXPLORE_PROC(udev->bus)) ? 1 : 2;
+
+	DPRINTF("Refcount = %d\n", (int)refcount); 
+
+	while (1) {
+		/* wait for any pending references to go away */
+		mtx_lock(&usb_ref_lock);
+		if (udev->refcount == refcount) {
+			/* prevent further refs being taken */
+			udev->refcount = USB_DEV_REF_MAX;
+			mtx_unlock(&usb_ref_lock);
+			break;
+		}
+		usbd_enum_unlock(udev);
+		cv_wait(&udev->ref_cv, &usb_ref_lock);
+		mtx_unlock(&usb_ref_lock);
+		(void) usbd_enum_lock(udev);
+	}
+#endif
+}
+
+/*------------------------------------------------------------------------*
+ *	usb_ref_restore_locked
+ *
+ * This function will restore the reference count value after a call
+ * to "usb_wait_pending_ref_locked()".
+ *------------------------------------------------------------------------*/
+static void
+usb_ref_restore_locked(struct usb_device *udev)
+{
+#if USB_HAVE_UGEN
+	const uint16_t refcount =
+	    usb_proc_is_called_from(
+	    USB_BUS_EXPLORE_PROC(udev->bus)) ? 1 : 2;
+
+	DPRINTF("Refcount = %d\n", (int)refcount); 
+
+	/* restore reference count and wakeup waiters, if any */
+	mtx_lock(&usb_ref_lock);
+	udev->refcount = refcount;
+	cv_broadcast(&udev->ref_cv);
+	mtx_unlock(&usb_ref_lock);
+#endif
+}
+
+/*------------------------------------------------------------------------*
  *	usb_unconfigure
  *
  * This function will free all USB interfaces and USB endpoints belonging
@@ -1121,6 +1180,9 @@ usb_detach_device(struct usb_device *ude
 
 	sx_assert(&udev->enum_sx, SA_LOCKED);
 
+	/* wait for pending refs to go away */
+	usb_wait_pending_ref_locked(udev);
+
 	/*
 	 * First detach the child to give the child's detach routine a
 	 * chance to detach the sub-devices in the correct order.
@@ -1147,6 +1209,8 @@ usb_detach_device(struct usb_device *ude
 		usb_detach_device_sub(udev, &iface->subdev,
 		    &iface->pnpinfo, flag);
 	}
+
+	usb_ref_restore_locked(udev);
 }
 
 /*------------------------------------------------------------------------*
@@ -2088,14 +2152,6 @@ usb_free_device(struct usb_device *udev,
 		udev->ugen_symlink = NULL;
 	}
 
-	/* wait for all pending references to go away: */
-	mtx_lock(&usb_ref_lock);
-	udev->refcount--;
-	while (udev->refcount != 0) {
-		cv_wait(&udev->ref_cv, &usb_ref_lock);
-	}
-	mtx_unlock(&usb_ref_lock);
-
 	usb_destroy_dev(udev->ctrl_dev);
 #endif
 
@@ -2612,8 +2668,14 @@ usb_fifo_free_wrap(struct usb_device *ud
 			/* no need to free this FIFO */
 			continue;
 		}
+		/* wait for pending refs to go away */
+		usb_wait_pending_ref_locked(udev);
+
 		/* free this FIFO */
 		usb_fifo_free(f);
+
+		/* restore refcount */
+		usb_ref_restore_locked(udev);
 	}
 }
 #endif

Modified: head/sys/dev/usb/usb_process.c
==============================================================================
--- head/sys/dev/usb/usb_process.c	Thu Mar 20 11:09:20 2014	(r263422)
+++ head/sys/dev/usb/usb_process.c	Thu Mar 20 13:53:24 2014	(r263423)
@@ -501,3 +501,15 @@ usb_proc_rewakeup(struct usb_process *up
 		cv_signal(&up->up_cv);
 	}
 }
+
+/*------------------------------------------------------------------------*
+ *	usb_proc_is_called_from
+ *
+ * This function will return non-zero if called from inside the USB
+ * process passed as first argument. Else this function returns zero.
+ *------------------------------------------------------------------------*/
+int
+usb_proc_is_called_from(struct usb_process *up)
+{
+	return (up->up_curtd == curthread);
+}

Modified: head/sys/dev/usb/usb_process.h
==============================================================================
--- head/sys/dev/usb/usb_process.h	Thu Mar 20 11:09:20 2014	(r263422)
+++ head/sys/dev/usb/usb_process.h	Thu Mar 20 13:53:24 2014	(r263423)
@@ -81,6 +81,7 @@ void	usb_proc_mwait(struct usb_process *
 void	usb_proc_free(struct usb_process *up);
 void   *usb_proc_msignal(struct usb_process *up, void *pm0, void *pm1);
 void	usb_proc_rewakeup(struct usb_process *up);
+int	usb_proc_is_called_from(struct usb_process *up);
 
 void	usb_proc_explore_mwait(struct usb_device *, void *, void *);
 void   *usb_proc_explore_msignal(struct usb_device *, void *, void *);



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201403201353.s2KDrOxZ048019>