Archive-Team
/
zfs
2
0
Fork 0
Code Issues Pull Requests Packages Projects Releases 103 Wiki Activity

Subclass tq_lock to eliminate a lockdep warning 

When taskq_dispatch() calls taskq_thread_spawn() to create a new thread
for a taskq, linux lockdep warns of possible recursive locking.  This is
a false positive.

One such call chain is as follows, when a taskq needs more threads:
	taskq_dispatch->taskq_thread_spawn->taskq_dispatch

The initial taskq_dispatch() holds tq_lock on the taskq that needed more
worker threads.  The later call into taskq_dispatch() takes
dynamic_taskq->tq_lock.  Without subclassing, lockdep believes these
could potentially be the same lock and complains.  A similar case occurs
when taskq_dispatch() then calls task_alloc().

This patch uses spin_lock_irqsave_nested() when taking tq_lock, with one
of two new lock subclasses:

subclass              taskq
TQ_LOCK_DYNAMIC       dynamic_taskq
TQ_LOCK_GENERAL       any other

Signed-off-by: Olaf Faaland <faaland1@llnl.gov>
Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Issue #480
This commit is contained in:
Olaf Faaland 2015-10-13 16:56:51 -07:00 committed by Brian Behlendorf
parent 628fc52137
commit 326172d854
2 changed files with 58 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
include/sys/taskq.h
View File

@ -55,6 +55,14 @@
#define TQ_NEW 0x04000000 #define TQ_NEW 0x04000000
#define TQ_FRONT 0x08000000 #define TQ_FRONT 0x08000000
/* spin_lock(lock) and spin_lock_nested(lock,0) are equivalent,
* so TQ_LOCK_DYNAMIC must not evaluate to 0
*/
typedef enum tq_lock_role {
TQ_LOCK_GENERAL = 0,
TQ_LOCK_DYNAMIC = 1,
} tq_lock_role_t;
typedef unsigned long taskqid_t; typedef unsigned long taskqid_t;
typedef void (task_func_t)(void *); typedef void (task_func_t)(void *);
@ -81,6 +89,7 @@ typedef struct taskq {
struct list_head tq_delay_list; /* delayed task_t's */ struct list_head tq_delay_list; /* delayed task_t's */
wait_queue_head_t tq_work_waitq; /* new work waitq */ wait_queue_head_t tq_work_waitq; /* new work waitq */
wait_queue_head_t tq_wait_waitq; /* wait waitq */ wait_queue_head_t tq_wait_waitq; /* wait waitq */
tq_lock_role_t tq_lock_class; /* class used when taking tq_lock */
} taskq_t; } taskq_t;
typedef struct taskq_ent { typedef struct taskq_ent {

70
module/spl/spl-taskq.c
View File

@ -113,7 +113,8 @@ retry:
*/ */
spin_unlock_irqrestore(&tq->tq_lock, tq->tq_lock_flags); spin_unlock_irqrestore(&tq->tq_lock, tq->tq_lock_flags);
schedule_timeout(HZ / 100); schedule_timeout(HZ / 100);
spin_lock_irqsave(&tq->tq_lock, tq->tq_lock_flags); spin_lock_irqsave_nested(&tq->tq_lock, tq->tq_lock_flags,
tq->tq_lock_class);
if (count < 100) { if (count < 100) {
count++; count++;
goto retry; goto retry;
@ -122,7 +123,8 @@ retry:
spin_unlock_irqrestore(&tq->tq_lock, tq->tq_lock_flags); spin_unlock_irqrestore(&tq->tq_lock, tq->tq_lock_flags);
t = kmem_alloc(sizeof(taskq_ent_t), task_km_flags(flags)); t = kmem_alloc(sizeof(taskq_ent_t), task_km_flags(flags));
spin_lock_irqsave(&tq->tq_lock, tq->tq_lock_flags); spin_lock_irqsave_nested(&tq->tq_lock, tq->tq_lock_flags,
tq->tq_lock_class);
if (t) { if (t) {
taskq_init_ent(t); taskq_init_ent(t);
@ -188,7 +190,8 @@ task_expire(unsigned long data)
taskq_t *tq = t->tqent_taskq; taskq_t *tq = t->tqent_taskq;
struct list_head *l; struct list_head *l;
spin_lock_irqsave(&tq->tq_lock, tq->tq_lock_flags); spin_lock_irqsave_nested(&tq->tq_lock, tq->tq_lock_flags,
tq->tq_lock_class);
if (t->tqent_flags & TQENT_FLAG_CANCEL) { if (t->tqent_flags & TQENT_FLAG_CANCEL) {
ASSERT(list_empty(&t->tqent_list)); ASSERT(list_empty(&t->tqent_list));
@ -379,7 +382,8 @@ taskq_wait_id_check(taskq_t *tq, taskqid_t id)
int active = 0; int active = 0;
int rc; int rc;
spin_lock_irqsave(&tq->tq_lock, tq->tq_lock_flags); spin_lock_irqsave_nested(&tq->tq_lock, tq->tq_lock_flags,
tq->tq_lock_class);
rc = (taskq_find(tq, id, &active) == NULL); rc = (taskq_find(tq, id, &active) == NULL);
spin_unlock_irqrestore(&tq->tq_lock, tq->tq_lock_flags); spin_unlock_irqrestore(&tq->tq_lock, tq->tq_lock_flags);
@ -402,7 +406,8 @@ taskq_wait_outstanding_check(taskq_t *tq, taskqid_t id)
{ {
int rc; int rc;
spin_lock_irqsave(&tq->tq_lock, tq->tq_lock_flags); spin_lock_irqsave_nested(&tq->tq_lock, tq->tq_lock_flags,
tq->tq_lock_class);
rc = (id < tq->tq_lowest_id); rc = (id < tq->tq_lowest_id);
spin_unlock_irqrestore(&tq->tq_lock, tq->tq_lock_flags); spin_unlock_irqrestore(&tq->tq_lock, tq->tq_lock_flags);
@ -429,7 +434,8 @@ taskq_wait_check(taskq_t *tq)
{ {
int rc; int rc;
spin_lock_irqsave(&tq->tq_lock, tq->tq_lock_flags); spin_lock_irqsave_nested(&tq->tq_lock, tq->tq_lock_flags,
tq->tq_lock_class);
rc = (tq->tq_lowest_id == tq->tq_next_id); rc = (tq->tq_lowest_id == tq->tq_next_id);
spin_unlock_irqrestore(&tq->tq_lock, tq->tq_lock_flags); spin_unlock_irqrestore(&tq->tq_lock, tq->tq_lock_flags);
@ -474,7 +480,8 @@ taskq_member(taskq_t *tq, void *t)
{ {
int found; int found;
spin_lock_irqsave(&tq->tq_lock, tq->tq_lock_flags); spin_lock_irqsave_nested(&tq->tq_lock, tq->tq_lock_flags,
tq->tq_lock_class);
found = taskq_member_impl(tq, t); found = taskq_member_impl(tq, t);
spin_unlock_irqrestore(&tq->tq_lock, tq->tq_lock_flags); spin_unlock_irqrestore(&tq->tq_lock, tq->tq_lock_flags);
@ -497,7 +504,8 @@ taskq_cancel_id(taskq_t *tq, taskqid_t id)
ASSERT(tq); ASSERT(tq);
spin_lock_irqsave(&tq->tq_lock, tq->tq_lock_flags); spin_lock_irqsave_nested(&tq->tq_lock, tq->tq_lock_flags,
tq->tq_lock_class);
t = taskq_find(tq, id, &active); t = taskq_find(tq, id, &active);
if (t && !active) { if (t && !active) {
list_del_init(&t->tqent_list); list_del_init(&t->tqent_list);
@ -519,7 +527,8 @@ taskq_cancel_id(taskq_t *tq, taskqid_t id)
if (timer_pending(&t->tqent_timer)) { if (timer_pending(&t->tqent_timer)) {
spin_unlock_irqrestore(&tq->tq_lock, tq->tq_lock_flags); spin_unlock_irqrestore(&tq->tq_lock, tq->tq_lock_flags);
del_timer_sync(&t->tqent_timer); del_timer_sync(&t->tqent_timer);
spin_lock_irqsave(&tq->tq_lock, tq->tq_lock_flags); spin_lock_irqsave_nested(&tq->tq_lock,
tq->tq_lock_flags, tq->tq_lock_class);
} }
if (!(t->tqent_flags & TQENT_FLAG_PREALLOC)) if (!(t->tqent_flags & TQENT_FLAG_PREALLOC))
@ -549,7 +558,8 @@ taskq_dispatch(taskq_t *tq, task_func_t func, void *arg, uint_t flags)
ASSERT(tq); ASSERT(tq);
ASSERT(func); ASSERT(func);
spin_lock_irqsave(&tq->tq_lock, tq->tq_lock_flags); spin_lock_irqsave_nested(&tq->tq_lock, tq->tq_lock_flags,
tq->tq_lock_class);
/* Taskq being destroyed and all tasks drained */ /* Taskq being destroyed and all tasks drained */
if (!(tq->tq_flags & TASKQ_ACTIVE)) if (!(tq->tq_flags & TASKQ_ACTIVE))
@ -605,7 +615,8 @@ taskq_dispatch_delay(taskq_t *tq, task_func_t func, void *arg,
ASSERT(tq); ASSERT(tq);
ASSERT(func); ASSERT(func);
spin_lock_irqsave(&tq->tq_lock, tq->tq_lock_flags); spin_lock_irqsave_nested(&tq->tq_lock, tq->tq_lock_flags,
tq->tq_lock_class);
/* Taskq being destroyed and all tasks drained */ /* Taskq being destroyed and all tasks drained */
if (!(tq->tq_flags & TASKQ_ACTIVE)) if (!(tq->tq_flags & TASKQ_ACTIVE))
@ -648,7 +659,8 @@ taskq_dispatch_ent(taskq_t *tq, task_func_t func, void *arg, uint_t flags,
ASSERT(tq); ASSERT(tq);
ASSERT(func); ASSERT(func);
spin_lock_irqsave(&tq->tq_lock, tq->tq_lock_flags); spin_lock_irqsave_nested(&tq->tq_lock, tq->tq_lock_flags,
tq->tq_lock_class);
/* Taskq being destroyed and all tasks drained */ /* Taskq being destroyed and all tasks drained */
if (!(tq->tq_flags & TASKQ_ACTIVE)) { if (!(tq->tq_flags & TASKQ_ACTIVE)) {
@ -740,13 +752,14 @@ taskq_thread_spawn_task(void *arg)
(void) taskq_thread_create(tq); (void) taskq_thread_create(tq);
spin_lock_irqsave(&tq->tq_lock, tq->tq_lock_flags); spin_lock_irqsave_nested(&tq->tq_lock, tq->tq_lock_flags,
tq->tq_lock_class);
tq->tq_nspawn--; tq->tq_nspawn--;
spin_unlock_irqrestore(&tq->tq_lock, tq->tq_lock_flags); spin_unlock_irqrestore(&tq->tq_lock, tq->tq_lock_flags);
} }
/* /*
* Spawn addition threads for dynamic taskqs (TASKQ_DYNMAIC) the current * Spawn addition threads for dynamic taskqs (TASKQ_DYNAMIC) the current
* number of threads is insufficient to handle the pending tasks. These * number of threads is insufficient to handle the pending tasks. These
* new threads must be created by the dedicated dynamic_taskq to avoid * new threads must be created by the dedicated dynamic_taskq to avoid
* deadlocks between thread creation and memory reclaim. The system_taskq * deadlocks between thread creation and memory reclaim. The system_taskq
@ -810,6 +823,7 @@ taskq_thread(void *args)
int seq_tasks = 0; int seq_tasks = 0;
ASSERT(tqt); ASSERT(tqt);
ASSERT(tqt->tqt_tq);
tq = tqt->tqt_tq; tq = tqt->tqt_tq;
current->flags |= PF_NOFREEZE; current->flags |= PF_NOFREEZE;
@ -821,7 +835,8 @@ taskq_thread(void *args)
sigprocmask(SIG_BLOCK, &blocked, NULL); sigprocmask(SIG_BLOCK, &blocked, NULL);
flush_signals(current); flush_signals(current);
spin_lock_irqsave(&tq->tq_lock, tq->tq_lock_flags); spin_lock_irqsave_nested(&tq->tq_lock, tq->tq_lock_flags,
tq->tq_lock_class);
/* Immediately exit if more threads than allowed were created. */ /* Immediately exit if more threads than allowed were created. */
if (tq->tq_nthreads >= tq->tq_maxthreads) if (tq->tq_nthreads >= tq->tq_maxthreads)
@ -848,7 +863,8 @@ taskq_thread(void *args)
schedule(); schedule();
seq_tasks = 0; seq_tasks = 0;
spin_lock_irqsave(&tq->tq_lock, tq->tq_lock_flags); spin_lock_irqsave_nested(&tq->tq_lock,
tq->tq_lock_flags, tq->tq_lock_class);
remove_wait_queue(&tq->tq_work_waitq, &wait); remove_wait_queue(&tq->tq_work_waitq, &wait);
} else { } else {
__set_current_state(TASK_RUNNING); __set_current_state(TASK_RUNNING);
@ -877,7 +893,8 @@ taskq_thread(void *args)
/* Perform the requested task */ /* Perform the requested task */
t->tqent_func(t->tqent_arg); t->tqent_func(t->tqent_arg);
spin_lock_irqsave(&tq->tq_lock, tq->tq_lock_flags); spin_lock_irqsave_nested(&tq->tq_lock,
tq->tq_lock_flags, tq->tq_lock_class);
tq->tq_nactive--; tq->tq_nactive--;
list_del_init(&tqt->tqt_active_list); list_del_init(&tqt->tqt_active_list);
tqt->tqt_task = NULL; tqt->tqt_task = NULL;
@ -999,9 +1016,11 @@ taskq_create(const char *name, int nthreads, pri_t pri,
INIT_LIST_HEAD(&tq->tq_delay_list); INIT_LIST_HEAD(&tq->tq_delay_list);
init_waitqueue_head(&tq->tq_work_waitq); init_waitqueue_head(&tq->tq_work_waitq);
init_waitqueue_head(&tq->tq_wait_waitq); init_waitqueue_head(&tq->tq_wait_waitq);
tq->tq_lock_class = TQ_LOCK_GENERAL;
if (flags & TASKQ_PREPOPULATE) { if (flags & TASKQ_PREPOPULATE) {
spin_lock_irqsave(&tq->tq_lock, tq->tq_lock_flags); spin_lock_irqsave_nested(&tq->tq_lock, tq->tq_lock_flags,
tq->tq_lock_class);
for (i = 0; i < minalloc; i++) for (i = 0; i < minalloc; i++)
task_done(tq, task_alloc(tq, TQ_PUSHPAGE | TQ_NEW)); task_done(tq, task_alloc(tq, TQ_PUSHPAGE | TQ_NEW));
@ -1040,7 +1059,8 @@ taskq_destroy(taskq_t *tq)
taskq_ent_t *t; taskq_ent_t *t;
ASSERT(tq); ASSERT(tq);
spin_lock_irqsave(&tq->tq_lock, tq->tq_lock_flags); spin_lock_irqsave_nested(&tq->tq_lock, tq->tq_lock_flags,
tq->tq_lock_class);
tq->tq_flags &= ~TASKQ_ACTIVE; tq->tq_flags &= ~TASKQ_ACTIVE;
spin_unlock_irqrestore(&tq->tq_lock, tq->tq_lock_flags); spin_unlock_irqrestore(&tq->tq_lock, tq->tq_lock_flags);
@ -1053,7 +1073,8 @@ taskq_destroy(taskq_t *tq)
taskq_wait(tq); taskq_wait(tq);
spin_lock_irqsave(&tq->tq_lock, tq->tq_lock_flags); spin_lock_irqsave_nested(&tq->tq_lock, tq->tq_lock_flags,
tq->tq_lock_class);
/* /*
* Signal each thread to exit and block until it does. Each thread * Signal each thread to exit and block until it does. Each thread
@ -1069,7 +1090,8 @@ taskq_destroy(taskq_t *tq)
kthread_stop(thread); kthread_stop(thread);
spin_lock_irqsave(&tq->tq_lock, tq->tq_lock_flags); spin_lock_irqsave_nested(&tq->tq_lock, tq->tq_lock_flags,
tq->tq_lock_class);
} }
while (!list_empty(&tq->tq_free_list)) { while (!list_empty(&tq->tq_free_list)) {
@ -1113,6 +1135,12 @@ spl_taskq_init(void)
return (1); return (1);
} }
/* This is used to annotate tq_lock, so
* taskq_dispatch -> taskq_thread_spawn -> taskq_dispatch
* does not trigger a lockdep warning re: possible recursive locking
*/
dynamic_taskq->tq_lock_class = TQ_LOCK_DYNAMIC;
return (0); return (0);
} }