Re: [PATCH RFC 0/3] workqueue: Take the pwq backend from the attrs

[Breno Leitao <leitao@debian.org>]

Hello Tejun,

On Fri, Sep 18, 2026 at 04:13:36PM -1000, Tejun Heo wrote:
> On Fri, Sep 18, 2026 at 07:25:32AM -0700, Breno Leitao wrote:
> >  * Keep wq->max_active and wq->percpu_max_active both current. The two
> >    backends meter work differently, and a pwq must not end up metered
> >    against a limit nobody set. (this is the semi-conflictual with my previous
> >    commit 27db9dd7f84f3a ("workqueue: Give percpu workqueues their own
> >    max_active")
> 
> Both should stay current but I don't think they should be the same number.

Ok, that means we are going to keep both as commit 27db9dd7f84f3a, and
keep both values set at a given time, and the values will be different.

> max_active means different things in the two domains, per-CPU on one side and
> across the whole workqueue on the other, so let's keep the two sets of values
> separate and link them through scaling.

Right, I will create a function that maps/scale one to another. Maybe
the following?

	 static int percpu_to_unbound_max_active(int percpu_max_active)
	 {
		 s64 max_active = (s64)percpu_max_active * num_possible_cpus();

		 return min_t(s64, max_active, WQ_MAX_ACTIVE);
	 }

	 static int unbound_to_percpu_max_active(int max_active)
	 {
		 return DIV_ROUND_UP(max_active, num_possible_cpus());
	 }

> On creation, the argument sets the
> values for the domain the workqueue starts in and the other domain is derived
> from it. Afterwards, each domain has its own interface, kernel and sysfs, and
> adjusting one updates the other accordingly. That way a switch always lands
> on a sensible limit without anyone having to think about it.

Sure, and probably call them from wq_adjust_max_active(), so,
independent of the one that is being set (percpu or unbound), it will
change the other as well according to the function above.

> >  * Create a ->concurrency_managed field in the wq attributes, used to
> >    decide whether to do concurrency management or not.
> >
> >  * Move WQ_PERCPU onto an UNBOUND workqueue with WQ_AFFN_CPU affinity and
> >    the newly created ->concurrency_managed. WQ_PERCPU is then only the
> >    promise that the workqueue stays on that backend.
> 
> I'd rather not build percpu on top of CPU scope. Unbound with strict CPU
> scope and percpu are different things. The pools, the metering and how the
> unbound cpumask applies all differ, and both should keep existing. So, how
> about making PERCPU its own scope? 

Ack! I will proceed with a newly created WQ_AFFN_PERCPU scope then.

> Whether concurrency management is then
> expressed as a flag or an attribute doesn't matter much as long as it can be
> turned on and off. WQ_PERCPU would mean that the workqueue can't leave the
> PERCPU scope while CM can still be toggled.

Oh, interesting, so, we are going to have CM toggable even for
WQ_PERCPU and also for WQ_UNBOUND+WQ_AFFN_PERCPU, is this right?

Once we have it, what will be the difference between
WQ_UNBOUND+WQ_AFFN_PERCPU+CM from WQ_PERCPU? They look exactly the same
from a user perspective, no?

> BH can be a scope value too for consistency. It's only selectable on creation
> and can't be switched into or out of, but having it in the same enum keeps
> things uniform.

Do you mean something like WQ_AFFN_BH or an entry in workqueue_attrs,
similar to affn_strict and the recently propsoed concurrency_managed?

I understand that scope means 'enum wq_affn_scope affn_scope', so, you
want a WQ_AFFN_BH, right?

> With scope carrying the backend, nice can apply verbatim on unbound and snap
> to normal or highpri when the workqueue is on percpu, picked from the current
> value. No need to restrict what can be written.

So, it means that we are going to expose the wq_sysfs_unbound_attrs to
WQ_PERCPU as well, and we are going to map nice to high priority queues
and normal queues. For instance:

  - echo -5 > nice succeeds and stores -5, whatever scope the wq is in
  - For WQ_PERCPU it runs on the highpri pool (nice -20 in practice)
  - echo 0 > nice, will turn the WQ_PERCPU workqueue into the normal
    pool.

Am I getting the idea right here?

> > Not done: the switching itself. concurrency_managed is fixed when the
> > workqueue is created and is not exported through sysfs, so nothing takes a
> > workqueue onto the backend or off it yet.
> 
> For PREFER_PERCPU type workqueues, which benefit from cmwq but don't depend
> on it for correctness, there's no reason to block switching in either
> direction. Having them follow the unbound cpumask when picking the queueing
> CPU even while on percpu, the way the last patch keys that on WQ_PERCPU
> rather than on the backend, makes sense for them.

let me back up a bit and talk about the scopes. The final state of the
scopes will be:

enum wq_affn_scope {
        WQ_AFFN_DFL,    
        WQ_AFFN_CPU,     
        WQ_AFFN_SMT,      
        WQ_AFFN_CACHE,     
        WQ_AFFN_CACHE_SHARD,
        WQ_AFFN_NUMA,       
        WQ_AFFN_SYSTEM,     
+       WQ_AFFN_PERCPU,                 /* one pod per CPU + CM + affinity */
+       WQ_AFFN_PEREFER_PERCPU,         /* one pod per CPU + CM + relaxed affinity */
+       WQ_AFFN_BH,			/* scope for WQ_BH */
}

Is this the final state you are envisioning?

Thanks for the excellent review and guidance,
--breno