From dfca46365afc030fb09bb40226514c500202dcdc Mon Sep 17 00:00:00 2001 From: Breno Leitao Date: Mon, 11 May 2026 05:14:18 -0700 Subject: workqueue: drop apply_wqattrs_lock()/unlock() wrappers The apply_wqattrs_lock()/unlock() helpers were introduced by commit a0111cf6710b ("workqueue: separate out and refactor the locking of applying attrs") to encapsulate the get_online_cpus() (later cpus_read_lock()) + mutex_lock(&wq_pool_mutex) acquire pair that was duplicated across the apply-attrs paths. Since commit 19af45757383 ("workqueue: Remove cpus_read_lock() from apply_wqattrs_lock()") removed the cpus_read_lock() (pwq creation and installation now operate on wq_online_cpumask, so CPU hotplug no longer needs to be excluded), the wrappers have been one-line forwarders to mutex_lock(&wq_pool_mutex)/mutex_unlock(&wq_pool_mutex). They no longer encode any non-trivial locking rule and obscure the fact that callers just take the existing wq_pool_mutex. This align with the "unnecessary" helpers that got discussed in [1] Inline the eight call sites and remove the wrappers. No functional change. Link: https://lore.kernel.org/all/afs_44-6ToJJVZTn@gmail.com/ [1] Signed-off-by: Breno Leitao Signed-off-by: Tejun Heo --- kernel/workqueue.c | 38 ++++++++++++++------------------------ 1 file changed, 14 insertions(+), 24 deletions(-) (limited to 'kernel/workqueue.c') diff --git a/kernel/workqueue.c b/kernel/workqueue.c index 3d2e3b2ec528..9adee917e2bb 100644 --- a/kernel/workqueue.c +++ b/kernel/workqueue.c @@ -5300,16 +5300,6 @@ static struct pool_workqueue *alloc_unbound_pwq(struct workqueue_struct *wq, return pwq; } -static void apply_wqattrs_lock(void) -{ - mutex_lock(&wq_pool_mutex); -} - -static void apply_wqattrs_unlock(void) -{ - mutex_unlock(&wq_pool_mutex); -} - /** * wq_calc_pod_cpumask - calculate a wq_attrs' cpumask for a pod * @attrs: the wq_attrs of the default pwq of the target workqueue @@ -5863,7 +5853,7 @@ static struct workqueue_struct *__alloc_workqueue(const char *fmt, * wq_pool_mutex protects the workqueues list, allocations of PWQs, * and the global freeze state. */ - apply_wqattrs_lock(); + mutex_lock(&wq_pool_mutex); if (alloc_and_link_pwqs(wq) < 0) goto err_unlock_free_node_nr_active; @@ -5877,7 +5867,7 @@ static struct workqueue_struct *__alloc_workqueue(const char *fmt, if (wq_online && init_rescuer(wq) < 0) goto err_unlock_destroy; - apply_wqattrs_unlock(); + mutex_unlock(&wq_pool_mutex); if ((wq->flags & WQ_SYSFS) && workqueue_sysfs_register(wq)) goto err_destroy; @@ -5885,7 +5875,7 @@ static struct workqueue_struct *__alloc_workqueue(const char *fmt, return wq; err_unlock_free_node_nr_active: - apply_wqattrs_unlock(); + mutex_unlock(&wq_pool_mutex); /* * Failed alloc_and_link_pwqs() may leave pending pwq->release_work, * flushing the pwq_release_worker ensures that the pwq_release_workfn() @@ -5900,7 +5890,7 @@ err_free_wq: kfree(wq); return NULL; err_unlock_destroy: - apply_wqattrs_unlock(); + mutex_unlock(&wq_pool_mutex); err_destroy: destroy_workqueue(wq); return NULL; @@ -7301,7 +7291,7 @@ static ssize_t wq_nice_store(struct device *dev, struct device_attribute *attr, struct workqueue_attrs *attrs; int ret = -ENOMEM; - apply_wqattrs_lock(); + mutex_lock(&wq_pool_mutex); attrs = wq_sysfs_prep_attrs(wq); if (!attrs) @@ -7314,7 +7304,7 @@ static ssize_t wq_nice_store(struct device *dev, struct device_attribute *attr, ret = -EINVAL; out_unlock: - apply_wqattrs_unlock(); + mutex_unlock(&wq_pool_mutex); free_workqueue_attrs(attrs); return ret ?: count; } @@ -7340,7 +7330,7 @@ static ssize_t wq_cpumask_store(struct device *dev, struct workqueue_attrs *attrs; int ret = -ENOMEM; - apply_wqattrs_lock(); + mutex_lock(&wq_pool_mutex); attrs = wq_sysfs_prep_attrs(wq); if (!attrs) @@ -7351,7 +7341,7 @@ static ssize_t wq_cpumask_store(struct device *dev, ret = apply_workqueue_attrs_locked(wq, attrs); out_unlock: - apply_wqattrs_unlock(); + mutex_unlock(&wq_pool_mutex); free_workqueue_attrs(attrs); return ret ?: count; } @@ -7387,13 +7377,13 @@ static ssize_t wq_affn_scope_store(struct device *dev, if (affn < 0) return affn; - apply_wqattrs_lock(); + mutex_lock(&wq_pool_mutex); attrs = wq_sysfs_prep_attrs(wq); if (attrs) { attrs->affn_scope = affn; ret = apply_workqueue_attrs_locked(wq, attrs); } - apply_wqattrs_unlock(); + mutex_unlock(&wq_pool_mutex); free_workqueue_attrs(attrs); return ret ?: count; } @@ -7418,13 +7408,13 @@ static ssize_t wq_affinity_strict_store(struct device *dev, if (sscanf(buf, "%d", &v) != 1) return -EINVAL; - apply_wqattrs_lock(); + mutex_lock(&wq_pool_mutex); attrs = wq_sysfs_prep_attrs(wq); if (attrs) { attrs->affn_strict = (bool)v; ret = apply_workqueue_attrs_locked(wq, attrs); } - apply_wqattrs_unlock(); + mutex_unlock(&wq_pool_mutex); free_workqueue_attrs(attrs); return ret ?: count; } @@ -7465,12 +7455,12 @@ static int workqueue_set_unbound_cpumask(cpumask_var_t cpumask) cpumask_and(cpumask, cpumask, cpu_possible_mask); if (!cpumask_empty(cpumask)) { ret = 0; - apply_wqattrs_lock(); + mutex_lock(&wq_pool_mutex); if (!cpumask_equal(cpumask, wq_unbound_cpumask)) ret = workqueue_apply_unbound_cpumask(cpumask); if (!ret) cpumask_copy(wq_requested_unbound_cpumask, cpumask); - apply_wqattrs_unlock(); + mutex_unlock(&wq_pool_mutex); } return ret; -- cgit v1.2.3 From 611583a76ea97991b0f65ec1ff099eac7fe0bae4 Mon Sep 17 00:00:00 2001 From: Breno Leitao Date: Sun, 24 May 2026 08:19:56 -0700 Subject: workqueue: drop spurious '*' from print_worker_info() fn declaration print_worker_info() declares its local 'fn' as work_func_t * but worker->current_func has type work_func_t (a function pointer). The extra level of indirection is wrong and only happens to be harmless today because every supported Linux architecture has sizeof(work_func_t) == sizeof(work_func_t *): copy_from_kernel_nofault() reads the correct number of bytes by accident, and %ps still resolves the printed address because the stored value is the function address regardless of declared type. On any future ABI where sizeof(void (*)()) differs from sizeof(void *), the nofault copy would transfer the wrong number of bytes and the subsequent %ps would print an incorrect address. Match the field type so the intent is explicit and the code does not silently rely on equal pointer sizes. Fixes: 3d1cb2059d93 ("workqueue: include workqueue info when printing debug dump of a worker task") Signed-off-by: Breno Leitao Signed-off-by: Tejun Heo --- kernel/workqueue.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'kernel/workqueue.c') diff --git a/kernel/workqueue.c b/kernel/workqueue.c index 9adee917e2bb..35b0c8f4f10f 100644 --- a/kernel/workqueue.c +++ b/kernel/workqueue.c @@ -6286,7 +6286,7 @@ EXPORT_SYMBOL_GPL(set_worker_desc); */ void print_worker_info(const char *log_lvl, struct task_struct *task) { - work_func_t *fn = NULL; + work_func_t fn = NULL; char name[WQ_NAME_LEN] = { }; char desc[WORKER_DESC_LEN] = { }; struct pool_workqueue *pwq = NULL; -- cgit v1.2.3 From 64d8eae3f89583821f599c0aa398dd2e69b31a89 Mon Sep 17 00:00:00 2001 From: Marco Crivellari Date: Fri, 29 May 2026 15:06:39 +0200 Subject: workqueue: Add warnings and fallback if system_{unbound}_wq is used Currently many users transitioned already to the new introduced workqueue (system_percpu_wq, system_dfl_wq), but there are new users who still use the older system_wq and system_unbound_wq. This change try to push this transition forward, by warning whether the old workqueues are used. Link: https://lore.kernel.org/all/20250221112003.1dSuoGyc@linutronix.de/ Suggested-by: Tejun Heo Signed-off-by: Marco Crivellari Signed-off-by: Tejun Heo --- kernel/workqueue.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) (limited to 'kernel/workqueue.c') diff --git a/kernel/workqueue.c b/kernel/workqueue.c index 35b0c8f4f10f..088cd9b70fca 100644 --- a/kernel/workqueue.c +++ b/kernel/workqueue.c @@ -2280,6 +2280,14 @@ static void __queue_work(int cpu, struct workqueue_struct *wq, unsigned int work_flags; unsigned int req_cpu = cpu; + /* + * NOTE: Check whether the used workqueue is deprecated and warn + */ + if (unlikely(wq->flags & __WQ_DEPRECATED)) + pr_warn_once("workqueue: work func %ps enqueued on deprecated workqueue. " + "Use system_{percpu|dfl}_wq instead.\n", + work->func); + /* * While a work item is PENDING && off queue, a task trying to * steal the PENDING will busy-loop waiting for it to either get @@ -8013,12 +8021,12 @@ void __init workqueue_init_early(void) ordered_wq_attrs[i] = attrs; } - system_wq = alloc_workqueue("events", WQ_PERCPU, 0); + system_wq = alloc_workqueue("events", WQ_PERCPU | __WQ_DEPRECATED, 0); system_percpu_wq = alloc_workqueue("events", WQ_PERCPU, 0); system_highpri_wq = alloc_workqueue("events_highpri", WQ_HIGHPRI | WQ_PERCPU, 0); system_long_wq = alloc_workqueue("events_long", WQ_PERCPU, 0); - system_unbound_wq = alloc_workqueue("events_unbound", WQ_UNBOUND, WQ_MAX_ACTIVE); + system_unbound_wq = alloc_workqueue("events_unbound", WQ_UNBOUND | __WQ_DEPRECATED, WQ_MAX_ACTIVE); system_dfl_wq = alloc_workqueue("events_unbound", WQ_UNBOUND, WQ_MAX_ACTIVE); system_freezable_wq = alloc_workqueue("events_freezable", WQ_FREEZABLE | WQ_PERCPU, 0); -- cgit v1.2.3 From 21c05ca88a548ca1353cbef189c97d4f03b90692 Mon Sep 17 00:00:00 2001 From: Marco Crivellari Date: Fri, 29 May 2026 15:06:40 +0200 Subject: workqueue: Add warnings and ensure one among WQ_PERCPU or WQ_UNBOUND is present Currently there are no checks in order to enforce the use of one between WQ_PERCPU or WQ_UNBOUND. So act as following: - if neither of them is present, set WQ_PERCPU - if both are present, remove WQ_PERCPU Along with this change, WARN_ONCE(), so that the code still uses both or neither of them, can be changed. Link: https://lore.kernel.org/all/20250221112003.1dSuoGyc@linutronix.de/ Suggested-by: Tejun Heo Signed-off-by: Marco Crivellari Signed-off-by: Tejun Heo --- kernel/workqueue.c | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) (limited to 'kernel/workqueue.c') diff --git a/kernel/workqueue.c b/kernel/workqueue.c index 088cd9b70fca..34c7c61c69aa 100644 --- a/kernel/workqueue.c +++ b/kernel/workqueue.c @@ -5802,7 +5802,7 @@ static struct workqueue_struct *__alloc_workqueue(const char *fmt, /* see the comment above the definition of WQ_POWER_EFFICIENT */ if ((flags & WQ_POWER_EFFICIENT) && wq_power_efficient) - flags |= WQ_UNBOUND; + flags = (flags & ~WQ_PERCPU) | WQ_UNBOUND; /* allocate wq and format name */ if (flags & WQ_UNBOUND) @@ -5826,6 +5826,23 @@ static struct workqueue_struct *__alloc_workqueue(const char *fmt, pr_warn_once("workqueue: name exceeds WQ_NAME_LEN. Truncating to: %s\n", wq->name); + /* + * One among WQ_PERCPU and WQ_UNBOUND must be set, but not both. + * - If neither is set, default to WQ_PERCPU + * - If both are set, default to WQ_UNBOUND + * + * This code can be removed after workqueue are unbound by default + */ + if (unlikely(!(flags & (WQ_UNBOUND | WQ_PERCPU)))) { + WARN_ONCE(1, "workqueue: %s is using neither WQ_PERCPU or WQ_UNBOUND. " + "Setting WQ_PERCPU.\n", wq->name); + flags |= WQ_PERCPU; + } else if (unlikely((flags & WQ_PERCPU) && (flags & WQ_UNBOUND))) { + WARN_ONCE(1, "workqueue: %s uses both WQ_PERCPU and WQ_UNBOUND. " + "Dropped WQ_PERCPU, keeping WQ_UNBOUND.\n", wq->name); + flags &= ~WQ_PERCPU; + } + if (flags & WQ_BH) { /* * BH workqueues always share a single execution context per CPU -- cgit v1.2.3