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

Carlos Bilbao
2025-02-09T16:20:00Z

Hello David,

On 2/9/25 00:44, David Airlie wrote:
> On Sun, Feb 9, 2025 at 9:55 AM Carlos Bilbao <bilbao@vt.edu> wrote:
>>
>> Hello,
>>
>> On 1/8/25 09:16, Miguel Ojeda wrote:
>>> On Wed, Jan 8, 2025 at 3:00 PM Christoph Hellwig <hch@lst.de> wrote:
>>>>
>>>> No rust code in kernel/dma, please.
>>>
>>> What do you suggest?
>>
>> This is it. What do people suggest? This thread has received a lot of
>> attention -- maybe it's an opportunity for the community to brainstorm.
>> Here's an idea:
>>
>> Some maintainers clearly hate wrappers/bindings to Rust, while others don't
>> mind as long as the R4L folks commit to the maintenance duties. Depending on
>> the subsystem, it will be a completely different conversation. It seems to
>> me that right now, every time the R4L folks interact with a new subsystem,
>> they need to understand the maintainer's stance. That looks like an
>> exhausting process.
>>
>
> That is the process we are committed to, everyone has a voice and gets
> to be heard, and we move forward in the appropriate manner with each
> maintainer. Bypassing maintainers is the last resort, not engaging at
> all with maintainers isn't a productive way forward either. Your below
> attempt is trying to create a technical solution to a social problem.
>
>> Would it make sense to have a C middleman that Rust calls instead of
>> binding directly to the C funcs? This dispatcher would provide a simpler,
>> stable API with fixed function signatures, even if the C functions change.
>> If a C func is removed/changed, only the C dispatcher would need to be
>> adapted, but until that happens, a new error could be returned to the Rust
>> side (not ideal, but Rust doesn't just break).
>>
>> This would obviously impose some limitations on the Rust side, but it might
>> be less conflictive than direct bindings. Also, in Abdiel's case here (for
>> example), he'd explicitly take on the maintenance of the Rust abstraction
>> and its DMA dispatchers, leaving no ambiguity about ownership.
>>
>
> This just adds overheads for everyone to little advantage for the
> cases where maintainers are fine with all the other approaches.

I'm not sure there's a need to worry about overhead in this hypothetical
case if the dispatcher is merely a translator and doesn't validate inputs,
copy buffers, or such. It would be less than calling from userspace.

What I meant to say (I wasn't clear before) was for this solution to apply
to cases where maintainers are firmly against all alternatives. Obviously,
if all the parties involved are happy, direct bindings are preferable.

>
> Also the bindings code in rust is usually pretty trivial to change,
> it's not like you can really abstract things that far, changing APIs
> is hard no matter what, understanding the nuances in every C driver
> can takes months of work adding rust bindings to your changes is
> rarely going to be the limiting factor. We have learned over the years
> to not make API changes that are subtle or hidden, and can leave out
> of tree or in development users broken in subtle ways and as long as
> we keep making API changes like that, updating the rust code is the
> least of anyone's worries. This isn't like having to learn Mandarin vs
> English, it's code written with a different syntax, it's not even
> perl.

I'm not sure there's a need to worry about overhead in this hypothetical
case if the dispatcher is merely a translator and doesn't validate inputs,
copy buffers, or such. It would be less than calling from userspace.

What I meant to say (and wasn't clear before) is that this solution could
apply to cases where maintainers are firmly against all better
alternatives. Of course, if all the parties involved are happy with direct
bindings, that's preferable. But as you said, this is a social problem, so
maybe it's not a one-size-fits-all solution.

I agree that in some cases, abstracting the API could be complicated, but
I’d focus on keeping it simple and providing Rust with the minimum subset
it needs to keep things moving forward. Abdiel's PR is a good example --
by keeping the functions in his CoherentAllocation, we keep that option,
but lose the granularity of DMA mapping attributes (which would be some
default in the C dispatcher).

I agree that making subtle API changes is risky, and it would def. be
something to keep an eye on.

And yes, with this approach, some Rust developers may eventually miss
certain DMA functionalities in the future; but that would still be better
than the current situation, where _all_ Rust devs are missing all DMA
capabilities.

>
> Dave.
>

Thanks,
Carlos