From: Lorenzo Delgado <lnsdev@proton.me>
To: Danilo Krummrich <dakr@kernel.org>, Miguel Ojeda <ojeda@kernel.org>
Cc: "Lorenzo Delgado" <lnsdev@proton.me>,
"Alice Ryhl" <aliceryhl@google.com>,
"Daniel Almeida" <daniel.almeida@collabora.com>,
"Alexandre Courbot" <acourbot@nvidia.com>,
"David Airlie" <airlied@gmail.com>,
"Simona Vetter" <simona@ffwll.ch>,
"Boqun Feng" <boqun@kernel.org>, "Gary Guo" <gary@garyguo.net>,
"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
"Benno Lossin" <lossin@kernel.org>,
"Andreas Hindborg" <a.hindborg@kernel.org>,
"Trevor Gross" <tmgross@umich.edu>,
"Tamir Duberstein" <tamird@kernel.org>,
"Onur Özkan" <work@onurozkan.dev>,
"Abdiel Janulgue" <abdiel.janulgue@gmail.com>,
"Robin Murphy" <robin.murphy@arm.com>,
"John Hubbard" <jhubbard@nvidia.com>,
"Timur Tabi" <ttabi@nvidia.com>,
rust-for-linux@vger.kernel.org, linux-kernel@vger.kernel.org,
driver-core@lists.linux.dev, dri-devel@lists.freedesktop.org,
nova-gpu@lists.linux.dev
Subject: Re: [PATCH] rust: io: convert ResourceSize into a transparent newtype
Date: Sun, 19 Jul 2026 10:40:19 +0000 [thread overview]
Message-ID: <20260719104012.364283-1-lnsdev@proton.me> (raw)
In-Reply-To: <DK0BPAHR8IS3.2KLSLH7K448RJ@kernel.org>
On Thu Jul 16, 2026 at 11:45 PM CEST, Danilo Krummrich wrote:
> On Sun Jul 12, 2026 at 1:36 PM CEST, Lorenzo Delgado wrote:
> > - let num_pages = usize::from_safe_cast(sg_entry.dma_len()).div_ceil(GSP_PAGE_SIZE);
> > + let num_pages = usize::try_from(sg_entry.dma_len())?.div_ceil(GSP_PAGE_SIZE);
>
> I think this is worse, as the conversion becomes fallible.
Agreed, that's a regression for nova-core, where the Kconfig guarantees
the value fits and the conversion should stay infallible. I'll drop the
try_from() there.
> You could implement From<ResourceSize> for u64 and then keep using
> usize::from_safe_cast() in nova-core.
I tried that, but it doesn't build. ResourceSize wraps resource_size_t,
which is u64 on 64-bit (CONFIG_PHYS_ADDR_T_64BIT), so the impl the patch
already has,
impl From<ResourceSize> for bindings::resource_size_t
is already From<ResourceSize> for u64 there, and a second one conflicts:
error[E0119]: conflicting implementations of trait
`From<ResourceSize>` for type `u64`
There's a simpler way that stays infallible and adds nothing to io.rs.
ResourceSize already has into_raw() (io/resource.rs uses it at the C
boundaries), so nova-core can do:
let num_pages =
usize::from_safe_cast(sg_entry.dma_len().into_raw()).div_ceil(GSP_PAGE_SIZE);
into_raw() gives back resource_size_t, and from_safe_cast handles that
as u32 or u64 depending on the config, so it stays infallible. I'll use
that in v2 unless you'd prefer something else.
> Alternatively, we could also consider moving the FromSafeCast trait to
> rust/kernel/num.rs and add FromSafeCast<ResourceSize> impls for usize.
> [...]
> However, by making it commonly availble I do see a risk with the
> cfg-gated impls silently breaking the build.
Agreed on the risk. A FromSafeCast<ResourceSize> for usize impl would
have to be cfg-gated like the u64 one, which is the same
silent-breakage-under-randconfig case you mention.
Since into_raw() keeps this patch self-contained, moving FromSafeCast
into the kernel crate is a separate change from the newtype conversion.
It seems worth doing on its own, and I'm happy to send it as its own
series so it gets reviewed as a new core API, but it doesn't need to
block this patch. I'll post v2 with the into_raw() change.
Thanks for the review.
Lorenzo
next prev parent reply other threads:[~2026-07-19 10:40 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-12 11:36 [PATCH] rust: io: convert ResourceSize into a transparent newtype Lorenzo Delgado
2026-07-16 21:45 ` Danilo Krummrich
2026-07-19 10:40 ` Lorenzo Delgado [this message]
2026-07-19 14:53 ` Danilo Krummrich
2026-07-19 20:53 ` Gary Guo
2026-07-19 20:09 ` [PATCH v2] " Lorenzo Delgado
2026-07-19 20:17 ` sashiko-bot
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=20260719104012.364283-1-lnsdev@proton.me \
--to=lnsdev@proton.me \
--cc=a.hindborg@kernel.org \
--cc=abdiel.janulgue@gmail.com \
--cc=acourbot@nvidia.com \
--cc=airlied@gmail.com \
--cc=aliceryhl@google.com \
--cc=bjorn3_gh@protonmail.com \
--cc=boqun@kernel.org \
--cc=dakr@kernel.org \
--cc=daniel.almeida@collabora.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=driver-core@lists.linux.dev \
--cc=gary@garyguo.net \
--cc=jhubbard@nvidia.com \
--cc=linux-kernel@vger.kernel.org \
--cc=lossin@kernel.org \
--cc=nova-gpu@lists.linux.dev \
--cc=ojeda@kernel.org \
--cc=robin.murphy@arm.com \
--cc=rust-for-linux@vger.kernel.org \
--cc=simona@ffwll.ch \
--cc=tamird@kernel.org \
--cc=tmgross@umich.edu \
--cc=ttabi@nvidia.com \
--cc=work@onurozkan.dev \
/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.