From 0bb43ca2823ab55a74565f6e17e2c36749cff3b9 Mon Sep 17 00:00:00 2001 From: Ned Bass Date: Thu, 19 Jan 2012 11:36:27 -0800 Subject: [PATCH] Revert "Taskq locking optimizations" This reverts commit ec2b41049f7f576aaa772b326d083e5971212d33. A race condition was introduced by which a wake_up() call can be lost after the taskq thread determines there is no pending work items, leading to deadlock: 1. taksq thread enables interrupts 2. dispatcher thread runs, queues work item, call wake_up() 3. taskq thread runs, adds self to waitq, sleeps This could easily happen if an interrupt for an IO completion was outstanding at the point where the taskq thread reenables interrupts, just before the call to add_wait_queue_exclusive(). The handler would run immediately within the race window. Signed-off-by: Brian Behlendorf Issue #32 --- module/spl/spl-taskq.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/module/spl/spl-taskq.c b/module/spl/spl-taskq.c index b0677666d..ece99aad6 100644 --- a/module/spl/spl-taskq.c +++ b/module/spl/spl-taskq.c @@ -286,11 +286,10 @@ __taskq_dispatch(taskq_t *tq, task_func_t func, void *arg, uint_t flags) ASSERT(!(t->tqent_flags & TQENT_FLAG_PREALLOC)); spin_unlock(&t->tqent_lock); + + wake_up(&tq->tq_work_waitq); out: spin_unlock_irqrestore(&tq->tq_lock, tq->tq_lock_flags); - if (rc > 0) - wake_up(&tq->tq_work_waitq); - SRETURN(rc); } EXPORT_SYMBOL(__taskq_dispatch); @@ -310,7 +309,6 @@ __taskq_dispatch_ent(taskq_t *tq, task_func_t func, void *arg, uint_t flags, /* Taskq being destroyed and all tasks drained */ if (!(tq->tq_flags & TQ_ACTIVE)) { t->tqent_id = 0; - spin_unlock_irqrestore(&tq->tq_lock, tq->tq_lock_flags); goto out; } @@ -334,10 +332,10 @@ __taskq_dispatch_ent(taskq_t *tq, task_func_t func, void *arg, uint_t flags, t->tqent_arg = arg; spin_unlock(&t->tqent_lock); - spin_unlock_irqrestore(&tq->tq_lock, tq->tq_lock_flags); wake_up(&tq->tq_work_waitq); out: + spin_unlock_irqrestore(&tq->tq_lock, tq->tq_lock_flags); SEXIT; } EXPORT_SYMBOL(__taskq_dispatch_ent); @@ -456,17 +454,17 @@ taskq_thread(void *args) while (!kthread_should_stop()) { + add_wait_queue(&tq->tq_work_waitq, &wait); if (list_empty(&tq->tq_pend_list) && list_empty(&tq->tq_prio_list)) { spin_unlock_irqrestore(&tq->tq_lock, tq->tq_lock_flags); - add_wait_queue_exclusive(&tq->tq_work_waitq, &wait); schedule(); - remove_wait_queue(&tq->tq_work_waitq, &wait); spin_lock_irqsave(&tq->tq_lock, tq->tq_lock_flags); } else { __set_current_state(TASK_RUNNING); } + remove_wait_queue(&tq->tq_work_waitq, &wait); if (!list_empty(&tq->tq_prio_list)) pend_list = &tq->tq_prio_list;