History

workqueue/lockdep: 'Fix' flush_work() annotation

The flush_work() annotation as introduced by commit: e159489baa71 ("workqueue: relax lockdep annotation on flush_work()") hits on the lockdep problem with recursive read locks. The situation as described is: Work W1: Work W2: Task: ARR(Q) ARR(Q) flush_workqueue(Q) A(W1) A(W2) A(Q) flush_work(W2) R(Q) A(W2) R(W2) if (special) A(Q) else ARR(Q) R(Q) where: A - acquire, ARR - acquire-read-recursive, R - release. Where under 'special' conditions we want to trigger a lock recursion deadlock, but otherwise allow the flush_work(). The allowing is done by using recursive read locks (ARR), but lockdep is broken for recursive stuff. However, there appears to be no need to acquire the lock if we're not 'special', so if we remove the 'else' clause things become much simpler and no longer need the recursion thing at all.

Peter Zijlstra committed 9 years ago

Commit a1d14931 parent e9149851 file · +11 −9Browse files

Showing the changes to kernel/workqueue.c (1 of 1 file). Show all

Unified · Split

kernel/workqueue.c+11−9
@@ -2091,7 +2091,7 @@ __acquires(&pool->lock) · 2090 unmodified lines
2091 2091
2092 spin_unlock_irq(&pool->lock);2092 spin_unlock_irq(&pool->lock);
2093 2093
2094 lock_map_acquire_read(&pwq->wq->lockdep_map);2094 lock_map_acquire(&pwq->wq->lockdep_map);
2095 lock_map_acquire(&lockdep_map);2095 lock_map_acquire(&lockdep_map);
2096 crossrelease_hist_start(XHLOCK_PROC);2096 crossrelease_hist_start(XHLOCK_PROC);
2097 trace_workqueue_execute_start(work);2097 trace_workqueue_execute_start(work);
@@ -2826,16 +2826,18 @@ static bool start_flush_work(struct work_struct *work, struct wq_barrier *barr) · 728 unmodified lines
2826 spin_unlock_irq(&pool->lock);2826 spin_unlock_irq(&pool->lock);
2827 2827
2828 /*2828 /*
2829 * If @max_active is 1 or rescuer is in use, flushing another work2829 * Force a lock recursion deadlock when using flush_work() inside a
2830 * item on the same workqueue may lead to deadlock. Make sure the2830 * single-threaded or rescuer equipped workqueue.
2831 * flusher is not running on the same workqueue by verifying write2831 *
2832 * access.2832 * For single threaded workqueues the deadlock happens when the work
2833 * is after the work issuing the flush_work(). For rescuer equipped
2834 * workqueues the deadlock happens when the rescuer stalls, blocking
2835 * forward progress.
2833 */2836 */
2834 if (pwq->wq->saved_max_active == 1 || pwq->wq->rescuer)2837 if (pwq->wq->saved_max_active == 1 || pwq->wq->rescuer) {
2835 lock_map_acquire(&pwq->wq->lockdep_map);2838 lock_map_acquire(&pwq->wq->lockdep_map);
2836 else2839 lock_map_release(&pwq->wq->lockdep_map);
2837 lock_map_acquire_read(&pwq->wq->lockdep_map);2840 }
2838 lock_map_release(&pwq->wq->lockdep_map);
2839 2841
2840 return true;2842 return true;
2841already_gone:2843already_gone: