All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
To: Tejun Heo <tj@kernel.org>
Cc: dri-devel@lists.freedesktop.org,
	Boris Brezillon <boris.brezillon@collabora.com>,
	Steven Price <steven.price@arm.com>,
	Liviu Dudau <liviu.dudau@arm.com>, Chia-I Wu <olvaffe@gmail.com>,
	Matthew Brost <matthew.brost@intel.com>,
	kernel-dev@igalia.com, linux-kernel@vger.kernel.org,
	Bradley Morgan <include@grrlz.net>, Chia-I Wu <olv@google.com>,
	Breno Leitao <leitao@debian.org>
Subject: Re: [RFC v4 1/2] workqueue: Add support for real-time workers
Date: Mon, 7 Sep 2026 16:00:47 +0100	[thread overview]
Message-ID: <8b0ea995-8299-4f68-9c06-2350a1d8fa2e@igalia.com> (raw)
In-Reply-To: <aoILFR3SNYBbMOOz@slm.duckdns.org>


On 16/08/2026 20:10, Tejun Heo wrote:
> (cc'ing Breno as he has been working in the area)
> 
> Hello,
> 
> Sorry about the delay.

Same here so no worries, summer holidays.. :)

> On Tue, Aug 04, 2026 at 11:19:24AM +0100, Tvrtko Ursulin wrote:
>>   struct workqueue_attrs {
>>   	/**
>> -	 * @nice: nice level
>> +	 * @prio: priority level
>> +	 */
>> +	enum wq_priority prio;
>> +
>> +	/**
>> +	 * @nice: nice level for WQ_PRIO_HIGH
>>   	 */
>>   	int nice;
> 
> I find this rather confusing. We're scattering the same internal state
> across multiple fields. If you look at scheduler code, rt and normal nice
> values are encoded into a single prio value, maybe we can do the same?

I thought it was more elegant to split the policy from the priority 
within a policy for call sites which only look up the policy. I can 
change it to encode all in a single integer or something. Alternative 
will be to call some helpers here and there which answers the "is this a 
rt policy" based on a range checks. At which point it felt one word in 
the struct is nicer than conditionals across the code. Your call what 
you prefer.

>>   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 +5637,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]);
> 
> And this looks a bit confusing too because there's no per-cpu counterpart
> but it looks like there should be.

I did not quite manage to follow you here. Was this not the per cpu 
part, with unbound and ordered down lower? You have to excuse me I am 
new in this code.

> I wonder whether this would look better after the percpu and unbound pool
> unification that Breno is working on.

I suppose this landed by now? I can see some changes so once we clarify 
the above opens I will rebase and adjust.

Regards,

Tvrtko


  reply	other threads:[~2026-09-07 15:00 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-04 10:19 [RFC v4 0/2] Realtime workqueues and panthor realtime submission Tvrtko Ursulin
2026-08-04 10:19 ` [RFC v4 1/2] workqueue: Add support for real-time workers Tvrtko Ursulin
2026-08-04 10:41   ` sashiko-bot
2026-08-16 19:10   ` Tejun Heo
2026-09-07 15:00     ` Tvrtko Ursulin [this message]
2026-09-08 22:51       ` Tejun Heo
2026-08-16 20:29   ` Bradley Morgan
2026-08-04 10:19 ` [RFC v4 2/2] drm/panthor: Create per queue priority workqueues Tvrtko Ursulin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8b0ea995-8299-4f68-9c06-2350a1d8fa2e@igalia.com \
    --to=tvrtko.ursulin@igalia.com \
    --cc=boris.brezillon@collabora.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=include@grrlz.net \
    --cc=kernel-dev@igalia.com \
    --cc=leitao@debian.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=liviu.dudau@arm.com \
    --cc=matthew.brost@intel.com \
    --cc=olv@google.com \
    --cc=olvaffe@gmail.com \
    --cc=steven.price@arm.com \
    --cc=tj@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.