[PATCH] workqueue: Warn on flushing system-wide workqueues

From: Tetsuo Handa
Date: Mon Feb 14 2022 - 08:28:21 EST


syzbot found a circular locking dependency which is caused by flushing
system_long_wq WQ [1]. Tejun Heo commented that it makes no sense at all
to call flush_workqueue() on the shared workqueues as the caller has no
idea what it's gonna end up waiting for.

Although there is flush_scheduled_work() which flushes system_wq WQ with
"Think twice before calling this function! It's very easy to get into
trouble if you don't take great care." warning message, it will be too
difficult to guarantee that all users safely flush system-wide WQs.

Therefore, let's change the direction to that developers had better use
their own WQs if flushing is inevitable. To give developers time to update
their modules, for now just emit a warning message when flush_workqueue()
is called on system-wide WQs. We will eventually convert this warning
message into WARN_ON() and kill flush_scheduled_work().

Link: https://syzkaller.appspot.com/bug?extid=831661966588c802aae9 [1]
Signed-off-by: Tetsuo Handa <penguin-kernel@xxxxxxxxxxxxxxxxxxx>
---
kernel/workqueue.c | 33 +++++++++++++++++++++++++++++++++
1 file changed, 33 insertions(+)

diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index 33f1106b4f99..5ef40b9a1842 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -2805,6 +2805,37 @@ static bool flush_workqueue_prep_pwqs(struct workqueue_struct *wq,
return wait;
}

+static void warn_if_flushing_global_workqueue(struct workqueue_struct *wq)
+{
+#ifdef CONFIG_PROVE_LOCKING
+ static DEFINE_RATELIMIT_STATE(flush_warn_rs, 600 * HZ, 1);
+ const char *name;
+
+ if (wq == system_wq)
+ name = "system_wq";
+ else if (wq == system_highpri_wq)
+ name = "system_highpri_wq";
+ else if (wq == system_long_wq)
+ name = "system_long_wq";
+ else if (wq == system_unbound_wq)
+ name = "system_unbound_wq";
+ else if (wq == system_freezable_wq)
+ name = "system_freezable_wq";
+ else if (wq == system_power_efficient_wq)
+ name = "system_power_efficient_wq";
+ else if (wq == system_freezable_power_efficient_wq)
+ name = "system_freezable_power_efficient_wq";
+ else
+ return;
+ ratelimit_set_flags(&flush_warn_rs, RATELIMIT_MSG_ON_RELEASE);
+ if (!__ratelimit(&flush_warn_rs))
+ return;
+ pr_warn("Since system-wide WQ is shared, flushing system-wide WQ can introduce unexpected locking dependency. Please replace %s usage in your code with your local WQ.\n",
+ name);
+ dump_stack();
+#endif
+}
+
/**
* flush_workqueue - ensure that any scheduled work has run to completion.
* @wq: workqueue to flush
@@ -2824,6 +2855,8 @@ void flush_workqueue(struct workqueue_struct *wq)
if (WARN_ON(!wq_online))
return;

+ warn_if_flushing_global_workqueue(wq);
+
lock_map_acquire(&wq->lockdep_map);
lock_map_release(&wq->lockdep_map);

--
2.32.0