Message-Id: <DDTRX3S7VYX1.1L0T28KA1NJ5M@nvidia.com>
Date: Tue, 28 Oct 2025 16:23:32 +0900
From: "Alexandre Courbot" <acourbot@...dia.com>
To: "Joel Fernandes" <joelagnelf@...dia.com>, "Alice Ryhl"
 <aliceryhl@...gle.com>, "David Airlie" <airlied@...il.com>, "Simona Vetter"
 <simona@...ll.ch>, "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" <lossin@...nel.org>, "Andreas
 Hindborg" <a.hindborg@...nel.org>, "Trevor Gross" <tmgross@...ch.edu>
Cc: "John Hubbard" <jhubbard@...dia.com>, "Alistair Popple"
 <apopple@...dia.com>, "Timur Tabi" <ttabi@...dia.com>, "Edwin Peer"
 <epeer@...dia.com>, <nouveau@...ts.freedesktop.org>,
 <dri-devel@...ts.freedesktop.org>, <linux-kernel@...r.kernel.org>,
 <rust-for-linux@...r.kernel.org>, "Danilo Krummrich" <dakr@...nel.org>,
 "Nouveau" <nouveau-bounces@...ts.freedesktop.org>
Subject: Re: [PATCH v2 5/7] gpu: nova-core: add extra integer conversion
 functions and traits

On Tue Oct 28, 2025 at 4:09 AM JST, Joel Fernandes wrote:
> Hello Alex,
>
> On 10/27/2025 8:54 AM, Alexandre Courbot wrote:
>> The core library's `From` implementations do not cover conversions
>> that are not portable or future-proof. For instance, even though it is
>> safe today, `From<usize>` is not implemented for `u64` because of the
>> possibility to support larger-than-64bit architectures in the future.
>> 
>> However, the kernel supports a narrower set of architectures, and in the
>> case of Nova we only support 64-bit. This makes it helpful and desirable
>> to provide more infallible conversions, lest we need to rely on the `as`
>> keyword and carry the risk of silently losing data.
>> 
>> Thus, introduce a new module `num` that provides safe const functions
>> performing more conversions allowed by the build target, as well as
>> `FromAs` and `IntoAs` traits that are just extensions of `From` and
>> `Into` to conversions that are known to be lossless.
>
> Why not just implement `From` and `Into` for the types missing it then, with
> adequate comments about why such conversions are Ok for the kernel, instead of
> introducing a new trait? This is exactly what `From`/`Into` is for right?

I wish we could. :) The link Danilo sent should clarify why this is not
possible.

<snip>
>> +impl_lossless_as!(u8 as { u16, u32, u64, usize });
>> +impl_lossless_as!(u16 as { u32, u64, usize });
>> +impl_lossless_as!(u32 as { u64, usize } );
>
> I prefer consistency with the notation in num.rs in the rust standard library:
> impl_from!(u16 => usize)
> But I am Ok with your notation as well, which is concise.

You're right, something like `impl_as` is both shorter and more
consistent.

>
>> +// `u64` and `usize` have the same size on 64-bit platforms.
>> +#[cfg(CONFIG_64BIT)]
>> +impl_lossless_as!(u64 as { usize } );
>> +
>> +// A `usize` fits into a `u64` on 32 and 64-bit platforms.
>> +#[cfg(any(CONFIG_32BIT, CONFIG_64BIT))]
>> +impl_lossless_as!(usize as { u64 });
>> +
>> +// A `usize` fits into a `u32` on 32-bit platforms.
>> +#[cfg(CONFIG_32BIT)]
>> +impl_lossless_as!(usize as { u32 });
>> +
>> +/// Extension trait providing guaranteed lossless conversion to `Self` from `T`.
>> +///
>> +/// The standard library's `From` implementations do not cover conversions that are not portable or
>> +/// future-proof. For instance, even though it is safe today, `From<usize>` is not implemented for
>> +/// [`u64`] because of the possibility to support larger-than-64bit architectures in the future.
>> +///
>> +/// The workaround is to either deal with the error handling of [`TryFrom`] for an operation that
>> +/// technically cannot fail, or to use the `as` keyword, which can silently strip data if the
>> +/// destination type is smaller than the source.
>> +///
>> +/// Both options are hardly acceptable for the kernel. It is also a much more architecture
>> +/// dependent environment, supporting only 32 and 64 bit architectures, with some modules
>> +/// explicitly depending on a specific bus width that could greatly benefit from infallible
>> +/// conversion operations.
>> +///
>> +/// Thus this extension trait that provides, for the architecture the kernel is built for, safe
>> +/// conversion between types for which such conversion is lossless.
>> +///
>> +/// In other words, this trait is implemented if, for the current build target and with `t: T`, the
>> +/// `t as Self` operation is completely lossless.
>> +///
>> +/// Prefer this over the `as` keyword to ensure no lossy conversions are performed.
>> +///
>> +/// If you need to perform a conversion in `const` context, use [`u64_as_usize`],
>> +/// [`u32_as_usize`], [`usize_as_u64`], etc.
>> +///
>> +/// # Examples
>> +///
>> +/// ```
>> +/// use crate::num::FromAs;
>> +///
>> +/// assert_eq!(usize::from_as(0xf00u32), 0xf00u32 as usize);
>
> This `from_as()` syntax will be very confusing for users IMO, honestly we should
> just keep it as `from()`, otherwise there is confusion and ambiguity around
> whether someone should use `::from()` or `::from_as()`. Please let us just keep
> all infallible conversions to use `from()`/`into()` and all infallible ones to
> use `try_from()`/`try_into()`. No need to additional `_as()` prefix, as it
> creates confusion. In the end of the day, the fact the conversion is lossless is
> not relevant, all conversions that don't use the `as` keyword should be expected
> to be lossless right?

I wanted to use `from`/`into` initially but this unfortunately clashes
with the `From` and `Into` traits and forces callers to disambiguate
which trait we want to use with a turbofish, which goes against the
intent of keeping the syntax short. I'm happy to consider better names
though.

Powered by blists - more mailing lists