Re: [PATCH 4/5] workqueue: build percpu pwqs through the attrs path

[Breno Leitao <leitao@debian.org>]

On Mon, Aug 31, 2026 at 12:23:10PM -1000, Tejun Heo wrote:
> Hello,
> 
> On Wed, Aug 19, 2026 at 07:37:38AM -0700, Breno Leitao wrote:
> >  	if (!(wq->flags & WQ_UNBOUND)) {
> > -		ret = alloc_and_link_percpu_pwqs(wq);
> > +		ret = apply_workqueue_attrs_locked(wq, percpu_std_wq_attrs[highpri]);
> 
> Applied 4-5 to wq/for-7.4.
> 
> One note: if pwq allocation fails partway through
> apply_wqattrs_prepare(), the already allocated pwqs are released through
> pwq_release_worker, which isn't created until workqueue_init(). Percpu
> workqueues are created before that, so an allocation failure in that
> window now oopses instead of failing cleanly. An allocation failing that
> early is unrecoverable anyway, so this doesn't need handling, but it
> probably deserves a comment.

Sure. Would a if() in put_pwq() cause harm? This is what I came up with.
Any chance it would be acceptable?


Thanks,
--breno

--
Author: Breno Leitao <leitao@debian.org>
Date:   Thu Sep 10 02:39:06 2026 -0700

    workqueue: warn when a pwq is released before workqueue_init()
    
    put_pwq() bounces the release to pwq_release_worker to get out from under
    pool->lock. workqueue_init() creates that kthread_worker well after
    workqueue_init_early() has built the system workqueues, so a pwq
    allocation failing partway through apply_wqattrs_prepare() unwinds
    through apply_wqattrs_cleanup() and dereferences a NULL worker.
    
    Running the release inline is not an option, that is what the bounce
    exists for. Warn and leak the pwq instead. The one that can get here is
    not linked to its workqueue yet and holds nothing but a pool reference,
    and the failed allocation takes the workqueue creation down anyway.
    
    Fixes: 967b494e2fd1 ("workqueue: Use a kthread_worker to release pool_workqueues")
    Suggested-by: Tejun Heo <tj@kernel.org>
    Link: https://lore.kernel.org/all/144b3dc986135ebe34f671cdd02bd8aa@kernel.org/
    Signed-off-by: Breno Leitao <leitao@debian.org>

diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index f868011e291f4b..47376593656a6b 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -1782,7 +1782,11 @@ static void put_pwq(struct pool_workqueue *pwq)
 	/*
 	 * @pwq can't be released under pool->lock, bounce to a dedicated
 	 * kthread_worker to avoid A-A deadlocks.
+	 * pwq_release_worker is created in workqueue_init(), and we might
+	 * get here before workqueue_init().
 	 */
+	if (WARN_ON_ONCE(!pwq_release_worker))
+		return;
 	kthread_queue_work(pwq_release_worker, &pwq->release_work);
 }
 
@@ -5930,6 +5934,10 @@ static int alloc_and_link_pwqs(struct workqueue_struct *wq)
 	if (!wq->cpu_pwq)
 		goto enomem;
 
+	/*
+	 * A pwq allocation failing before workqueue_init() leaks the pwqs
+	 * already allocated, see put_pwq().
+	 */
 	if (!(wq->flags & WQ_UNBOUND)) {
 		ret = apply_workqueue_attrs_locked(wq, percpu_std_wq_attrs[highpri]);
 	} else if (wq->flags & __WQ_ORDERED) {