Message-ID: <20260211132442.721db4d8@fedora>
Date: Wed, 11 Feb 2026 13:24:42 +0100
From: Boris Brezillon <boris.brezillon@...labora.com>
To: Philipp Stanner <phasta@...lbox.org>
Cc: phasta@...nel.org, David Airlie <airlied@...il.com>, Simona Vetter
 <simona@...ll.ch>, Danilo Krummrich <dakr@...nel.org>, Alice Ryhl
 <aliceryhl@...gle.com>, Gary Guo <gary@...yguo.net>, Benno Lossin
 <lossin@...nel.org>, Christian König
 <christian.koenig@....com>, Daniel Almeida <daniel.almeida@...labora.com>,
 Joel Fernandes <joelagnelf@...dia.com>, linux-kernel@...r.kernel.org,
 dri-devel@...ts.freedesktop.org, rust-for-linux@...r.kernel.org
Subject: Re: [RFC PATCH 3/4] rust/drm: Add DRM Jobqueue

On Wed, 11 Feb 2026 13:14:11 +0100
Philipp Stanner <phasta@...lbox.org> wrote:

> On Wed, 2026-02-11 at 12:59 +0100, Boris Brezillon wrote:
> > On Wed, 11 Feb 2026 12:19:56 +0100
> > Philipp Stanner <phasta@...lbox.org> wrote:
> >   
> > > On Wed, 2026-02-11 at 12:07 +0100, Boris Brezillon wrote:  
> > > > On Wed, 11 Feb 2026 11:47:27 +0100
> > > > Philipp Stanner <phasta@...lbox.org> wrote:
> > > >     
> > > > > On Tue, 2026-02-10 at 15:57 +0100, Boris Brezillon wrote:    
> > > > > > On Tue,  3 Feb 2026 09:14:02 +0100
> > > > > > Philipp Stanner <phasta@...nel.org> wrote:
> > > > > >       
> > > > > > > +/// A jobqueue Job.
> > > > > > > +///
> > > > > > > +/// You can stuff your data in it. The job will be borrowed back to your driver
> > > > > > > +/// once the time has come to run it.
> > > > > > > +///
> > > > > > > +/// Jobs are consumed by [`Jobqueue::submit_job`] by value (ownership transfer).
> > > > > > > +/// You can set multiple [`DmaFence`] as dependencies for a job. It will only
> > > > > > > +/// get run once all dependency fences have been signaled.
> > > > > > > +///
> > > > > > > +/// Jobs cost credits. Jobs will only be run if there are is enough capacity in
> > > > > > > +/// the jobqueue for the job's credits. It is legal to specify jobs costing 0
> > > > > > > +/// credits, effectively disabling that mechanism.
> > > > > > > +#[pin_data]
> > > > > > > +pub struct Job<T: 'static + Send> {
> > > > > > > +    cost: u32,
> > > > > > > +    #[pin]
> > > > > > > +    pub data: T,
> > > > > > > +    done_fence: Option<ARef<DmaFence<i32>>>,
> > > > > > > +    hardware_fence: Option<ARef<DmaFence<i32>>>,
> > > > > > > +    nr_of_deps: AtomicU32,
> > > > > > > +    dependencies: List<Dependency>,      
> > > > > > 
> > > > > > Given how tricky Lists are in rust, I'd recommend going for an XArray,
> > > > > > like we have on the C side. There's a bit of overhead when the job only
> > > > > > has a few deps, but I think simplicity beats memory-usage-optimizations
> > > > > > in that case (especially since the overhead exists and is accepted in
> > > > > > C).      
> > > > > 
> > > > > I mean, the list is now already implemented and works. Considering the
> > > > > XArray would have made sense during the development difficulties.    
> > > > 
> > > > I'm sure it does, but that's still more code/tricks to maintain than
> > > > what you'd have with the XArray abstraction.    
> > > 
> > > The solution than will rather be to make the linked list implementation
> > > better.
> > > 
> > > A list is the correct data structure in a huge number of use cases in
> > > the kernel. We should not begin here to defer to other structures
> > > because of convenience.
> > > 
> > > Btw. lists in Rust being so horrible has been repeatedly a reason why
> > > some other hackers argued that Rust as a language is not suitable for
> > > kernel development.
> > > 
> > > So getting that right seems more desirable than capitulating.  
> > 
> > I'm not capitulating, and I'm not saying "No list, never!" either. I'm
> > saying, if there's something that fits the bill and is easier to use,
> > maybe we should consider it...
> >   
> > >   
> > > >     
> > > > > 
> > > > > If it were to make sense we could certainly replace the list with an
> > > > > xarray, but I don't see an advantage. The JQ just needs to iterate over
> > > > > the dependencies to register its events on them, and on drop to
> > > > > deregister them perhaps.
> > > > > 
> > > > > We have many jobs, but likely only few dependencies per job, so the
> > > > > lower memory footprint seems desirable and the XArray's advantages
> > > > > don't come to play – except maybe if we'd want to consider to avoid the
> > > > > current unsafe-rawpointer solution to obtain the job, since obtaining a
> > > > > job from an Xarray is far faster than by list iteration.    
> > > > 
> > > > I don't think we need O(1) for picking random deps in a job, because
> > > > that's not something we need at all: the dep list here is used as a
> > > > FIFO.
> > > >     
> > > 
> > > Wrong. The dep list here has no ordering requirements at all. JQ does
> > > not care in which order it registers its events, it just cares about
> > > dealing with dep-fences racing.  
> > 
> > What I mean is that it's used as a FIFO right now, not that deps have to
> > be processed in order.  
> 
> Yeah, but it being a FIFO is irrelevant :)

I do think it's relevant actually. If the implementation does it as a
FIFO, then that means a container that's capable of providing a FIFO
abstraction is good enough :P. The fact that in theory it can be random
order dep checking is not important, because the implementation does
the check in dependency addition order, and it rightfully does so to
keep things simple => if we have to wait for all the deps anyway,
what's the point of trying to give them a fancy
will-likely-be-signaled-first-order.

> 
> >   
> > > 
> > > You could (de-)register your callbacks in random order, it does not
> > > matter.  
> > 
> > Again, that's not my point, and I think we're just saying the same
> > thing here: the list seems to be a good match for this dependency
> > array/list, because right now deps are processed in order. Now, being
> > the right construct in one language doesn't mean it's the right
> > construct in another language.
> >   
> > > 
> > > List and Xarray might be useful for the unsafe related to the
> > > DependencyWaker. There you could avoid a raw pointer by getting the job
> > > through a list iteration or through the hypothetical XArray.
> > > 
> > > Please take a look at my detailed code comments for DependencyWaker.  
> > 
> > Sure, I'll have a closer look.
> >   
> > >   
> > > >  There's the per-dep overhead of the ListLinks object maybe, but
> > > > it's certainly acceptable. And I don't think cache locality matters
> > > > either, because the XArray stores pointers too, so we'll still be one
> > > > deref away from the DmaFence. No, my main concern was maintainability,
> > > > because managing lists in rust is far from trivial, and as a developer,
> > > > I try to avoid using concepts the language I rely on is not friendly
> > > > with.    
> > > 
> > > This would be a decision with wide implications, as detailed above.
> > > 
> > > If we were to admit that lists just don't work in Rust, then wouldn't
> > > the consequent decision to remove them all together?  
> > 
> > I'm not going as far as saying they don't work, I'm just saying they
> > are trickier to use, and that's a fact.
> >   
> > > 
> > > "Lists in kernel-Rust are not supported. Too difficult to maintain.
> > > We're sorry. Use XArray et al. instead :("  
> > 
> > No, there are patterns where an XArray wouldn't be a good fit. For
> > instance, LRU lists where objects get moved between lists depending on
> > their usage pattern. If we were to use XArrays for that, that would
> > imply potential allocations in paths where we don't want them. In this
> > dep array case, the deps are added at submit time, and they get
> > progressively dropped, so the array can't grow, it can only ever
> > shrink, and XArray allows it to shrink from both ends (and even have
> > holes), so it sounds like a good match too. Not saying a perfect match,
> > not saying better than a list in general, but an XArray is easier to
> > use than a list **in rust**, and the fact it fits the bill (if it does
> > in C, it should in rust too) had me thinking that maybe it's what we
> > should use.  
> 
> Yoah, you have valid points.
> 
> Since XArray allows for dropping the unsafe {} without the performance
> penalty of a list-iteration, I think your idea for this particular case
> is good after all and can be put on the TODO list.
> 
> I'm not sure how soon I have the cycles for implementing that, though.
> Looks as if a ton of work is coming at us for dma_fence.

Let me know how we (the Tyr devs) can help with that.

Powered by blists - more mailing lists