Re: [PATCH] rust: time: make Delta division and remainder fail consistently

[FUJITA Tomonori <tomo@flapping.org>]

On Tue, 29 Sep 2026 02:39:25 +0800
chenhan <chenhan0017.work@gmail.com> wrote:

> On 64-bit, Delta division and remainder use Rust operators, which panic
> for a zero divisor and for the i64::MIN / -1 overflow case. On 32-bit,
> the C helpers do not provide the same behavior, so the same API calls can
> return architecture-dependent values or emit a divide-by-zero diagnostic.
> 
> Check both invalid inputs before selecting the architecture-specific
> implementation. This gives both APIs the same panic conditions as i64's
> `/` and `%` operators and documents them in the public API.
> 
> Tested on x86-64 and ARMv7 QEMU with CONFIG_SAMPLE_RUST_REPRO=y: zero
> divisors and i64::MIN / -1 panic for both APIs, while 10 / 3 returns 3
> and 10 % 3 returns 1 on both architectures.
> 
> Fixes: 4521438fb076 ("rust: time: Implement basic arithmetic operations for Delta")
> Closes: https://github.com/Rust-for-Linux/linux/issues/1254
> Assisted-by: LLM
> Signed-off-by: chenhan <chenhan0017.work@gmail.com>
> ---
>  rust/kernel/time.rs | 103 +++++++++++++++++++++++++++++++-------------
>  1 file changed, 72 insertions(+), 31 deletions(-)
> 
> diff --git a/rust/kernel/time.rs b/rust/kernel/time.rs
> index 6c0a5e8090d0..55c90365ba73 100644
> --- a/rust/kernel/time.rs
> +++ b/rust/kernel/time.rs
> @@ -405,21 +405,65 @@ fn mul_assign(&mut self, rhs: i64) {
>      }
>  }
>  
> +#[inline]
> +fn div_s64_or_panic(dividend: i64, divisor: i64) -> i64 {

Do you have other users in mind? If not, each helper has only one
caller, so they could go directly into div() and rem_nanos().

> +    if divisor == 0 {
> +        panic!("attempt to divide by zero");
> +    }
> +
> +    if dividend == i64::MIN && divisor == -1 {
> +        panic!("attempt to divide with overflow");
> +    }
> +
> +    #[cfg(CONFIG_64BIT)]
> +    {
> +        dividend / divisor
> +    }
> +
> +    #[cfg(not(CONFIG_64BIT))]
> +    {
> +        // SAFETY: `divisor` is non-zero, and both operands are passed by value.
> +        unsafe { bindings::div64_s64(dividend, divisor) }
> +    }
> +}
> +
> +#[inline]
> +fn rem_s64_or_panic(dividend: i64, divisor: i32) -> i64 {

Ditto.

> +    if divisor == 0 {
> +        panic!("attempt to calculate the remainder with a divisor of zero");
> +    }
> +
> +    if dividend == i64::MIN && divisor == -1 {
> +        panic!("attempt to calculate the remainder with overflow");
> +    }
> +
> +    #[cfg(CONFIG_64BIT)]
> +    {
> +        dividend % i64::from(divisor)
> +    }
> +
> +    #[cfg(not(CONFIG_64BIT))]
> +    {
> +        let mut rem = 0;
> +
> +        // SAFETY: `rem` points to a local variable and `divisor` is non-zero.
> +        unsafe { bindings::div_s64_rem(dividend, divisor, &mut rem) };
> +
> +        i64::from(rem)
> +    }
> +}
> +
> +/// # Panics
> +///
> +/// Panics if `rhs` is zero, or if the quotient overflows, i.e. if `self` is
> +/// `Delta::from_nanos(i64::MIN)` and `rhs` is `Delta::from_nanos(-1)`. Both
> +/// cases panic on 32-bit as well as on 64-bit; see `div_s64_or_panic()`.

div_s64_or_panic() is private, so you should not mention it in the
public documentation.

Also, shouldn't the `# Panics` section be right above fn div()?


>  impl ops::Div for Delta {
>      type Output = i64;
>  
>      #[inline]
>      fn div(self, rhs: Self) -> Self::Output {
> -        #[cfg(CONFIG_64BIT)]
> -        {
> -            self.value / rhs.value
> -        }
> -
> -        #[cfg(not(CONFIG_64BIT))]
> -        {
> -            // SAFETY: This function is always safe to call regardless of the input values
> -            unsafe { bindings::div64_s64(self.value, rhs.value) }
> -        }
> +        div_s64_or_panic(self.value, rhs.value)
>      }
>  }
>  
> @@ -554,29 +598,26 @@ pub fn as_millis_ceil(self) -> i64 {
>          }
>      }
>  
> -    /// Return `self % dividend` where `dividend` is in nanoseconds.
> +    /// Return `self % divisor`, where `divisor` is a number of nanoseconds.
> +    ///
> +    /// The result has the sign of `self`, and its magnitude is strictly smaller
> +    /// than that of `divisor`.
>      ///
> -    /// The kernel doesn't have any emulation for `s64 % s64` on 32 bit platforms, so this is
> -    /// limited to 32 bit dividends.
> +    /// `divisor` is a 32-bit integer because the helper called on 32-bit
> +    /// platforms, `div_s64_rem()`, takes an `s32` divisor. The dividend (`self`)
> +    /// is a full `i64` on every architecture, so the width restriction applies
> +    /// to all configurations, not only to 32-bit ones.
> +    ///
> +    /// # Panics
> +    ///
> +    /// Panics if `divisor` is zero, or if the division overflows, i.e. if `self`
> +    /// is `Delta::from_nanos(i64::MIN)` and `divisor` is `-1`. These are the same
> +    /// inputs [`ops::Div`] panics on, and the same ones `i64`'s `%` operator

The inputs are not the same. One takes i64 and the other takes i32.

> +    /// panics on. See `rem_s64_or_panic()`.
>      #[inline]
> -    pub fn rem_nanos(self, dividend: i32) -> Self {
> -        #[cfg(CONFIG_64BIT)]
> -        {
> -            Self {
> -                value: 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 {
> -                value: i64::from(rem),
> -            }
> +    pub fn rem_nanos(self, divisor: i32) -> Self {
> +        Self {
> +            value: rem_s64_or_panic(self.as_nanos(), divisor),
>          }
>      }
>  }
> 
> base-commit: d266640c6c760c9bc215bf5a3ece122ca488b6f5
> -- 
> 2.34.1
> 
>