[RFC v3 0/2] Realtime workqueues and panthor realtime submission

[RFC v3 0/2] Realtime workqueues and panthor realtime submission

[Tvrtko Ursulin]

This is a continuation of the previous discussion which was here:
https://lore.kernel.org/dri-devel/20260702143745.79293-1-tvrtko.ursulin@igalia.com/

Work is now converted to a much simpler approach by adding real-time scheduling
workqueues based on Tejun's feedback

To re-cap, when an userspace thread submits GPU work, due how the DRM scheduler
uses workqueues to feed the GPU, and regardless of the GPU rendering context
priority, or the CPU scheduling priority of the userspace thread itself, the
use of workqueues adds (a lot of) latency to the submit path.

When CPU is busy with enough backround load this translates to severe latency
spikes measured as time between userspace submitting work and GPU actually being
given that work to execute.

With the panthor workqueue upgraded to use WQ_HIGHPRI and varying the CPU
priority of the submit thread, the test program from
https://gitlab.freedesktop.org/panfrost/linux/-/work_items/49 reproduces these
kind of latencies:

         .    N    RT
    M   27   28    32
  95%  163  246   809
  98%  924  991  1882

Legend:

   M = Median submit latency in us
 95% = Percentile latency in us

   . = Userspace submit thread SCHED_OTHER
   N = -||= nice -1
  RT = -||- FIFO 1

Upgrading the panthor workqueues so that the realtime GPU priority queue uses
WQ_RTPRI, submit latency becomes completely controlled with the median of 14us
and 95 and 98-th percentiles at 23us and 25us respectively.

Important to note is that VK_QUEUE_GLOBAL_PRIORITY_REALTIME already required
the userspace to have CAP_SYS_NICE, meaning access to real-time workqueues is
effectively also guared behind this capability. The implementation of WQ_RTPRI
itself also adds some limites on the maximum number of worker threads in order
to prevent scheduling starvation.

v2:
 * See patch 1 changelog.

v3:
 * See patch 1 changelog + apologies for v3 following so quickly after v2.
   I have found a race condition as I expanded the testing to a second platform.

Tvrtko Ursulin (2):
  workqueue: Add support for real-time workers
  drm/panthor: Create per queue priority workqueues

 drivers/gpu/drm/panthor/panthor_sched.c |  38 +++++-
 include/linux/workqueue.h               |  23 +++-
 kernel/workqueue.c                      | 152 ++++++++++++++++++------
 3 files changed, 168 insertions(+), 45 deletions(-)

-- 
2.54.0

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

[Tvrtko Ursulin]

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.

For these situations the existing WQ_HIGHPRI does not bring a noticeable
improvement and a stronger hint is needed.

Lets add WQ_RTPRI which creates workers with a SCHED_FIFO scheduling class
to improve this.

We use a minimum priority level since we only care about winning the
contest against normal background CPU load. We also limit the number of
instantiated threads, both per workqueue to a maximum of two, or one on a
dual processor machine, and system-wide to one less than the number of
CPUs. The idea of that is to prevent system wide starvation caused by
potentially misbehaving work items.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Cc: Boris Brezillon <boris.brezillon@collabora.com>
Cc: Chia-I Wu <olv@google.com>
Cc: Liviu Dudau <liviu.dudau@arm.com>
Cc: Matthew Brost <matthew.brost@intel.com>
Cc: Steven Price <steven.price@arm.com>
Cc: Tejun Heo <tj@kernel.org>
---
v2:
 * Limit WQ_RTPRI to unbound workqueues and make it have strict CPU
   affinitity. (Tejun)
 * Fixed commit message typos. (AI)
 * Fixed sysfs handling, max_active setting and user modified nice
   application. (AI)

v3:
 * Fix worker->pool null pointer dereference race by moving the
   global decrement to detach_dying_workers().
 * Rebase for upstream changes.
---
 include/linux/workqueue.h |  23 ++++--
 kernel/workqueue.c        | 152 +++++++++++++++++++++++++++++---------
 2 files changed, 135 insertions(+), 40 deletions(-)

diff --git a/include/linux/workqueue.h b/include/linux/workqueue.h
index a283766a192a..e9ab53568e0c 100644
--- a/include/linux/workqueue.h
+++ b/include/linux/workqueue.h
@@ -140,6 +140,13 @@ enum wq_affn_scope {
 	WQ_AFFN_NR_TYPES,
 };
 
+enum wq_priority {
+	WQ_PRIO_NORMAL = 0,
+	WQ_PRIO_HIGH = 1,
+	WQ_PRIO_RT = 2,
+	NUM_WQ_PRIO, /* Keep last */
+};
+
 /**
  * struct workqueue_attrs - A struct for workqueue attributes.
  *
@@ -147,7 +154,12 @@ enum wq_affn_scope {
  */
 struct workqueue_attrs {
 	/**
-	 * @nice: nice level
+	 * @prio: priority level
+	 */
+	enum wq_priority prio;
+
+	/**
+	 * @nice: nice level for WQ_PRIO_HIGH
 	 */
 	int nice;
 
@@ -374,8 +386,9 @@ enum wq_flags {
 	WQ_FREEZABLE		= 1 << 2, /* freeze during suspend */
 	WQ_MEM_RECLAIM		= 1 << 3, /* may be used for memory reclaim */
 	WQ_HIGHPRI		= 1 << 4, /* high priority */
-	WQ_CPU_INTENSIVE	= 1 << 5, /* cpu intensive workqueue */
-	WQ_SYSFS		= 1 << 6, /* visible in sysfs, see workqueue_sysfs_register() */
+	WQ_RTPRI		= 1 << 5, /* real-time priority, valid only with WQ_UNBOUND */
+	WQ_CPU_INTENSIVE	= 1 << 6, /* cpu intensive workqueue */
+	WQ_SYSFS		= 1 << 7, /* visible in sysfs, see workqueue_sysfs_register() */
 
 	/*
 	 * Per-cpu workqueues are generally preferred because they tend to
@@ -402,8 +415,8 @@ enum wq_flags {
 	 *
 	 * http://thread.gmane.org/gmane.linux.kernel/1480396
 	 */
-	WQ_POWER_EFFICIENT	= 1 << 7,
-	WQ_PERCPU		= 1 << 8, /* bound to a specific cpu */
+	WQ_POWER_EFFICIENT	= 1 << 8,
+	WQ_PERCPU		= 1 << 9, /* bound to a specific cpu */
 
 	__WQ_DESTROYING		= 1 << 15, /* internal: workqueue is destroying */
 	__WQ_DRAINING		= 1 << 16, /* internal: workqueue is draining */
diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index 78068ae8f28a..30fed46a3164 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -509,10 +509,10 @@ static DEFINE_IDR(worker_pool_idr);	/* PR: idr of all pools */
 static DEFINE_HASHTABLE(unbound_pool_hash, UNBOUND_POOL_HASH_ORDER);
 
 /* I: attributes used when instantiating standard unbound pools on demand */
-static struct workqueue_attrs *unbound_std_wq_attrs[NR_STD_WORKER_POOLS];
+static struct workqueue_attrs *unbound_std_wq_attrs[NR_STD_WORKER_POOLS + 1];
 
 /* I: attributes used when instantiating ordered pools on demand */
-static struct workqueue_attrs *ordered_wq_attrs[NR_STD_WORKER_POOLS];
+static struct workqueue_attrs *ordered_wq_attrs[NR_STD_WORKER_POOLS + 1];
 
 /*
  * I: kthread_worker to release pwq's. pwq release needs to be bounced to a
@@ -546,6 +546,8 @@ EXPORT_SYMBOL_GPL(system_bh_highpri_wq);
 struct workqueue_struct *system_dfl_long_wq __ro_after_init;
 EXPORT_SYMBOL_GPL(system_dfl_long_wq);
 
+static atomic_t total_rtpri_workers = ATOMIC_INIT(0);
+
 static int worker_thread(void *__worker);
 static void workqueue_sysfs_unregister(struct workqueue_struct *wq);
 static void show_pwq(struct pool_workqueue *pwq);
@@ -1236,9 +1238,7 @@ static bool assign_work(struct work_struct *work, struct worker *worker,
 
 static struct irq_work *bh_pool_irq_work(struct worker_pool *pool)
 {
-	int high = pool->attrs->nice == HIGHPRI_NICE_LEVEL ? 1 : 0;
-
-	return &per_cpu(bh_pool_irq_works, pool->cpu)[high];
+	return &per_cpu(bh_pool_irq_works, pool->cpu)[pool->attrs->prio];
 }
 
 static void kick_bh_pool(struct worker_pool *pool)
@@ -1251,7 +1251,7 @@ static void kick_bh_pool(struct worker_pool *pool)
 		return;
 	}
 #endif
-	if (pool->attrs->nice == HIGHPRI_NICE_LEVEL)
+	if (pool->attrs->prio >= WQ_PRIO_HIGH)
 		raise_softirq_irqoff(HI_SOFTIRQ);
 	else
 		raise_softirq_irqoff(TASKLET_SOFTIRQ);
@@ -2809,13 +2809,16 @@ static int format_worker_id(char *buf, size_t size, struct worker *worker,
 				 worker->rescue_wq->name);
 
 	if (pool) {
-		if (pool->cpu >= 0)
+		if (pool->cpu >= 0) {
+			const char *suffix[NUM_WQ_PRIO] = { "", "H", "R" };
+
 			return scnprintf(buf, size, "kworker/%d:%d%s",
 					 pool->cpu, worker->id,
-					 pool->attrs->nice < 0  ? "H" : "");
-		else
+					 suffix[pool->attrs->prio]);
+		} else {
 			return scnprintf(buf, size, "kworker/u%d:%d",
 					 pool->id, worker->id);
+		}
 	} else {
 		return scnprintf(buf, size, "kworker/dying");
 	}
@@ -2838,12 +2841,30 @@ static struct worker *create_worker(struct worker_pool *pool)
 	struct worker *worker;
 	int id;
 
+	/*
+	 * Do not consume all CPUs with RT workers to avoid scheduler
+	 * starvation.
+	 */
+	if (pool->attrs->prio == WQ_PRIO_RT) {
+		unsigned int max = num_online_cpus();
+
+		if (max > 2)
+			max = max - 1;
+		else
+			max = 1;
+
+		if (atomic_inc_return(&total_rtpri_workers) > max) {
+			atomic_dec(&total_rtpri_workers);
+			return NULL;
+		}
+	}
+
 	/* ID is needed to determine kthread name */
 	id = ida_alloc(&pool->worker_ida, GFP_KERNEL);
 	if (id < 0) {
 		pr_err_once("workqueue: Failed to allocate a worker ID: %pe\n",
 			    ERR_PTR(id));
-		return NULL;
+		goto fail_ida;
 	}
 
 	worker = alloc_worker(pool->node);
@@ -2871,7 +2892,11 @@ static struct worker *create_worker(struct worker_pool *pool)
 			goto fail;
 		}
 
-		set_user_nice(worker->task, pool->attrs->nice);
+		if (pool->attrs->prio == WQ_PRIO_RT)
+			sched_set_fifo_low(worker->task);
+		else
+			set_user_nice(worker->task, pool->attrs->nice);
+
 		kthread_bind_mask(worker->task, pool_allowed_cpus(pool));
 	}
 
@@ -2896,6 +2921,9 @@ static struct worker *create_worker(struct worker_pool *pool)
 
 	return worker;
 
+fail_ida:
+	if (pool->attrs->prio == WQ_PRIO_RT)
+		atomic_dec(&total_rtpri_workers);
 fail:
 	ida_free(&pool->worker_ida, id);
 	kfree(worker);
@@ -2906,8 +2934,11 @@ static void detach_dying_workers(struct list_head *cull_list)
 {
 	struct worker *worker;
 
-	list_for_each_entry(worker, cull_list, entry)
+	list_for_each_entry(worker, cull_list, entry) {
+		if (worker->pool->attrs->prio == WQ_PRIO_RT)
+			atomic_dec(&total_rtpri_workers);
 		detach_worker(worker);
+	}
 }
 
 static void reap_dying_workers(struct list_head *cull_list)
@@ -3773,7 +3804,7 @@ static void drain_dead_softirq_workfn(struct work_struct *work)
 	 * don't hog this CPU's BH.
 	 */
 	if (repeat) {
-		if (pool->attrs->nice == HIGHPRI_NICE_LEVEL)
+		if (pool->attrs->prio >= WQ_PRIO_HIGH)
 			queue_work(system_bh_highpri_wq, work);
 		else
 			queue_work(system_bh_wq, work);
@@ -3805,7 +3836,7 @@ void workqueue_softirq_dead(unsigned int cpu)
 		dead_work.pool = pool;
 		init_completion(&dead_work.done);
 
-		if (pool->attrs->nice == HIGHPRI_NICE_LEVEL)
+		if (pool->attrs->prio >= WQ_PRIO_HIGH)
 			queue_work(system_bh_highpri_wq, &dead_work.work);
 		else
 			queue_work(system_bh_wq, &dead_work.work);
@@ -4780,6 +4811,7 @@ struct workqueue_attrs *alloc_workqueue_attrs_noprof(void)
 static void copy_workqueue_attrs(struct workqueue_attrs *to,
 				 const struct workqueue_attrs *from)
 {
+	to->prio = from->prio;
 	to->nice = from->nice;
 	cpumask_copy(to->cpumask, from->cpumask);
 	cpumask_copy(to->__pod_cpumask, from->__pod_cpumask);
@@ -4811,6 +4843,7 @@ static u32 wqattrs_hash(const struct workqueue_attrs *attrs)
 {
 	u32 hash = 0;
 
+	hash = jhash_1word(attrs->prio, hash);
 	hash = jhash_1word(attrs->nice, hash);
 	hash = jhash_1word(attrs->affn_strict, hash);
 	hash = jhash(cpumask_bits(attrs->__pod_cpumask),
@@ -4825,6 +4858,8 @@ static u32 wqattrs_hash(const struct workqueue_attrs *attrs)
 static bool wqattrs_equal(const struct workqueue_attrs *a,
 			  const struct workqueue_attrs *b)
 {
+	if (a->prio != b->prio)
+		return false;
 	if (a->nice != b->nice)
 		return false;
 	if (a->affn_strict != b->affn_strict)
@@ -5601,11 +5636,17 @@ static void unbound_wq_update_pwq(struct workqueue_struct *wq, int cpu)
 
 static int alloc_and_link_pwqs(struct workqueue_struct *wq)
 {
-	bool highpri = wq->flags & WQ_HIGHPRI;
-	int cpu, ret;
+	int prio, cpu, ret;
 
 	lockdep_assert_held(&wq_pool_mutex);
 
+	if (wq->flags & WQ_RTPRI)
+		prio = WQ_PRIO_RT;
+	else if (wq->flags & WQ_HIGHPRI)
+		prio = WQ_PRIO_HIGH;
+	else
+		prio = WQ_PRIO_NORMAL;
+
 	wq->cpu_pwq = alloc_percpu(struct pool_workqueue *);
 	if (!wq->cpu_pwq)
 		goto enomem;
@@ -5622,7 +5663,7 @@ static int alloc_and_link_pwqs(struct workqueue_struct *wq)
 			struct pool_workqueue **pwq_p;
 			struct worker_pool *pool;
 
-			pool = &(per_cpu_ptr(pools, cpu)[highpri]);
+			pool = &(per_cpu_ptr(pools, cpu)[prio]);
 			pwq_p = per_cpu_ptr(wq->cpu_pwq, cpu);
 
 			*pwq_p = kmem_cache_alloc_node(pwq_cache, GFP_KERNEL,
@@ -5642,14 +5683,14 @@ static int alloc_and_link_pwqs(struct workqueue_struct *wq)
 	if (wq->flags & __WQ_ORDERED) {
 		struct pool_workqueue *dfl_pwq;
 
-		ret = apply_workqueue_attrs_locked(wq, ordered_wq_attrs[highpri]);
+		ret = apply_workqueue_attrs_locked(wq, ordered_wq_attrs[prio]);
 		/* there should only be single pwq for ordering guarantee */
 		dfl_pwq = rcu_access_pointer(wq->dfl_pwq);
 		WARN(!ret && (wq->pwqs.next != &dfl_pwq->pwqs_node ||
 			      wq->pwqs.prev != &dfl_pwq->pwqs_node),
 		     "ordering guarantee broken for workqueue %s\n", wq->name);
 	} else {
-		ret = apply_workqueue_attrs_locked(wq, unbound_std_wq_attrs[highpri]);
+		ret = apply_workqueue_attrs_locked(wq, unbound_std_wq_attrs[prio]);
 	}
 
 	if (ret)
@@ -5814,6 +5855,12 @@ static struct workqueue_struct *__alloc_workqueue(const char *fmt,
 			return NULL;
 	}
 
+	if (flags & WQ_RTPRI) {
+		if (WARN_ON_ONCE((flags & (WQ_HIGHPRI | WQ_UNBOUND)) !=
+			         WQ_UNBOUND))
+			return NULL;
+	}
+
 	/* see the comment above the definition of WQ_POWER_EFFICIENT */
 	if ((flags & WQ_POWER_EFFICIENT) && wq_power_efficient)
 		flags = (flags & ~WQ_PERCPU) | WQ_UNBOUND;
@@ -5857,7 +5904,17 @@ static struct workqueue_struct *__alloc_workqueue(const char *fmt,
 		flags &= ~WQ_PERCPU;
 	}
 
-	if (flags & WQ_BH) {
+	if (flags & WQ_RTPRI) {
+		/*
+		 * RT workqueues are limited to max half of possible CPUs to
+		 * avoid scheduling starvation and have strict CPU affinity for
+		 * low latency execution.
+		 */
+		max_active = min_t(int, max_active,
+				   DIV_ROUND_UP(num_possible_cpus(), 2));
+		wq->unbound_attrs->affn_scope = WQ_AFFN_CPU;
+		wq->unbound_attrs->affn_strict = true;
+	} else if (flags & WQ_BH) {
 		/*
 		 * BH workqueues always share a single execution context per CPU
 		 * and don't impose any max_active limit.
@@ -6359,17 +6416,26 @@ void print_worker_info(const char *log_lvl, struct task_struct *task)
 	}
 }
 
+static void pr_cont_bh_suffix(enum wq_priority prio)
+{
+	if (prio == WQ_PRIO_RT)
+		pr_cont("bh-rt");
+	else if (prio == WQ_PRIO_HIGH)
+		pr_cont("bh-hi");
+	else
+		pr_cont("bh");
+}
+
 static void pr_cont_pool_info(struct worker_pool *pool)
 {
 	pr_cont(" cpus=%*pbl", nr_cpumask_bits, pool->attrs->cpumask);
 	if (pool->node != NUMA_NO_NODE)
 		pr_cont(" node=%d", pool->node);
-	pr_cont(" flags=0x%x", pool->flags);
+	pr_cont(" flags=0x%x ", pool->flags);
 	if (pool->flags & POOL_BH)
-		pr_cont(" bh%s",
-			pool->attrs->nice == HIGHPRI_NICE_LEVEL ? "-hi" : "");
+		pr_cont_bh_suffix(pool->attrs->prio);
 	else
-		pr_cont(" nice=%d", pool->attrs->nice);
+		pr_cont("prio=%d", pool->attrs->prio);
 }
 
 static void pr_cont_worker_id(struct worker *worker)
@@ -6377,8 +6443,7 @@ static void pr_cont_worker_id(struct worker *worker)
 	struct worker_pool *pool = worker->pool;
 
 	if (pool->flags & POOL_BH)
-		pr_cont("bh%s",
-			pool->attrs->nice == HIGHPRI_NICE_LEVEL ? "-hi" : "");
+		pr_cont_bh_suffix(pool->attrs->prio);
 	else
 		pr_cont("%d%s", task_pid_nr(worker->task),
 			worker->rescue_wq ? "(RESCUER)" : "");
@@ -7302,7 +7367,11 @@ static ssize_t wq_nice_show(struct device *dev, struct device_attribute *attr,
 	int written;
 
 	mutex_lock(&wq->mutex);
-	written = scnprintf(buf, PAGE_SIZE, "%d\n", wq->unbound_attrs->nice);
+	if (wq->unbound_attrs->prio != WQ_PRIO_RT)
+		written = scnprintf(buf, PAGE_SIZE, "%d\n",
+				    wq->unbound_attrs->nice);
+	else
+		written = -EINVAL;
 	mutex_unlock(&wq->mutex);
 
 	return written;
@@ -7328,13 +7397,20 @@ static ssize_t wq_nice_store(struct device *dev, struct device_attribute *attr,
 {
 	struct workqueue_struct *wq = dev_to_wq(dev);
 	struct workqueue_attrs *attrs;
-	int ret = -ENOMEM;
+	int ret;
 
 	mutex_lock(&wq_pool_mutex);
 
 	attrs = wq_sysfs_prep_attrs(wq);
-	if (!attrs)
+	if (!attrs) {
+		ret = -ENOMEM;
 		goto out_unlock;
+	}
+
+	if (attrs->prio == WQ_PRIO_RT) {
+		ret = -EINVAL;
+		goto out_unlock;
+	}
 
 	if (sscanf(buf, "%d", &attrs->nice) == 1 &&
 	    attrs->nice >= MIN_NICE && attrs->nice <= MAX_NICE)
@@ -7942,12 +8018,13 @@ static void __init restrict_unbound_cpumask(const char *name, const struct cpuma
 	cpumask_and(wq_unbound_cpumask, wq_unbound_cpumask, mask);
 }
 
-static void __init init_cpu_worker_pool(struct worker_pool *pool, int cpu, int nice)
+static void __init init_cpu_worker_pool(struct worker_pool *pool, int cpu, enum wq_priority prio, int nice)
 {
 	BUG_ON(init_worker_pool(pool));
 	pool->cpu = cpu;
 	cpumask_copy(pool->attrs->cpumask, cpumask_of(cpu));
 	cpumask_copy(pool->attrs->__pod_cpumask, cpumask_of(cpu));
+	pool->attrs->prio = prio;
 	pool->attrs->nice = nice;
 	pool->attrs->affn_strict = true;
 	pool->node = cpu_to_node(cpu);
@@ -7971,7 +8048,8 @@ static void __init init_cpu_worker_pool(struct worker_pool *pool, int cpu, int n
 void __init workqueue_init_early(void)
 {
 	struct wq_pod_type *pt = &wq_pod_types[WQ_AFFN_SYSTEM];
-	int std_nice[NR_STD_WORKER_POOLS] = { 0, HIGHPRI_NICE_LEVEL };
+	int std_prio[NR_STD_WORKER_POOLS + 1] = { 0, WQ_PRIO_HIGH, WQ_PRIO_RT };
+	int std_nice[NR_STD_WORKER_POOLS + 1] = { 0, HIGHPRI_NICE_LEVEL, 0 };
 	void (*irq_work_fns[NR_STD_WORKER_POOLS])(struct irq_work *) =
 		{ bh_pool_kick_normal, bh_pool_kick_highpri };
 	int i, cpu;
@@ -8023,22 +8101,25 @@ void __init workqueue_init_early(void)
 
 		i = 0;
 		for_each_bh_worker_pool(pool, cpu) {
-			init_cpu_worker_pool(pool, cpu, std_nice[i]);
+			init_cpu_worker_pool(pool, cpu, std_prio[i], std_nice[i]);
 			pool->flags |= POOL_BH;
 			init_irq_work(bh_pool_irq_work(pool), irq_work_fns[i]);
 			i++;
 		}
 
 		i = 0;
-		for_each_cpu_worker_pool(pool, cpu)
-			init_cpu_worker_pool(pool, cpu, std_nice[i++]);
+		for_each_cpu_worker_pool(pool, cpu) {
+			init_cpu_worker_pool(pool, cpu, std_prio[i], std_nice[i]);
+			i++;
+		}
 	}
 
 	/* create default unbound and ordered wq attrs */
-	for (i = 0; i < NR_STD_WORKER_POOLS; i++) {
+	for (i = 0; i < NR_STD_WORKER_POOLS + 1; i++) {
 		struct workqueue_attrs *attrs;
 
 		BUG_ON(!(attrs = alloc_workqueue_attrs()));
+		attrs->prio = std_prio[i];
 		attrs->nice = std_nice[i];
 		unbound_std_wq_attrs[i] = attrs;
 
@@ -8047,6 +8128,7 @@ void __init workqueue_init_early(void)
 		 * guaranteed by max_active which is enforced by pwqs.
 		 */
 		BUG_ON(!(attrs = alloc_workqueue_attrs()));
+		attrs->prio = std_prio[i];
 		attrs->nice = std_nice[i];
 		attrs->ordered = true;
 		ordered_wq_attrs[i] = attrs;
-- 
2.54.0

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
- [Critical] Uninitialized pointer `worker` is freed if `ida_alloc()` fails in `create_worker()`, causing memory corruption.
- [High] Worker pools can become permanently deadlocked when the global RT worker limit is reached due to an infinite sleep loop in the worker manager.
- [High] The global `total_rtpri_workers` atomic counter is permanently leaked when worker allocation or thread creation fails.
- [High] Strict CPU affinity meant for RT workqueues is immediately discarded and overwritten by standard NUMA attributes.
- [High] Creating an RT workqueue with WQ_AFFN_CPU affinity deterministically fails due to hitting the total_rtpri_workers limit on initialization.
- [Medium] The safety limit on max_active designed to prevent RT scheduler starvation can be bypassed via sysfs.
--

--- Patch [1]: [RFC v3 1/2] workqueue: Add support for real-time workers ---
commit 9672bfd2ee579be7697316d4370a3f3836fac4ce
Author: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>

workqueue: Add support for real-time workers

Adds WQ_RTPRI for real-time priority unbound workqueues to avoid latency
spikes from background CPU load. Limits RT workers system-wide to prevent
starvation.

> diff --git a/kernel/workqueue.c b/kernel/workqueue.c
> index 78068ae8f28a6..30fed46a31640 100644
> --- a/kernel/workqueue.c
> +++ b/kernel/workqueue.c
[ ... ]
> @@ -2838,12 +2841,30 @@ static struct worker *create_worker(struct worker_pool *pool)
>  	struct worker *worker;
>  	int id;
>  
> +	/*
> +	 * Do not consume all CPUs with RT workers to avoid scheduler
> +	 * starvation.
> +	 */
> +	if (pool->attrs->prio == WQ_PRIO_RT) {
> +		unsigned int max = num_online_cpus();
> +
> +		if (max > 2)
> +			max = max - 1;
> +		else
> +			max = 1;
> +
> +		if (atomic_inc_return(&total_rtpri_workers) > max) {
> +			atomic_dec(&total_rtpri_workers);
> +			return NULL;

[Severity: High]
Does returning NULL here cause a deadlock when called from
maybe_create_worker()?

maybe_create_worker() runs in a while (true) loop and expects create_worker()
failures to be temporary memory allocation issues:

maybe_create_worker() {
    while (true) {
        if (create_worker(pool) || !need_to_create_worker(pool))
            break;
        schedule_timeout_interruptible(CREATE_COOLDOWN);
    }
}

If the total_rtpri_workers limit is reached, create_worker() deterministically
returns NULL. Does this cause the manager thread to enter an infinite sleep
loop and deadlock the pool?

> +		}
> +	}
> +
>  	/* ID is needed to determine kthread name */
>  	id = ida_alloc(&pool->worker_ida, GFP_KERNEL);
>  	if (id < 0) {
>  		pr_err_once("workqueue: Failed to allocate a worker ID: %pe\n",
>  			    ERR_PTR(id));
> -		return NULL;
> +		goto fail_ida;

[Severity: Critical]
Is the uninitialized pointer worker freed if ida_alloc() fails?

At this point worker has not been allocated. If ida_alloc() fails, the code
jumps to fail_ida, which falls through to fail:

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

Will this call kfree() on a garbage pointer?

>  	}
>  
>  	worker = alloc_worker(pool->node);
> @@ -2871,7 +2892,11 @@ static struct worker *create_worker(struct worker_pool *pool)
>  			goto fail;

[Severity: High]
Does jumping to fail skip the total_rtpri_workers decrement?

If alloc_worker() or kthread_create_on_node() fails, they jump to the fail
label, which is placed below fail_ida. Doesn't this skip the atomic_dec() and
permanently leak the global total_rtpri_workers counter?

>  		}
>  
> -		set_user_nice(worker->task, pool->attrs->nice);
> +		if (pool->attrs->prio == WQ_PRIO_RT)
> +			sched_set_fifo_low(worker->task);
> +		else
> +			set_user_nice(worker->task, pool->attrs->nice);
> +
>  		kthread_bind_mask(worker->task, pool_allowed_cpus(pool));
>  	}
[ ... ]
> @@ -5622,7 +5663,7 @@ static int alloc_and_link_pwqs(struct workqueue_struct *wq)
[ ... ]
>  	if (wq->flags & __WQ_ORDERED) {
>  		struct pool_workqueue *dfl_pwq;
>  
> -		ret = apply_workqueue_attrs_locked(wq, ordered_wq_attrs[highpri]);
> +		ret = apply_workqueue_attrs_locked(wq, ordered_wq_attrs[prio]);
>  		/* there should only be single pwq for ordering guarantee */
>  		dfl_pwq = rcu_access_pointer(wq->dfl_pwq);
>  		WARN(!ret && (wq->pwqs.next != &dfl_pwq->pwqs_node ||
>  			      wq->pwqs.prev != &dfl_pwq->pwqs_node),
>  		     "ordering guarantee broken for workqueue %s\n", wq->name);
>  	} else {
> -		ret = apply_workqueue_attrs_locked(wq, unbound_std_wq_attrs[highpri]);
> +		ret = apply_workqueue_attrs_locked(wq, unbound_std_wq_attrs[prio]);

[Severity: High]
Does this overwrite the strict CPU affinity intended for RT workqueues?

In __alloc_workqueue(), wq->unbound_attrs is correctly configured with
WQ_AFFN_CPU and affn_strict = true for RT workqueues. However,
alloc_and_link_pwqs() immediately calls apply_workqueue_attrs_locked()
with unbound_std_wq_attrs[prio]. Does this replace the strict affinity
with the default NUMA-scoped non-strict attributes?

>  	}
>  
>  	if (ret)
[ ... ]
> @@ -5857,7 +5904,17 @@ static struct workqueue_struct *__alloc_workqueue(const char *fmt,
>  		flags &= ~WQ_PERCPU;
>  	}
>  
> -	if (flags & WQ_BH) {
> +	if (flags & WQ_RTPRI) {
> +		/*
> +		 * RT workqueues are limited to max half of possible CPUs to
> +		 * avoid scheduling starvation and have strict CPU affinity for
> +		 * low latency execution.
> +		 */
> +		max_active = min_t(int, max_active,
> +				   DIV_ROUND_UP(num_possible_cpus(), 2));

[Severity: Medium]
Can this max_active safety limit be bypassed via sysfs?

The sysfs max_active attribute uses wq_clamp_max_active(), which does not
recognize WQ_RTPRI and broadly clamps the value to 512 (WQ_MAX_ACTIVE).

Does this allow a privileged user to overwrite this initialization cap and
potentially cause RT scheduler starvation?

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

[Severity: High]
Does applying WQ_AFFN_CPU affinity deterministically fail to initialize due to
the global RT worker limit?

If WQ_AFFN_CPU is applied, apply_wqattrs_prepare() will loop over
for_each_possible_cpu() and create a distinct pool for every CPU.

Since total_rtpri_workers is globally capped to num_online_cpus() - 1, won't
attempting to sequentially spawn num_possible_cpus() workers deterministically
hit the hard limit and fail during initialization?

> +	} else if (flags & WQ_BH) {
>  		/*
>  		 * BH workqueues always share a single execution context per CPU
>  		 * and don't impose any max_active limit.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260717090209.26931-1-tvrtko.ursulin@igalia.com?part=1

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

[Tvrtko Ursulin]

On 17/07/2026 10:02, Tvrtko Ursulin via Kernel-dev 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.
> 
> For these situations the existing WQ_HIGHPRI does not bring a noticeable
> improvement and a stronger hint is needed.
> 
> Lets add WQ_RTPRI which creates workers with a SCHED_FIFO scheduling class
> to improve this.
> 
> We use a minimum priority level since we only care about winning the
> contest against normal background CPU load. We also limit the number of
> instantiated threads, both per workqueue to a maximum of two, or one on a
> dual processor machine, and system-wide to one less than the number of
> CPUs. The idea of that is to prevent system wide starvation caused by
> potentially misbehaving work items.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
> Cc: Boris Brezillon <boris.brezillon@collabora.com>
> Cc: Chia-I Wu <olv@google.com>
> Cc: Liviu Dudau <liviu.dudau@arm.com>
> Cc: Matthew Brost <matthew.brost@intel.com>
> Cc: Steven Price <steven.price@arm.com>
> Cc: Tejun Heo <tj@kernel.org>
> ---
> v2:
>   * Limit WQ_RTPRI to unbound workqueues and make it have strict CPU
>     affinitity. (Tejun)
>   * Fixed commit message typos. (AI)
>   * Fixed sysfs handling, max_active setting and user modified nice
>     application. (AI)
> 
> v3:
>   * Fix worker->pool null pointer dereference race by moving the
>     global decrement to detach_dying_workers().
>   * Rebase for upstream changes.
> ---
>   include/linux/workqueue.h |  23 ++++--
>   kernel/workqueue.c        | 152 +++++++++++++++++++++++++++++---------
>   2 files changed, 135 insertions(+), 40 deletions(-)
> 
> diff --git a/include/linux/workqueue.h b/include/linux/workqueue.h
> index a283766a192a..e9ab53568e0c 100644
> --- a/include/linux/workqueue.h
> +++ b/include/linux/workqueue.h
> @@ -140,6 +140,13 @@ enum wq_affn_scope {
>   	WQ_AFFN_NR_TYPES,
>   };
>   
> +enum wq_priority {
> +	WQ_PRIO_NORMAL = 0,
> +	WQ_PRIO_HIGH = 1,
> +	WQ_PRIO_RT = 2,
> +	NUM_WQ_PRIO, /* Keep last */
> +};
> +
>   /**
>    * struct workqueue_attrs - A struct for workqueue attributes.
>    *
> @@ -147,7 +154,12 @@ enum wq_affn_scope {
>    */
>   struct workqueue_attrs {
>   	/**
> -	 * @nice: nice level
> +	 * @prio: priority level
> +	 */
> +	enum wq_priority prio;
> +
> +	/**
> +	 * @nice: nice level for WQ_PRIO_HIGH
>   	 */
>   	int nice;
>   
> @@ -374,8 +386,9 @@ enum wq_flags {
>   	WQ_FREEZABLE		= 1 << 2, /* freeze during suspend */
>   	WQ_MEM_RECLAIM		= 1 << 3, /* may be used for memory reclaim */
>   	WQ_HIGHPRI		= 1 << 4, /* high priority */
> -	WQ_CPU_INTENSIVE	= 1 << 5, /* cpu intensive workqueue */
> -	WQ_SYSFS		= 1 << 6, /* visible in sysfs, see workqueue_sysfs_register() */
> +	WQ_RTPRI		= 1 << 5, /* real-time priority, valid only with WQ_UNBOUND */
> +	WQ_CPU_INTENSIVE	= 1 << 6, /* cpu intensive workqueue */
> +	WQ_SYSFS		= 1 << 7, /* visible in sysfs, see workqueue_sysfs_register() */
>   
>   	/*
>   	 * Per-cpu workqueues are generally preferred because they tend to
> @@ -402,8 +415,8 @@ enum wq_flags {
>   	 *
>   	 * http://thread.gmane.org/gmane.linux.kernel/1480396
>   	 */
> -	WQ_POWER_EFFICIENT	= 1 << 7,
> -	WQ_PERCPU		= 1 << 8, /* bound to a specific cpu */
> +	WQ_POWER_EFFICIENT	= 1 << 8,
> +	WQ_PERCPU		= 1 << 9, /* bound to a specific cpu */
>   
>   	__WQ_DESTROYING		= 1 << 15, /* internal: workqueue is destroying */
>   	__WQ_DRAINING		= 1 << 16, /* internal: workqueue is draining */
> diff --git a/kernel/workqueue.c b/kernel/workqueue.c
> index 78068ae8f28a..30fed46a3164 100644
> --- a/kernel/workqueue.c
> +++ b/kernel/workqueue.c
> @@ -509,10 +509,10 @@ static DEFINE_IDR(worker_pool_idr);	/* PR: idr of all pools */
>   static DEFINE_HASHTABLE(unbound_pool_hash, UNBOUND_POOL_HASH_ORDER);
>   
>   /* I: attributes used when instantiating standard unbound pools on demand */
> -static struct workqueue_attrs *unbound_std_wq_attrs[NR_STD_WORKER_POOLS];
> +static struct workqueue_attrs *unbound_std_wq_attrs[NR_STD_WORKER_POOLS + 1];
>   
>   /* I: attributes used when instantiating ordered pools on demand */
> -static struct workqueue_attrs *ordered_wq_attrs[NR_STD_WORKER_POOLS];
> +static struct workqueue_attrs *ordered_wq_attrs[NR_STD_WORKER_POOLS + 1];
>   
>   /*
>    * I: kthread_worker to release pwq's. pwq release needs to be bounced to a
> @@ -546,6 +546,8 @@ EXPORT_SYMBOL_GPL(system_bh_highpri_wq);
>   struct workqueue_struct *system_dfl_long_wq __ro_after_init;
>   EXPORT_SYMBOL_GPL(system_dfl_long_wq);
>   
> +static atomic_t total_rtpri_workers = ATOMIC_INIT(0);
> +
>   static int worker_thread(void *__worker);
>   static void workqueue_sysfs_unregister(struct workqueue_struct *wq);
>   static void show_pwq(struct pool_workqueue *pwq);
> @@ -1236,9 +1238,7 @@ static bool assign_work(struct work_struct *work, struct worker *worker,
>   
>   static struct irq_work *bh_pool_irq_work(struct worker_pool *pool)
>   {
> -	int high = pool->attrs->nice == HIGHPRI_NICE_LEVEL ? 1 : 0;
> -
> -	return &per_cpu(bh_pool_irq_works, pool->cpu)[high];
> +	return &per_cpu(bh_pool_irq_works, pool->cpu)[pool->attrs->prio];
>   }
>   
>   static void kick_bh_pool(struct worker_pool *pool)
> @@ -1251,7 +1251,7 @@ static void kick_bh_pool(struct worker_pool *pool)
>   		return;
>   	}
>   #endif
> -	if (pool->attrs->nice == HIGHPRI_NICE_LEVEL)
> +	if (pool->attrs->prio >= WQ_PRIO_HIGH)
>   		raise_softirq_irqoff(HI_SOFTIRQ);
>   	else
>   		raise_softirq_irqoff(TASKLET_SOFTIRQ);
> @@ -2809,13 +2809,16 @@ static int format_worker_id(char *buf, size_t size, struct worker *worker,
>   				 worker->rescue_wq->name);
>   
>   	if (pool) {
> -		if (pool->cpu >= 0)
> +		if (pool->cpu >= 0) {
> +			const char *suffix[NUM_WQ_PRIO] = { "", "H", "R" };
> +
>   			return scnprintf(buf, size, "kworker/%d:%d%s",
>   					 pool->cpu, worker->id,
> -					 pool->attrs->nice < 0  ? "H" : "");
> -		else
> +					 suffix[pool->attrs->prio]);
> +		} else {
>   			return scnprintf(buf, size, "kworker/u%d:%d",
>   					 pool->id, worker->id);
> +		}
>   	} else {
>   		return scnprintf(buf, size, "kworker/dying");
>   	}
> @@ -2838,12 +2841,30 @@ static struct worker *create_worker(struct worker_pool *pool)
>   	struct worker *worker;
>   	int id;
>   
> +	/*
> +	 * Do not consume all CPUs with RT workers to avoid scheduler
> +	 * starvation.
> +	 */
> +	if (pool->attrs->prio == WQ_PRIO_RT) {
> +		unsigned int max = num_online_cpus();
> +
> +		if (max > 2)
> +			max = max - 1;
> +		else
> +			max = 1;
> +
> +		if (atomic_inc_return(&total_rtpri_workers) > max) {
> +			atomic_dec(&total_rtpri_workers);
> +			return NULL;

Sashiko is warning me here this can create a deadlock when called from 
maybe_create_worker(). My apparently naive idea was that this would make 
it wait for a bit, until the load on the currently instantiated workers 
subsides, at which point need_to_create_worker() would say "not any 
more" and allow it to exit.

But even more importantly, it is also warning me that strict CPU 
affinity creates per CPU threads.

So my idea of global limit will just not work.

Tejun, how does this sound to you? Are you okay without having the 
global limit of RT threads?

I will await sending v4 with other fixes until we can close on this one. 
I also have some vacation pending so there may be a delay in my replies.

> +		}
> +	}
> +
>   	/* ID is needed to determine kthread name */
>   	id = ida_alloc(&pool->worker_ida, GFP_KERNEL);
>   	if (id < 0) {
>   		pr_err_once("workqueue: Failed to allocate a worker ID: %pe\n",
>   			    ERR_PTR(id));
> -		return NULL;
> +		goto fail_ida;
>   	}
>   
>   	worker = alloc_worker(pool->node);
> @@ -2871,7 +2892,11 @@ static struct worker *create_worker(struct worker_pool *pool)
>   			goto fail;
>   		}
>   
> -		set_user_nice(worker->task, pool->attrs->nice);
> +		if (pool->attrs->prio == WQ_PRIO_RT)
> +			sched_set_fifo_low(worker->task);
> +		else
> +			set_user_nice(worker->task, pool->attrs->nice);
> +
>   		kthread_bind_mask(worker->task, pool_allowed_cpus(pool));
>   	}
>   
> @@ -2896,6 +2921,9 @@ static struct worker *create_worker(struct worker_pool *pool)
>   
>   	return worker;
>   
> +fail_ida:
> +	if (pool->attrs->prio == WQ_PRIO_RT)
> +		atomic_dec(&total_rtpri_workers);
>   fail:
>   	ida_free(&pool->worker_ida, id);
>   	kfree(worker);

I failed on onion unwind here by not reversing the order. Fixed locally.

> @@ -2906,8 +2934,11 @@ static void detach_dying_workers(struct list_head *cull_list)
>   {
>   	struct worker *worker;
>   
> -	list_for_each_entry(worker, cull_list, entry)
> +	list_for_each_entry(worker, cull_list, entry) {
> +		if (worker->pool->attrs->prio == WQ_PRIO_RT)
> +			atomic_dec(&total_rtpri_workers);
>   		detach_worker(worker);
> +	}
>   }
>   
>   static void reap_dying_workers(struct list_head *cull_list)
> @@ -3773,7 +3804,7 @@ static void drain_dead_softirq_workfn(struct work_struct *work)
>   	 * don't hog this CPU's BH.
>   	 */
>   	if (repeat) {
> -		if (pool->attrs->nice == HIGHPRI_NICE_LEVEL)
> +		if (pool->attrs->prio >= WQ_PRIO_HIGH)
>   			queue_work(system_bh_highpri_wq, work);
>   		else
>   			queue_work(system_bh_wq, work);
> @@ -3805,7 +3836,7 @@ void workqueue_softirq_dead(unsigned int cpu)
>   		dead_work.pool = pool;
>   		init_completion(&dead_work.done);
>   
> -		if (pool->attrs->nice == HIGHPRI_NICE_LEVEL)
> +		if (pool->attrs->prio >= WQ_PRIO_HIGH)
>   			queue_work(system_bh_highpri_wq, &dead_work.work);
>   		else
>   			queue_work(system_bh_wq, &dead_work.work);
> @@ -4780,6 +4811,7 @@ struct workqueue_attrs *alloc_workqueue_attrs_noprof(void)
>   static void copy_workqueue_attrs(struct workqueue_attrs *to,
>   				 const struct workqueue_attrs *from)
>   {
> +	to->prio = from->prio;
>   	to->nice = from->nice;
>   	cpumask_copy(to->cpumask, from->cpumask);
>   	cpumask_copy(to->__pod_cpumask, from->__pod_cpumask);
> @@ -4811,6 +4843,7 @@ static u32 wqattrs_hash(const struct workqueue_attrs *attrs)
>   {
>   	u32 hash = 0;
>   
> +	hash = jhash_1word(attrs->prio, hash);
>   	hash = jhash_1word(attrs->nice, hash);
>   	hash = jhash_1word(attrs->affn_strict, hash);
>   	hash = jhash(cpumask_bits(attrs->__pod_cpumask),
> @@ -4825,6 +4858,8 @@ static u32 wqattrs_hash(const struct workqueue_attrs *attrs)
>   static bool wqattrs_equal(const struct workqueue_attrs *a,
>   			  const struct workqueue_attrs *b)
>   {
> +	if (a->prio != b->prio)
> +		return false;
>   	if (a->nice != b->nice)
>   		return false;
>   	if (a->affn_strict != b->affn_strict)
> @@ -5601,11 +5636,17 @@ static void unbound_wq_update_pwq(struct workqueue_struct *wq, int cpu)
>   
>   static int alloc_and_link_pwqs(struct workqueue_struct *wq)
>   {
> -	bool highpri = wq->flags & WQ_HIGHPRI;
> -	int cpu, ret;
> +	int prio, cpu, ret;
>   
>   	lockdep_assert_held(&wq_pool_mutex);
>   
> +	if (wq->flags & WQ_RTPRI)
> +		prio = WQ_PRIO_RT;
> +	else if (wq->flags & WQ_HIGHPRI)
> +		prio = WQ_PRIO_HIGH;
> +	else
> +		prio = WQ_PRIO_NORMAL;
> +
>   	wq->cpu_pwq = alloc_percpu(struct pool_workqueue *);
>   	if (!wq->cpu_pwq)
>   		goto enomem;
> @@ -5622,7 +5663,7 @@ static int alloc_and_link_pwqs(struct workqueue_struct *wq)
>   			struct pool_workqueue **pwq_p;
>   			struct worker_pool *pool;
>   
> -			pool = &(per_cpu_ptr(pools, cpu)[highpri]);
> +			pool = &(per_cpu_ptr(pools, cpu)[prio]);
>   			pwq_p = per_cpu_ptr(wq->cpu_pwq, cpu);
>   
>   			*pwq_p = kmem_cache_alloc_node(pwq_cache, GFP_KERNEL,
> @@ -5642,14 +5683,14 @@ static int alloc_and_link_pwqs(struct workqueue_struct *wq)
>   	if (wq->flags & __WQ_ORDERED) {
>   		struct pool_workqueue *dfl_pwq;
>   
> -		ret = apply_workqueue_attrs_locked(wq, ordered_wq_attrs[highpri]);
> +		ret = apply_workqueue_attrs_locked(wq, ordered_wq_attrs[prio]);
>   		/* there should only be single pwq for ordering guarantee */
>   		dfl_pwq = rcu_access_pointer(wq->dfl_pwq);
>   		WARN(!ret && (wq->pwqs.next != &dfl_pwq->pwqs_node ||
>   			      wq->pwqs.prev != &dfl_pwq->pwqs_node),
>   		     "ordering guarantee broken for workqueue %s\n", wq->name);
>   	} else {
> -		ret = apply_workqueue_attrs_locked(wq, unbound_std_wq_attrs[highpri]);
> +		ret = apply_workqueue_attrs_locked(wq, unbound_std_wq_attrs[prio]);
>   	}
>   
>   	if (ret)
> @@ -5814,6 +5855,12 @@ static struct workqueue_struct *__alloc_workqueue(const char *fmt,
>   			return NULL;
>   	}
>   
> +	if (flags & WQ_RTPRI) {
> +		if (WARN_ON_ONCE((flags & (WQ_HIGHPRI | WQ_UNBOUND)) !=
> +			         WQ_UNBOUND))
> +			return NULL;
> +	}
> +
>   	/* see the comment above the definition of WQ_POWER_EFFICIENT */
>   	if ((flags & WQ_POWER_EFFICIENT) && wq_power_efficient)
>   		flags = (flags & ~WQ_PERCPU) | WQ_UNBOUND;
> @@ -5857,7 +5904,17 @@ static struct workqueue_struct *__alloc_workqueue(const char *fmt,
>   		flags &= ~WQ_PERCPU;
>   	}
>   
> -	if (flags & WQ_BH) {
> +	if (flags & WQ_RTPRI) {
> +		/*
> +		 * RT workqueues are limited to max half of possible CPUs to
> +		 * avoid scheduling starvation and have strict CPU affinity for
> +		 * low latency execution.
> +		 */
> +		max_active = min_t(int, max_active,
> +				   DIV_ROUND_UP(num_possible_cpus(), 2));
> +		wq->unbound_attrs->affn_scope = WQ_AFFN_CPU;
> +		wq->unbound_attrs->affn_strict = true;

Sashiko let me know this will get overwritten by default attributes so I 
have moved it to where latter are initialized in workqueue_init_early().

Regards,

Tvrtko

> +	} else if (flags & WQ_BH) {
>   		/*
>   		 * BH workqueues always share a single execution context per CPU
>   		 * and don't impose any max_active limit.
> @@ -6359,17 +6416,26 @@ void print_worker_info(const char *log_lvl, struct task_struct *task)
>   	}
>   }
>   
> +static void pr_cont_bh_suffix(enum wq_priority prio)
> +{
> +	if (prio == WQ_PRIO_RT)
> +		pr_cont("bh-rt");
> +	else if (prio == WQ_PRIO_HIGH)
> +		pr_cont("bh-hi");
> +	else
> +		pr_cont("bh");
> +}
> +
>   static void pr_cont_pool_info(struct worker_pool *pool)
>   {
>   	pr_cont(" cpus=%*pbl", nr_cpumask_bits, pool->attrs->cpumask);
>   	if (pool->node != NUMA_NO_NODE)
>   		pr_cont(" node=%d", pool->node);
> -	pr_cont(" flags=0x%x", pool->flags);
> +	pr_cont(" flags=0x%x ", pool->flags);
>   	if (pool->flags & POOL_BH)
> -		pr_cont(" bh%s",
> -			pool->attrs->nice == HIGHPRI_NICE_LEVEL ? "-hi" : "");
> +		pr_cont_bh_suffix(pool->attrs->prio);
>   	else
> -		pr_cont(" nice=%d", pool->attrs->nice);
> +		pr_cont("prio=%d", pool->attrs->prio);
>   }
>   
>   static void pr_cont_worker_id(struct worker *worker)
> @@ -6377,8 +6443,7 @@ static void pr_cont_worker_id(struct worker *worker)
>   	struct worker_pool *pool = worker->pool;
>   
>   	if (pool->flags & POOL_BH)
> -		pr_cont("bh%s",
> -			pool->attrs->nice == HIGHPRI_NICE_LEVEL ? "-hi" : "");
> +		pr_cont_bh_suffix(pool->attrs->prio);
>   	else
>   		pr_cont("%d%s", task_pid_nr(worker->task),
>   			worker->rescue_wq ? "(RESCUER)" : "");
> @@ -7302,7 +7367,11 @@ static ssize_t wq_nice_show(struct device *dev, struct device_attribute *attr,
>   	int written;
>   
>   	mutex_lock(&wq->mutex);
> -	written = scnprintf(buf, PAGE_SIZE, "%d\n", wq->unbound_attrs->nice);
> +	if (wq->unbound_attrs->prio != WQ_PRIO_RT)
> +		written = scnprintf(buf, PAGE_SIZE, "%d\n",
> +				    wq->unbound_attrs->nice);
> +	else
> +		written = -EINVAL;
>   	mutex_unlock(&wq->mutex);
>   
>   	return written;
> @@ -7328,13 +7397,20 @@ static ssize_t wq_nice_store(struct device *dev, struct device_attribute *attr,
>   {
>   	struct workqueue_struct *wq = dev_to_wq(dev);
>   	struct workqueue_attrs *attrs;
> -	int ret = -ENOMEM;
> +	int ret;
>   
>   	mutex_lock(&wq_pool_mutex);
>   
>   	attrs = wq_sysfs_prep_attrs(wq);
> -	if (!attrs)
> +	if (!attrs) {
> +		ret = -ENOMEM;
>   		goto out_unlock;
> +	}
> +
> +	if (attrs->prio == WQ_PRIO_RT) {
> +		ret = -EINVAL;
> +		goto out_unlock;
> +	}
>   
>   	if (sscanf(buf, "%d", &attrs->nice) == 1 &&
>   	    attrs->nice >= MIN_NICE && attrs->nice <= MAX_NICE)
> @@ -7942,12 +8018,13 @@ static void __init restrict_unbound_cpumask(const char *name, const struct cpuma
>   	cpumask_and(wq_unbound_cpumask, wq_unbound_cpumask, mask);
>   }
>   
> -static void __init init_cpu_worker_pool(struct worker_pool *pool, int cpu, int nice)
> +static void __init init_cpu_worker_pool(struct worker_pool *pool, int cpu, enum wq_priority prio, int nice)
>   {
>   	BUG_ON(init_worker_pool(pool));
>   	pool->cpu = cpu;
>   	cpumask_copy(pool->attrs->cpumask, cpumask_of(cpu));
>   	cpumask_copy(pool->attrs->__pod_cpumask, cpumask_of(cpu));
> +	pool->attrs->prio = prio;
>   	pool->attrs->nice = nice;
>   	pool->attrs->affn_strict = true;
>   	pool->node = cpu_to_node(cpu);
> @@ -7971,7 +8048,8 @@ static void __init init_cpu_worker_pool(struct worker_pool *pool, int cpu, int n
>   void __init workqueue_init_early(void)
>   {
>   	struct wq_pod_type *pt = &wq_pod_types[WQ_AFFN_SYSTEM];
> -	int std_nice[NR_STD_WORKER_POOLS] = { 0, HIGHPRI_NICE_LEVEL };
> +	int std_prio[NR_STD_WORKER_POOLS + 1] = { 0, WQ_PRIO_HIGH, WQ_PRIO_RT };
> +	int std_nice[NR_STD_WORKER_POOLS + 1] = { 0, HIGHPRI_NICE_LEVEL, 0 };
>   	void (*irq_work_fns[NR_STD_WORKER_POOLS])(struct irq_work *) =
>   		{ bh_pool_kick_normal, bh_pool_kick_highpri };
>   	int i, cpu;
> @@ -8023,22 +8101,25 @@ void __init workqueue_init_early(void)
>   
>   		i = 0;
>   		for_each_bh_worker_pool(pool, cpu) {
> -			init_cpu_worker_pool(pool, cpu, std_nice[i]);
> +			init_cpu_worker_pool(pool, cpu, std_prio[i], std_nice[i]);
>   			pool->flags |= POOL_BH;
>   			init_irq_work(bh_pool_irq_work(pool), irq_work_fns[i]);
>   			i++;
>   		}
>   
>   		i = 0;
> -		for_each_cpu_worker_pool(pool, cpu)
> -			init_cpu_worker_pool(pool, cpu, std_nice[i++]);
> +		for_each_cpu_worker_pool(pool, cpu) {
> +			init_cpu_worker_pool(pool, cpu, std_prio[i], std_nice[i]);
> +			i++;
> +		}
>   	}
>   
>   	/* create default unbound and ordered wq attrs */
> -	for (i = 0; i < NR_STD_WORKER_POOLS; i++) {
> +	for (i = 0; i < NR_STD_WORKER_POOLS + 1; i++) {
>   		struct workqueue_attrs *attrs;
>   
>   		BUG_ON(!(attrs = alloc_workqueue_attrs()));
> +		attrs->prio = std_prio[i];
>   		attrs->nice = std_nice[i];
>   		unbound_std_wq_attrs[i] = attrs;
>   
> @@ -8047,6 +8128,7 @@ void __init workqueue_init_early(void)
>   		 * guaranteed by max_active which is enforced by pwqs.
>   		 */
>   		BUG_ON(!(attrs = alloc_workqueue_attrs()));
> +		attrs->prio = std_prio[i];
>   		attrs->nice = std_nice[i];
>   		attrs->ordered = true;
>   		ordered_wq_attrs[i] = attrs;

[RFC v3 2/2] drm/panthor: Create per queue priority workqueues

[Tvrtko Ursulin]

Split the single workqueue shared between the driver internal logic and
DRM scheduler use into separate ones, where the DRM scheduler one is
created per GPU priority level using the appropriate mapping to
workqueue priorities.

Low and medium GPU priority are served by a normal workqueue,
high is server by a WQ_HIGHPRI instance, while realtime GPU priority is
using the newly added WQ_RTPRI flag for lowest possible latency.

These workqueues are device global and for all three we set the maximum
concurrency to two in order to keep the GPU optimally fed with work.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Cc: Boris Brezillon <boris.brezillon@collabora.com>
Cc: Chia-I Wu <olv@google.com>
Cc: Liviu Dudau <liviu.dudau@arm.com>
Cc: Matthew Brost <matthew.brost@intel.com>
Cc: Steven Price <steven.price@arm.com>
Cc: Tejun Heo <tj@kernel.org>
---
 drivers/gpu/drm/panthor/panthor_sched.c | 38 +++++++++++++++++++++----
 1 file changed, 33 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/panthor/panthor_sched.c b/drivers/gpu/drm/panthor/panthor_sched.c
index 5832dccfc093..70e84ebb6c66 100644
--- a/drivers/gpu/drm/panthor/panthor_sched.c
+++ b/drivers/gpu/drm/panthor/panthor_sched.c
@@ -152,11 +152,18 @@ struct panthor_scheduler {
 	 *
 	 * Used for the scheduler tick, group update or other kind of FW
 	 * event processing that can't be handled in the threaded interrupt
-	 * path. Also passed to the drm_gpu_scheduler instances embedded
-	 * in panthor_queue.
+	 * path.
 	 */
 	struct workqueue_struct *wq;
 
+	/**
+	 * @submit_wq: Per priority workqueues for the DRM scheduler
+	 *
+	 * Passed to the drm_gpu_scheduler instances embedded
+	 * in panthor_queue based on the queue priority.
+	 */
+	struct workqueue_struct *submit_wq[PANTHOR_CSG_PRIORITY_COUNT];
+
 	/**
 	 * @heap_alloc_wq: Workqueue used to schedule tiler_oom works.
 	 *
@@ -3500,7 +3507,6 @@ group_create_queue(struct panthor_group *group,
 {
 	struct drm_sched_init_args sched_args = {
 		.ops = &panthor_queue_sched_ops,
-		.submit_wq = group->ptdev->scheduler->wq,
 		/*
 		 * The credit limit argument tells us the total number of
 		 * instructions across all CS slots in the ringbuffer, with
@@ -3593,8 +3599,14 @@ group_create_queue(struct panthor_group *group,
 		goto err_free_queue;
 	}
 
+	if (group->priority >= ARRAY_SIZE(group->ptdev->scheduler->submit_wq) ||
+	    !group->ptdev->scheduler->submit_wq[group->priority]) {
+		ret = -EINVAL;
+		goto err_free_queue;
+	}
+
 	sched_args.name = queue->name;
-
+	sched_args.submit_wq = group->ptdev->scheduler->submit_wq[group->priority];
 	ret = drm_sched_init(&queue->scheduler, &sched_args);
 	if (ret)
 		goto err_free_queue;
@@ -4084,6 +4096,15 @@ static void panthor_sched_fini(struct drm_device *ddev, void *res)
 	if (!sched || !sched->csg_slot_count)
 		return;
 
+	if (sched->submit_wq[PANTHOR_CSG_PRIORITY_MEDIUM])
+		destroy_workqueue(sched->submit_wq[PANTHOR_CSG_PRIORITY_MEDIUM]);
+
+	if (sched->submit_wq[PANTHOR_CSG_PRIORITY_HIGH])
+		destroy_workqueue(sched->submit_wq[PANTHOR_CSG_PRIORITY_HIGH]);
+
+	if (sched->submit_wq[PANTHOR_CSG_PRIORITY_RT])
+		destroy_workqueue(sched->submit_wq[PANTHOR_CSG_PRIORITY_RT]);
+
 	if (sched->wq)
 		destroy_workqueue(sched->wq);
 
@@ -4185,7 +4206,14 @@ int panthor_sched_init(struct panthor_device *ptdev)
 	 */
 	sched->heap_alloc_wq = alloc_workqueue("panthor-heap-alloc", WQ_UNBOUND, 0);
 	sched->wq = alloc_workqueue("panthor-csf-sched", WQ_MEM_RECLAIM | WQ_UNBOUND, 0);
-	if (!sched->wq || !sched->heap_alloc_wq) {
+	sched->submit_wq[PANTHOR_CSG_PRIORITY_MEDIUM] = alloc_workqueue("panthor-drm", WQ_MEM_RECLAIM | WQ_UNBOUND, 2);
+	sched->submit_wq[PANTHOR_CSG_PRIORITY_LOW] = sched->submit_wq[PANTHOR_CSG_PRIORITY_MEDIUM];
+	sched->submit_wq[PANTHOR_CSG_PRIORITY_HIGH] = alloc_workqueue("panthor-drm-high", WQ_HIGHPRI | WQ_MEM_RECLAIM | WQ_UNBOUND, 2);
+	sched->submit_wq[PANTHOR_CSG_PRIORITY_RT] = alloc_workqueue("panthor-drm-rt", WQ_RTPRI | WQ_MEM_RECLAIM | WQ_UNBOUND, 2);
+	if (!sched->wq || !sched->heap_alloc_wq ||
+	    !sched->submit_wq[PANTHOR_CSG_PRIORITY_MEDIUM] ||
+	    !sched->submit_wq[PANTHOR_CSG_PRIORITY_HIGH] ||
+	    !sched->submit_wq[PANTHOR_CSG_PRIORITY_RT]) {
 		panthor_sched_fini(&ptdev->base, sched);
 		drm_err(&ptdev->base, "Failed to allocate the workqueues");
 		return -ENOMEM;
-- 
2.54.0

Re: [RFC v3 2/2] drm/panthor: Create per queue priority workqueues

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Asynchronous teardown of `panthor_group` via `panthor_cleanup_wq` can race with device unplug and `drm_dev_release`, causing a Use-After-Free of the DRM device and workqueues.
--

--- Patch [2]: [RFC v3 2/2] drm/panthor: Create per queue priority workqueues ---
commit 55f41e3219b2c06b949efdf0212b58b264d95a7b
Author: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>

drm/panthor: Create per queue priority workqueues

This commit splits the single workqueue used by the DRM scheduler into
multiple priority-based workqueues (low, medium, high, rt) to better serve
different GPU priority levels.

> diff --git a/drivers/gpu/drm/panthor/panthor_sched.c b/drivers/gpu/drm/panthor/panthor_sched.c
> index 5832dccfc0932..70e84ebb6c663 100644
> --- a/drivers/gpu/drm/panthor/panthor_sched.c
> +++ b/drivers/gpu/drm/panthor/panthor_sched.c

[ ... ]

> @@ -4084,6 +4096,15 @@ static void panthor_sched_fini(struct drm_device *ddev, void *res)
>  	if (!sched || !sched->csg_slot_count)
>  		return;
>  
> +	if (sched->submit_wq[PANTHOR_CSG_PRIORITY_MEDIUM])
> +		destroy_workqueue(sched->submit_wq[PANTHOR_CSG_PRIORITY_MEDIUM]);
> +
> +	if (sched->submit_wq[PANTHOR_CSG_PRIORITY_HIGH])
> +		destroy_workqueue(sched->submit_wq[PANTHOR_CSG_PRIORITY_HIGH]);
> +
> +	if (sched->submit_wq[PANTHOR_CSG_PRIORITY_RT])
> +		destroy_workqueue(sched->submit_wq[PANTHOR_CSG_PRIORITY_RT]);
> +

[Severity: High]
This is a pre-existing issue, but can this synchronous destruction race
with the asynchronous teardown of the queue?

When a user closes the device, the driver can schedule group_release on
the global panthor_cleanup_wq without holding a reference to the DRM
device:

group_release()
    ...
    queue_work(panthor_cleanup_wq, &group->release_work);

If this was the last reference, drm_dev_release can run immediately and
call panthor_sched_fini, which destroys these new submit_wq workqueues
and frees the ptdev allocation. 

Meanwhile, the asynchronous release work executing group_free_queue can
attempt to access the destroyed queue->scheduler.submit_wq, or access
the already-freed ptdev memory:

panthor_kernel_bo_destroy()
    ...
    ptdev = container_of(bo->obj->dev, struct panthor_device, base);

Could this lead to a use-after-free since the release work isn't flushed
before the device teardown?

>  	if (sched->wq)
>  		destroy_workqueue(sched->wq);
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260717090209.26931-1-tvrtko.ursulin@igalia.com?part=2