--- a/sys/sys/taskqueue.h +++ b/sys/sys/taskqueue.h @@ -55,11 +55,14 @@ */ typedef void (*taskqueue_enqueue_fn)(void *context); +#define TASKQ_RUNNING 0x0001 /* ta_func currently executing */ + struct task { STAILQ_ENTRY(task) ta_link; /* link for queue */ struct taskqueue *ta_queue; /* taskqueue enqueued on */ int ta_pending; /* count times queued */ int ta_priority; /* priority of task in queue */ + int ta_flags; /* TASK_* flags, see above */ task_fn_t *ta_func; /* task handler */ void *ta_context; /* argument for handler */ }; @@ -99,6 +102,7 @@ { .ta_queue = NULL, \ .ta_pending = 0, \ .ta_priority = (priority), \ + .ta_flags = 0, \ .ta_func = (func), \ .ta_context = (context) } @@ -115,6 +119,7 @@ (task)->ta_queue = NULL; \ (task)->ta_pending = 0; \ (task)->ta_priority = (priority); \ + (task)->ta_flags = 0; \ (task)->ta_func = (func); \ (task)->ta_context = (context); \ } while (0) --- a/sys/kern/subr_taskqueue.c +++ b/sys/kern/subr_taskqueue.c @@ -54,7 +54,6 @@ taskqueue_enqueue_fn tq_enqueue; void *tq_context; - struct task *tq_running; struct spinlock tq_lock; struct thread **tq_threads; int tq_tcount; @@ -152,6 +151,14 @@ { TQ_LOCK(queue); queue->tq_flags &= ~TQ_FLAGS_ACTIVE; + /* + * Wait for any armed timeout-task callouts to complete so they + * cannot fire into the queue after it is freed. Completing + * callouts try to enqueue their task, which is rejected with + * EPIPE because TQ_FLAGS_ACTIVE is already clear. + */ + while (queue->tq_callouts != 0) + TQ_SLEEP(queue, &queue->tq_callouts, "tqfree"); taskqueue_run(queue, 1); taskqueue_terminate(queue->tq_threads, queue); TQ_UNLOCK(queue); @@ -325,6 +332,7 @@ { struct taskqueue *queue; struct timeout_task *timeout_task; + int wake_freer; timeout_task = arg; queue = timeout_task->t.ta_queue; @@ -333,8 +341,11 @@ KASSERT((timeout_task->f & DT_CALLOUT_ARMED) != 0, ("Stray timeout")); timeout_task->f &= ~DT_CALLOUT_ARMED; queue->tq_callouts--; + wake_freer = (queue->tq_callouts == 0); taskqueue_enqueue_locked(queue, &timeout_task->t); TQ_UNLOCK(queue); + if (wake_freer) + wakeup(&queue->tq_callouts); } int @@ -398,17 +409,28 @@ while (STAILQ_FIRST(&queue->tq_queue)) { /* * Carefully remove the first task from the queue and - * zero its pending count. + * zero its pending count. Mark it TASKQ_RUNNING under the + * queue spinlock so that concurrent runners (multi-thread + * queues, taskqueue_free() racing the service thread) each + * track their own task. */ task = STAILQ_FIRST(&queue->tq_queue); STAILQ_REMOVE_HEAD(&queue->tq_queue, ta_link); pending = task->ta_pending; task->ta_pending = 0; - queue->tq_running = task; + atomic_set_int(&task->ta_flags, TASKQ_RUNNING); TQ_UNLOCK(queue); task->ta_func(task->ta_context, pending); - queue->tq_running = NULL; + /* + * Clear TASKQ_RUNNING *before* wakeup() and while holding + * the queue spinlock: sleepers check the flag under the + * same spinlock and interlock their sleep with it + * (ssleep), so the wakeup cannot be lost. + */ + TQ_LOCK(queue); + atomic_clear_int(&task->ta_flags, TASKQ_RUNNING); + TQ_UNLOCK(queue); wakeup(task); TQ_LOCK(queue); } @@ -426,7 +448,7 @@ if (pendp != NULL) *pendp = task->ta_pending; task->ta_pending = 0; - return (task == queue->tq_running ? EBUSY : 0); + return ((task->ta_flags & TASKQ_RUNNING) ? EBUSY : 0); } int @@ -490,7 +512,8 @@ taskqueue_drain(struct taskqueue *queue, struct task *task) { TQ_LOCK(queue); - while (task->ta_pending != 0 || task == queue->tq_running) + while (task->ta_pending != 0 || + (task->ta_flags & TASKQ_RUNNING) != 0) TQ_SLEEP(queue, task, "-"); TQ_UNLOCK(queue); } @@ -509,7 +532,8 @@ if (queue == NULL) return; TQ_LOCK(queue); - if (task->ta_pending == 0 && task != queue->tq_running) { + if (task->ta_pending == 0 && + (task->ta_flags & TASKQ_RUNNING) == 0) { TQ_UNLOCK(queue); return; } @@ -616,6 +640,15 @@ TQ_LOCK(tq); while ((tq->tq_flags & TQ_FLAGS_ACTIVE) != 0) { taskqueue_run(tq, 1); + /* + * Re-check TQ_FLAGS_ACTIVE (under the spinlock we still + * hold from taskqueue_run) before parking. Otherwise a + * wakeup() delivered by taskqueue_terminate() while we + * were inside ta_func is lost and both threads sleep + * forever. + */ + if ((tq->tq_flags & TQ_FLAGS_ACTIVE) == 0) + break; TQ_SLEEP(tq, tq, "tqthr"); }