Re: [RFC v2 1/2] workqueue: Add support for real-time workers

[Tvrtko Ursulin]

Hi,

Is this a human review, or AI review, or combined?

On 17/07/2026 12:25, Bradley Morgan wrote:
Hi Tvrtko,

Applies to master. On linux-next one hunk rejects, the WQ_RTPRI block
in __alloc_workqueue(), the rest is clean. Rebase v3 onto the wq tree,
please..

Fixed in v3 already.


Things to fix:

+fail_ida:
+       if (pool->attrs->prio == WQ_PRIO_RT)
+               atomic_dec(&total_rtpri_workers);
  fail:
        ida_free(&pool->worker_ida, id);
        kfree(worker);

Labels swapped. The ida_alloc() failure falls into fail:, so ida_free()
gets a negative id and kfree() an uninitialized pointer, and the
later failure paths leak the counter. The fix is something like,

        id = ida_alloc(&pool->worker_ida, GFP_KERNEL);
        if (id < 0)
                goto fail_dec;
        ...
fail:
        ida_free(&pool->worker_ida, id);
        kfree(worker);
fail_dec:
        if (pool->attrs->prio == WQ_PRIO_RT)
                atomic_dec(&total_rtpri_workers);
        return NULL;

Already fixed locally after Sashiko flagged the onion unwind fail.


+               if (worker->pool->attrs->prio == WQ_PRIO_RT)
+                       atomic_dec(&total_rtpri_workers);

worker->pool is NULL here, the WORKER_DIE path clears it before
kthread_stop_put() returns, so every cull oopses.

Flag the worker at creation, a bool rtpri in struct worker:

        worker->rtpri = pool->attrs->prio == WQ_PRIO_RT;

and in reap_dying_workers():

-               if (worker->pool->attrs->prio == WQ_PRIO_RT)
+               if (worker->rtpri)
                        atomic_dec(&total_rtpri_workers);

Ditto but differently.

+               wq->unbound_attrs->affn_scope = WQ_AFFN_CPU;
+               wq->unbound_attrs->affn_strict = true;

Its a dead store, apply_wqattrs_commit() overwrites both, so the headline
v2
change never engages. Drop these two lines and mark the RT entries in
the workqueue_init_early() attrs loops instead, for both
unbound_std_wq_attrs and ordered_wq_attrs:

                attrs->prio = std_prio[i];
                attrs->nice = std_nice[i];

+               if (attrs->prio == WQ_PRIO_RT) {
+                       attrs->affn_scope = WQ_AFFN_CPU;
+                       attrs->affn_strict = true;
+               }

Already done locally exactly like this.

Questions: the changelog says two workers per workqueue, but the code clamps 
max_active, which caps work items, not threads. At the global cap 
create_worker() fails silently and maybe_create_worker() retries forever, so 
the pool stalls; with strict pods the last pod on an N CPU
box may never get a worker. pr_warn() or fall back to a normal worker?

Yeah it's a design open. I will probably just drop the global limits if 
Tejun agrees.

Also the rescuer is not FIFO, so WQ_MEM_RECLAIM plus WQ_RTPRI loses
the latency claim exactly under memory pressure.

Sashiko commented on this already. But I don't think latency is super 
relevant under memory pressure.

Nits: the "R" suffix branch is dead code, RT is unbound only. The

Curiosly lkml Sashiko missed this but I am aware of it. It is leftover 
from v1. And it's not a branch but an extra array element.

affinity sysfs files can still undo the forced affinity.

I am opting letting the admins shoot themselves in the foot.

Id prefer NUM_WQ_PRIO over NR_STD_WORKER_POOLS + 1. workqueue.rst needs
WQ_RTPRI.

I was on the fence. Either way is not ideal.

RST noted.

With the rebase and fixes, please add:

Reviewed-by: Bradley Morgan <incl...@grrlz.net> # kernel/

Thanks for the patch!

Thank you for reading through it!

Regards,

Tvrtko