[PATCH wq/for-7.3] workqueue: Annotate cb_lock nesting when draining a dead BH pool

[Tejun Heo]

On PREEMPT_RT, bh_worker() wraps work item execution in pool->cb_lock to
provide a handshake for canceling BH work items. When a CPU goes down,
drain_dead_softirq_workfn() runs the dead pool's bh_worker() nested inside
the local pool's bh_worker(), acquiring the cb_locks of two different pools
without a nesting annotation. lockdep reports possible recursive locking:

============================================
WARNING: possible recursive locking detected
--------------------------------------------
ktimers/0/16 is trying to acquire lock:
ffff8880b873a990 (&pool->cb_lock){+...}-{3:3}, at: bh_worker+0x7d/0x880

but task is already holding lock:
ffff8880b863a990 (&pool->cb_lock){+...}-{3:3}, at: bh_worker+0x7d/0x880

Call Trace:
bh_worker+0x7d/0x880 kernel/workqueue.c:3688
drain_dead_softirq_workfn+0x95/0x220 kernel/workqueue.c:3763
process_scheduled_works+0xa8e/0x14e0 kernel/workqueue.c:3405
bh_worker+0x46a/0x880 kernel/workqueue.c:3708
tasklet_action+0xc/0x70 kernel/softirq.c:965

The nesting can't deadlock. A pool's bh_worker() runs nested only while the
pool's CPU is dead, entered from a live pool's bh_worker() on the draining
CPU, so the ordering is always live to dead. CPU hotplug operations are
serialized and the drain is synchronous, so the nesting depth never exceeds
two. Annotate the inner acquisition with SINGLE_DEPTH_NESTING.

Signed-off-by: Tejun Heo <t...@kernel.org>
Reported-by: syzbot+1bd201...@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=1bd20115328f8254ed62
Fixes: ad7c7f4b9c6c ("workqueue: Provide a handshake for canceling BH workers")
Cc: sta...@vger.kernel.org # v6.18+
---
kernel/workqueue.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)

--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -3141,7 +3141,16 @@ restart:
#ifdef CONFIG_PREEMPT_RT
static void worker_lock_callback(struct worker_pool *pool)
{
- spin_lock(&pool->cb_lock);
+ /*
+ * SINGLE_DEPTH_NESTING is for a dead pool's bh_worker() running from
+ * drain_dead_softirq_workfn() inside a live pool's bh_worker(). The
+ * unlocked read is stable: the flag is only set while @pool's CPU is
+ * dead, inside a serialized hotplug operation. data_race() as the value
+ * only affects the lockdep annotation and the read can be elided when
+ * lockdep is disabled.
+ */
+ spin_lock_nested(&pool->cb_lock,
+ data_race(pool->flags) & POOL_BH_DRAINING ? SINGLE_DEPTH_NESTING : 0);
}

static void worker_unlock_callback(struct worker_pool *pool)

[Tejun Heo]

Applied to wq/for-7.3-fixes.

Thanks.

--
tejun