From: "Danilo Krummrich" <dakr@kernel.org>
To: "Alice Ryhl" <aliceryhl@google.com>
Cc: "Tejun Heo" <tj@kernel.org>, "Miguel Ojeda" <ojeda@kernel.org>,
"Lai Jiangshan" <jiangshanlai@gmail.com>,
"Gary Guo" <gary@garyguo.net>,
"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
"Andreas Hindborg" <a.hindborg@kernel.org>,
"Trevor Gross" <tmgross@umich.edu>,
"Daniel Almeida" <daniel.almeida@collabora.com>,
"John Hubbard" <jhubbard@nvidia.com>,
"Philipp Stanner" <phasta@kernel.org>,
rust-for-linux@vger.kernel.org, linux-kernel@vger.kernel.org,
"Boqun Feng" <boqun@kernel.org>,
"Benno Lossin" <lossin@kernel.org>,
"Tamir Duberstein" <tamird@kernel.org>
Subject: Re: [PATCH v3 2/2] rust: workqueue: add creation of workqueues
Date: Sat, 28 Feb 2026 15:43:02 +0100 [thread overview]
Message-ID: <DGQOAM62LGC8.2S5KJZBQU9OR8@kernel.org> (raw)
In-Reply-To: <aaLmzp0mMPpE45yQ@google.com>
On Sat Feb 28, 2026 at 1:59 PM CET, Alice Ryhl wrote:
> On Fri, Feb 27, 2026 at 08:23:44PM +0100, Danilo Krummrich wrote:
>> On Fri Feb 27, 2026 at 8:05 PM CET, Alice Ryhl wrote:
>> > On Fri, Feb 27, 2026 at 04:30:59PM +0100, Danilo Krummrich wrote:
>> >> On Fri Feb 27, 2026 at 3:53 PM CET, Alice Ryhl wrote:
>> >> > + #[inline]
>> >> > + pub fn max_active(mut self, max_active: u32) -> Builder {
>> >> > + self.max_active = i32::try_from(max_active).unwrap_or(i32::MAX);
>> >>
>> >> The workqueue code prints a warning for max_active > WQ_MAX_ACTIVE. Maybe use
>> >> debug_assert()?
>> >
>> > What's wrong with just making use of the C-side warning?
>>
>> IIRC, we have the same pattern in other Rust code that we use debug_assert()
>> when a value got clamped, e.g. in udelay().
>
> In udelay(), the clamping happens on the Rust side, so it makes sense
> that Rust is the one to warn about it.
>
> Here, the clamping happens in C code. To warn about it, I'd have to
> duplicate the existing C-side check to clamp in Rust.
That's fair, although I also think that it is not unreasonable. Given that this
uses the builder pattern, I think it would be nice to ensure that nothing
"invalid" can be built in the first place.
Maybe we can use a bounded integer?
>> >> It's also a bit unfortunate that alloc_ordered_workqueue() becomes
>> >> .max_active(1).
>> >>
>> >> At the same time having a separate ordered() method competes with max_active().
>> >>
>> >> Mybe a type state, i.e. Builder<Ordered> that doesn't have max_active()?
>> >
>> > Sorry I'm a bit confused by this. Why does an ordered() compete with
>> > max_active()?
>>
>> Because you could get an inconsistent state with __WQ_ORDERED and
>> max_active > 1.
>>
>> It also conflicts with sysfs() I think [1].
>>
>> [1] https://elixir.bootlin.com/linux/v6.19.3/source/kernel/workqueue.c#L7417
>
> And I guess the further argument is that we have a use-case for ordered
> workqueues?
In the context of
GPU drivers often need to create their own workqueues for various
reasons. Add the ability to do so.
I think we do.
Depending on the final implementation details and the driver it may be needed by
the job queue.
They are also pretty common outside the scheduler use-case in GPU drivers. I
think panthor has one as well, so you might also need one in Tyr. In nova-core I
expect this to be used in MM code.
But even without that, I think it would be reasonble to consider ordered queues
for this abstraction, since alloc_ordered_workqueue() and
create_singlethread_workqueue() seem to have more users than the non-ordered
constructors (without checking whether alloc_workqueue() is also used directly
to create ordered queues).
next prev parent reply other threads:[~2026-02-28 14:43 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-02-27 14:53 [PATCH v3 0/2] Creation of workqueues in Rust Alice Ryhl
2026-02-27 14:53 ` [PATCH v3 1/2] rust: workqueue: restrict delayed work to global wqs Alice Ryhl
2026-02-27 15:10 ` Danilo Krummrich
2026-02-27 15:47 ` Gary Guo
2026-02-27 17:09 ` Tejun Heo
2026-02-27 19:01 ` Alice Ryhl
2026-02-27 19:08 ` Tejun Heo
2026-02-27 19:19 ` Alice Ryhl
2026-02-27 19:24 ` Tejun Heo
2026-02-27 19:28 ` Alice Ryhl
2026-02-27 19:46 ` Tejun Heo
2026-02-27 20:36 ` Alice Ryhl
2026-02-27 21:25 ` Tejun Heo
2026-02-27 14:53 ` [PATCH v3 2/2] rust: workqueue: add creation of workqueues Alice Ryhl
2026-02-27 15:30 ` Danilo Krummrich
2026-02-27 16:00 ` Gary Guo
2026-02-27 16:12 ` Danilo Krummrich
2026-02-27 19:05 ` Alice Ryhl
2026-02-27 19:23 ` Danilo Krummrich
2026-02-28 12:59 ` Alice Ryhl
2026-02-28 14:43 ` Danilo Krummrich [this message]
2026-03-01 11:55 ` Alice Ryhl
2026-03-02 12:45 ` Philipp Stanner
2026-02-27 16:04 ` Gary Guo
2026-02-27 19:08 ` Alice Ryhl
2026-02-28 2:24 ` kernel test robot
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=DGQOAM62LGC8.2S5KJZBQU9OR8@kernel.org \
--to=dakr@kernel.org \
--cc=a.hindborg@kernel.org \
--cc=aliceryhl@google.com \
--cc=bjorn3_gh@protonmail.com \
--cc=boqun@kernel.org \
--cc=daniel.almeida@collabora.com \
--cc=gary@garyguo.net \
--cc=jhubbard@nvidia.com \
--cc=jiangshanlai@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=lossin@kernel.org \
--cc=ojeda@kernel.org \
--cc=phasta@kernel.org \
--cc=rust-for-linux@vger.kernel.org \
--cc=tamird@kernel.org \
--cc=tj@kernel.org \
--cc=tmgross@umich.edu \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox