public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
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).

  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