Re: [PATCH v3 2/2] rust: workqueue: add creation of workqueues

[Danilo Krummrich]

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).