[PATCH wq/for-7.4 0/2] workqueue: give percpu its own max_active and derive attrs from flags

[PATCH wq/for-7.4 0/2] workqueue: give percpu its own max_active and derive attrs from flags

[Breno Leitao]

Phase two of unifying the percpu and unbound workqueue paths, following
the discussion at [1].

wq->max_active means one thing on a concurrency managed per-cpu pool and
another on an unbound one. Patch 1 gives the per-cpu case its own field,
as Tejun suggested.

Patch 2 builds the standard attrs from the workqueue flags instead of
picking one of three static arrays, which leaves alloc_and_link_pwqs()
with a single apply_workqueue_attrs_locked() call for every workqueue

No visible change intented for our workqueue users.

[1] https://lore.kernel.org/all/20260714-tejun1-v1-0-024d59241386@debian.org/

Signed-off-by: Breno Leitao <leitao@debian.org>
---
Breno Leitao (2):
      workqueue: give percpu workqueues their own max_active
      workqueue: derive the standard attrs from the workqueue flags

 kernel/workqueue.c | 120 ++++++++++++++++++++++++++---------------------------
 1 file changed, 59 insertions(+), 61 deletions(-)
---
base-commit: f1639cde5f5c1bee25498ef988227ee8cf5ddb04
change-id: 20260910-realsplit_last_v2-3a73d7986db4

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

[PATCH wq/for-7.4 1/2] workqueue: give percpu workqueues their own max_active

[Breno Leitao]

wq->max_active means two different things, depending on how the workqueue
is backed. On a concurrency managed per-cpu pool it caps pwq->nr_active,
so it is a per-cpu limit. On an unbound pool it feeds
wq_node_nr_active() and is shared by the whole workqueue.

Tejun once said [1]:

	It kinda sucks that max_active's meaning is different across the
	boundary tho. Maybe percpu max_active should be separate into
	its own field, idk.

Add wq->percpu_max_active for the first meaning and leave max_active as
the system wide one. wq_adjust_max_active() updates whichever of the two
applies, and min_active stays with max_active as only the unbound side
uses it.

Suggested-by: Tejun Heo <tj@kernel.org>
Link: https://lore.kernel.org/all/amESSqf0TMmzhFGz@slm.duckdns.org/ [1]
Signed-off-by: Breno Leitao <leitao@debian.org>
---
 kernel/workqueue.c | 44 +++++++++++++++++++++++++++-----------------
 1 file changed, 27 insertions(+), 17 deletions(-)

diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index f868011e291f4b..8232a60e107cf7 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -389,6 +389,7 @@ struct workqueue_struct {
 
 	/* See alloc_workqueue() function comment for info on min/max_active */
 	int			max_active;	/* WO: max active works */
+	int			percpu_max_active; /* WO: max active works per cpu */
 	int			min_active;	/* WO: min active works */
 	int			saved_max_active; /* WQ: saved max_active */
 	int			saved_min_active; /* WQ: saved min_active */
@@ -1855,10 +1856,10 @@ static bool pwq_tryinc_nr_active(struct pool_workqueue *pwq, bool fill)
 
 	/*
 	 * A concurrency-managed per-cpu pool accounts nr_active per pwq, so
-	 * pwq->nr_active against wq->max_active is sufficient.
+	 * pwq->nr_active against wq->percpu_max_active is sufficient.
 	 */
 	if (is_percpu_pool(pool)) {
-		obtained = pwq->nr_active < READ_ONCE(wq->max_active);
+		obtained = pwq->nr_active < READ_ONCE(wq->percpu_max_active);
 		goto out;
 	}
 
@@ -6016,9 +6017,9 @@ static int init_rescuer(struct workqueue_struct *wq)
  * wq_adjust_max_active - update a wq's max_active to the current setting
  * @wq: target workqueue
  *
- * If @wq isn't freezing, set @wq->max_active to the saved_max_active and
- * activate inactive work items accordingly. If @wq is freezing, clear
- * @wq->max_active to zero.
+ * If @wq isn't freezing, set the limit that applies to @wq's backing to the
+ * saved_max_active and activate inactive work items accordingly. If @wq is
+ * freezing, clear it to zero.
  */
 static void wq_adjust_max_active(struct workqueue_struct *wq)
 {
@@ -6035,20 +6036,25 @@ static void wq_adjust_max_active(struct workqueue_struct *wq)
 		new_min = wq->saved_min_active;
 	}
 
-	if (wq->max_active == new_max && wq->min_active == new_min)
-		return;
-
 	/*
-	 * Update @wq->max/min_active and then kick inactive work items if more
-	 * active work items are allowed. This doesn't break work item ordering
+	 * Update the limit 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.
 	 */
-	WRITE_ONCE(wq->max_active, new_max);
-	WRITE_ONCE(wq->min_active, new_min);
+	if (wq->flags & WQ_UNBOUND) {
+		if (wq->max_active == new_max && wq->min_active == new_min)
+			return;
 
-	if (wq->flags & WQ_UNBOUND)
+		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->percpu_max_active, new_max);
+	}
 
 	if (new_max == 0)
 		return;
@@ -6145,10 +6151,14 @@ static struct workqueue_struct *__alloc_workqueue(const char *fmt,
 
 	/* init wq */
 	wq->flags = flags;
-	wq->max_active = max_active;
-	wq->min_active = min(max_active, WQ_DFL_MIN_ACTIVE);
-	wq->saved_max_active = wq->max_active;
-	wq->saved_min_active = wq->min_active;
+	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->saved_max_active = max_active;
 	mutex_init(&wq->mutex);
 	atomic_set(&wq->nr_pwqs_to_flush, 0);
 	INIT_LIST_HEAD(&wq->pwqs);

-- 
2.53.0-Meta

[PATCH wq/for-7.4 2/2] workqueue: derive the standard attrs from the workqueue flags

[Breno Leitao]

alloc_and_link_pwqs() picks one of three static attrs arrays indexed by
WQ_HIGHPRI, and branches to a separate apply_workqueue_attrs_locked()
call for percpu, ordered and unbound workqueues. The arrays differ only
in the nice level and the ordered bit, which the flags already say.

Build the attrs from the flags instead. alloc_wq_std_attrs() sets nice
for WQ_HIGHPRI and ordered for __WQ_ORDERED, which collapses the three
branches into one call and drops the three arrays along with their
workqueue_init_early() loop.

The ordering WARN now sits behind its own __WQ_ORDERED test rather than
the unbound branch. __WQ_ORDERED is internal and every macro that sets it
pairs it with WQ_UNBOUND, so the WARN covers the same workqueues as
before.

The attrs are allocated per creation now. apply_wqattrs_prepare() already
allocates one on the same path, including in workqueue_init_early(), so
that is not a new constraint, and it only copies what it is given, so the
caller can free them once apply_workqueue_attrs_locked() returns.

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

diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index 8232a60e107cf7..39187f424c34e7 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -533,15 +533,6 @@ static DEFINE_IDR(worker_pool_idr);	/* PR: idr of all pools */
 /* PL: hash of all unbound pools keyed by pool->attrs */
 static DEFINE_HASHTABLE(unbound_pool_hash, UNBOUND_POOL_HASH_ORDER);
 
-/* I: attributes used when instantiating standard unbound pools on demand */
-static struct workqueue_attrs *unbound_std_wq_attrs[NR_STD_WORKER_POOLS];
-
-/* I: attributes used when instantiating ordered pools on demand */
-static struct workqueue_attrs *ordered_wq_attrs[NR_STD_WORKER_POOLS];
-
-/* I: attributes of percpu workqueues, which are backed by the static pools */
-static struct workqueue_attrs *percpu_std_wq_attrs[NR_STD_WORKER_POOLS];
-
 /*
  * I: kthread_worker to release pwq's. pwq release needs to be bounced to a
  * process context while holding a pool lock. Bounce to a dedicated kthread
@@ -5920,9 +5911,27 @@ static void unbound_wq_update_pwq(struct workqueue_struct *wq, int cpu)
 	put_pwq_unlocked(old_pwq);
 }
 
+/* the attrs @wq asked for, as spelled by its flags */
+static struct workqueue_attrs *alloc_wq_std_attrs(struct workqueue_struct *wq)
+{
+	struct workqueue_attrs *attrs;
+
+	attrs = alloc_workqueue_attrs();
+	if (!attrs)
+		return NULL;
+
+	if (wq->flags & WQ_HIGHPRI)
+		attrs->nice = HIGHPRI_NICE_LEVEL;
+
+	if (wq->flags & __WQ_ORDERED)
+		attrs->ordered = true;
+
+	return attrs;
+}
+
 static int alloc_and_link_pwqs(struct workqueue_struct *wq)
 {
-	bool highpri = wq->flags & WQ_HIGHPRI;
+	struct workqueue_attrs *attrs;
 	int ret;
 
 	lockdep_assert_held(&wq_pool_mutex);
@@ -5931,23 +5940,24 @@ static int alloc_and_link_pwqs(struct workqueue_struct *wq)
 	if (!wq->cpu_pwq)
 		goto enomem;
 
-	if (!(wq->flags & WQ_UNBOUND)) {
-		ret = apply_workqueue_attrs_locked(wq, percpu_std_wq_attrs[highpri]);
-	} else if (wq->flags & __WQ_ORDERED) {
-		struct pool_workqueue *dfl_pwq;
+	attrs = alloc_wq_std_attrs(wq);
+	if (!attrs)
+		goto enomem;
+
+	ret = apply_workqueue_attrs_locked(wq, attrs);
+	free_workqueue_attrs(attrs);
+	if (ret)
+		goto enomem;
+
+	if (wq->flags & __WQ_ORDERED) {
+		struct pool_workqueue *dfl_pwq = rcu_access_pointer(wq->dfl_pwq);
 
-		ret = apply_workqueue_attrs_locked(wq, ordered_wq_attrs[highpri]);
 		/* there should only be single pwq for ordering guarantee */
-		dfl_pwq = rcu_access_pointer(wq->dfl_pwq);
-		WARN(!ret && (wq->pwqs.next != &dfl_pwq->pwqs_node ||
-			      wq->pwqs.prev != &dfl_pwq->pwqs_node),
+		WARN(wq->pwqs.next != &dfl_pwq->pwqs_node ||
+		     wq->pwqs.prev != &dfl_pwq->pwqs_node,
 		     "ordering guarantee broken for workqueue %s\n", wq->name);
-	} else {
-		ret = apply_workqueue_attrs_locked(wq, unbound_std_wq_attrs[highpri]);
 	}
 
-	if (ret)
-		goto enomem;
 	return 0;
 
 enomem:
@@ -8378,28 +8388,6 @@ void __init workqueue_init_early(void)
 			init_cpu_worker_pool(pool, cpu, std_nice[i++]);
 	}
 
-	/* create default unbound, ordered and percpu wq attrs */
-	for (i = 0; i < NR_STD_WORKER_POOLS; i++) {
-		struct workqueue_attrs *attrs;
-
-		BUG_ON(!(attrs = alloc_workqueue_attrs()));
-		attrs->nice = std_nice[i];
-		unbound_std_wq_attrs[i] = attrs;
-
-		/*
-		 * An ordered wq should have only one pwq as ordering is
-		 * guaranteed by max_active which is enforced by pwqs.
-		 */
-		BUG_ON(!(attrs = alloc_workqueue_attrs()));
-		attrs->nice = std_nice[i];
-		attrs->ordered = true;
-		ordered_wq_attrs[i] = attrs;
-
-		BUG_ON(!(attrs = alloc_workqueue_attrs()));
-		attrs->nice = std_nice[i];
-		percpu_std_wq_attrs[i] = attrs;
-	}
-
 	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",

-- 
2.53.0-Meta

Re: [PATCH wq/for-7.4 0/2] workqueue: give percpu its own max_active and derive attrs from flags

[Tejun Heo]

> Breno Leitao (2):
>       workqueue: give percpu workqueues their own max_active
>       workqueue: derive the standard attrs from the workqueue flags

Applied 1-2 to wq/for-7.4.

Thanks.
--
tejun