[PATCH RFC 1/3] workqueue: Maintain both max_active limits

[Breno Leitao]

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@xxxxxxxxxxxxxxx/
Signed-off-by: Breno Leitao <leitao@xxxxxxxxxx>
---
 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