Re: [PATCH v4 3/3] rust: workqueue: add creation of workqueues

[Gary Guo]

On Thu Mar 12, 2026 at 10:56 PM GMT, Alice Ryhl wrote:
> On Thu, Mar 12, 2026 at 04:39:00PM +0000, Gary Guo wrote:
>> > +    flags: bindings::wq_flags,
>> > +    max_active: i32,
>> > +    _type: PhantomData<T>,
>> 
>> Hmm, it is somewhat awkward to me that we are having a `PhantomData<T>` here,
>> as `PhantomData` is documented to "behave as it it owns a `T`", but all the
>> possible `T`s that we use here are uninhabited.
>
> I don't think this is an issue.

Fair. I also don't think it's an issue, just awkward.

I would rather make the uninhabited enums be just unit structs, though (like what we do
for atomic ordering and device contexts).

>
>> > +    /// Build a single-threaded workqueue that executes jobs in order.
>> > +    ///
>> > +    /// # Examples
>> > +    ///
>> > +    /// ```
>> > +    /// use kernel::workqueue::Queue;
>> > +    ///
>> > +    /// let wq = Queue::new_ordered().build(c"my-wq")?;
>> > +    /// wq.try_spawn(GFP_KERNEL, || pr_info!("Hello from ordered wq"))?;
>> > +    /// # Ok::<(), Error>(())
>> > +    /// ```
>> > +    #[inline]
>> > +    #[doc(alias = "alloc_ordered_workqueue")]
>> > +    #[doc(alias = "__WQ_ORDERED")]
>> > +    pub fn new_ordered() -> Builder<TypeOrdered> {
>> > +        Builder {
>> > +            flags: bindings::wq_flags_WQ_UNBOUND | bindings::wq_flags___WQ_ORDERED,
>> > +            max_active: 0,
>> 
>> This should be 1 instead of 0.
>
> Hmm, it's weird that the doc-test didn't catch this.

It seems that there's no check for this in the __alloc_workqueue (which I guess
makes sense, because there's a C macro for allocating ordered workqueue which
doesn't take max_active and pass 1 along).

The only assertion is in workqueue_set_max_active to prevent changing it after
creation.

Best,
Gary

>
>> > +/// An owned kernel work queue.
>> > +///
>> > +/// Dropping a workqueue blocks on all pending work.
>> > +///
>> > +/// # Invariants
>> > +///
>> > +/// `queue` points at a valid workqueue that is owned by this `OwnedQueue`.
>> > +pub struct OwnedQueue {
>> > +    queue: NonNull<Queue>,
>> > +}
>> 
>> This looks like something that can become `Owned<Queue>` when Andreas' series
>> land?
>
> That's right.
>
> Alice