Skip to content

Commit

Permalink
Taskq locking optimizations
Browse files Browse the repository at this point in the history
Testing has shown that tq->tq_lock can be highly contended when a
large number of small work items are dispatched.  The lock hold time
is reduced by the following changes:

1) Use exclusive threads in the work_waitq

When a single work item is dispatched we only need to wake a single
thread to service it.  The current implementation uses non-exclusive
threads so all threads are woken when the dispatcher calls wake_up().
If a large number of threads are in the queue this overhead can become
non-negligible.

2) Conditionally add/remove threads from work waitq outside of tq_lock

Taskq threads need only add themselves to the work wait queue if there
are no pending work items.  Furthermore, the add and remove function
calls can be made outside of the taskq lock since the wait queues are
protected from concurrent access by their own spinlocks.

3) Call wake_up() outside of tq->tq_lock

Again, the wait queues are protected by their own spinlock, so the
dispatcher functions can drop tq->tq_lock before calling wake_up().

A new splat test taskq:contention was added in a prior commit to measure
the impact of these changes.  The following table summarizes the
results using data from the kernel lock profiler.

                        tq_lock time    %diff   Wall clock (s)  %diff
original:               39117614.10     0       41.72           0
exclusive threads:      31871483.61     18.5    34.2            18.0
unlocked add/rm waitq:  13794303.90     64.7    16.17           61.2
unlocked wake_up():     1589172.08      95.9    16.61           60.2

Each row reflects the average result over 5 test runs.
/proc/lock_stats was zeroed out before and collected after each run.
Column 1 is the cumulative hold time in microseconds for tq->tq_lock.
The tests are cumulative; each row reflects the code changes of the
previous rows.  %diff is calculated with respect to "original" as
100*(orig-new)/orig.

Although calling wake_up() outside of the taskq lock dramatically
reduced the taskq lock hold time, the test actually took slightly more
wall clock time.  This is because the point of contention shifts from
the taskq lock to the wait queue lock.  But the change still seems
worthwhile since it removes our taskq implementation as a bottleneck,
assuming the small increase in wall clock time to be statistical
noise.

Signed-off-by: Brian Behlendorf <[email protected]>
Closes openzfs#32
  • Loading branch information
nedbass authored and behlendorf committed Jan 18, 2012
1 parent cf5d23f commit ec2b410
Showing 1 changed file with 7 additions and 5 deletions.
12 changes: 7 additions & 5 deletions module/spl/spl-taskq.c
Original file line number Diff line number Diff line change
Expand Up @@ -286,10 +286,11 @@ __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);
Expand All @@ -309,6 +310,7 @@ __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;
}

Expand All @@ -332,10 +334,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);
Expand Down Expand Up @@ -454,17 +456,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;
Expand Down

0 comments on commit ec2b410

Please sign in to comment.