Message-Id: <485A6C23-9E6A-45C6-8F3F-377E538F052E@collabora.com>
Date: Tue, 26 Aug 2025 11:28:23 -0300
From: Daniel Almeida <daniel.almeida@...labora.com>
To: Lyude Paul <lyude@...hat.com>
Cc: rust-for-linux@...r.kernel.org,
 linux-kernel@...r.kernel.org,
 Thomas Gleixner <tglx@...utronix.de>,
 Andreas Hindborg <a.hindborg@...nel.org>,
 FUJITA Tomonori <fujita.tomonori@...il.com>,
 Boqun Feng <boqun.feng@...il.com>,
 Frederic Weisbecker <frederic@...nel.org>,
 Anna-Maria Behnsen <anna-maria@...utronix.de>,
 John Stultz <jstultz@...gle.com>,
 Stephen Boyd <sboyd@...nel.org>,
 Miguel Ojeda <ojeda@...nel.org>,
 Alex Gaynor <alex.gaynor@...il.com>,
 Gary Guo <gary@...yguo.net>,
 Björn Roy Baron <bjorn3_gh@...tonmail.com>,
 Benno Lossin <lossin@...nel.org>,
 Alice Ryhl <aliceryhl@...gle.com>,
 Trevor Gross <tmgross@...ch.edu>,
 Danilo Krummrich <dakr@...nel.org>
Subject: Re: [PATCH v3 2/2] rust: time: Implement basic arithmetic operations
 for Delta



> On 20 Aug 2025, at 17:26, Lyude Paul <lyude@...hat.com> wrote:
> 
> While rvkms is only going to be using a few of these, since Deltas are
> basically the same as i64 it's easy enough to just implement all of the
> basic arithmetic operations for Delta types.
> 
> Keep in mind there's one quirk here - the kernel has no support for
> i64 % i64 on 32 bit platforms, the closest we have is i64 % i32 through
> div_s64_rem(). So, instead of implementing ops::Rem or ops::RemAssign we
> simply provide Delta::rem_nanos().
> 
> Signed-off-by: Lyude Paul <lyude@...hat.com>
> 
> ---
> V2:
> * Don't forget to make sure that we inline all of these
> * Drop ops::Rem and ops::RemAssign implementations for Delta, replace with
>  Delta::rem_nanos() instead. It turns out that there's actually no way to
>  perform i64 % i64 on 32 bit platforms in the kernel at the moment, the
>  closest that we have is div_s64_rem() which only allows a 32 bit divisor.
> * Actually use the kernel arithmetic helpers for division/remainders so
>  that this code works on both 32 and 64 bit platforms.
> V3:
> * Change the output type for Div to i64, drop DivAssign
> * Change Mul/MulAssign to accept i64, not another Delta
> * Fix parameter name in rem_nanos (ns -> dividend)
> 
> rust/kernel/time.rs | 98 +++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 98 insertions(+)
> 
> diff --git a/rust/kernel/time.rs b/rust/kernel/time.rs
> index 4bd7a8a009f3e..e64c5b13152dd 100644
> --- a/rust/kernel/time.rs
> +++ b/rust/kernel/time.rs
> @@ -265,6 +265,78 @@ pub struct Delta {
>     nanos: i64,
> }
> 
> +impl ops::Add for Delta {
> +    type Output = Self;
> +
> +    #[inline]
> +    fn add(self, rhs: Self) -> Self {
> +        Self {
> +            nanos: self.nanos + rhs.nanos,
> +        }
> +    }
> +}
> +
> +impl ops::AddAssign for Delta {
> +    #[inline]
> +    fn add_assign(&mut self, rhs: Self) {
> +        self.nanos += rhs.nanos;
> +    }
> +}
> +
> +impl ops::Sub for Delta {
> +    type Output = Self;
> +
> +    #[inline]
> +    fn sub(self, rhs: Self) -> Self::Output {
> +        Self {
> +            nanos: self.nanos - rhs.nanos,
> +        }
> +    }
> +}
> +
> +impl ops::SubAssign for Delta {
> +    #[inline]
> +    fn sub_assign(&mut self, rhs: Self) {
> +        self.nanos -= rhs.nanos;
> +    }
> +}
> +
> +impl ops::Mul<i64> for Delta {
> +    type Output = Self;
> +
> +    #[inline]
> +    fn mul(self, rhs: i64) -> Self::Output {
> +        Self {
> +            nanos: self.nanos * rhs,
> +        }
> +    }
> +}
> +
> +impl ops::MulAssign<i64> for Delta {
> +    #[inline]
> +    fn mul_assign(&mut self, rhs: i64) {
> +        self.nanos *= rhs;
> +    }
> +}
> +
> +impl ops::Div for Delta {
> +    type Output = i64;
> +
> +    #[inline]
> +    fn div(self, rhs: Self) -> Self::Output {
> +        #[cfg(CONFIG_64BIT)]
> +        {
> +            self.nanos / rhs.nanos
> +        }
> +
> +        #[cfg(not(CONFIG_64BIT))]
> +        {
> +            // SAFETY: This function is always safe to call regardless of the input values
> +            unsafe { bindings::div64_s64(self.nanos, rhs.nanos) }
> +        }
> +    }
> +}
> +
> impl Delta {
>     /// A span of time equal to zero.
>     pub const ZERO: Self = Self { nanos: 0 };
> @@ -353,4 +425,30 @@ pub fn as_millis(self) -> i64 {
>             bindings::ktime_to_ms(self.as_nanos())
>         }
>     }
> +
> +    /// Return `self % dividend` where `dividend` is in nanoseconds.
> +    ///
> +    /// The kernel doesn't have any emulation for `s64 % s64` on 32 bit platforms, so this is
> +    /// limited to 32 bit dividends.
> +    #[inline]
> +    pub fn rem_nanos(self, dividend: i32) -> Self {
> +        #[cfg(CONFIG_64BIT)]
> +        {
> +            Self {
> +                nanos: self.as_nanos() % i64::from(dividend),
> +            }
> +        }
> +
> +        #[cfg(not(CONFIG_64BIT))]
> +        {
> +            let mut rem = 0;
> +
> +            // SAFETY: `rem` is in the stack, so we can always provide a valid pointer to it.
> +            unsafe { bindings::div_s64_rem(self.as_nanos(), dividend, &mut rem) };
> +
> +            Self {
> +                nanos: i64::from(rem),
> +            }
> +        }
> +    }
> }
> -- 
> 2.50.0
> 
> 

Reviewed-by: Daniel Almeida <daniel.almeida@...labora.com>

Powered by blists - more mailing lists