PERFORCE change 118675 for review

[Hans Petter Selasky]

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

Change 118675 by hselasky at hselasky_mini_itx on 2007/04/23 19:18:51

	Make the USB stack smarter by allowing
	NULL pointer arguments to be passed
	to usbd_transfer_start() and
	usbd_transfer_stop(). At the same
	time add some more comments describing
	why.

Affected files ...

.. //depot/projects/usb/src/sys/dev/usb/usb_transfer.c#21 edit

Differences ...

==== //depot/projects/usb/src/sys/dev/usb/usb_transfer.c#21 (text+ko) ====

@@ -383,7 +383,6 @@
 	while(n_setup--)
 	{
 	    xfer = pxfer[n_setup];
-	    pxfer[n_setup] = NULL;
 
 	    if(xfer)
 	    {
@@ -391,6 +390,27 @@
 		{
 		    mtx_lock(xfer->priv_mtx);
 
+		    /* HINT: when you start/stop a transfer,
+		     * it might be a good idea to directly
+		     * use the "pxfer[]" structure:
+		     *
+		     * usbd_transfer_start(sc->pxfer[0]);
+		     * usbd_transfer_stop(sc->pxfer[0]);
+		     *
+		     * That way, if your code has many parts
+		     * that will not stop running under the
+		     * same lock, in other words "priv_mtx",
+		     * the usbd_transfer_start and
+		     * usbd_transfer_stop functions will
+		     * simply return when they detect a NULL
+		     * pointer argument.
+		     *
+		     * To avoid any races we clear the "pxfer[]"
+		     * pointer while holding the private mutex
+		     * of the driver:
+		     */
+		    pxfer[n_setup] = NULL;
+
 		    usbd_transfer_stop(xfer);
 
 		    /* NOTE: default pipe does not
@@ -400,6 +420,9 @@
 		    xfer->pipe->refcount--;
 
 		    mtx_unlock(xfer->priv_mtx);
+		} else {
+		    /* clear the transfer pointer */
+		    pxfer[n_setup] = NULL;
 		}
 
 		__callout_drain(&(xfer->timeout_handle));
@@ -423,6 +446,8 @@
 
 		    if (info->setup_refcount == 0) {
 
+		        /* wait for any USB callbacks to return */
+
 		        while (info->memory_refcount > 0) {
 			    error = msleep(info, info->usb_mtx, 0, 
 					   "usb_mem_wait", 0);
@@ -708,9 +733,11 @@
 }
 
 /*---------------------------------------------------------------------------*
- *	usbd_transfer_start - start a USB transfer
+ *	usbd_transfer_start - start an USB transfer
  *
- * NOTE: this function can be called any number of times
+ * NOTE: Calling this function more than one time will only
+ *       result in a single transfer start, until the USB transfer
+ *       completes.
  * NOTE: if USBD_SYNCHRONOUS is set in "xfer->flags", then this
  *       function will sleep for transfer completion
  * NOTE: if USBD_USE_POLLING is set in "xfer->flags", then this
@@ -719,6 +746,11 @@
 void
 usbd_transfer_start(struct usbd_xfer *xfer)
 {
+	if (xfer == NULL) {
+	    /* transfer is gone */
+	    return;
+	}
+
 	mtx_assert(xfer->priv_mtx, MA_OWNED);
 
 	if(!(xfer->flags & USBD_DEV_OPEN))
@@ -793,13 +825,19 @@
 }
 
 /*---------------------------------------------------------------------------*
- *	usbd_transfer_stop - stop a USB transfer
+ *	usbd_transfer_stop - stop an USB transfer
  *
- * NOTE: this function can be called any number of times
+ * NOTE: Calling this function more than one time will only
+ *       result in a single transfer stop.
  *---------------------------------------------------------------------------*/
 void
 usbd_transfer_stop(struct usbd_xfer *xfer)
 {
+	if (xfer == NULL) {
+	    /* transfer is gone */
+	    return;
+	}
+
 	mtx_assert(xfer->priv_mtx, MA_OWNED);
 
 	if(xfer->flags & USBD_DEV_OPEN)