Message-ID: <4c885320-4fb7-4b6b-9dee-be65d1d6ec42@nvidia.com>
Date: Fri, 11 Jul 2025 19:45:58 -0400
From: Joel Fernandes <joelagnelf@...dia.com>
To: Miguel Ojeda <miguel.ojeda.sandonis@...il.com>
Cc: Danilo Krummrich <dakr@...nel.org>, abdiel.janulgue@...il.com,
 daniel.almeida@...labora.com, robin.murphy@....com, a.hindborg@...nel.org,
 ojeda@...nel.org, alex.gaynor@...il.com, boqun.feng@...il.com,
 gary@...yguo.net, bjorn3_gh@...tonmail.com, lossin@...nel.org,
 aliceryhl@...gle.com, tmgross@...ch.edu, bhelgaas@...gle.com,
 kwilczynski@...nel.org, gregkh@...uxfoundation.org, rafael@...nel.org,
 rust-for-linux@...r.kernel.org, linux-pci@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/5] rust: dma: add DMA addressing capabilities



On 7/11/2025 4:14 PM, Miguel Ojeda wrote:
> On Fri, Jul 11, 2025 at 9:35 PM Joel Fernandes <joelagnelf@...dia.com> wrote:
>>
>> 2. Since the Rust code is wrapping around the C code, the data race is
>> happening entirely on the C side right? So can we just rely on KCSAN to catch
>> concurrency issues instead of marking the callers as unsafe? I feel the
>> unsafe { } really might make the driver code ugly.
>>
>> 3. Maybe we could document this issue than enforce it via unsafe? My concern
>> is wrapping unsafe { } makes the calling code ugly.
> 
> Yeah, this sort of dilemma comes up from time to time, yeah, e.g. see:
> 
>     https://lore.kernel.org/rust-for-linux/CANiq72k_NNFsQ=GGCsur34CTYhSFC0m=mHS83mTB8HQCDBcW=w@mail.gmail.com/
>     https://lore.kernel.org/rust-for-linux/CANiq72m3WFj9Eb2iRUM3mLFibWW+cupAoNQt+cqtNa4O9=jq7Q@mail.gmail.com/
> 
> In short: that is the job of `unsafe {}` -- if we start to avoid it
> just to make code prettier, then it loses its power.
> 
> There are few alternatives/notes, though:
> 
>   - If there is a way to somehow guarantee or check that something is
> safe, perhaps with a tool like Klint, then that could allow us to
> avoid `unsafe`.
> 
>   - If the blocks are very repetitive in a single user and don't add
> any value, then one could consider having users write a single `unsafe
> {}` where they promise to uphold X instead of requiring it in further
> calls.
> 
>   - Worst case, we can promote something to the potential ASH list
> idea ("Acknowledged Soundness Holes"): a list where we document things
> that do not require `unsafe {}` that should require it but don't for
> strong practical reasons.
> 
>> 5. In theory, all rust bindings wrappers are unsafe and we do mark it around
>> the bindings call, right? But in this case, we're also making the calling
>> code of the unsafe caller as unsafe. C code is 'unsafe' obviously from Rust
>> PoV but I am not sure we worry about the internal implementation-unsafety of
>> the C code because then maybe most bindings wrappers would need to be unsafe,
>> not only these DMA ones.
> 
> It is orthogonal -- you may have a safe function that uses `unsafe {}`
> inside (e.g. to call a C function), but also you will see unsafe
> functions with just safe code inside. And, of course, safe functions
> with only safe code inside and unsafe functions with `unsafe {}`
> blocks inside.
> 
> In other words, a safe function does not mean unsafe code is used or
> not inside. Similarly, an unsafe function does not mean unsafe code is
> used (or not) inside either.
> 
> This is the usual "two meaning of `unsafe`" -- that of e.g. functions
> (where it means there are safety preconditions for calling a function)
> and that of e.g. `unsafe {}` blocks (where it means the caller must
> play by the rules, e.g. satisfy the safety preconditions to call an
> unsafe function).
> 
> So a Rust function that calls C functions (and thus uses `unsafe {}`)
> may or may not need to be unsafe -- it all depends on the case. That
> is, it depends on whether callers can cause UB or not. And similarly,
> a Rust function that does not use unsafe (including not calling C
> functions) can still be very much unsafe, because other code may rely
> on that code for soundness (e.g. an unsafe method that assigns to an
> internal pointer which then other safe methods rely on).
> 
Thanks for this clarification and write up! I need it so thank you very much Miguel!

 - Joel

Powered by blists - more mailing lists