Re: [PATCH 2/2] workqueue: defer wake_up_process() outside pool->lock on hot paths

[Breno Leitao <leitao@debian.org>]

On Wed, May 27, 2026 at 05:35:00PM +0200, Sebastian Andrzej Siewior wrote:
> On 2026-05-27 07:51:17 [-0700], Breno Leitao wrote:
> > @@ -3447,7 +3459,13 @@ static int worker_thread(void *__worker)
> >  		return 0;
> >  	}
> >  
> > -	worker_leave_idle(worker);
> > +	/*
> > +	 * Kicked workers have already been removed from pool->idle_list
> > +	 * by kick_pool(); only first-time wakeups (via create_worker())
> > +	 * still arrive with WORKER_IDLE set.
> > +	 */
> > +	if (worker->flags & WORKER_IDLE)
> > +		worker_leave_idle(worker);
> 
> Couldn't create_worker() be aligned here not set the idle flag and wake
> the thread a few lines later? Then we wouldn't have to conditionally
> clear the idle flag here (which sort of NULL renders the flag check in
> worker_leave_idle()).

I tried exactly that and it regresses worker creation, so I'd rather
keep create_worker() as-is and leave the check in woke_up: conditional.

create_worker() deliberately returns with the new worker still on
pool->idle_list (counted in pool->nr_idle), and maybe_create_worker()
depends on that.  After a successful create_worker() it re-checks:

      need_to_create_worker() = need_more_worker() && !may_start_working()

and may_start_working() is just pool->nr_idle.  That nr_idle is the
signal "I already created a worker that will pick up the pending work,
stop creating".

If create_worker() leaves the worker !WORKER_IDLE before the wakeup (or
never enters idle), it returns with nr_idle unchanged.  A fresh worker
starts WORKER_PREP, so it doesn't bump nr_running until it actually
runs.  So until the woken kthread schedules in, the manager keeps
seeing:

      need_more_worker()  == true   (worklist non-empty, nr_running 0)
      may_start_working() == false  (nr_idle 0)

and loops/goto restart, creating extra workers until one of the woken
ones runs.  Under scheduler latency that's a burst of surplus kworkers
(eventually culled).

So I kept create_worker() untouched and only claim the worker (off
idle_list) in kick_pool(), with the conditional worker_leave_idle() in
woke_up: covering the create_worker() path.

--breno