[src/trunk]: src/sys/dev/usb Track transfer state better to avoid races betwe...

[skrll <skrll%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/5d0367cf0d1f
branches:  trunk
changeset: 784959:5d0367cf0d1f
user:      skrll <skrll%NetBSD.org@localhost>
date:      Fri Feb 15 17:07:09 2013 +0000

description:
Track transfer state better to avoid races between the workqueue and
aborting.

diffstat:

 sys/dev/usb/dwc_otg.c    |  57 ++++++++++++++++++++++++++++++++++-------------
 sys/dev/usb/dwc_otgvar.h |  12 +++++++--
 2 files changed, 50 insertions(+), 19 deletions(-)

diffs (175 lines):

diff -r fd213a02b231 -r 5d0367cf0d1f sys/dev/usb/dwc_otg.c
--- a/sys/dev/usb/dwc_otg.c     Fri Feb 15 17:04:15 2013 +0000
+++ b/sys/dev/usb/dwc_otg.c     Fri Feb 15 17:07:09 2013 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: dwc_otg.c,v 1.45 2013/02/04 21:37:05 skrll Exp $       */
+/*     $NetBSD: dwc_otg.c,v 1.46 2013/02/15 17:07:09 skrll Exp $       */
 
 /*-
  * Copyright (c) 2012 Hans Petter Selasky. All rights reserved.
@@ -60,7 +60,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: dwc_otg.c,v 1.45 2013/02/04 21:37:05 skrll Exp $");
+__KERNEL_RCSID(0, "$NetBSD: dwc_otg.c,v 1.46 2013/02/15 17:07:09 skrll Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -423,13 +423,20 @@
 
        mutex_spin_enter(&sc->sc_intr_lock);
        while ((dxfer = TAILQ_FIRST(&sc->sc_complete)) != NULL) {
-               TAILQ_REMOVE(&sc->sc_complete, dxfer, xnext);
-
+               KASSERT(dxfer->state == DXFER_COMPLETING);
+
+               /*
+                * dwc_otg_abort_xfer will remove this transfer from the
+                * sc_complete queue
+                */
                if (dxfer->xfer.hcflags & UXFER_ABORTING) {
                        cv_broadcast(&dxfer->xfer.hccv);
                        continue;
                }
 
+               TAILQ_REMOVE(&sc->sc_complete, dxfer, xnext);
+               dxfer->state = DXFER_DONE;
+
                mutex_spin_exit(&sc->sc_intr_lock);
                usb_transfer_complete(&dxfer->xfer);
                mutex_spin_enter(&sc->sc_intr_lock);
@@ -641,9 +648,23 @@
        xfer->status = status;  /* make software ignore it */
        callout_stop(&xfer->timeout_handle);
 
-       if (dxfer->active) {
+       switch (dxfer->state) {
+       case DXFER_INIT:
+               dxfer->state = DXFER_ABORTING;
+               break;
+       case DXFER_WORKQ:
+               /* Give the workqueue a chance */
+               break;
+       case DXFER_ACTIVE:
                TAILQ_REMOVE(&sc->sc_active, dxfer, xnext);
-               dxfer->active = false;
+               dxfer->state = DXFER_ABORTING;
+               break;
+       case DXFER_COMPLETING:
+               TAILQ_REMOVE(&sc->sc_complete, dxfer, xnext);
+               dxfer->state = DXFER_ABORTING;
+               break;
+       default:
+               KASSERT(false);
        }
        mutex_spin_exit(&sc->sc_intr_lock);
 
@@ -651,10 +672,11 @@
                dwc_otg_host_channel_free(dxfer->td_transfer_cache);
        }
 
-       while (dxfer->queued) {
+       while (dxfer->state != DXFER_ABORTING) {
                cv_wait(&xfer->hccv, &sc->sc_lock);
        }
 
+       dxfer->state = DXFER_DONE;
        /*
         * Step 2: Execute callback.
         */
@@ -1671,13 +1693,14 @@
                dwc_otg_timer(sc);
        } else {
                KASSERT(dwork->xfer != NULL);
-               KASSERT(dxfer->queued == true);
+               KASSERT(dxfer->state == DXFER_WORKQ);
 
                if (!(xfer->hcflags & UXFER_ABORTING)) {
                        dwc_otg_start_standard_chain(xfer);
+               } else {
+                       dxfer->state = DXFER_ABORTING;
+                       cv_broadcast(&xfer->hccv);
                }
-               dxfer->queued = false;
-               cv_broadcast(&xfer->hccv);
        }
        mutex_exit(&sc->sc_lock);
 }
@@ -3853,7 +3876,7 @@
 
 //     DPRINTF(("%s: xfer->length %d\n", __func__, xfer->length));
 
-       dxfer->queued = false;
+       dxfer->state = DXFER_INIT;
 
        /* get first again */
        td = dxfer->td_transfer_first;
@@ -3962,12 +3985,14 @@
 
        /* poll one time - will turn on interrupts */
        mutex_spin_enter(&sc->sc_intr_lock);
+       dxfer->state = DXFER_STARTED;
+
        if (dwc_otg_xfer_do_fifo(xfer)) {
 
                KASSERT(mutex_owned(&sc->sc_lock));
                /* put transfer on interrupt queue */
 
-               dxfer->active = true;
+               dxfer->state = DXFER_ACTIVE;
                TAILQ_INSERT_TAIL(&sc->sc_active, dxfer, xnext);
 
                /* start timeout, if any */
@@ -4059,12 +4084,12 @@
                dwc_otg_host_channel_free(td);
 
        xfer->status = err;
-       if (dxfer->active) {
+       if (dxfer->state == DXFER_ACTIVE) {
                TAILQ_REMOVE(&sc->sc_active, dxfer, xnext);
-               dxfer->active = false;
        }
        callout_stop(&xfer->timeout_handle);
 
+       dxfer->state = DXFER_COMPLETING;
        TAILQ_INSERT_TAIL(&sc->sc_complete, dxfer, xnext);
 
        usb_schedsoftintr(&sc->sc_bus);
@@ -4406,8 +4431,8 @@
        if (sc->sc_bus.use_polling) {
                dwc_otg_start_standard_chain(xfer);
        } else {
-               KASSERT(dxfer->queued == false);
-               dxfer->queued = true;
+               KASSERT(dxfer->state == DXFER_INIT);
+               dxfer->state = DXFER_WORKQ;
                workqueue_enqueue(sc->sc_wq, (struct work *)&dxfer->work, NULL);
        }
 }
diff -r fd213a02b231 -r 5d0367cf0d1f sys/dev/usb/dwc_otgvar.h
--- a/sys/dev/usb/dwc_otgvar.h  Fri Feb 15 17:04:15 2013 +0000
+++ b/sys/dev/usb/dwc_otgvar.h  Fri Feb 15 17:07:09 2013 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: dwc_otgvar.h,v 1.11 2013/02/04 21:29:14 skrll Exp $ */
+/*     $NetBSD: dwc_otgvar.h,v 1.12 2013/02/15 17:07:09 skrll Exp $ */
 
 /* $FreeBSD: src/sys/dev/usb/controller/dwc_otg.h,v 1.12 2012/09/27 15:23:38 hselasky Exp $ */
 /*-
@@ -158,8 +158,14 @@
        struct usbd_xfer xfer;                  /* Needs to be first */
        struct usb_task abort_task;
        TAILQ_ENTRY(dwc_otg_xfer) xnext;        /* list of active/complete xfers */
-       bool            queued;                 /* pending workqueue */
-       bool            active;                 /* still active */
+       int     state;
+#define DXFER_INIT             0
+#define DXFER_WORKQ            1
+#define DXFER_STARTED          2
+#define DXFER_ACTIVE           3
+#define DXFER_COMPLETING       4
+#define DXFER_ABORTING         5
+#define DXFER_DONE             6
 
        void            *td_start[1];
        dwc_otg_td_t    *td_transfer_first;