mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH RFC 0/3] workqueue: Take the pwq backend from the attrs
@ 2026-09-18 14:25 Breno Leitao
  2026-09-18 14:25 ` [PATCH RFC 1/3] workqueue: Maintain both max_active limits Breno Leitao
                   ` (4 more replies)
  0 siblings, 5 replies; 7+ messages in thread
From: Breno Leitao @ 2026-09-18 14:25 UTC (permalink / raw)
  To: Tejun Heo, Lai Jiangshan
  Cc: linux-kernel, marco.crivellari, Breno Leitao, kernel-team

From the top of the mountain, Tejun said [1]:

  WQ_PERCPU would indicate that the wq must stay per-cpu for correctness,
  and we'd also want to allow concurrency management to workqueues which
  want to be percpu for performance reasons but can be switched into other
  affinity scopes for isolation, so it should move together with whether
  the backend needs concurrency management or not instead of WQ_PERCPU
  expressed at creation time.

This is an RFC patchset that tries to translate that into opinionated
code.

Design principles:

 * Keep wq->max_active and wq->percpu_max_active both current. The two
   backends meter work differently, and a pwq must not end up metered
   against a limit nobody set. (this is the semi-conflictual with my previous
   commit 27db9dd7f84f3a ("workqueue: Give percpu workqueues their own
   max_active")

 * Create a ->concurrency_managed field in the wq attributes, used to
   decide whether to do concurrency management or not.

 * Move WQ_PERCPU onto an UNBOUND workqueue with WQ_AFFN_CPU affinity and
   the newly created ->concurrency_managed. WQ_PERCPU is then only the
   promise that the workqueue stays on that backend.

This is not for acceptance, given this is drastic and it was not tested
enought, it is more to share what I have in mind and get comments from
the community.

Not done: the switching itself. concurrency_managed is fixed when the
workqueue is created and is not exported through sysfs, so nothing takes a
workqueue onto the backend or off it yet. WQ_BH is still its own path
rather than a third backend, the static per-cpu pools are still looked up
outside unbound_pool_hash, and callers still pass WQ_PERCPU rather than
asking for the attrs they want.

Link: https://lore.kernel.org/all/amESSqf0TMmzhFGz@slm.duckdns.org/ [1]

Signed-off-by: Breno Leitao <leitao@debian.org>
---
Breno Leitao (3):
      workqueue: Maintain both max_active limits
      workqueue: Add a concurrency_managed workqueue attribute
      workqueue: Back every workqueue with the unbound machinery

 include/linux/workqueue.h |  10 +++++
 kernel/workqueue.c        | 107 +++++++++++++++++++++++++++-------------------
 2 files changed, 72 insertions(+), 45 deletions(-)
---
base-commit: 9d4815c14f7faf789aeaa63515024168daf0390c
change-id: 20260914-wq_final-bcfbcd3b0f79

Best regards,
--  
Breno Leitao <leitao@debian.org>


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH RFC 1/3] workqueue: Maintain both max_active limits
  2026-09-18 14:25 [PATCH RFC 0/3] workqueue: Take the pwq backend from the attrs Breno Leitao
@ 2026-09-18 14:25 ` Breno Leitao
  2026-09-18 14:25 ` [PATCH RFC 2/3] workqueue: Add a concurrency_managed workqueue attribute Breno Leitao
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 7+ messages in thread
From: Breno Leitao @ 2026-09-18 14:25 UTC (permalink / raw)
  To: Tejun Heo, Lai Jiangshan
  Cc: linux-kernel, marco.crivellari, Breno Leitao, kernel-team

I've done commit 27db9dd7f84f3a ("workqueue: Give percpu workqueues
their own max_active"), but, later I found the solution was not that
simple.

I want to keep only one active, either percpu_max_active or max_active.
But there is no instant at which the backend flips,
apply_wqattrs_commit() swaps the slots one CPU at a time:

        for_each_possible_cpu(cpu)
                ctx->pwq_tbl[cpu] = install_pwq(ctx->wq, cpu,
                                                ctx->pwq_tbl[cpu]);

__queue_work() reads a slot under rcu_read_lock() plus pool->lock, it
never takes wq->mutex. So partway through that loop, CPU 0 already has
a concurrency-managed pwq while CPU 7 still has a pod-backed one, and
work can be queued to either.

So, let's keep both values the same, which is silly for now. Maybe we
should revert commit 27db9dd7f84f3a ("workqueue: Give percpu workqueues
their own max_active"). Not convinced yet.

Link: https://lore.kernel.org/all/amESSqf0TMmzhFGz@slm.duckdns.org/
Signed-off-by: Breno Leitao <leitao@debian.org>
---
 kernel/workqueue.c | 39 ++++++++++++++++++---------------------
 1 file changed, 18 insertions(+), 21 deletions(-)

diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index e618108c6127da..1c4f8bdd1cd509 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -6055,24 +6055,25 @@ static void wq_adjust_max_active(struct workqueue_struct *wq)
 	}
 
 	/*
-	 * Update the limit and then kick inactive work items if more active
+	 * Update both limits and then kick inactive work items if more active
 	 * work items are allowed. This doesn't break work item ordering
 	 * because new work items are always queued behind existing inactive
 	 * work items if there are any.
+	 *
+	 * Which one a pwq honours follows its pool, see pwq_tryinc_nr_active().
+	 * Keeping both current means a pwq is never metered against a limit
+	 * that was never set.
 	 */
-	if (wq->flags & WQ_UNBOUND) {
-		if (wq->max_active == new_max && wq->min_active == new_min)
-			return;
+	if (wq->max_active == new_max && wq->min_active == new_min &&
+	    wq->percpu_max_active == new_max)
+		return;
 
-		WRITE_ONCE(wq->max_active, new_max);
-		WRITE_ONCE(wq->min_active, new_min);
-		wq_update_node_max_active(wq, -1);
-	} else {
-		if (wq->percpu_max_active == new_max)
-			return;
+	WRITE_ONCE(wq->max_active, new_max);
+	WRITE_ONCE(wq->min_active, new_min);
+	WRITE_ONCE(wq->percpu_max_active, new_max);
 
-		WRITE_ONCE(wq->percpu_max_active, new_max);
-	}
+	if (wq->flags & WQ_UNBOUND)
+		wq_update_node_max_active(wq, -1);
 
 	if (new_max == 0)
 		return;
@@ -6169,14 +6170,11 @@ static struct workqueue_struct *__alloc_workqueue(const char *fmt,
 
 	/* init wq */
 	wq->flags = flags;
-	if (flags & WQ_UNBOUND) {
-		wq->max_active = max_active;
-		wq->min_active = min(max_active, WQ_DFL_MIN_ACTIVE);
-		wq->saved_min_active = wq->min_active;
-	} else {
-		wq->percpu_max_active = max_active;
-	}
+	wq->max_active = max_active;
+	wq->min_active = min(max_active, WQ_DFL_MIN_ACTIVE);
+	wq->percpu_max_active = max_active;
 	wq->saved_max_active = max_active;
+	wq->saved_min_active = wq->min_active;
 	mutex_init(&wq->mutex);
 	atomic_set(&wq->nr_pwqs_to_flush, 0);
 	INIT_LIST_HEAD(&wq->pwqs);
@@ -6452,8 +6450,7 @@ void workqueue_set_max_active(struct workqueue_struct *wq, int max_active)
 	mutex_lock(&wq->mutex);
 
 	wq->saved_max_active = max_active;
-	if (wq->flags & WQ_UNBOUND)
-		wq->saved_min_active = min(wq->saved_min_active, max_active);
+	wq->saved_min_active = min(wq->saved_min_active, max_active);
 
 	wq_adjust_max_active(wq);
 

-- 
2.53.0-Meta


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH RFC 2/3] workqueue: Add a concurrency_managed workqueue attribute
  2026-09-18 14:25 [PATCH RFC 0/3] workqueue: Take the pwq backend from the attrs Breno Leitao
  2026-09-18 14:25 ` [PATCH RFC 1/3] workqueue: Maintain both max_active limits Breno Leitao
@ 2026-09-18 14:25 ` Breno Leitao
  2026-09-18 15:25   ` Marco Crivellari
  2026-09-18 14:25 ` [PATCH RFC 3/3] workqueue: Back every workqueue with the unbound machinery Breno Leitao
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 7+ messages in thread
From: Breno Leitao @ 2026-09-18 14:25 UTC (permalink / raw)
  To: Tejun Heo, Lai Jiangshan
  Cc: linux-kernel, marco.crivellari, Breno Leitao, kernel-team

Tejun wanted to have a CM affinity (WQ_AFFN_CPU_CM), but I am proposing
to have an attribute field, that we can set with WQ_AFFN_CPU, depending
on whether we want to have CM or not.

Create workqueue_attrs->concurrency_managed, and use it to decide if we
do CM. It always comes with WQ_AFFN_CPU and affn_strict, since
alloc_wq_std_attrs() sets the three together for a WQ_PERCPU workqueue,
which is the only thing that sets it. It sits below the divider, so
wqattrs_clear_for_pool() clears it and it does not become part of the
pool hash.

This is not set by sysfs (for now?!), and apply_workqueue_attrs() refuses
attrs that carry it, so it is fixed when the workqueue is created, for
now.

So, a workqueue has 4 "attributes" with this patch:

  * cpumask
  	- which CPUs the workqueue may use at all
  * affn_scope
  	- cpu / smt / cache / cache_shard / numa / system
  * affn_strict
	- whether the pod boundary is a hard bind or a hint
  * concurrency_managed
  	- CM enabled or now.

Signed-off-by: Breno Leitao <leitao@debian.org>
---
 include/linux/workqueue.h | 10 ++++++++++
 kernel/workqueue.c        | 19 ++++++++++++++++---
 2 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/include/linux/workqueue.h b/include/linux/workqueue.h
index a283766a192aaf..a7fc9b0f72af2e 100644
--- a/include/linux/workqueue.h
+++ b/include/linux/workqueue.h
@@ -204,6 +204,16 @@ struct workqueue_attrs {
 	 */
 	enum wq_affn_scope affn_scope;
 
+	/**
+	 * @concurrency_managed: use the concurrency managed per-cpu pools
+	 *
+	 * Those keep at most one worker running per CPU and account max_active
+	 * per CPU rather than per node. Set from %WQ_PERCPU when the workqueue
+	 * is created and fixed for its lifetime, so it always comes with
+	 * %WQ_AFFN_CPU and @affn_strict.
+	 */
+	bool concurrency_managed;
+
 	/**
 	 * @ordered: work items must be executed one by one in queueing order
 	 */
diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index 1c4f8bdd1cd509..f86e8d0770ca22 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -5026,6 +5026,7 @@ static void copy_workqueue_attrs(struct workqueue_attrs *to,
 	 * get_unbound_pool() explicitly clears the fields.
 	 */
 	to->affn_scope = from->affn_scope;
+	to->concurrency_managed = from->concurrency_managed;
 	to->ordered = from->ordered;
 }
 
@@ -5036,6 +5037,7 @@ static void copy_workqueue_attrs(struct workqueue_attrs *to,
 static void wqattrs_clear_for_pool(struct workqueue_attrs *attrs)
 {
 	attrs->affn_scope = WQ_AFFN_NR_TYPES;
+	attrs->concurrency_managed = false;
 	attrs->ordered = false;
 	if (attrs->affn_strict)
 		cpumask_copy(attrs->cpumask, cpu_possible_mask);
@@ -5607,9 +5609,9 @@ static struct pool_workqueue *alloc_pwq(struct workqueue_struct *wq,
 
 	lockdep_assert_held(&wq_pool_mutex);
 
-	WARN_ON_ONCE((wq->flags & WQ_PERCPU) && cpu < 0);
+	WARN_ON_ONCE(attrs->concurrency_managed && cpu < 0);
 
-	if (cpu >= 0 && (wq->flags & WQ_PERCPU)) {
+	if (cpu >= 0 && attrs->concurrency_managed) {
 		pool = get_percpu_pool(wq, cpu);
 	} else {
 		pool = get_unbound_pool(attrs);
@@ -5737,7 +5739,7 @@ apply_wqattrs_prepare(struct workqueue_struct *wq,
 	copy_workqueue_attrs(new_attrs, attrs);
 	wqattrs_actualize_cpumask(new_attrs, unbound_cpumask);
 	cpumask_copy(new_attrs->__pod_cpumask, new_attrs->cpumask);
-	if (!(wq->flags & WQ_PERCPU)) {
+	if (!new_attrs->concurrency_managed) {
 		ctx->dfl_pwq = alloc_unbound_pwq(wq, new_attrs);
 		if (!ctx->dfl_pwq)
 			goto out_free;
@@ -5843,6 +5845,10 @@ int apply_workqueue_attrs(struct workqueue_struct *wq,
 	if (WARN_ON(!(wq->flags & WQ_UNBOUND)))
 		return -EINVAL;
 
+	/* concurrency management comes from WQ_PERCPU, it is not applied */
+	if (WARN_ON(attrs->concurrency_managed))
+		return -EINVAL;
+
 	mutex_lock(&wq_pool_mutex);
 	ret = apply_workqueue_attrs_locked(wq, attrs);
 	mutex_unlock(&wq_pool_mutex);
@@ -5934,6 +5940,13 @@ static struct workqueue_attrs *alloc_wq_std_attrs(struct workqueue_struct *wq)
 	if (wq->flags & __WQ_ORDERED)
 		attrs->ordered = true;
 
+	/* a percpu workqueue wants a concurrency managed pwq on every CPU */
+	if (wq->flags & WQ_PERCPU) {
+		attrs->affn_scope = WQ_AFFN_CPU;
+		attrs->affn_strict = true;
+		attrs->concurrency_managed = true;
+	}
+
 	return attrs;
 }
 

-- 
2.53.0-Meta


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH RFC 3/3] workqueue: Back every workqueue with the unbound machinery
  2026-09-18 14:25 [PATCH RFC 0/3] workqueue: Take the pwq backend from the attrs Breno Leitao
  2026-09-18 14:25 ` [PATCH RFC 1/3] workqueue: Maintain both max_active limits Breno Leitao
  2026-09-18 14:25 ` [PATCH RFC 2/3] workqueue: Add a concurrency_managed workqueue attribute Breno Leitao
@ 2026-09-18 14:25 ` Breno Leitao
  2026-09-18 15:23 ` [PATCH RFC 0/3] workqueue: Take the pwq backend from the attrs Marco Crivellari
  2026-09-19  2:13 ` Tejun Heo
  4 siblings, 0 replies; 7+ messages in thread
From: Breno Leitao @ 2026-09-18 14:25 UTC (permalink / raw)
  To: Tejun Heo, Lai Jiangshan
  Cc: linux-kernel, marco.crivellari, Breno Leitao, kernel-team

Now all workqueues will be WQ_UNBOUND, including WQ_PERCPU, and
the attributes will define the affinity and concurrency management.

It gives WQ_PERCPU a single meaning. It no longer selects per-cpu pools;
it only asserts "this workqueue is concurrency managed and may not leave
that backend."

This opens up space for a lot of optimizations down the line, but I want
to stop here to discuss if this is the right approach.

Signed-off-by: Breno Leitao <leitao@debian.org>
---
 kernel/workqueue.c | 51 +++++++++++++++++++++++++++++----------------------
 1 file changed, 29 insertions(+), 22 deletions(-)

diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index f86e8d0770ca22..da4d3e6cc5ee91 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -1669,7 +1669,7 @@ static bool is_percpu_pool(struct worker_pool *pool)
 static struct wq_node_nr_active *wq_node_nr_active(struct workqueue_struct *wq,
 						   int node)
 {
-	BUG_ON(!(wq->flags & WQ_UNBOUND));
+	BUG_ON(wq->flags & WQ_PERCPU);
 
 	if (node == NUMA_NO_NODE)
 		node = nr_node_ids;
@@ -2453,10 +2453,10 @@ static void __queue_work(int cpu, struct workqueue_struct *wq,
 retry:
 	/* pwq which will be used unless @work is executing elsewhere */
 	if (req_cpu == WORK_CPU_UNBOUND) {
-		if (wq->flags & WQ_UNBOUND)
-			cpu = wq_select_unbound_cpu(raw_smp_processor_id());
-		else
+		if (wq->flags & WQ_PERCPU)
 			cpu = raw_smp_processor_id();
+		else
+			cpu = wq_select_unbound_cpu(raw_smp_processor_id());
 	}
 
 	pwq = rcu_dereference(*per_cpu_ptr(wq->cpu_pwq, cpu));
@@ -2500,7 +2500,7 @@ static void __queue_work(int cpu, struct workqueue_struct *wq,
 	 * on it, so the retrying is guaranteed to make forward-progress.
 	 */
 	if (unlikely(!pwq->refcnt)) {
-		if (wq->flags & WQ_UNBOUND) {
+		if (!(wq->flags & WQ_PERCPU)) {
 			raw_spin_unlock(&pool->lock);
 			cpu_relax();
 			goto retry;
@@ -2669,7 +2669,7 @@ bool queue_work_node(int node, struct workqueue_struct *wq,
 	 * workqueue_select_cpu_near would need to be updated to allow for
 	 * some round robin type logic.
 	 */
-	WARN_ON_ONCE(!(wq->flags & WQ_UNBOUND));
+	WARN_ON_ONCE(wq->flags & WQ_PERCPU);
 
 	local_irq_save(irq_flags);
 
@@ -5297,7 +5297,7 @@ static void rcu_free_wq(struct rcu_head *rcu)
 	struct workqueue_struct *wq =
 		container_of(rcu, struct workqueue_struct, rcu);
 
-	if (wq->flags & WQ_UNBOUND)
+	if (!(wq->flags & WQ_PERCPU))
 		free_node_nr_active(wq->node_nr_active);
 
 	free_flush_pnodes(wq);
@@ -5799,7 +5799,7 @@ static void apply_wqattrs_commit(struct apply_wqattrs_ctx *ctx)
 		ctx->dfl_pwq = install_pwq(ctx->wq, -1, ctx->dfl_pwq);
 
 	/* update node_nr_active->max, which only unbound workqueues have */
-	if (ctx->wq->flags & WQ_UNBOUND)
+	if (!(ctx->wq->flags & WQ_PERCPU))
 		wq_update_node_max_active(ctx->wq, -1);
 
 	mutex_unlock(&ctx->wq->mutex);
@@ -5841,8 +5841,8 @@ int apply_workqueue_attrs(struct workqueue_struct *wq,
 {
 	int ret;
 
-	/* only unbound workqueues can change attributes */
-	if (WARN_ON(!(wq->flags & WQ_UNBOUND)))
+	/* a percpu workqueue is pinned to the concurrency managed backend */
+	if (WARN_ON(wq->flags & WQ_PERCPU))
 		return -EINVAL;
 
 	/* concurrency management comes from WQ_PERCPU, it is not applied */
@@ -5882,7 +5882,7 @@ static void unbound_wq_update_pwq(struct workqueue_struct *wq, int cpu)
 
 	lockdep_assert_held(&wq_pool_mutex);
 
-	if (!(wq->flags & WQ_UNBOUND) || wq->attrs->ordered)
+	if (wq->attrs->ordered || wq->attrs->concurrency_managed)
 		return;
 
 	/*
@@ -6085,7 +6085,7 @@ static void wq_adjust_max_active(struct workqueue_struct *wq)
 	WRITE_ONCE(wq->min_active, new_min);
 	WRITE_ONCE(wq->percpu_max_active, new_max);
 
-	if (wq->flags & WQ_UNBOUND)
+	if (!(wq->flags & WQ_PERCPU))
 		wq_update_node_max_active(wq, -1);
 
 	if (new_max == 0)
@@ -6170,6 +6170,13 @@ static struct workqueue_struct *__alloc_workqueue(const char *fmt,
 		flags &= ~WQ_PERCPU;
 	}
 
+	/*
+	 * Every workqueue is backed by the unbound machinery now. WQ_PERCPU no
+	 * longer picks a backend, it only says the workqueue is concurrency
+	 * managed and may not leave that backend.
+	 */
+	flags |= WQ_UNBOUND;
+
 	if (flags & WQ_BH) {
 		/*
 		 * BH workqueues always share a single execution context per CPU
@@ -6200,7 +6207,7 @@ static struct workqueue_struct *__alloc_workqueue(const char *fmt,
 	if (alloc_flush_pnodes(wq) < 0)
 		goto err_free_wq;
 
-	if (flags & WQ_UNBOUND) {
+	if (!(flags & WQ_PERCPU)) {
 		if (alloc_node_nr_active(wq->node_nr_active) < 0)
 			goto err_free_wq;
 	}
@@ -6239,7 +6246,7 @@ static struct workqueue_struct *__alloc_workqueue(const char *fmt,
 	 */
 	if (pwq_release_worker)
 		kthread_flush_worker(pwq_release_worker);
-	if (wq->flags & WQ_UNBOUND)
+	if (!(wq->flags & WQ_PERCPU))
 		free_node_nr_active(wq->node_nr_active);
 err_free_wq:
 	free_workqueue_attrs(wq->attrs);
@@ -6488,8 +6495,7 @@ EXPORT_SYMBOL_GPL(workqueue_set_max_active);
 void workqueue_set_min_active(struct workqueue_struct *wq, int min_active)
 {
 	/* min_active is only meaningful for non-ordered unbound workqueues */
-	if (WARN_ON((wq->flags & (WQ_BH | WQ_UNBOUND | __WQ_ORDERED)) !=
-		    WQ_UNBOUND))
+	if (WARN_ON(wq->flags & (WQ_BH | WQ_PERCPU | __WQ_ORDERED)))
 		return;
 
 	mutex_lock(&wq->mutex);
@@ -7185,7 +7191,7 @@ int workqueue_online_cpu(unsigned int cpu)
 	list_for_each_entry(wq, &workqueues, list) {
 		struct workqueue_attrs *attrs = wq->attrs;
 
-		if (wq->flags & WQ_UNBOUND) {
+		if (!(wq->flags & WQ_PERCPU)) {
 			const struct wq_pod_type *pt = wqattrs_pod_type(attrs);
 			int tcpu;
 
@@ -7220,7 +7226,7 @@ int workqueue_offline_cpu(unsigned int cpu)
 	list_for_each_entry(wq, &workqueues, list) {
 		struct workqueue_attrs *attrs = wq->attrs;
 
-		if (wq->flags & WQ_UNBOUND) {
+		if (!(wq->flags & WQ_PERCPU)) {
 			const struct wq_pod_type *pt = wqattrs_pod_type(attrs);
 			int tcpu;
 
@@ -7395,7 +7401,8 @@ static int workqueue_apply_unbound_cpumask(const cpumask_var_t unbound_cpumask)
 	lockdep_assert_held(&wq_pool_mutex);
 
 	list_for_each_entry(wq, &workqueues, list) {
-		if (!(wq->flags & WQ_UNBOUND) || (wq->flags & __WQ_DESTROYING))
+		if ((wq->flags & __WQ_DESTROYING) ||
+		    wq->attrs->concurrency_managed)
 			continue;
 
 		ctx = apply_wqattrs_prepare(wq, wq->attrs, unbound_cpumask);
@@ -7556,7 +7563,7 @@ static ssize_t per_cpu_show(struct device *dev, struct device_attribute *attr,
 {
 	struct workqueue_struct *wq = dev_to_wq(dev);
 
-	return scnprintf(buf, PAGE_SIZE, "%d\n", (bool)!(wq->flags & WQ_UNBOUND));
+	return scnprintf(buf, PAGE_SIZE, "%d\n", (bool)(wq->flags & WQ_PERCPU));
 }
 static DEVICE_ATTR_RO(per_cpu);
 
@@ -7932,7 +7939,7 @@ int workqueue_sysfs_register(struct workqueue_struct *wq)
 		return ret;
 	}
 
-	if (wq->flags & WQ_UNBOUND) {
+	if (!(wq->flags & WQ_PERCPU)) {
 		struct device_attribute *attr;
 
 		for (attr = wq_sysfs_unbound_attrs; attr->attr.name; attr++) {
@@ -8800,7 +8807,7 @@ void __init workqueue_init_topology(void)
 	list_for_each_entry(wq, &workqueues, list) {
 		for_each_online_cpu(cpu)
 			unbound_wq_update_pwq(wq, cpu);
-		if (wq->flags & WQ_UNBOUND) {
+		if (!(wq->flags & WQ_PERCPU)) {
 			mutex_lock(&wq->mutex);
 			wq_update_node_max_active(wq, -1);
 			mutex_unlock(&wq->mutex);

-- 
2.53.0-Meta


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH RFC 0/3] workqueue: Take the pwq backend from the attrs
  2026-09-18 14:25 [PATCH RFC 0/3] workqueue: Take the pwq backend from the attrs Breno Leitao
                   ` (2 preceding siblings ...)
  2026-09-18 14:25 ` [PATCH RFC 3/3] workqueue: Back every workqueue with the unbound machinery Breno Leitao
@ 2026-09-18 15:23 ` Marco Crivellari
  2026-09-19  2:13 ` Tejun Heo
  4 siblings, 0 replies; 7+ messages in thread
From: Marco Crivellari @ 2026-09-18 15:23 UTC (permalink / raw)
  To: Breno Leitao; +Cc: Tejun Heo, Lai Jiangshan, linux-kernel, kernel-team

Hello,

On Fri, Sep 18, 2026 at 4:25 PM Breno Leitao <leitao@debian.org> wrote:
> [...]
> Not done: the switching itself. concurrency_managed is fixed when the
> workqueue is created and is not exported through sysfs, so nothing takes a
> workqueue onto the backend or off it yet. WQ_BH is still its own path
> rather than a third backend, the static per-cpu pools are still looked up
> outside unbound_pool_hash, and callers still pass WQ_PERCPU rather than
> asking for the attrs they want.

I personally think it is better that way indeed, with WQ_PERCPU present.

I started an RFC about WQ_PREFER_PERCPU, still not published: the idea
would be an unbound workqueue that can be per-CPU (using affinities)
but without CM; so when CPUs are isolated, these WQ_PREFER_PERCPU WQs
can act as unbound. So the CM flag you added makes sense to me.

I tested my code a bit on the wq/for-7.4 branch, and it seems to be
working, but I will wait to see what happens with this series, before
sending mine.

Let's hear Tejun opinion about all of this; I'm also interested
regarding my series, so I can change if I'm not heading in the right
direction.

Thanks!
-- 

Marco Crivellari

SUSE Labs

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH RFC 2/3] workqueue: Add a concurrency_managed workqueue attribute
  2026-09-18 14:25 ` [PATCH RFC 2/3] workqueue: Add a concurrency_managed workqueue attribute Breno Leitao
@ 2026-09-18 15:25   ` Marco Crivellari
  0 siblings, 0 replies; 7+ messages in thread
From: Marco Crivellari @ 2026-09-18 15:25 UTC (permalink / raw)
  To: Breno Leitao; +Cc: Tejun Heo, Lai Jiangshan, linux-kernel, kernel-team

On Fri, Sep 18, 2026 at 4:25 PM Breno Leitao <leitao@debian.org> wrote:
> [...]
>   * concurrency_managed
>         - CM enabled or now.

Just noticed you typed "now" instead of "not". :-)

-- 

Marco Crivellari

SUSE Labs

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH RFC 0/3] workqueue: Take the pwq backend from the attrs
  2026-09-18 14:25 [PATCH RFC 0/3] workqueue: Take the pwq backend from the attrs Breno Leitao
                   ` (3 preceding siblings ...)
  2026-09-18 15:23 ` [PATCH RFC 0/3] workqueue: Take the pwq backend from the attrs Marco Crivellari
@ 2026-09-19  2:13 ` Tejun Heo
  4 siblings, 0 replies; 7+ messages in thread
From: Tejun Heo @ 2026-09-19  2:13 UTC (permalink / raw)
  To: Breno Leitao
  Cc: Lai Jiangshan, Marco Crivellari, linux-kernel, kernel-team, Tejun Heo

Hello, Breno.

On Fri, Sep 18, 2026 at 07:25:32AM -0700, Breno Leitao wrote:
>  * Keep wq->max_active and wq->percpu_max_active both current. The two
>    backends meter work differently, and a pwq must not end up metered
>    against a limit nobody set. (this is the semi-conflictual with my previous
>    commit 27db9dd7f84f3a ("workqueue: Give percpu workqueues their own
>    max_active")

Both should stay current but I don't think they should be the same number.
max_active means different things in the two domains, per-CPU on one side and
across the whole workqueue on the other, so let's keep the two sets of values
separate and link them through scaling. On creation, the argument sets the
values for the domain the workqueue starts in and the other domain is derived
from it. Afterwards, each domain has its own interface, kernel and sysfs, and
adjusting one updates the other accordingly. That way a switch always lands
on a sensible limit without anyone having to think about it.

>  * Create a ->concurrency_managed field in the wq attributes, used to
>    decide whether to do concurrency management or not.
>
>  * Move WQ_PERCPU onto an UNBOUND workqueue with WQ_AFFN_CPU affinity and
>    the newly created ->concurrency_managed. WQ_PERCPU is then only the
>    promise that the workqueue stays on that backend.

I'd rather not build percpu on top of CPU scope. Unbound with strict CPU
scope and percpu are different things. The pools, the metering and how the
unbound cpumask applies all differ, and both should keep existing. So, how
about making PERCPU its own scope? Whether concurrency management is then
expressed as a flag or an attribute doesn't matter much as long as it can be
turned on and off. WQ_PERCPU would mean that the workqueue can't leave the
PERCPU scope while CM can still be toggled.

BH can be a scope value too for consistency. It's only selectable on creation
and can't be switched into or out of, but having it in the same enum keeps
things uniform.

With scope carrying the backend, nice can apply verbatim on unbound and snap
to normal or highpri when the workqueue is on percpu, picked from the current
value. No need to restrict what can be written.

> Not done: the switching itself. concurrency_managed is fixed when the
> workqueue is created and is not exported through sysfs, so nothing takes a
> workqueue onto the backend or off it yet.

For PREFER_PERCPU type workqueues, which benefit from cmwq but don't depend
on it for correctness, there's no reason to block switching in either
direction. Having them follow the unbound cpumask when picking the queueing
CPU even while on percpu, the way the last patch keys that on WQ_PERCPU
rather than on the backend, makes sense for them.

Thanks.

--
tejun

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-09-19  2:13 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-18 14:25 [PATCH RFC 0/3] workqueue: Take the pwq backend from the attrs Breno Leitao
2026-09-18 14:25 ` [PATCH RFC 1/3] workqueue: Maintain both max_active limits Breno Leitao
2026-09-18 14:25 ` [PATCH RFC 2/3] workqueue: Add a concurrency_managed workqueue attribute Breno Leitao
2026-09-18 15:25   ` Marco Crivellari
2026-09-18 14:25 ` [PATCH RFC 3/3] workqueue: Back every workqueue with the unbound machinery Breno Leitao
2026-09-18 15:23 ` [PATCH RFC 0/3] workqueue: Take the pwq backend from the attrs Marco Crivellari
2026-09-19  2:13 ` Tejun Heo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®