Date: Thu, 27 Mar 2014 07:03:51 +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-9@freebsd.org Subject: svn commit: r263800 - stable/9/sys/dev/usb Message-ID: <201403270703.s2R73pNn037223@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: hselasky Date: Thu Mar 27 07:03:50 2014 New Revision: 263800 URL: http://svnweb.freebsd.org/changeset/base/263800 Log: MFC r263423: 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. Modified: stable/9/sys/dev/usb/usb_dev.c stable/9/sys/dev/usb/usb_device.c stable/9/sys/dev/usb/usb_process.c stable/9/sys/dev/usb/usb_process.h Directory Properties: stable/9/sys/ (props changed) stable/9/sys/dev/ (props changed) Modified: stable/9/sys/dev/usb/usb_dev.c ============================================================================== --- stable/9/sys/dev/usb/usb_dev.c Thu Mar 27 06:59:56 2014 (r263799) +++ stable/9/sys/dev/usb/usb_dev.c Thu Mar 27 07:03:50 2014 (r263800) @@ -210,12 +210,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); @@ -289,9 +290,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"); @@ -357,10 +357,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: stable/9/sys/dev/usb/usb_device.c ============================================================================== --- stable/9/sys/dev/usb/usb_device.c Thu Mar 27 06:59:56 2014 (r263799) +++ stable/9/sys/dev/usb/usb_device.c Thu Mar 27 07:03:50 2014 (r263800) @@ -431,6 +431,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( + &udev->bus->explore_proc) ? 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( + &udev->bus->explore_proc) ? 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 @@ -1091,6 +1150,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. @@ -1117,6 +1179,8 @@ usb_detach_device(struct usb_device *ude usb_detach_device_sub(udev, &iface->subdev, &iface->pnpinfo, flag); } + + usb_ref_restore_locked(udev); } /*------------------------------------------------------------------------* @@ -2054,14 +2118,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 @@ -2578,8 +2634,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: stable/9/sys/dev/usb/usb_process.c ============================================================================== --- stable/9/sys/dev/usb/usb_process.c Thu Mar 27 06:59:56 2014 (r263799) +++ stable/9/sys/dev/usb/usb_process.c Thu Mar 27 07:03:50 2014 (r263800) @@ -493,3 +493,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: stable/9/sys/dev/usb/usb_process.h ============================================================================== --- stable/9/sys/dev/usb/usb_process.h Thu Mar 27 06:59:56 2014 (r263799) +++ stable/9/sys/dev/usb/usb_process.h Thu Mar 27 07:03:50 2014 (r263800) @@ -79,6 +79,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?201403270703.s2R73pNn037223>