From: "Alexandre Courbot" <acourbot@nvidia.com>
To: "Christian S. Lima" <christiansantoslima21@gmail.com>,
"Miguel Ojeda" <ojeda@kernel.org>,
"Alex Gaynor" <alex.gaynor@gmail.com>,
"Boqun Feng" <boqun.feng@gmail.com>,
"Gary Guo" <gary@garyguo.net>,
"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
"Benno Lossin" <benno.lossin@proton.me>,
"Andreas Hindborg" <a.hindborg@kernel.org>,
"Alice Ryhl" <aliceryhl@google.com>,
"Trevor Gross" <tmgross@umich.edu>,
"Danilo Krummrich" <dakr@kernel.org>,
rust-for-linux@vger.kernel.org, linux-kernel@vger.kernel.org,
~lkcamp/patches@lists.sr.ht, richard120310@gmail.com
Subject: Re: [PATCH v9] rust: transmute: Add methods for FromBytes trait
Date: Tue, 12 Aug 2025 23:24:25 +0900 [thread overview]
Message-ID: <DC0INEKNZPSE.WLEQH24SLW1I@nvidia.com> (raw)
In-Reply-To: <20250811213851.65644-1-christiansantoslima21@gmail.com>
On Tue Aug 12, 2025 at 6:38 AM JST, Christian S. Lima wrote:
> The two methods added take a slice of bytes and return those bytes in a
> specific type. These methods are useful when we need to transform the
> stream of bytes into specific type.
>
> The `Frombytessized` trait was added to make it easier to implement other
typo: FromBytesSized.
> user defined types within the codebase. With the current implementation,
> there's no way to interact without implementing `from_bytes` and
> `from_mut_bytes` for every new type, and this would end up generating a lot
> of duplicate code. By using FromBytesSized as a proxy trait, we can avoid
> this without generating a direct dependency. If necessary, the user can
> simply implement `FromBytes` if needed. For more context, please check the
> [1] and [2].
>
> [1] https://lore.kernel.org/rust-for-linux/DANSZ6Q476EC.3GY00K717QVUL@nvidia.com/
> [2] https://lore.kernel.org/rust-for-linux/DAOESYD6F287.3U3M64X0S1WN5@nvidia.com/
>
> Link: https://github.com/Rust-for-Linux/linux/issues/1119
> Suggested-by: Alexandre Courbot <acourbot@nvidia.com>
> Signed-off-by: Christian S. Lima <christiansantoslima21@gmail.com>
I mentioned it on v8 [1] and v7 [2], but the tests that break due to
this change need to be updated by this patch as well. This includes:
* The doctests in `rust/kernel/dma.rs`,
* The `samples/rust/rust_dma.rs` sample,
* The example for `FromBytes` introduced by this patch which uses `?` without
defining a function.
Please make sure to compile with `CONFIG_RUST_KERNEL_DOCTESTS` and
`CONFIG_SAMPLE_RUST_DMA` to see the failures.
Also, now that we are on 6.17-rc1, the types in `nova-core` that implement
`FromBytes` will also fail to build unless they are switched to
`FromBytesSized`. Which I guess speaks in favor of taking this into the Nova
tree if there are not other planned user for this cycle?
[1]
https://lore.kernel.org/rust-for-linux/DB5KEWX9EJ2Q.3CX5EGS66OVHH@nvidia.com/
[2]
https://lore.kernel.org/rust-for-linux/DANSZ6Q476EC.3GY00K717QVUL@nvidia.com/
In case this helps, here is the diff I needed to apply to get the tests
to build:
diff --git a/rust/kernel/dma.rs b/rust/kernel/dma.rs
index 1801836f31455..3797c70c13040 100644
--- a/rust/kernel/dma.rs
+++ b/rust/kernel/dma.rs
@@ -597,7 +597,7 @@ unsafe impl<T: AsBytes + FromBytes + Send> Send for CoherentAllocation<T> {}
/// struct MyStruct { field: u32, }
///
/// // SAFETY: All bit patterns are acceptable values for `MyStruct`.
-/// unsafe impl kernel::transmute::FromBytes for MyStruct{};
+/// unsafe impl kernel::transmute::FromBytesSized for MyStruct{};
/// // SAFETY: Instances of `MyStruct` have no uninitialized portions.
/// unsafe impl kernel::transmute::AsBytes for MyStruct{};
///
@@ -641,7 +641,7 @@ macro_rules! dma_read {
/// struct MyStruct { member: u32, }
///
/// // SAFETY: All bit patterns are acceptable values for `MyStruct`.
-/// unsafe impl kernel::transmute::FromBytes for MyStruct{};
+/// unsafe impl kernel::transmute::FromBytesSized for MyStruct{};
/// // SAFETY: Instances of `MyStruct` have no uninitialized portions.
/// unsafe impl kernel::transmute::AsBytes for MyStruct{};
///
diff --git a/rust/kernel/transmute.rs b/rust/kernel/transmute.rs
index ba21fe49e4f07..e7bd698ec99cd 100644
--- a/rust/kernel/transmute.rs
+++ b/rust/kernel/transmute.rs
@@ -16,6 +16,7 @@
/// ```
/// use kernel::transmute::FromBytes;
///
+/// # fn test() -> Option<()> {
/// let foo = [1, 2, 3, 4];
///
/// let result = u32::from_bytes(&foo)?;
@@ -25,6 +26,7 @@
///
/// #[cfg(target_endian = "big")]
/// assert_eq!(*result, 0x1020304);
+/// # Some(()) }
/// ```
///
/// # Safety
diff --git a/samples/rust/rust_dma.rs b/samples/rust/rust_dma.rs
index 9fb3ae1dd8587..36ad877d4804d 100644
--- a/samples/rust/rust_dma.rs
+++ b/samples/rust/rust_dma.rs
@@ -65,7 +65,7 @@ fn new(h: u32, b: u32) -> Self {
// SAFETY: All bit patterns are acceptable values for `MyStruct`.
unsafe impl kernel::transmute::AsBytes for MyStruct {}
// SAFETY: Instances of `MyStruct` have no uninitialized portions.
-unsafe impl kernel::transmute::FromBytes for MyStruct {}
+unsafe impl kernel::transmute::FromBytesSized for MyStruct {}
kernel::pci_device_table!(
PCI_TABLE,
-- end of diff --
Otherwise the patch in itself looks good to me. It is used in nova-core (and a
critical dependency for this cycle :)), so although I cannot give my
Reviewed-by until the tests are fixed, it is at least
Tested-by: Alexandre Courbot <acourbot@nvidia.com>
next prev parent reply other threads:[~2025-08-12 14:24 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-08-11 21:38 [PATCH v9] rust: transmute: Add methods for FromBytes trait Christian S. Lima
2025-08-12 14:24 ` Alexandre Courbot [this message]
2025-08-12 18:00 ` Christian
2025-08-18 11:28 ` Alexandre Courbot
2025-08-18 12:20 ` Alice Ryhl
2025-08-18 19:04 ` Christian
2025-08-21 11:13 ` Alexandre Courbot
2025-08-21 13:50 ` Miguel Ojeda
2025-08-22 3:09 ` Alexandre Courbot
2025-08-22 8:07 ` Miguel Ojeda
2025-08-22 13:49 ` Alexandre Courbot
2025-08-22 20:08 ` Miguel Ojeda
2025-08-13 1:06 ` kernel test robot
2025-08-14 8:06 ` Benno Lossin
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=DC0INEKNZPSE.WLEQH24SLW1I@nvidia.com \
--to=acourbot@nvidia.com \
--cc=a.hindborg@kernel.org \
--cc=alex.gaynor@gmail.com \
--cc=aliceryhl@google.com \
--cc=benno.lossin@proton.me \
--cc=bjorn3_gh@protonmail.com \
--cc=boqun.feng@gmail.com \
--cc=christiansantoslima21@gmail.com \
--cc=dakr@kernel.org \
--cc=gary@garyguo.net \
--cc=linux-kernel@vger.kernel.org \
--cc=ojeda@kernel.org \
--cc=richard120310@gmail.com \
--cc=rust-for-linux@vger.kernel.org \
--cc=tmgross@umich.edu \
--cc=~lkcamp/patches@lists.sr.ht \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.