Re: [PATCH v8 06/10] rust: io: use generic read/write accessors for primitive accesses

["Gary Guo" <gary@garyguo.net>]

On Tue Mar 10, 2026 at 1:59 AM GMT, Alexandre Courbot wrote:
> On Tue Mar 10, 2026 at 10:57 AM JST, Alexandre Courbot wrote:
>> On Tue Mar 10, 2026 at 12:29 AM JST, Gary Guo wrote:
>>> On Mon Mar 9, 2026 at 3:14 PM GMT, Alexandre Courbot wrote:
>>>> By providing the required `IoLoc` implementations on `usize`, we can
>>>> leverage the generic accessors and reduce the number of unsafe blocks in
>>>> the module.
>>>>
>>>> This also allows us to directly call the generic `read/write/update`
>>>> methods with primitive types, so add examples illustrating this.
>>>>
>>>> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
>>>> ---
>>>>  rust/kernel/io.rs | 199 +++++++++++++++++++++++++++++++++++-------------------
>>>>  1 file changed, 131 insertions(+), 68 deletions(-)
>>>>
>>>> diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs
>>>> index 1db6572f4a42..ed6fab001a39 100644
>>>> --- a/rust/kernel/io.rs
>>>> +++ b/rust/kernel/io.rs
>>>> @@ -197,6 +197,25 @@ pub trait IoLoc<T> {
>>>>      fn offset(&self) -> usize;
>>>>  }
>>>>  
>>>> +/// Implements [`IoLoc<$ty>`] for [`usize`], allowing to use `usize` as a parameter of
>>>> +/// [`Io::read`] and [`Io::write`].
>>>> +macro_rules! impl_usize_ioloc {
>>>> +    ($($ty:ty),*) => {
>>>> +        $(
>>>> +            impl IoLoc<$ty> for usize {
>>>> +                type IoType = $ty;
>>>> +
>>>
>>> #[inline(always)]
>>>
>>> the fact that this is a pointer is somewhat uneasy to me. I wonder if Clippy
>>> with its inlining tweak would cause optimisation failure here.
>>>
>>> Could this be just `fn offset(self)`?
>>
>> Yes, this was a remnant from a previous design where I needed to either
>> make `IoLoc` require `Copy` or use a reference here, so I opted for the
>> latter.
>>
>> With the current code though we are using `IoLoc`s exactly one time, so
>> we can consume `self` without requiring `Copy`.
>
> I also guess this method should be renamed `into_offset` then?

Calling this `offset` is fine, given that this is a trait method and is for I/O
plumbing and not really conversion. But I'm fine with `into_offset` too.

Also, most of the `IoLoc` types would be `Copy` anyway (even if it's not
required as a bound), neither makes a difference.

Best,
Gary