[src/trunk]: src/sys/dev/usb Fix usb task queue locking.

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

details:   https://anonhg.NetBSD.org/src/rev/f3987b450ddb
branches:  trunk
changeset: 797439:f3987b450ddb
user:      riastradh <riastradh%NetBSD.org@localhost>
date:      Thu Jul 17 18:42:37 2014 +0000

description:
Fix usb task queue locking.

diffstat:

 sys/dev/usb/usb.c   |  30 +++++++++++++++++++++---------
 sys/dev/usb/usbdi.h |   6 +++---
 2 files changed, 24 insertions(+), 12 deletions(-)

diffs (101 lines):

diff -r 7233565c49ce -r f3987b450ddb sys/dev/usb/usb.c
--- a/sys/dev/usb/usb.c Thu Jul 17 17:52:39 2014 +0000
+++ b/sys/dev/usb/usb.c Thu Jul 17 18:42:37 2014 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: usb.c,v 1.149 2014/03/16 05:20:29 dholland Exp $       */
+/*     $NetBSD: usb.c,v 1.150 2014/07/17 18:42:37 riastradh Exp $      */
 
 /*
  * Copyright (c) 1998, 2002, 2008, 2012 The NetBSD Foundation, Inc.
@@ -37,7 +37,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: usb.c,v 1.149 2014/03/16 05:20:29 dholland Exp $");
+__KERNEL_RCSID(0, "$NetBSD: usb.c,v 1.150 2014/07/17 18:42:37 riastradh Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_compat_netbsd.h"
@@ -355,28 +355,40 @@
 {
        struct usb_taskq *taskq;
 
+       KASSERT(0 <= queue);
+       KASSERT(queue < USB_NUM_TASKQS);
        taskq = &usb_taskq[queue];
        mutex_enter(&taskq->lock);
-       if (task->queue == -1) {
+       if (atomic_cas_uint(&task->queue, USB_NUM_TASKQS, queue) ==
+           USB_NUM_TASKQS) {
                DPRINTFN(2,("usb_add_task: task=%p\n", task));
                TAILQ_INSERT_TAIL(&taskq->tasks, task, next);
                task->queue = queue;
+               cv_signal(&taskq->cv);
        } else {
                DPRINTFN(3,("usb_add_task: task=%p on q\n", task));
        }
-       cv_signal(&taskq->cv);
        mutex_exit(&taskq->lock);
 }
 
+/*
+ * XXX This does not wait for completion!  Most uses need such an
+ * operation.  Urgh...
+ */
 void
 usb_rem_task(usbd_device_handle dev, struct usb_task *task)
 {
+       unsigned queue;
 
-       if (task->queue != -1) {
-               struct usb_taskq *taskq = &usb_taskq[task->queue];
+       while ((queue = task->queue) != USB_NUM_TASKQS) {
+               struct usb_taskq *taskq = &usb_taskq[queue];
                mutex_enter(&taskq->lock);
-               TAILQ_REMOVE(&taskq->tasks, task, next);
-               task->queue = -1;
+               if (__predict_true(task->queue == queue)) {
+                       TAILQ_REMOVE(&taskq->tasks, task, next);
+                       task->queue = USB_NUM_TASKQS;
+                       mutex_exit(&taskq->lock);
+                       break;
+               }
                mutex_exit(&taskq->lock);
        }
 }
@@ -444,7 +456,7 @@
                DPRINTFN(2,("usb_task_thread: woke up task=%p\n", task));
                if (task != NULL) {
                        TAILQ_REMOVE(&taskq->tasks, task, next);
-                       task->queue = -1;
+                       task->queue = USB_NUM_TASKQS;
                        mutex_exit(&taskq->lock);
 
                        if (!(task->flags & USB_TASKQ_MPSAFE))
diff -r 7233565c49ce -r f3987b450ddb sys/dev/usb/usbdi.h
--- a/sys/dev/usb/usbdi.h       Thu Jul 17 17:52:39 2014 +0000
+++ b/sys/dev/usb/usbdi.h       Thu Jul 17 18:42:37 2014 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: usbdi.h,v 1.89 2013/09/26 07:25:31 skrll Exp $ */
+/*     $NetBSD: usbdi.h,v 1.90 2014/07/17 18:42:37 riastradh Exp $     */
 /*     $FreeBSD: src/sys/dev/usb/usbdi.h,v 1.18 1999/11/17 22:33:49 n_hibma Exp $      */
 
 /*
@@ -202,7 +202,7 @@
        TAILQ_ENTRY(usb_task) next;
        void (*fun)(void *);
        void *arg;
-       int queue;
+       volatile unsigned queue;
        int flags;
 };
 #define        USB_TASKQ_HC            0
@@ -213,7 +213,7 @@
 
 void usb_add_task(usbd_device_handle, struct usb_task *, int);
 void usb_rem_task(usbd_device_handle, struct usb_task *);
-#define usb_init_task(t, f, a, fl) ((t)->fun = (f), (t)->arg = (a), (t)->queue = -1, (t)->flags = (fl))
+#define usb_init_task(t, f, a, fl) ((t)->fun = (f), (t)->arg = (a), (t)->queue = USB_NUM_TASKQS, (t)->flags = (fl))
 
 struct usb_devno {
        u_int16_t ud_vendor;