PERFORCE change 144475 for review

Hans Petter Selasky hselasky at FreeBSD.org
Wed Jul 2 09:47:18 UTC 2008


http://perforce.freebsd.org/chv.cgi?CH=144475

Change 144475 by hselasky at hselasky_laptop001 on 2008/07/02 09:47:12

	
	Fix some LORs. When starting and stopping an USB transfer it is safest
	to defer the callback to the callback thread, and not do all callbacks
	from the current execution environment. That way the USB callback is
	always executed from the callback thread/process: See: 
	xfer->usb2_root->done_p (struct usb2_process)

Affected files ...

.. //depot/projects/usb/src/sys/dev/usb2/controller/usb2_controller.c#5 edit
.. //depot/projects/usb/src/sys/dev/usb2/core/usb2_device.c#6 edit
.. //depot/projects/usb/src/sys/dev/usb2/core/usb2_hub.c#8 edit
.. //depot/projects/usb/src/sys/dev/usb2/core/usb2_request.c#5 edit
.. //depot/projects/usb/src/sys/dev/usb2/core/usb2_transfer.c#9 edit
.. //depot/projects/usb/src/sys/dev/usb2/core/usb2_transfer.h#4 edit

Differences ...

==== //depot/projects/usb/src/sys/dev/usb2/controller/usb2_controller.c#5 (text+ko) ====

@@ -234,9 +234,7 @@
 	 * Free USB Root device, but not any sub-devices, hence they
 	 * are freed by the caller of this function:
 	 */
-	sx_xlock(udev->default_sx + 1);
 	usb2_detach_device(udev, USB_IFACE_INDEX_ANY, 0);
-	sx_unlock(udev->default_sx + 1);
 	usb2_free_device(udev);
 
 	mtx_unlock(&Giant);

==== //depot/projects/usb/src/sys/dev/usb2/core/usb2_device.c#6 (text+ko) ====

@@ -856,6 +856,7 @@
 {
 	struct usb2_interface *iface;
 	uint8_t i;
+	uint8_t do_unlock;
 
 	if (udev == NULL) {
 		/* nothing to do */
@@ -863,7 +864,13 @@
 	}
 	DPRINTF(3, "udev=%p\n", udev);
 
-	sx_assert(udev->default_sx + 1, SA_LOCKED);
+	/* automatic locking */
+	if (sx_xlocked(udev->default_sx + 1)) {
+		do_unlock = 0;
+	} else {
+		do_unlock = 1;
+		sx_xlock(udev->default_sx + 1);
+	}
 
 	/*
 	 * First detach the child to give the child's detach routine a
@@ -890,6 +897,10 @@
 		}
 		usb2_detach_device_sub(udev, &(iface->subdev), free_subdev);
 	}
+
+	if (do_unlock) {
+		sx_unlock(udev->default_sx + 1);
+	}
 	return;
 }
 

==== //depot/projects/usb/src/sys/dev/usb2/core/usb2_hub.c#8 (text+ko) ====

@@ -313,9 +313,7 @@
 	/* detach any existing devices */
 
 	if (child) {
-		sx_xlock(child->default_sx + 1);
 		usb2_detach_device(child, USB_IFACE_INDEX_ANY, 1);
-		sx_unlock(child->default_sx + 1);
 		usb2_free_device(child);
 		child = NULL;
 	}
@@ -409,9 +407,7 @@
 
 error:
 	if (child) {
-		sx_xlock(child->default_sx + 1);
 		usb2_detach_device(child, USB_IFACE_INDEX_ANY, 1);
-		sx_unlock(child->default_sx + 1);
 		usb2_free_device(child);
 		child = NULL;
 	}
@@ -806,9 +802,7 @@
 		 * Subdevices are not freed, because the caller of
 		 * uhub_detach() will do that.
 		 */
-		sx_xlock(child->default_sx + 1);
 		usb2_detach_device(child, USB_IFACE_INDEX_ANY, 0);
-		sx_unlock(child->default_sx + 1);
 		usb2_free_device(child);
 		child = NULL;
 	}

==== //depot/projects/usb/src/sys/dev/usb2/core/usb2_request.c#5 (text+ko) ====

@@ -365,7 +365,7 @@
 
 		usb2_transfer_start(xfer);
 
-		while (xfer->flags_int.transferring) {
+		while (usb2_transfer_pending(xfer)) {
 			if ((flags & USB_USE_POLLING) || cold) {
 				usb2_do_poll(udev->default_xfer, USB_DEFAULT_XFER_MAX);
 			} else {

==== //depot/projects/usb/src/sys/dev/usb2/core/usb2_transfer.c#9 (text+ko) ====

@@ -125,6 +125,7 @@
 static void usb2_control_transfer_init(struct usb2_xfer *xfer);
 static uint8_t usb2_start_hardware_sub(struct usb2_xfer *xfer);
 static void usb2_callback_proc(struct usb2_proc_msg *_pm);
+static void usb2_callback_ss_done_defer(struct usb2_xfer *xfer);
 static void usb2_callback_wrapper(struct usb2_xfer_queue *pq);
 static void usb2_dma_delay_done_cb(void *arg);
 static void usb2_transfer_start_cb(void *arg);
@@ -1164,6 +1165,9 @@
 		/* the size of the SETUP structure is hardcoded ! */
 
 		if (xfer->frlengths[0] != sizeof(struct usb2_device_request)) {
+			DPRINTF(-1, "Wrong framelength %u != %u\n",
+			    xfer->frlengths[0], sizeof(struct
+			    usb2_device_request));
 			goto error;
 		}
 		/* check USB mode */
@@ -1470,7 +1474,7 @@
 	}
 	mtx_lock(xfer->usb2_mtx);
 	/* call the USB transfer callback */
-	usb2_command_wrapper(&(xfer->usb2_root->done_q), xfer);
+	usb2_callback_ss_done_defer(xfer);
 	mtx_unlock(xfer->usb2_mtx);
 	return;
 }
@@ -1540,6 +1544,46 @@
 }
 
 /*------------------------------------------------------------------------*
+ *	usb2_transfer_pending
+ *
+ * This function will check if an USB transfer is pending which is a
+ * little bit complicated!
+ * Return values:
+ * 0: Not pending
+ * 1: Pending: The USB transfer will receive a callback in the future.
+ *------------------------------------------------------------------------*/
+uint8_t
+usb2_transfer_pending(struct usb2_xfer *xfer)
+{
+	struct usb2_xfer_root *info;
+	struct usb2_xfer_queue *pq;
+
+	mtx_assert(xfer->priv_mtx, MA_OWNED);
+
+	if (xfer->flags_int.transferring) {
+		/* trivial case */
+		return (1);
+	}
+	mtx_lock(xfer->usb2_mtx);
+	if (xfer->wait_queue) {
+		/* we are waiting on a queue somewhere */
+		mtx_unlock(xfer->usb2_mtx);
+		return (1);
+	}
+	info = xfer->usb2_root;
+	pq = &(info->done_q);
+
+	if (pq->curr == xfer) {
+		/* we are currently scheduled for callback */
+		mtx_unlock(xfer->usb2_mtx);
+		return (1);
+	}
+	/* we are not pending */
+	mtx_unlock(xfer->usb2_mtx);
+	return (0);
+}
+
+/*------------------------------------------------------------------------*
  *	usb2_transfer_drain
  *
  * This function will stop the USB transfer and wait for any
@@ -1564,7 +1608,7 @@
 
 	usb2_transfer_stop(xfer);
 
-	while (xfer->flags_int.transferring) {
+	while (usb2_transfer_pending(xfer)) {
 		xfer->flags_int.draining = 1;
 		/*
 		 * Wait until the current outstanding USB
@@ -1645,6 +1689,43 @@
 }
 
 /*------------------------------------------------------------------------*
+ *	usb2_callback_ss_done_defer
+ *
+ * This function will defer the start, stop and done callback to the
+ * correct thread.
+ *------------------------------------------------------------------------*/
+static void
+usb2_callback_ss_done_defer(struct usb2_xfer *xfer)
+{
+	struct usb2_xfer_root *info = xfer->usb2_root;
+	struct usb2_xfer_queue *pq = &(info->done_q);
+
+	if (!mtx_owned(xfer->usb2_mtx)) {
+		panic("%s: called unlocked!\n", __FUNCTION__);
+	}
+	if (pq->curr != xfer) {
+		usb2_transfer_enqueue(pq, xfer);
+	}
+	if (!pq->recurse_1) {
+
+		/*
+	         * We have to postpone the callback due to the fact we
+	         * will have a Lock Order Reversal, LOR, if we try to
+	         * proceed !
+	         */
+		if (usb2_proc_msignal(&(info->done_p),
+		    &(info->done_m[0]), &(info->done_m[1]))) {
+			/* ignore */
+		}
+	} else {
+		/* clear second recurse flag */
+		pq->recurse_2 = 0;
+	}
+	return;
+
+}
+
+/*------------------------------------------------------------------------*
  *	usb2_callback_wrapper
  *
  * This is a wrapper for USB callbacks. This wrapper does some
@@ -1663,11 +1744,10 @@
 	}
 	if (!mtx_owned(xfer->priv_mtx)) {
 		/*
-		       * Cases that end up here:
-		       *
-		       * 5) HW interrupt done callback or other source.
-		       */
-
+	       	 * Cases that end up here:
+		 *
+		 * 5) HW interrupt done callback or other source.
+		 */
 		DPRINTF(2, "case 5\n");
 
 		/*
@@ -1675,7 +1755,6 @@
 	         * will have a Lock Order Reversal, LOR, if we try to
 	         * proceed !
 	         */
-
 		if (usb2_proc_msignal(&(info->done_p),
 		    &(info->done_m[0]), &(info->done_m[1]))) {
 			/* ignore */
@@ -1700,6 +1779,11 @@
 	/* set correct USB state for callback */
 	if (!xfer->flags_int.transferring) {
 		xfer->usb2_state = USB_ST_SETUP;
+		if (!xfer->flags_int.started) {
+			/* we got stopped before we even got started */
+			mtx_lock(xfer->usb2_mtx);
+			goto done;
+		}
 	} else {
 
 		if (usb2_callback_wrapper_sub(xfer)) {
@@ -1905,7 +1989,7 @@
 	}
 
 	/* call the USB transfer callback */
-	usb2_command_wrapper(&(xfer->usb2_root->done_q), xfer);
+	usb2_callback_ss_done_defer(xfer);
 	return;
 }
 

==== //depot/projects/usb/src/sys/dev/usb2/core/usb2_transfer.h#4 (text+ko) ====

@@ -63,10 +63,8 @@
 	uint32_t dma_nframes;		/* number of page caches to load */
 	uint32_t dma_currframe;		/* currect page cache number */
 	uint32_t dma_frlength_0;	/* length of page cache zero */
-
 	uint8_t	dma_error;		/* set if virtual memory could not be
 					 * loaded */
-
 	uint8_t	done_sleep;		/* set if done thread is sleeping */
 };
 
@@ -103,6 +101,7 @@
 
 /* function prototypes */
 
+uint8_t	usb2_transfer_pending(struct usb2_xfer *xfer);
 uint8_t	usb2_transfer_setup_sub_malloc(struct usb2_setup_params *parm, struct usb2_page_search *info, struct usb2_page_cache **ppc, uint32_t size, uint32_t align);
 void	usb2_command_wrapper(struct usb2_xfer_queue *pq, struct usb2_xfer *xfer);
 void	usb2_pipe_enter(struct usb2_xfer *xfer);


More information about the p4-projects mailing list