svn commit: r278291 - in stable/9/sys/dev/usb: . controller

Hans Petter Selasky hselasky at FreeBSD.org
Thu Feb 5 21:38:01 UTC 2015


Author: hselasky
Date: Thu Feb  5 21:37:59 2015
New Revision: 278291
URL: https://svnweb.freebsd.org/changeset/base/278291

Log:
  MFC r277136:
  Resolve a special case deadlock: When two or more threads are
  simultaneously detaching kernel drivers on the same USB device we can
  get stuck in the "usb_wait_pending_ref_locked()" function because the
  conditions needed for allowing detach are not met.
  
  While at it ensure that "flag_iserror" is only written when "priv_mtx"
  is locked, which is protecting it.

Modified:
  stable/9/sys/dev/usb/controller/usb_controller.c
  stable/9/sys/dev/usb/usb_bus.h
  stable/9/sys/dev/usb/usb_dev.c
  stable/9/sys/dev/usb/usb_device.c
  stable/9/sys/dev/usb/usb_device.h
Directory Properties:
  stable/9/sys/   (props changed)
  stable/9/sys/dev/   (props changed)

Modified: stable/9/sys/dev/usb/controller/usb_controller.c
==============================================================================
--- stable/9/sys/dev/usb/controller/usb_controller.c	Thu Feb  5 21:25:11 2015	(r278290)
+++ stable/9/sys/dev/usb/controller/usb_controller.c	Thu Feb  5 21:37:59 2015	(r278291)
@@ -56,6 +56,7 @@
 #include <dev/usb/usb_busdma.h>
 #include <dev/usb/usb_dynamic.h>
 #include <dev/usb/usb_device.h>
+#include <dev/usb/usb_dev.h>
 #include <dev/usb/usb_hub.h>
 
 #include <dev/usb/usb_controller.h>
@@ -205,6 +206,11 @@ usb_detach(device_t dev)
 	usb_proc_mwait(&bus->explore_proc,
 	    &bus->detach_msg[0], &bus->detach_msg[1]);
 
+#if USB_HAVE_UGEN
+	/* Wait for cleanup to complete */
+	usb_proc_mwait(&bus->explore_proc,
+	    &bus->cleanup_msg[0], &bus->cleanup_msg[1]);
+#endif
 	USB_BUS_UNLOCK(bus);
 
 	/* Get rid of USB callback processes */
@@ -613,6 +619,32 @@ usb_bus_shutdown(struct usb_proc_msg *pm
 	USB_BUS_LOCK(bus);
 }
 
+/*------------------------------------------------------------------------*
+ *	usb_bus_cleanup
+ *
+ * This function is used to cleanup leftover USB character devices.
+ *------------------------------------------------------------------------*/
+#if USB_HAVE_UGEN
+static void
+usb_bus_cleanup(struct usb_proc_msg *pm)
+{
+	struct usb_bus *bus;
+	struct usb_fs_privdata *pd;
+
+	bus = ((struct usb_bus_msg *)pm)->bus;
+
+	while ((pd = LIST_FIRST(&bus->pd_cleanup_list)) != NULL) {
+
+		LIST_REMOVE(pd, pd_next);
+		USB_BUS_UNLOCK(bus);
+
+		usb_destroy_dev_sync(pd);
+
+		USB_BUS_LOCK(bus);
+	}
+}
+#endif
+
 static void
 usb_power_wdog(void *arg)
 {
@@ -793,6 +825,14 @@ usb_attach_sub(device_t dev, struct usb_
 	bus->shutdown_msg[1].hdr.pm_callback = &usb_bus_shutdown;
 	bus->shutdown_msg[1].bus = bus;
 
+#if USB_HAVE_UGEN
+	LIST_INIT(&bus->pd_cleanup_list);
+	bus->cleanup_msg[0].hdr.pm_callback = &usb_bus_cleanup;
+	bus->cleanup_msg[0].bus = bus;
+	bus->cleanup_msg[1].hdr.pm_callback = &usb_bus_cleanup;
+	bus->cleanup_msg[1].bus = bus;
+#endif
+
 	/* Create USB explore and callback processes */
 
 	if (usb_proc_create(&bus->giant_callback_proc,

Modified: stable/9/sys/dev/usb/usb_bus.h
==============================================================================
--- stable/9/sys/dev/usb/usb_bus.h	Thu Feb  5 21:25:11 2015	(r278290)
+++ stable/9/sys/dev/usb/usb_bus.h	Thu Feb  5 21:37:59 2015	(r278291)
@@ -27,6 +27,8 @@
 #ifndef _USB_BUS_H_
 #define	_USB_BUS_H_
 
+struct usb_fs_privdata;
+
 /*
  * The following structure defines the USB explore message sent to the USB
  * explore process.
@@ -73,6 +75,10 @@ struct usb_bus {
 	struct usb_bus_msg resume_msg[2];
 	struct usb_bus_msg reset_msg[2];
 	struct usb_bus_msg shutdown_msg[2];
+#if USB_HAVE_UGEN
+	struct usb_bus_msg cleanup_msg[2];
+	LIST_HEAD(,usb_fs_privdata) pd_cleanup_list;
+#endif
 	/*
 	 * This mutex protects the USB hardware:
 	 */

Modified: stable/9/sys/dev/usb/usb_dev.c
==============================================================================
--- stable/9/sys/dev/usb/usb_dev.c	Thu Feb  5 21:25:11 2015	(r278290)
+++ stable/9/sys/dev/usb/usb_dev.c	Thu Feb  5 21:37:59 2015	(r278291)
@@ -290,8 +290,8 @@ error:
 		usbd_enum_unlock(cpd->udev);
 
 	if (crd->is_uref) {
-		cpd->udev->refcount--;
-		cv_broadcast(&cpd->udev->ref_cv);
+		if (--(cpd->udev->refcount) == 0)
+			cv_broadcast(&cpd->udev->ref_cv);
 	}
 	mtx_unlock(&usb_ref_lock);
 	DPRINTFN(2, "fail\n");
@@ -362,8 +362,8 @@ usb_unref_device(struct usb_cdev_privdat
 	}
 	if (crd->is_uref) {
 		crd->is_uref = 0;
-		cpd->udev->refcount--;
-		cv_broadcast(&cpd->udev->ref_cv);
+		if (--(cpd->udev->refcount) == 0)
+			cv_broadcast(&cpd->udev->ref_cv);
 	}
 	mtx_unlock(&usb_ref_lock);
 }
@@ -589,12 +589,12 @@ usb_fifo_free(struct usb_fifo *f)
 
 	/* decrease refcount */
 	f->refcount--;
-	/* prevent any write flush */
-	f->flag_iserror = 1;
 	/* need to wait until all callers have exited */
 	while (f->refcount != 0) {
 		mtx_unlock(&usb_ref_lock);	/* avoid LOR */
 		mtx_lock(f->priv_mtx);
+		/* prevent write flush, if any */
+		f->flag_iserror = 1;
 		/* get I/O thread out of any sleep state */
 		if (f->flag_sleeping) {
 			f->flag_sleeping = 0;

Modified: stable/9/sys/dev/usb/usb_device.c
==============================================================================
--- stable/9/sys/dev/usb/usb_device.c	Thu Feb  5 21:25:11 2015	(r278290)
+++ stable/9/sys/dev/usb/usb_device.c	Thu Feb  5 21:37:59 2015	(r278291)
@@ -430,68 +430,29 @@ usb_endpoint_foreach(struct usb_device *
 	return (NULL);
 }
 
-#if USB_HAVE_UGEN
-static uint16_t
-usb_get_refcount(struct usb_device *udev)
-{
-	if (usb_proc_is_called_from(&udev->bus->explore_proc) ||
-	    usb_proc_is_called_from(&udev->bus->control_xfer_proc))
-		return (1);
-	return (2);
-}
-#endif
-
 /*------------------------------------------------------------------------*
- *	usb_wait_pending_ref_locked
+ *	usb_wait_pending_refs
  *
  * 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.
+ * returning. This function is used before freeing a USB device.
  *------------------------------------------------------------------------*/
 static void
-usb_wait_pending_ref_locked(struct usb_device *udev)
+usb_wait_pending_refs(struct usb_device *udev)
 {
 #if USB_HAVE_UGEN
-	const uint16_t refcount = usb_get_refcount(udev);
-
-	DPRINTF("Refcount = %d\n", (int)refcount); 
+	DPRINTF("Refcount = %d\n", (int)udev->refcount); 
 
+	mtx_lock(&usb_ref_lock);
+	udev->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 */
+		if (udev->refcount == 0) {
+			/* prevent further refs being taken, if any */
 			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_get_refcount(udev);
-
-	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
 }
@@ -1156,9 +1117,6 @@ 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.
@@ -1185,8 +1143,6 @@ usb_detach_device(struct usb_device *ude
 		usb_detach_device_sub(udev, &iface->subdev,
 		    &iface->pnpinfo, flag);
 	}
-
-	usb_ref_restore_locked(udev);
 }
 
 /*------------------------------------------------------------------------*
@@ -1999,14 +1955,46 @@ usb_make_dev(struct usb_device *udev, co
 }
 
 void
+usb_destroy_dev_sync(struct usb_fs_privdata *pd)
+{
+	DPRINTFN(1, "Destroying device at ugen%d.%d\n",
+	    pd->bus_index, pd->dev_index);
+
+	/*
+	 * Destroy character device synchronously. After this
+	 * all system calls are returned. Can block.
+	 */
+	destroy_dev(pd->cdev);
+
+	free(pd, M_USBDEV);
+}
+
+void
 usb_destroy_dev(struct usb_fs_privdata *pd)
 {
+	struct usb_bus *bus;
+
 	if (pd == NULL)
 		return;
 
-	destroy_dev(pd->cdev);
+	mtx_lock(&usb_ref_lock);
+	bus = devclass_get_softc(usb_devclass_ptr, pd->bus_index);
+	mtx_unlock(&usb_ref_lock);
 
-	free(pd, M_USBDEV);
+	if (bus == NULL) {
+		usb_destroy_dev_sync(pd);
+		return;
+	}
+
+	/* make sure we can re-use the device name */
+	delist_dev(pd->cdev);
+
+	USB_BUS_LOCK(bus);
+	LIST_INSERT_HEAD(&bus->pd_cleanup_list, pd, pd_next);
+	/* get cleanup going */
+	usb_proc_msignal(&bus->explore_proc,
+	    &bus->cleanup_msg[0], &bus->cleanup_msg[1]);
+	USB_BUS_UNLOCK(bus);
 }
 
 static void
@@ -2157,6 +2145,9 @@ usb_free_device(struct usb_device *udev,
 	    &udev->cs_msg[0], &udev->cs_msg[1]);
 	USB_BUS_UNLOCK(udev->bus);
 
+	/* wait for all references to go away */
+	usb_wait_pending_refs(udev);
+	
 	sx_destroy(&udev->enum_sx);
 	sx_destroy(&udev->sr_sx);
 
@@ -2642,14 +2633,8 @@ 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: stable/9/sys/dev/usb/usb_device.h
==============================================================================
--- stable/9/sys/dev/usb/usb_device.h	Thu Feb  5 21:25:11 2015	(r278290)
+++ stable/9/sys/dev/usb/usb_device.h	Thu Feb  5 21:37:59 2015	(r278291)
@@ -281,6 +281,7 @@ struct usb_device *usb_alloc_device(devi
 struct usb_fs_privdata *usb_make_dev(struct usb_device *, const char *,
 		    int, int, int, uid_t, gid_t, int);
 void	usb_destroy_dev(struct usb_fs_privdata *);
+void	usb_destroy_dev_sync(struct usb_fs_privdata *);
 #endif
 usb_error_t	usb_probe_and_attach(struct usb_device *udev,
 		    uint8_t iface_index);


More information about the svn-src-stable-9 mailing list