Module Name: src
Committed By: riastradh
Date: Sun Jul 29 01:59:46 UTC 2018
Modified Files:
src/sys/dev/usb: usb.c usbdi.h
Log Message:
New function usb_rem_task_wait(dev, task, queue).
If task is scheduled to run, removes it from the queue. If it may
have already begun to run, waits for it to complete. Caller must
guarantee it will not switch to another queue. If caller guarantees
it will not be scheduled again, then usb_rem_task_wait guarantees it
is not running on return.
This will enable us to fix a litany of bugs in detach where we
currently fail to wait for a pending task.
To generate a diff of this commit:
cvs rdiff -u -r1.169 -r1.170 src/sys/dev/usb/usb.c
cvs rdiff -u -r1.92 -r1.93 src/sys/dev/usb/usbdi.h
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: src/sys/dev/usb/usb.c
diff -u src/sys/dev/usb/usb.c:1.169 src/sys/dev/usb/usb.c:1.170
--- src/sys/dev/usb/usb.c:1.169 Fri Jun 29 17:48:24 2018
+++ src/sys/dev/usb/usb.c Sun Jul 29 01:59:46 2018
@@ -1,4 +1,4 @@
-/* $NetBSD: usb.c,v 1.169 2018/06/29 17:48:24 msaitoh Exp $ */
+/* $NetBSD: usb.c,v 1.170 2018/07/29 01:59:46 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.169 2018/06/29 17:48:24 msaitoh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: usb.c,v 1.170 2018/07/29 01:59:46 riastradh Exp $");
#ifdef _KERNEL_OPT
#include "opt_usb.h"
@@ -146,6 +146,7 @@ struct usb_taskq {
kcondvar_t cv;
struct lwp *task_thread_lwp;
const char *name;
+ struct usb_task *current_task;
};
static struct usb_taskq usb_taskq[USB_NUM_TASKQS];
@@ -294,6 +295,7 @@ usb_once_init(void)
mutex_init(&taskq->lock, MUTEX_DEFAULT, IPL_USB);
cv_init(&taskq->cv, "usbtsk");
taskq->name = taskq_names[i];
+ taskq->current_task = NULL;
if (kthread_create(PRI_NONE, KTHREAD_MPSAFE, NULL,
usb_task_thread, taskq, &taskq->task_thread_lwp,
"%s", taskq->name)) {
@@ -426,8 +428,14 @@ usb_add_task(struct usbd_device *dev, st
}
/*
- * XXX This does not wait for completion! Most uses need such an
- * operation. Urgh...
+ * usb_rem_task(dev, task)
+ *
+ * If task is queued to run, remove it from the queue.
+ *
+ * Caller is _not_ guaranteed that the task is not running when
+ * this is done.
+ *
+ * Never sleeps.
*/
void
usb_rem_task(struct usbd_device *dev, struct usb_task *task)
@@ -449,6 +457,85 @@ usb_rem_task(struct usbd_device *dev, st
}
}
+/*
+ * usb_taskq_wait(taskq, task)
+ *
+ * Wait for taskq to finish executing task, if it is executing
+ * task. Caller must hold the taskq lock.
+ */
+static void
+usb_taskq_wait(struct usb_taskq *taskq, struct usb_task *task)
+{
+
+ KASSERT(mutex_owned(&taskq->lock));
+
+ while (taskq->current_task == task)
+ cv_wait(&taskq->cv, &taskq->lock);
+
+ KASSERT(taskq->current_task != task);
+}
+
+/*
+ * usb_rem_task_wait(dev, task, queue)
+ *
+ * If task is scheduled to run, remove it from the queue. If it
+ * may have already begun to run, wait for it to complete.
+ *
+ * Caller MUST guarantee that task will not be scheduled on a
+ * _different_ queue, at least until after this returns.
+ *
+ * If caller guarantees that task will not be scheduled on the
+ * same queue before this returns, then caller is guaranteed that
+ * the task is not running at all when this returns.
+ *
+ * May sleep.
+ */
+void
+usb_rem_task_wait(struct usbd_device *dev, struct usb_task *task, int queue)
+{
+ struct usb_taskq *taskq;
+ int queue1;
+
+ USBHIST_FUNC(); USBHIST_CALLED(usbdebug);
+ ASSERT_SLEEPABLE();
+ KASSERT(0 <= queue);
+ KASSERT(queue < USB_NUM_TASKQS);
+
+ taskq = &usb_taskq[queue];
+
+ if ((queue1 = task->queue) == USB_NUM_TASKQS) {
+ /*
+ * It is not on the queue, but it may have already run.
+ * Wait for it.
+ */
+ mutex_enter(&taskq->lock);
+ usb_taskq_wait(taskq, task);
+ mutex_exit(&taskq->lock);
+ } else {
+ /*
+ * It may be on the queue (and not another one), but
+ * the state may have changed by now because we don't
+ * have the queue locked. Lock and reload.
+ */
+ KASSERTMSG(queue1 == queue,
+ "task %p on q%d expected on q%d", task, queue1, queue);
+ mutex_enter(&taskq->lock);
+ queue1 = task->queue;
+ if (queue1 == queue) {
+ /* Still queued, not run. Just remove it. */
+ TAILQ_REMOVE(&taskq->tasks, task, next);
+ task->queue = USB_NUM_TASKQS;
+ } else {
+ /* Already ran. Wait for it. */
+ KASSERTMSG(queue1 == USB_NUM_TASKQS,
+ "task %p on q%d expected on q%d",
+ task, queue1, queue);
+ usb_taskq_wait(taskq, task);
+ }
+ mutex_exit(&taskq->lock);
+ }
+}
+
void
usb_event_thread(void *arg)
{
@@ -517,6 +604,7 @@ usb_task_thread(void *arg)
mpsafe = ISSET(task->flags, USB_TASKQ_MPSAFE);
TAILQ_REMOVE(&taskq->tasks, task, next);
task->queue = USB_NUM_TASKQS;
+ taskq->current_task = task;
mutex_exit(&taskq->lock);
if (!mpsafe)
@@ -527,6 +615,10 @@ usb_task_thread(void *arg)
KERNEL_UNLOCK_ONE(curlwp);
mutex_enter(&taskq->lock);
+ KASSERTMSG(taskq->current_task == task,
+ "somebody scribbled on usb taskq %p", taskq);
+ taskq->current_task = NULL;
+ cv_broadcast(&taskq->cv);
}
}
mutex_exit(&taskq->lock);
Index: src/sys/dev/usb/usbdi.h
diff -u src/sys/dev/usb/usbdi.h:1.92 src/sys/dev/usb/usbdi.h:1.93
--- src/sys/dev/usb/usbdi.h:1.92 Sun Aug 14 14:42:22 2016
+++ src/sys/dev/usb/usbdi.h Sun Jul 29 01:59:46 2018
@@ -1,4 +1,4 @@
-/* $NetBSD: usbdi.h,v 1.92 2016/08/14 14:42:22 skrll Exp $ */
+/* $NetBSD: usbdi.h,v 1.93 2018/07/29 01:59:46 riastradh Exp $ */
/* $FreeBSD: src/sys/dev/usb/usbdi.h,v 1.18 1999/11/17 22:33:49 n_hibma Exp $ */
/*
@@ -219,6 +219,7 @@ struct usb_task {
void usb_add_task(struct usbd_device *, struct usb_task *, int);
void usb_rem_task(struct usbd_device *, struct usb_task *);
+void usb_rem_task_wait(struct usbd_device *, struct usb_task *, int);
#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 {
