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);
_______________________________________________
svn-src-stable-9@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-stable-9
To unsubscribe, send any mail to "svn-src-stable-9-unsubscr...@freebsd.org"
