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

[Tejun Heo]

Hello,

The following is a Claude-generated review.

On Thu, Oct 01, 2026 at 07:48:48PM +0100, Tvrtko Ursulin wrote:
> For use cases such as the DRM scheduler submitting work to the GPU on
> behalf of low latency userspace applications, where latter have sufficient
> privileges to have had successfully obtained realtime Vulkan global
> priority, competing with random background CPU load can create large
> latency spikes which gets in the way of a smooth user experience.

Can you add the compositor / DRM master usage model from the v5 discussion
here? The cover letter also still says CAP_SYS_NICE is required while
group_priority_permit() accepts DRM master too.

> +	HIGHPRI_PRIORITY	= NICE_TO_PRIO(MIN_NICE),
> +	RT_PRIORITY		= MAX_RT_PRIO - 1,

MAX_RT_PRIO - 1 is what rt_priority 0 would map to, while
sched_set_fifo_low() gives the workers prio 98. Nothing depends on it today,
but maybe MAX_RT_PRIO - 2 with a comment tying it to sched_set_fifo_low(),
or soften the kerneldoc which says the encoding matches task_struct->prio?
Also, s/analoguous/analogous/ there.

> +	if (wq->flags & WQ_RT) {
> +		attrs->prio = RT_PRIORITY;
> +		/*
> +		 * RT workqueues have strict CPU affinity for low
> +		 * latency execution.
> +		 */
> +		attrs->affn_scope = WQ_AFFN_CPU;
> +		attrs->affn_strict = true;

apply_workqueue_attrs() is unrestricted, so a caller can hand a WQ_RT wq the
attrs from alloc_workqueue_attrs() and turn it into a normal non-strict wq
while the flag stays set. Maybe apply_wqattrs_prepare() should force prio
and affinity for WQ_RT instead of restricting only the sysfs side?

> +	if (flags & WQ_RT) {
> +		if (WARN_ON_ONCE((flags & (WQ_HIGHPRI | WQ_UNBOUND)) !=
> +			         WQ_UNBOUND))
> +			return NULL;
> +	}

alloc_ordered_workqueue() with WQ_RT passes this and ends up with a single
pool spanning all CPUs because ordered wqs use dfl_pwq everywhere, while the
doc and sysfs say strict per-CPU. Should __WQ_ORDERED be rejected too? The
nested ifs can also be a single condition.

> -		pr_cont(" nice=%d", pool->attrs->nice);
> +		pr_cont(" nice=%d", PRIO_TO_NICE(pool->attrs->prio));

This prints nice=-21 for RT pools. Can you show "rt" here like nice_show()?

> +	/* Do not allow cpumask changes for RT workers. */
> +	if (wq->flags & WQ_RT)
> +		return -EINVAL;

The three stores check WQ_RT and return -EINVAL while nice goes read-only
through is_visible below. Can all four go through
wq_sysfs_unbound_group_visible() returning 0444 for WQ_RT? That drops the
three store checks. Note that the global cpumask still applies to WQ_RT wqs
through workqueue_apply_unbound_cpumask(), so the comment overstates a bit.
The interface comment at the top of the sysfs section also still says nice
is RW int.

> +	/* Do not allow priority changes for RT workers. */
> +	if ((wq->flags & WQ_RT) && !strcmp(attr->name, "nice"))
> +		return 0444;

attr == &dev_attr_nice.attr would avoid the strcmp.

> +    prio = pool.attrs.prio.value_()
> +    if prio == rt_prio:
> +        prio = 'rt'
> +    print(f'pool[{pi:0{max_pool_id_len}}] flags=0x{pool.flags.value_():02x} ref={pool.refcnt.value_():{max_ref_len}} prio={prio:3} ', end='')

Can we keep printing nice for non-RT pools? prio=120 is the internal
encoding, sysfs and the pool dumps print nice, and the example output in
workqueue.rst would go stale. prog['RT_PRIORITY'] would match the rest of
the file too.

One more thing which isn't in the diff. The rescuer of a WQ_RT |
WQ_MEM_RECLAIM wq, which is what panthor-drm-rt is, still runs at nice -20,
so under memory pressure the RT wq's work items run as CFS. Should
rescuer_thread() use sched_set_fifo_low() for WQ_RT?

Thanks.

--
tejun