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

Carlos Bilbao
2025-02-09T16:28:28Z

On 2/9/25 10:19, Carlos Bilbao wrote:

> 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 accidentally copied this twice after rephrasing, not trying to be
annoying x).

>
> 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