From: Lai Jiangshan <jiangshan.ljs@antgroup.com>

wq_barrier work items are internal and do not count as user work items.
They do not participate in flush_workqueue() except for a sanity check,
since wq_barrier owns the PWQ reference. Therefore, any work color is
acceptable; just use the latest pwq->work_color.

See the commit d812796eb390 ("workqueue: Assign a color to barrier work
items").

Signed-off-by: Lai Jiangshan <jiangshan.ljs@antgroup.com>
---
 kernel/workqueue.c          | 9 ++-------
 kernel/workqueue_internal.h | 1 -
 2 files changed, 2 insertions(+), 8 deletions(-)

diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index 253311af47c6..4d817c7144cf 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -3187,7 +3187,6 @@ __acquires(&pool->lock)
 	if (worker->task)
 		worker->current_at = worker->task->se.sum_exec_runtime;
 	work_data = *work_data_bits(work);
-	worker->current_color = get_work_color(work_data);
 
 	/*
 	 * Record wq name for cmdline and debug reporting, may get
@@ -3308,7 +3307,6 @@ __acquires(&pool->lock)
 	worker->current_work = NULL;
 	worker->current_func = NULL;
 	worker->current_pwq = NULL;
-	worker->current_color = INT_MAX;
 
 	/* must be the last step, see the function comment */
 	pwq_dec_nr_in_flight(pwq, work_data);
@@ -3796,7 +3794,6 @@ static void insert_wq_barrier(struct pool_workqueue *pwq,
 {
 	static __maybe_unused struct lock_class_key bh_key, thr_key;
 	unsigned int work_flags = 0;
-	unsigned int work_color;
 	struct list_head *head;
 
 	/*
@@ -3826,19 +3823,17 @@ static void insert_wq_barrier(struct pool_workqueue *pwq,
 	 */
 	if (worker) {
 		head = worker->scheduled.next;
-		work_color = worker->current_color;
 	} else {
 		unsigned long *bits = work_data_bits(target);
 
 		head = target->entry.next;
 		/* there can already be other linked works, inherit and set */
 		work_flags |= *bits & WORK_STRUCT_LINKED;
-		work_color = get_work_color(*bits);
 		__set_bit(WORK_STRUCT_LINKED_BIT, bits);
 	}
 
-	pwq->nr_in_flight[work_color]++;
-	work_flags |= work_color_to_flags(work_color);
+	pwq->nr_in_flight[pwq->work_color]++;
+	work_flags |= work_color_to_flags(pwq->work_color);
 
 	insert_work(pwq, &barr->work, head, work_flags);
 }
diff --git a/kernel/workqueue_internal.h b/kernel/workqueue_internal.h
index f6275944ada7..76d2d99b26ab 100644
--- a/kernel/workqueue_internal.h
+++ b/kernel/workqueue_internal.h
@@ -32,7 +32,6 @@ struct worker {
 	work_func_t		current_func;	/* K: function */
 	struct pool_workqueue	*current_pwq;	/* K: pwq */
 	u64			current_at;	/* K: runtime at start or last wakeup */
-	unsigned int		current_color;	/* K: color */
 
 	int			sleeping;	/* S: is worker sleeping? */
 
-- 
2.19.1.6.gb485710b

Hello,

On Tue, Nov 25, 2025 at 06:31:24PM +0800, Lai Jiangshan wrote:
> From: Lai Jiangshan <jiangshan.ljs@antgroup.com>
> 
> wq_barrier work items are internal and do not count as user work items.
> They do not participate in flush_workqueue() except for a sanity check,
> since wq_barrier owns the PWQ reference. Therefore, any work color is
> acceptable; just use the latest pwq->work_color.

Maybe it is but I really don't like it. It becomes a lot harder to think
about and that makes things more fragile in the long term. I think it should
either not participate at all or do something straightforward like
inheriting the color of the work item it's flushing.

Even just thinking about the original problem that added flush color to
barrier work items becomes trickier. We can no longer just think that "oh
yeah, no new work items and all barriers match the target work items, so
flushing should wait for the whole thing". It now becomes "what happens if a
new flush_work() is queued after the latest flush_workqueue()? does that
still wait for that new barrier item?". In fact, I'm not sure it does. So,
please don't do this. Let's keep things conceptually straightforward as much
as possible even if that costs a bit more code. Code is often a lot cheaper
than cognitive overhead.

Thanks.

-- 
tejun

Hello, Tejun,

On Wed, Dec 3, 2025 at 1:37 AM Tejun Heo <tj@kernel.org> wrote:
> "oh
> yeah, no new work items and all barriers match the target work items, so
> flushing should wait for the whole thing".

This is only true after commit d812796eb390 ("workqueue: Assign
a color to barrier work items") is applied.

Users should not depend on this behavior; if they do, their code
is already fragile.

Anyway, I’ll withdraw this patchset.

Thanks
Lai