Alexandre Courbot
2025-Oct-27 12:20 UTC
[PATCH 5/7] gpu: nova-core: add extra conversion functions and traits
On Mon Oct 27, 2025 at 1:44 AM JST, Miguel Ojeda wrote:> On Sun, Oct 26, 2025 at 3:40?PM Alexandre Courbot <acourbot at nvidia.com> wrote: >> >> +/// Infallibly converts a `usize` to `u64`. >> +/// >> +/// This conversion is always lossless as Linux only supports 32-bit and 64-bit platforms, thus a >> +/// `usize` is always smaller than or of the same size as a `u64`. >> +/// >> +/// Prefer this over the `as` keyword to ensure no lossy conversions are performed. >> +/// >> +/// This is for use from a `const` context. For non `const` use, prefer the [`FromAs`] and >> +/// [`IntoAs`] traits. >> +pub(crate) const fn usize_as_u64(value: usize) -> u64 { >> + kernel::static_assert!(size_of::<u64>() >= size_of::<usize>()); >> + >> + value as u64 >> +} > > Since you have the static asserts, this is fine today. > > However, we may actually get 128-bit architectures in the > not-so-distant future -- Matthew suggests to be ready by 2035: > > https://lwn.net/Articles/908026/ > > So this one in particular may actually not be true "soon" -- we also > had related discussions about these assumptions, e.g.: > > https://lore.kernel.org/rust-for-linux/CANiq72m9AeqFKHrRniQ5Nr9vPv2MmUMHFTuuj5ydmqo+OYn60A at mail.gmail.com/ > > So we should consider having the `cfg` already to prevent people from > assuming it will be always available, and likely a note in its docs, > i.e. we may introducing trouble to port later on to new architectures. > Similarly, the docs of the trait may need rewording. > > What do you think?Do you mean adding a `#[cfg(any(CONFIG_32BIT, CONFIG_64BIT))]`? That sounds like a good idea. The static asserts will break whenever one of these functions needs to be protected by more conditional compilation anyway, but for consistency I agree it would make sense to add it now.> > Regarding the `.into_as()` name, it makes sense, but it can be a bit > surprising when reading out of context... The standalone functions are > super clear, in comparison. But I am not sure what could be better. > `into_in_this_arch()` or similar could emphasize that this will only > work in certain architectures, i.e. it is "an `into()` for this arch" > rather than the general one. > That would go well with the idea that you didn't implement it for > other obvious types, which I guess was to avoid developers using this > instead of `into()` by mistake, right?Exactly, the trait implementation is limited to conversions not already covered by `From` (because if there is a `From` implementation, it is obviously the preferred way to do it). The const functions, by contrast, need to cover all safe conversions as we cannot use `From` in a const context yet. I am happy to take suggestions for naming (I also think the current name is not great) - we could also consider dropping the trait altogether, but I find it more convenient for non-const contexts.> > (By the way, please use intra-doc links on the primitives too.)Thanks, it never occured to me that we could. ^_^;