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. The "destroy_dev()"
function waits for all system calls involving the given character
device to return. Character device system calls may lock the USB
enumeration lock, which is also held when "destroy_dev()" is
called. This can sometimes lead to a deadlock not noticed by
WITNESS. The current solution is to ensure the calling thread is the
only one holding the USB enumeration lock and prevent other threads
from getting refs while a USB device detach is ongoing. This turned
out not to be sufficient. To solve this deadlock we could use
"destroy_dev_sched()" to schedule the device destruction in the
background, but then we don't know when it is safe to free() the
private data of the character device. Instead a callback function is
executed by the USB explore process to kill off any leftover USB
character devices synchronously after the USB device explore code is
finished and the USB enumeration lock is no longer locked. This makes
porting easier and also ensures us that character devices must
eventually go away after a USB device detach.

While at it ensure that "flag_iserror" is only written when "priv_mtx"
is locked, which is protecting it.

MFC after:	5 days

1 files changed, 40 insertions, 0 deletions

diff --git a/sys/dev/usb/controller/usb_controller.c b/sys/dev/usb/controller/usb_controller.c
index b93d541..92ea6c5 100644
--- a/sys/dev/usb/controller/usb_controller.c
+++ b/sys/dev/usb/controller/usb_controller.c
@@ -59,6 +59,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>
@@ -219,6 +220,11 @@ usb_detach(device_t dev)
 	usb_proc_mwait(USB_BUS_EXPLORE_PROC(bus),
 	    &bus->detach_msg[0], &bus->detach_msg[1]);
 
+#if USB_HAVE_UGEN
+	/* Wait for cleanup to complete */
+	usb_proc_mwait(USB_BUS_EXPLORE_PROC(bus),
+	    &bus->cleanup_msg[0], &bus->cleanup_msg[1]);
+#endif
 	USB_BUS_UNLOCK(bus);
 
 #if USB_HAVE_PER_BUS_PROCESS
@@ -631,6 +637,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)
 {
@@ -813,6 +845,14 @@ usb_attach_sub(device_t dev, struct usb_bus *bus)
 	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
+
 #if USB_HAVE_PER_BUS_PROCESS
 	/* Create USB explore and callback processes */