Re: [PATCH v8 2/2] rust: add dma coherent allocator abstraction.

Simona Vetter
2025-01-17T13:56:13Z

On Thu, Jan 16, 2025 at 04:57:56PM +0100, Danilo Krummrich wrote:
> On Thu, Jan 16, 2025 at 01:57:50PM +0000, Robin Murphy wrote:
> > On 2025-01-16 1:17 pm, Danilo Krummrich wrote:
> > > On Fri, Jan 10, 2025 at 11:41:54AM +0100, Danilo Krummrich wrote:
> > > > On Fri, Jan 10, 2025 at 09:39:55AM +0100, Christoph Hellwig wrote:
> > > > > On Thu, Jan 09, 2025 at 09:49:47AM +0100, Danilo Krummrich wrote:
> > > > > > On Thu, Jan 09, 2025 at 09:08:12AM +0100, Christoph Hellwig wrote:
> > > > > > > On Wed, Jan 08, 2025 at 04:21:33PM +0100, Danilo Krummrich wrote:
> > > > > > > > What does "your code" mean? Duplicated in every driver?
> > > > > > >
> > > > > > > Yes, interfaces to the DMA API should stay in readable C code and not
> > > > > > > in weird bindings so that it reminds greppable and maintainable.
> > > > > > >
> > > > > >
> > > > > > Rust drivers shouldn't use C APIs directly, but rather use an abstraction of the
> > > > > > corresponding C API.
> > > > >
> > > > > Don't force me to deal with your shiny language of the day.
> > > >
> > > > Again, no one asks you to deal with or maintain this piece of Rust code.
> > > >
> > > > > Maintaining
> > > > > multi-language projects is a pain I have no interest in dealing with.
> > > > > If you want to use something that's not C, be that assembly or rust you
> > > > > write to C interfaces and deal with the impedence mismatch yourself as
> > > > > far as I'm concerned.
> > > > >
> > > >
> > > > This is exactly what we're doing and proposing here, isn't it?
> > > >
> > > > We wrote a single piece of Rust code that abstracts the C API for all Rust
> > > > drivers, which we offer to maintain ourselves.
> > > >
> > > > What else are you asking for?
> > >
> > > Since there hasn't been a reply so far, I assume that we're good with
> > > maintaining the DMA Rust abstractions separately.
> >
> > Indeed, FWIW it seems like the appropriate level of abstraction to me,
> > judging by the other wrappers living in rust/kernel/ already. As far as the
> > interaction with C code goes, it appears to be a pretty straightforward
> > midlayer consumer of the DMA API much like others we already have (e.g.
> > videobuf2-dma-*), just one which happens to be a language binding rather
> > than some other kind of functional abstraction.
> >
> > There is a realistic chance that the C API will evolve in ways which break
> > the binding, but as long as a) that won't break non-Rust builds, and b) Rust
> > folks are happy to take responsibility for un-breaking the Rust build if and
> > when it happens, then that seems reasonable IMO.
>
> Surely you can expect maintainers of the Rust abstraction to help with
> integrating API changes -- this isn't different compared to driver / component
> maintainers helping with integrating fundamental API changes for their affected
> driver / component, like you've mentioned videobuf2-dma stuff.
>
> At last year's LPC I held a talk [1] about Rust in the kernel and there was a
> question at the end where I was asked how I think about cases where Rust
> abstraction break caused C API changes.
>
> My answer was that I'm not too concerned about this. We can just rely on what
> already happens every day in kernel development: people work together and
> collaborate. There are a lot of very core components that are widely used and
> depending on the complexity of the change may require the help of the users to
> integrate changes. So, I don't think with Rust abstractions we're adding
> anything that the kernel does not already has a strategy to deal with.

I think when we know that an api change is in the works we should try to
anticipate that, and not add another complex user of a feature that's
likely going to disappear. For dma-api I guess the big thing is the
dma/page split that's been talked about since forever, instead of mixing
it all up into the scatterlist structure.

But with rust abstractions it should be pretty easy to make sure the rust
side interface is really clean here, and so any refactoring limited to
just the middlelayer. Plus this patch here is not even close to tackling
the sg based dma-api parts.

Same applies for rust wrapping dma-buf interfaces, that's just more of the
same.
-Sima

>
> - Danilo
>
> [1] https://youtu.be/3Igmx28B3BQ?si=wD0CP-zku4U6fAsN
>
> >
> > Thanks,
> > Robin.
> >
> > >
> > > Hence, the next version of this patch series will have the corresponding
> > > maintainer entry.
> > >
> > > - Danilo
> >

--
Simona Vetter
Software Engineer, Intel Corporation