Re: [PATCH] wfx: avoid flush_workqueue(system_highpri_wq) usage

Tetsuo Handa
2022-05-01T10:23:38Z

On 2022/05/01 17:53, Kalle Valo wrote:
> So now the thread is created every time the module loaded, even if
> there's no device available.

Excuse me, but what thread? alloc_workqueue() without WQ_MEM_RECLAIM flag
does not create a thread, and therefore consumes little resource where
there's no device available does not matter.

> Also I'm not really a fan of global
> variables (wfx_wq). I would rather create a workqueue per device in
> wfx_probe() or use the workqueue provided by mac80211.

Whatever is fine. wfx is the last user of flush_workqueue(system_*_wq) and
I want to know whether I can include system_highpri_wq into below patch.

Subject: [PATCH] workqueue: wrap flush_workqueue() using a macro

While a conversion to stop flushing of kernel-global workqueues is in
progress, we can warn users who are not aware of this conversion, by
wrapping flush_workqueue() as a macro and injecting BUILD_BUG_ON() tests.

---
include/linux/workqueue.h | 27 +++++++++++++++++++++++++++
kernel/workqueue.c | 1 +
2 files changed, 28 insertions(+)

diff --git a/include/linux/workqueue.h b/include/linux/workqueue.h
index 7b13fae377e2..3eaf19262d19 100644
--- a/include/linux/workqueue.h
+++ b/include/linux/workqueue.h
@@ -654,4 +654,31 @@ int workqueue_offline_cpu(unsigned int cpu);
void __init workqueue_init_early(void);
void __init workqueue_init(void);

+/*
+ * Detect attempt to flush system-wide workqueues at compile time when possible.
+ * See https://lkml.kernel.org/r/49925af7-78a8-a3dd-bce6-cfc02e1a9236@I-love.SAKURA.ne.jp for
+ * reasons and steps for converting system-wide workqueues into local workqueues.
+ * Checks for system_wq and system_highpri_wq will be added after
+ * all in-tree users stopped flushing these workqueues.
+ */
+#define flush_workqueue(wq) \
+({ \
+ BUILD_BUG_ON_MSG(__builtin_constant_p(&(wq) == &system_long_wq) && \
+ &(wq) == &system_long_wq, \
+ "Please avoid flushing system_long_wq."); \
+ BUILD_BUG_ON_MSG(__builtin_constant_p(&(wq) == &system_unbound_wq) && \
+ &(wq) == &system_unbound_wq, \
+ "Please avoid flushing system_unbound_wq."); \
+ BUILD_BUG_ON_MSG(__builtin_constant_p(&(wq) == &system_freezable_wq) && \
+ &(wq) == &system_freezable_wq, \
+ "Please avoid flushing system_freezable_wq."); \
+ BUILD_BUG_ON_MSG(__builtin_constant_p(&(wq) == &system_power_efficient_wq) && \
+ &(wq) == &system_power_efficient_wq, \
+ "Please avoid flushing system_power_efficient_wq."); \
+ BUILD_BUG_ON_MSG(__builtin_constant_p(&(wq) == &system_freezable_power_efficient_wq) && \
+ &(wq) == &system_freezable_power_efficient_wq, \
+ "Please avoid flushing system_freezable_power_efficient_wq."); \
+ flush_workqueue(wq); \
+})
+
#endif
diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index b447012df177..de53813a92d2 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -2813,6 +2813,7 @@ static void warn_flush_attempt(struct workqueue_struct *wq)
* This function sleeps until all work items which were queued on entry
* have finished execution, but it is not livelocked by new incoming ones.
*/
+#undef flush_workqueue
void flush_workqueue(struct workqueue_struct *wq)
{
struct wq_flusher this_flusher = {
--
2.34.1