Re: [PATCH v3 03/16] rust: mem: add `AsRepr` and `AsReprMut`
"Alexandre Courbot" <[email protected]>
| Newsgroups | dev.linux.lists.nova-gpu,dev.linux.lists.driver-core,org.freedesktop.lists.dri-devel,org.kernel.vger.linux-kernel,org.kernel.vger.linux-pci,org.kernel.vger.rust-for-linux |
|---|---|
| Message-ID | <[email protected]> |
On Wed Aug 26, 2026 at 8:50 PM JST, Gary Guo wrote: > On Wed Aug 26, 2026 at 12:25 PM BST, Alexandre Courbot wrote: >> On Wed Aug 19, 2026 at 8:09 PM JST, Gary Guo wrote: >>> Some API like atomics and I/O operate on primitives only; therefore other >>> types would need to converted to these primitive first. Add two traits >>> `AsRepr` and `AsReprMut` to indicate that the type can be turned into a >>> primitive for these operations. >>> >>> Conceptually, `T: AsRepr` means that `&T` can be viewed as `&T::Repr` and >>> thus it has only a round-trip transmutability requirement. `T: AsReprMut` >>> means that `&mut T` can be viewed as `&mut T::Repr` and thus it needs to >>> support bi-directional transmutability. >>> >>> To avoid duplicating implementation, all repr types are normalized to >>> unsigned integers. >>> >>> Signed-off-by: Gary Guo <[email protected]> >>> --- >>> rust/kernel/mem.rs | 148 +++++++++++++++++++++++++++++++++++++++++++++++++++++ >>> 1 file changed, 148 insertions(+) >>> >>> diff --git a/rust/kernel/mem.rs b/rust/kernel/mem.rs >>> index a0901cbe1b2d..5bce381d8895 100644 >>> --- a/rust/kernel/mem.rs >>> +++ b/rust/kernel/mem.rs >>> @@ -35,3 +35,151 @@ pub const fn transmute<Src: IntoBytes, Dst: FromBytes>(val: Src) -> Dst { >>> // SAFETY: transmute is safe with `IntoBytes` and `FromBytes` bounds. >>> unsafe { transmute_unchecked(val) } >>> } >>> + >>> +/// Type that is layout-compatible with a primitive representation. >>> +/// >>> +/// # Round-trip transmutability >>> +/// >>> +/// `T` is round-trip transmutable to `U` if and only if both of these properties hold: >>> +/// >>> +/// - Any valid bit pattern for `T` is also a valid bit pattern for `U`. >>> +/// - Transmuting a value of type `T` to `U` and then to `T` again >>> +/// yields a value that is in all aspects equivalent to the original value. >>> +/// >>> +/// # Safety >>> +/// >>> +/// - [`Self`] must have the same size and alignment as [`Self::Repr`]. >>> +/// - [`Self`] must be [round-trip transmutable] to [`Self::Repr`]. >>> +/// >>> +/// [round-trip transmutable]: AsRepr#round-trip-transmutability >>> +pub unsafe trait AsRepr: Sized { >> >> I'm a bit confused by the naming of this trait - the commit message >> mentions that it means that "`&T` can be viewed as `&T::Repr`", but such >> a method doesn't exist. Instead we have an `AsRepr` trait with an >> `into_repr` method that makes a copy. >> >> Can we either rename this to `IntoRepr` (and `AsReprMut` into `FromRepr` >> I guess?), or maybe better, have `fn as_repr(this: &Self) -> >> &Self::Repr` (on top of which `into_repr` could be implemented if >> useful) so the names of the trait and provided method align, while also >> making the commit message match the actual API? I haven't tried but >> since all the representations we are working with are primitives that >> implement `Copy`, I intuitively think it should work just as well. >> >> Same would apply to `AsReprMut`. >> >> Since this is becoming core infrastructure, I guess `as_repr` will also >> become valuable when we want to do in-place access (with e.g. atomics) >> or use it with larger types that we don't want to copy. > > Good point. I'll add a `Repr: Copy` bound and switch impl of `as_repr`. > > `AsRepr` is meant as "have a primitive representation", which I imagine to be > most likely just integers. I didn't say "integers" specifically because we > cannot lose provenance on pointers so pointers are not integer represented. > > So I do think we might also want to make `Copy` a supertrait of `AsRepr`? > `AtomicType` current requires it. Yes, unless we have a use for move-only register values or something of the sort, I guess requiring `Copy` would be pretty harmless here. <...> >>> +#[cfg(target_pointer_width = "32")] >>> +const _: () = { >>> + // SAFETY: usize has the same size and alignment with u32, and is round-trip transmutable to it. >>> + unsafe impl AsRepr for usize { >>> + type Repr = u32; >>> + } >>> + >>> + // SAFETY: isize has the same size and alignment with u32, and is round-trip transmutable to it. >>> + unsafe impl AsRepr for isize { >>> + type Repr = u32; >>> + } >>> + >>> + // SAFETY: usize is transmutable from u32. >>> + unsafe impl AsReprMut for usize {} >>> + // SAFETY: isize is transmutable from u32. >>> + unsafe impl AsReprMut for isize {} >>> +}; >>> + >>> +#[cfg(target_pointer_width = "64")] >>> +const _: () = { >>> + // SAFETY: usize has the same size and alignment with u64, and is round-trip transmutable to it. >>> + unsafe impl AsRepr for usize { >>> + type Repr = u64; >>> + } >>> + >>> + // SAFETY: isize has the same size and alignment with u64, and is round-trip transmutable to it. >>> + unsafe impl AsRepr for isize { >>> + type Repr = u64; >>> + } >>> + >>> + // SAFETY: usize is transmutable from u64. >>> + unsafe impl AsReprMut for usize {} >>> + // SAFETY: isize is transmutable from u64. >>> + unsafe impl AsReprMut for isize {} >>> +}; >> >> By making these available, aren't we running into the same >> non-portability issue [1] that we discussed on the `casts` module? >> >> [1] https://lore.kernel.org/all/[email protected]/ > > I added it because `Atomic` would require it. However, it does make it possible > to create non-portable if `usize` is used on a `I/O` type that does not > implement `IoCapable<u64>` on 64-bit platforms. > > I guess an option is to have `usize` stay as `usize` and we add a forwarding > `usize` impl in atomic module instead. That sounds like a good way to work around the problem - it's basically how we address it with casts if I understand your intent correctly.