diff options
author | Peter Zijlstra <peterz@infradead.org> | 2017-08-23 13:23:30 +0200 |
---|---|---|
committer | Ingo Molnar <mingo@kernel.org> | 2017-08-25 11:06:33 +0200 |
commit | e6f3faa734a00c606b7b06c6b9f15e5627d3245b (patch) | |
tree | 4c3f0047d1fa1796442512e03147c40c026f25a8 /kernel/workqueue.c | |
parent | a1d14934ea4b9db816a8dbfeab1c3e7204a0d871 (diff) | |
download | linux-e6f3faa734a00c606b7b06c6b9f15e5627d3245b.tar.gz linux-e6f3faa734a00c606b7b06c6b9f15e5627d3245b.tar.bz2 linux-e6f3faa734a00c606b7b06c6b9f15e5627d3245b.zip |
locking/lockdep: Fix workqueue crossrelease annotation
The new completion/crossrelease annotations interact unfavourable with
the extant flush_work()/flush_workqueue() annotations.
The problem is that when a single work class does:
wait_for_completion(&C)
and
complete(&C)
in different executions, we'll build dependencies like:
lock_map_acquire(W)
complete_acquire(C)
and
lock_map_acquire(W)
complete_release(C)
which results in the dependency chain: W->C->W, which lockdep thinks
spells deadlock, even though there is no deadlock potential since
works are ran concurrently.
One possibility would be to change the work 'lock' to recursive-read,
but that would mean hitting a lockdep limitation on recursive locks.
Also, unconditinoally switching to recursive-read here would fail to
detect the actual deadlock on single-threaded workqueues, which do
have a problem with this.
For now, forcefully disregard these locks for crossrelease.
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Acked-by: Tejun Heo <tj@kernel.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: boqun.feng@gmail.com
Cc: byungchul.park@lge.com
Cc: david@fromorbit.com
Cc: johannes@sipsolutions.net
Cc: oleg@redhat.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Diffstat (limited to 'kernel/workqueue.c')
-rw-r--r-- | kernel/workqueue.c | 23 |
1 files changed, 22 insertions, 1 deletions
diff --git a/kernel/workqueue.c b/kernel/workqueue.c index 8ad214dc15a9..c0331891dec1 100644 --- a/kernel/workqueue.c +++ b/kernel/workqueue.c @@ -2093,7 +2093,28 @@ __acquires(&pool->lock) lock_map_acquire(&pwq->wq->lockdep_map); lock_map_acquire(&lockdep_map); - crossrelease_hist_start(XHLOCK_PROC); + /* + * Strictly speaking we should do start(PROC) without holding any + * locks, that is, before these two lock_map_acquire()'s. + * + * However, that would result in: + * + * A(W1) + * WFC(C) + * A(W1) + * C(C) + * + * Which would create W1->C->W1 dependencies, even though there is no + * actual deadlock possible. There are two solutions, using a + * read-recursive acquire on the work(queue) 'locks', but this will then + * hit the lockdep limitation on recursive locks, or simly discard + * these locks. + * + * AFAICT there is no possible deadlock scenario between the + * flush_work() and complete() primitives (except for single-threaded + * workqueues), so hiding them isn't a problem. + */ + crossrelease_hist_start(XHLOCK_PROC, true); trace_workqueue_execute_start(work); worker->current_func(work); /* |