skipping locks, mutex_owned, usb

Andriy Gapon avg at FreeBSD.org
Thu Sep 1 13:53:42 UTC 2011


on 31/08/2011 21:15 Hans Petter Selasky said the following:
> What Warner says is correct. The code was written when SCHEDULER_STOPPED() was 
> not implemented. Really the usbd_transfer_poll should simply return if 
> SCHEDULER_STOPPED() is not set.

Rather than simply return I would instead KASSERT that either kdb_active or
SCHEDULER_STOPPED().

> And then those mtx_unlock()/mtx_lock() cases 
> can simply be removed.

Yep.  So for now I simply removed those:

    [wip] usb_transfer: adapt for SCHEDULER_STOPPED()

    - remove unneeded dropping of locks
    - check SCHEDULER_STOPPED along with mutex_owned()
      which may be unreliable after panic

    Maybe to do: KASSERT that usbd_transfer_poll is called only
    when kdb_active or SCHEDULER_STOPPED

diff --git a/sys/dev/usb/usb_transfer.c b/sys/dev/usb/usb_transfer.c
index d4c2408..ef50dc4 100644
--- a/sys/dev/usb/usb_transfer.c
+++ b/sys/dev/usb/usb_transfer.c
@@ -2150,7 +2150,7 @@ usbd_callback_wrapper(struct usb_xfer_queue *pq)
 	struct usb_xfer_root *info = xfer->xroot;

 	USB_BUS_LOCK_ASSERT(info->bus, MA_OWNED);
-	if (!mtx_owned(info->xfer_mtx)) {
+	if (!SCHEDULER_STOPPED() && !mtx_owned(info->xfer_mtx)) {
 		/*
 	       	 * Cases that end up here:
 		 *
@@ -3094,8 +3094,6 @@ usbd_transfer_poll(struct usb_xfer **ppxfer, uint16_t max)
 	struct usb_device *udev;
 	struct usb_proc_msg *pm;
 	uint16_t n;
-	uint16_t drop_bus;
-	uint16_t drop_xfer;

 	for (n = 0; n != max; n++) {
 		/* Extra checks to avoid panic */
@@ -3115,20 +3113,6 @@ usbd_transfer_poll(struct usb_xfer **ppxfer, uint16_t max)
 		if (udev->bus->methods->xfer_poll == NULL)
 			continue;	/* no poll method */

-		/* make sure that the BUS mutex is not locked */
-		drop_bus = 0;
-		while (mtx_owned(&xroot->udev->bus->bus_mtx)) {
-			mtx_unlock(&xroot->udev->bus->bus_mtx);
-			drop_bus++;
-		}
-
-		/* make sure that the transfer mutex is not locked */
-		drop_xfer = 0;
-		while (mtx_owned(xroot->xfer_mtx)) {
-			mtx_unlock(xroot->xfer_mtx);
-			drop_xfer++;
-		}
-
 		/* Make sure cv_signal() and cv_broadcast() is not called */
 		udev->bus->control_xfer_proc.up_msleep = 0;
 		udev->bus->explore_proc.up_msleep = 0;
@@ -3157,14 +3141,6 @@ usbd_transfer_poll(struct usb_xfer **ppxfer, uint16_t max)
 		(pm->pm_callback) (pm);

 		USB_BUS_UNLOCK(xroot->bus);
-
-		/* restore transfer mutex */
-		while (drop_xfer--)
-			mtx_lock(xroot->xfer_mtx);
-
-		/* restore BUS mutex */
-		while (drop_bus--)
-			mtx_lock(&xroot->udev->bus->bus_mtx);
 	}
 }



-- 
Andriy Gapon


More information about the freebsd-arch mailing list