From: Joel Fernandes <joelagnelf@nvidia.com>
To: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>
Cc: Danilo Krummrich <dakr@kernel.org>,
abdiel.janulgue@gmail.com, daniel.almeida@collabora.com,
robin.murphy@arm.com, a.hindborg@kernel.org, ojeda@kernel.org,
alex.gaynor@gmail.com, boqun.feng@gmail.com, gary@garyguo.net,
bjorn3_gh@protonmail.com, lossin@kernel.org,
aliceryhl@google.com, tmgross@umich.edu, bhelgaas@google.com,
kwilczynski@kernel.org, gregkh@linuxfoundation.org,
rafael@kernel.org, rust-for-linux@vger.kernel.org,
linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/5] rust: dma: add DMA addressing capabilities
Date: Fri, 11 Jul 2025 19:45:58 -0400 [thread overview]
Message-ID: <4c885320-4fb7-4b6b-9dee-be65d1d6ec42@nvidia.com> (raw)
In-Reply-To: <CANiq72nP=u49vhj7+Z_digM+gKk0_=oAWUofbmyntyPsKy=+ew@mail.gmail.com>
On 7/11/2025 4:14 PM, Miguel Ojeda wrote:
> On Fri, Jul 11, 2025 at 9:35 PM Joel Fernandes <joelagnelf@nvidia.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
next prev parent reply other threads:[~2025-07-11 23:46 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-07-10 19:45 [PATCH 0/5] dma::Device trait and DMA mask Danilo Krummrich
2025-07-10 19:45 ` [PATCH 1/5] rust: dma: implement `dma::Device` trait Danilo Krummrich
2025-07-10 19:45 ` [PATCH 2/5] rust: dma: add DMA addressing capabilities Danilo Krummrich
2025-07-11 19:35 ` Joel Fernandes
2025-07-11 19:54 ` Danilo Krummrich
2025-07-11 23:40 ` Joel Fernandes
2025-07-11 20:14 ` Miguel Ojeda
2025-07-11 23:45 ` Joel Fernandes [this message]
2025-07-16 3:18 ` Alexandre Courbot
2025-07-16 8:04 ` Danilo Krummrich
2025-07-16 8:12 ` Alexandre Courbot
2025-07-16 9:15 ` Greg KH
2025-07-16 10:08 ` Danilo Krummrich
2025-07-16 10:20 ` Alice Ryhl
2025-07-10 19:45 ` [PATCH 3/5] rust: pci: implement the `dma::Device` trait Danilo Krummrich
2025-07-10 19:45 ` [PATCH 4/5] rust: platform: " Danilo Krummrich
2025-07-10 19:45 ` [PATCH 5/5] rust: samples: dma: set DMA mask Danilo Krummrich
2025-07-14 13:00 ` [PATCH 0/5] dma::Device trait and " Abdiel Janulgue
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4c885320-4fb7-4b6b-9dee-be65d1d6ec42@nvidia.com \
--to=joelagnelf@nvidia.com \
--cc=a.hindborg@kernel.org \
--cc=abdiel.janulgue@gmail.com \
--cc=alex.gaynor@gmail.com \
--cc=aliceryhl@google.com \
--cc=bhelgaas@google.com \
--cc=bjorn3_gh@protonmail.com \
--cc=boqun.feng@gmail.com \
--cc=dakr@kernel.org \
--cc=daniel.almeida@collabora.com \
--cc=gary@garyguo.net \
--cc=gregkh@linuxfoundation.org \
--cc=kwilczynski@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=lossin@kernel.org \
--cc=miguel.ojeda.sandonis@gmail.com \
--cc=ojeda@kernel.org \
--cc=rafael@kernel.org \
--cc=robin.murphy@arm.com \
--cc=rust-for-linux@vger.kernel.org \
--cc=tmgross@umich.edu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®