summaryrefslogtreecommitdiffstats
path: root/kernel/workqueue.c
diff options
context:
space:
mode:
authorPeter Zijlstra <peterz@infradead.org>2017-08-23 13:23:30 +0200
committerIngo Molnar <mingo@kernel.org>2017-08-25 11:06:33 +0200
commite6f3faa734a00c606b7b06c6b9f15e5627d3245b (patch)
tree4c3f0047d1fa1796442512e03147c40c026f25a8 /kernel/workqueue.c
parenta1d14934ea4b9db816a8dbfeab1c3e7204a0d871 (diff)
downloadlinux-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.c23
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);
/*