Date: Thu, 5 Feb 2015 21:18:45 +0000 (UTC) From: Hans Petter Selasky <hselasky@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-10@freebsd.org Subject: svn commit: r278289 - in stable/10/sys/dev/usb: . controller Message-ID: <201502052118.t15LIjmi053890@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: hselasky Date: Thu Feb 5 21:18:44 2015 New Revision: 278289 URL: https://svnweb.freebsd.org/changeset/base/278289 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/10/sys/dev/usb/controller/usb_controller.c stable/10/sys/dev/usb/usb_bus.h stable/10/sys/dev/usb/usb_dev.c stable/10/sys/dev/usb/usb_device.c stable/10/sys/dev/usb/usb_device.h Directory Properties: stable/10/ (props changed) Modified: stable/10/sys/dev/usb/controller/usb_controller.c ============================================================================== --- stable/10/sys/dev/usb/controller/usb_controller.c Thu Feb 5 20:59:48 2015 (r278288) +++ stable/10/sys/dev/usb/controller/usb_controller.c Thu Feb 5 21:18:44 2015 (r278289) @@ -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> @@ -221,6 +222,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 @@ -633,6 +639,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) { @@ -815,6 +847,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 + #if USB_HAVE_PER_BUS_PROCESS /* Create USB explore and callback processes */ Modified: stable/10/sys/dev/usb/usb_bus.h ============================================================================== --- stable/10/sys/dev/usb/usb_bus.h Thu Feb 5 20:59:48 2015 (r278288) +++ stable/10/sys/dev/usb/usb_bus.h Thu Feb 5 21:18:44 2015 (r278289) @@ -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. @@ -83,6 +85,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/10/sys/dev/usb/usb_dev.c ============================================================================== --- stable/10/sys/dev/usb/usb_dev.c Thu Feb 5 20:59:48 2015 (r278288) +++ stable/10/sys/dev/usb/usb_dev.c Thu Feb 5 21:18:44 2015 (r278289) @@ -294,8 +294,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"); @@ -366,8 +366,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); } @@ -593,12 +593,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/10/sys/dev/usb/usb_device.c ============================================================================== --- stable/10/sys/dev/usb/usb_device.c Thu Feb 5 20:59:48 2015 (r278288) +++ stable/10/sys/dev/usb/usb_device.c Thu Feb 5 21:18:44 2015 (r278289) @@ -451,68 +451,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(USB_BUS_EXPLORE_PROC(udev->bus)) || - usb_proc_is_called_from(USB_BUS_CONTROL_XFER_PROC(udev->bus))) - 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 } @@ -1189,9 +1150,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. @@ -1218,8 +1176,6 @@ usb_detach_device(struct usb_device *ude usb_detach_device_sub(udev, &iface->subdev, &iface->pnpinfo, flag); } - - usb_ref_restore_locked(udev); } /*------------------------------------------------------------------------* @@ -2036,14 +1992,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(USB_BUS_EXPLORE_PROC(bus), + &bus->cleanup_msg[0], &bus->cleanup_msg[1]); + USB_BUS_UNLOCK(bus); } static void @@ -2194,6 +2182,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); @@ -2679,14 +2670,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/10/sys/dev/usb/usb_device.h ============================================================================== --- stable/10/sys/dev/usb/usb_device.h Thu Feb 5 20:59:48 2015 (r278288) +++ stable/10/sys/dev/usb/usb_device.h Thu Feb 5 21:18:44 2015 (r278289) @@ -293,6 +293,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);
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201502052118.t15LIjmi053890>