Message-Id: <DC828042PKDV.16NDVKIGBNTJH@nvidia.com>
Date: Thu, 21 Aug 2025 20:13:20 +0900
From: "Alexandre Courbot" <acourbot@...dia.com>
To: "Christian" <christiansantoslima21@...il.com>, "Miguel Ojeda"
 <ojeda@...nel.org>
Cc: "Miguel Ojeda" <ojeda@...nel.org>, "Alex Gaynor"
 <alex.gaynor@...il.com>, "Boqun Feng" <boqun.feng@...il.com>, "Gary Guo"
 <gary@...yguo.net>, Björn Roy Baron
 <bjorn3_gh@...tonmail.com>, "Benno Lossin" <benno.lossin@...ton.me>,
 "Andreas Hindborg" <a.hindborg@...nel.org>, "Alice Ryhl"
 <aliceryhl@...gle.com>, "Trevor Gross" <tmgross@...ch.edu>, "Danilo
 Krummrich" <dakr@...nel.org>, <rust-for-linux@...r.kernel.org>,
 <linux-kernel@...r.kernel.org>, <~lkcamp/patches@...ts.sr.ht>,
 <richard120310@...il.com>
Subject: Re: [PATCH v9] rust: transmute: Add methods for FromBytes trait

Hi Christian,

On Tue Aug 19, 2025 at 4:04 AM JST, Christian wrote:
> Hi, Alexandre.
>
> On Mon, Aug 18, 2025 at 8:28 AM Alexandre Courbot <acourbot@...dia.com> wrote:
>>
>> On Wed Aug 13, 2025 at 3:00 AM JST, Christian wrote:
>> > Hi, Alexandre.
>> >
>> >> 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.
>> >
>> > Sorry for my inattention, I'll fix this in the next version.
>>
>> Ah, as it turns out you might not need to.
>>
>> We discussed this patch a bit during the DRM sync, and the consensus was
>> that it would probably be better to keep things a bit simpler for the
>> first version. The `FromBytesSized` trait in particular was not very
>> popular; a better long-term way to provide implementations for
>> `FromBytes` would be to use a derive macro, but that's out of scope for
>> now.
>>
>> Instead, we agreed that the following would make a good first version:
>>
>> - Make the `FromBytes` trait depend on `Sized`,
>> - Provide default implementations for `from_bytes` and `from_bytes_mut`
>>   directly in the `FromBytes` trait,
>> - No implementation for slices for now,
>> - Consequently, no user code will break due to the addition of the
>>   methods, which is a big plus.
>>
>> The simpler version that would result from this covers all the immediate
>> use-cases and would be easier to merge, which will give us some time to
>> think about how to handle the non-sized use cases (probably via a derive
>> macro).
>>
>> Do you think you could write the next version along these lines?
>
> Yes, no problem.

Thanks - I also noticed a few other things while building with rustc
1.78 and clippy enabled:

- The `is_aligned` method for pointer types has only been stabilized
  with 1.79, meaning we need to explicitly enable the feature in
  `rust/kernel/lib.rs`:

diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index ed53169e795c0..c859a8984bae1 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -18,6 +18,7 @@
 //
 // Stable since Rust 1.79.0.
 #![feature(inline_const)]
+#![feature(pointer_is_aligned)]
 //
 // Stable since Rust 1.81.0.
 #![feature(lint_reasons)]

(adding Miguel as a heads-up, and in case enabling a feature is not as easy as
I think :))

- Clippy was no happy with our use of `foo` in the doctest. This is easy to
fix:

diff --git a/rust/kernel/transmute.rs b/rust/kernel/transmute.rs
index 9bb707aabdc9a..9e4413fe9c32a 100644
--- a/rust/kernel/transmute.rs
+++ b/rust/kernel/transmute.rs
@@ -14,9 +14,9 @@
 /// ```
 /// use kernel::transmute::FromBytes;
 ///
-/// let foo = [1, 2, 3, 4];
+/// let raw = [1, 2, 3, 4];
 ///
-/// let result = u32::from_bytes(&foo).unwrap();
+/// let result = u32::from_bytes(&raw).unwrap();
 ///
 /// #[cfg(target_endian = "little")]
 /// assert_eq!(*result, 0x4030201);

- The patch adding `size_of*` to the prelude is not available yet, so for
safety we should probably import them:

diff --git a/rust/kernel/transmute.rs b/rust/kernel/transmute.rs
index 9e4413fe9c32a..3320e9e95057f 100644
--- a/rust/kernel/transmute.rs
+++ b/rust/kernel/transmute.rs
@@ -2,6 +2,8 @@

 //! Traits for transmuting types.

+use core::mem::{size_of, size_of_val};
+
 /// Types for which any bit pattern is valid.
 ///
 /// Not all types are valid for all values. For example, a `bool` must be either zero or one, so

With that, and the simplification discussed above, everything should be ok.

Do you think you can send the next iteration soonish? We have a consequent
patch series on nova-core that I would like to send which depends on this, and
since it is also very handy to have generally speaking it would be nice to have
it secured for the next merge window.

Powered by blists - more mailing lists