[PATCH v3 0/3] rust: time: fix Delta arithmetic and add math helpers

[PATCH v3 0/3] rust: time: fix Delta arithmetic and add math helpers

[chenhan]

Hi,

This revision separates the Delta arithmetic fix from establishing the
math module, following the v2 discussion.

Patch 1 corrects the rem_nanos() parameter name and documentation, as
Miguel requested. Patch 2 adds the panic checks in the existing methods.
Patch 3 moves that implementation to public math::div64_s64() and
math::rem_s64() helpers and uses them from Delta.

For stable, patch 2 depends on patch 1; both carry stable Cc, and patch 1
is marked as a prerequisite. The math refactor is not marked for stable.
The first two patches can be taken without the third. Backport suitability
remains for the maintainers to decide; I have not audited all stable trees.

Each signed helper still has one production call site, in Delta. The
current pwm_th1520 conversions use unsigned arithmetic and are not users
of these helpers. Patch 3 follows Andreas' request to establish math.rs.
Tomonori, it adds no unsigned helpers or PWM changes; if your math module
lands first, I can rebase this patch onto it. The helpers are public so
they are available to driver crates as well as the kernel crate.

Changes since v2:
- Split the parameter correction, behavior fix and math extraction.
- Explain in SAFETY why the conditions hold: the checks above exclude
  zero and i64::MIN / -1, and &mut rem supplies the valid writable pointer.
- Drop the redundant issue-comment Link trailers, retaining Closes.
- Remove the unrelated "a number of" wording, C implementation details
  from public documentation, and the "even though ..." panic explanation.

No in-tree calls to Delta's division or rem_nanos() were found at the
base or rust-next 7c306f00835a. This covered tracked Rust files, named
calls and division expressions in Delta/Instant users, including elapsed()
results. Fixed-divisor time conversions, unsigned jiffies arithmetic and
generic bounded-integer operations are not extra users of these helpers.

Testing used a built-in Rust module whose init function passes black_box
operands to the actual Delta APIs (a and b are i64, d is i32):

    Delta::from_nanos(black_box(a)) / Delta::from_nanos(black_box(b))
    Delta::from_nanos(black_box(a)).rem_nanos(black_box(d)).as_nanos()

A boot-parameter selector chooses the test. One boot compares 26 valid
divisions and 21 valid remainders against literal expected values; four
separate boots exercise the invalid inputs because each panic stops its
test. Core expectations are:

    dividend    divisor    division    remainder (nanoseconds)
    10          0          panic       panic
    i64::MIN    -1         panic       panic
    10          3          3           1
    -10         3          -3          -1

Other valid cases cover signed extrema, i32::MIN divisors and full-width
i64 division operands. Patches 1-2 passed on x86-64 and ARMv7 QEMU with
CONFIG_RUST_OVERFLOW_CHECKS=y. The full series passed on both architectures
with that option enabled and disabled: 30 boots, 282 valid comparisons and
24 expected panics in total. Kernel builds and rustdoc passed on both
architectures. The test module is validation code, not part of this series.

Base: timekeeping-next 2ea0119f72dba597aa8a98cbdb72c564bfc5cb38.
Rechecked on October 17: this is still the timekeeping-next tip, and
neither it nor rust-next has a math module. When applying patch 3 to
rust-next 7c306f00835a, the new "pub mod mem;" line overlaps the module
registration hunk. Keep both modules, with math between maple_tree and
mem. Patches 1 and 2 apply there unchanged. Runtime testing above is on
the stated timekeeping-next base.

V2: https://lore.kernel.org/rust-for-linux/20261001042942.109012-1-chenhan0017.work@gmail.com/

Best regards,
chenhan

chenhan (3):
  rust: time: rename the rem_nanos() divisor
  rust: time: make Delta division and remainder fail consistently
  rust: math: add signed division and remainder helpers

 rust/kernel/lib.rs  |  1 +
 rust/kernel/math.rs | 64 +++++++++++++++++++++++++++++++++++++++++++++
 rust/kernel/time.rs | 46 +++++++++++---------------------
 3 files changed, 80 insertions(+), 31 deletions(-)
 create mode 100644 rust/kernel/math.rs


base-commit: 2ea0119f72dba597aa8a98cbdb72c564bfc5cb38
-- 
2.34.1

[PATCH v3 1/3] rust: time: rename the rem_nanos() divisor

[chenhan]

The argument to rem_nanos() is the divisor, while self holds the
dividend. Rename the argument and correct the documentation. Drop the
architecture-specific implementation explanation from the public API
documentation; the argument type already states the divisor's width.

This does not change the function's type or behavior. It is a preparation
for documenting and checking the invalid divisors in the next patch.

Cc: stable@vger.kernel.org # prerequisite for the Delta division/remainder fix
Assisted-by: LLM
Signed-off-by: chenhan <chenhan0017.work@gmail.com>
---
 rust/kernel/time.rs | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/rust/kernel/time.rs b/rust/kernel/time.rs
index 5ae377e70dd8..16b44e82d2ac 100644
--- a/rust/kernel/time.rs
+++ b/rust/kernel/time.rs
@@ -614,16 +614,13 @@ pub fn to_jiffies_timeout(self) -> Delta<Jiffy> {
         Delta::<Jiffy>::from_jiffies(jiffies)
     }
 
-    /// 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.
+    /// Return `self % divisor` where `divisor` is in nanoseconds.
     #[inline]
-    pub fn rem_nanos(self, dividend: i32) -> Self {
+    pub fn rem_nanos(self, divisor: i32) -> Self {
         #[cfg(CONFIG_64BIT)]
         {
             Self {
-                value: self.as_nanos() % i64::from(dividend),
+                value: self.as_nanos() % i64::from(divisor),
             }
         }
 
@@ -632,7 +629,7 @@ pub fn rem_nanos(self, dividend: i32) -> Self {
             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) };
+            unsafe { bindings::div_s64_rem(self.as_nanos(), divisor, &mut rem) };
 
             Self {
                 value: i64::from(rem),
-- 
2.34.1

[PATCH v3 2/3] rust: time: make Delta division and remainder fail consistently

[chenhan]

Delta's division and rem_nanos() use Rust operators on 64-bit and C
helpers on 32-bit. The report shows that rem_nanos(0) can return 10 for
a 10 ns dividend on ARMv7, while the same call panics on x86-64. The
i64::MIN / -1 case also differs, and neither API documents these inputs.

Check for zero divisors and i64::MIN with -1 before the architecture-
specific code, and document the panic conditions on each method. This
matches Rust's signed division and remainder operators, including when
overflow checks are disabled. Valid-input results and API signatures
are unchanged; invalid inputs now panic on 32-bit too.

No in-tree calls to either API were found at the base commit. The checks
stay in the existing methods so the fix does not depend on introducing
a math module.

Fixes: 4521438fb076 ("rust: time: Implement basic arithmetic operations for Delta")
Reported-by: Georgios Androutsopoulos <georgeandrout13@gmail.com>
Closes: https://github.com/Rust-for-Linux/linux/issues/1254
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: chenhan <chenhan0017.work@gmail.com>
---
 rust/kernel/time.rs | 30 ++++++++++++++++++++++++++++--
 1 file changed, 28 insertions(+), 2 deletions(-)

diff --git a/rust/kernel/time.rs b/rust/kernel/time.rs
index 16b44e82d2ac..4415e91a5227 100644
--- a/rust/kernel/time.rs
+++ b/rust/kernel/time.rs
@@ -412,8 +412,20 @@ fn mul_assign(&mut self, rhs: i64) {
 impl ops::Div for Delta {
     type Output = i64;
 
+    /// # Panics
+    ///
+    /// Panics if `rhs` is zero, or if `self` represents [`i64::MIN`] nanoseconds
+    /// and `rhs` represents `-1` nanosecond.
     #[inline]
     fn div(self, rhs: Self) -> Self::Output {
+        if rhs.value == 0 {
+            panic!("attempt to divide by zero");
+        }
+
+        if self.value == i64::MIN && rhs.value == -1 {
+            panic!("attempt to divide with overflow");
+        }
+
         #[cfg(CONFIG_64BIT)]
         {
             self.value / rhs.value
@@ -421,7 +433,7 @@ fn div(self, rhs: Self) -> Self::Output {
 
         #[cfg(not(CONFIG_64BIT))]
         {
-            // SAFETY: This function is always safe to call regardless of the input values
+            // SAFETY: The checks above exclude a zero divisor and `i64::MIN / -1`.
             unsafe { bindings::div64_s64(self.value, rhs.value) }
         }
     }
@@ -615,8 +627,21 @@ pub fn to_jiffies_timeout(self) -> Delta<Jiffy> {
     }
 
     /// Return `self % divisor` where `divisor` is in nanoseconds.
+    ///
+    /// # Panics
+    ///
+    /// Panics if `divisor` is zero, or if `self` represents [`i64::MIN`]
+    /// nanoseconds and `divisor` is `-1`.
     #[inline]
     pub fn rem_nanos(self, divisor: i32) -> Self {
+        if divisor == 0 {
+            panic!("attempt to calculate the remainder with a divisor of zero");
+        }
+
+        if self.value == i64::MIN && divisor == -1 {
+            panic!("attempt to calculate the remainder with overflow");
+        }
+
         #[cfg(CONFIG_64BIT)]
         {
             Self {
@@ -628,7 +653,8 @@ pub fn rem_nanos(self, divisor: i32) -> Self {
         {
             let mut rem = 0;
 
-            // SAFETY: `rem` is in the stack, so we can always provide a valid pointer to it.
+            // SAFETY: The checks above exclude a zero divisor and `i64::MIN / -1`.
+            // `&mut rem` provides a valid, aligned, writable pointer for this call.
             unsafe { bindings::div_s64_rem(self.as_nanos(), divisor, &mut rem) };
 
             Self {
-- 
2.34.1

[PATCH v3 3/3] rust: math: add signed division and remainder helpers

[chenhan]

Delta's division and remainder implementations operate on integers and
do not depend on time units. Move them into a math module to provide a
common home for arithmetic helpers, with Delta as the initial user.

Expose div64_s64() and rem_s64() so driver crates can use these operations
without constructing time values. The division helper takes two i64
operands; the remainder helper takes an i64 dividend and an i32 divisor
and returns an i64. Both keep Rust's signed operator semantics, including
the unconditional panic checks, and use the existing C helpers on 32-bit.

Keep the Delta API and its panic documentation unchanged. This is separate
from the preceding behavior fix so that introducing the math module is
not required for a backport.

Assisted-by: LLM
Signed-off-by: chenhan <chenhan0017.work@gmail.com>
---
 rust/kernel/lib.rs  |  1 +
 rust/kernel/math.rs | 64 +++++++++++++++++++++++++++++++++++++++++++++
 rust/kernel/time.rs | 49 ++++------------------------------
 3 files changed, 70 insertions(+), 44 deletions(-)
 create mode 100644 rust/kernel/math.rs

diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index 4d5c96ddc49c..60c2af705b47 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -98,6 +98,7 @@
 pub mod kunit;
 pub mod list;
 pub mod maple_tree;
+pub mod math;
 pub mod miscdevice;
 pub mod mm;
 pub mod module;
diff --git a/rust/kernel/math.rs b/rust/kernel/math.rs
new file mode 100644
index 000000000000..34ca097caac9
--- /dev/null
+++ b/rust/kernel/math.rs
@@ -0,0 +1,64 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Integer arithmetic helpers.
+
+/// Divides an [`i64`] dividend by an [`i64`] divisor, rounding towards zero.
+///
+/// # Panics
+///
+/// Panics if `divisor` is zero, or if `dividend` is [`i64::MIN`] and `divisor` is `-1`.
+#[inline]
+pub fn div64_s64(dividend: i64, divisor: i64) -> i64 {
+    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: The checks above exclude a zero divisor and `i64::MIN / -1`.
+        unsafe { bindings::div64_s64(dividend, divisor) }
+    }
+}
+
+/// Returns the remainder of an [`i64`] dividend divided by an [`i32`] divisor.
+///
+/// A non-zero remainder has the sign of `dividend`.
+///
+/// # Panics
+///
+/// Panics if `divisor` is zero, or if `dividend` is [`i64::MIN`] and `divisor` is `-1`.
+#[inline]
+pub fn rem_s64(dividend: i64, divisor: i32) -> i64 {
+    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: The checks above exclude a zero divisor and `i64::MIN / -1`.
+        // `&mut rem` provides a valid, aligned, writable pointer for this call.
+        unsafe { bindings::div_s64_rem(dividend, divisor, &mut rem) };
+
+        i64::from(rem)
+    }
+}
diff --git a/rust/kernel/time.rs b/rust/kernel/time.rs
index 4415e91a5227..e297d327953f 100644
--- a/rust/kernel/time.rs
+++ b/rust/kernel/time.rs
@@ -27,6 +27,8 @@
 use core::marker::PhantomData;
 use core::ops;
 
+use crate::math;
+
 pub mod delay;
 pub mod hrtimer;
 
@@ -418,24 +420,7 @@ impl ops::Div for Delta {
     /// and `rhs` represents `-1` nanosecond.
     #[inline]
     fn div(self, rhs: Self) -> Self::Output {
-        if rhs.value == 0 {
-            panic!("attempt to divide by zero");
-        }
-
-        if self.value == i64::MIN && rhs.value == -1 {
-            panic!("attempt to divide with overflow");
-        }
-
-        #[cfg(CONFIG_64BIT)]
-        {
-            self.value / rhs.value
-        }
-
-        #[cfg(not(CONFIG_64BIT))]
-        {
-            // SAFETY: The checks above exclude a zero divisor and `i64::MIN / -1`.
-            unsafe { bindings::div64_s64(self.value, rhs.value) }
-        }
+        math::div64_s64(self.value, rhs.value)
     }
 }
 
@@ -634,32 +619,8 @@ pub fn to_jiffies_timeout(self) -> Delta<Jiffy> {
     /// nanoseconds and `divisor` is `-1`.
     #[inline]
     pub fn rem_nanos(self, divisor: i32) -> Self {
-        if divisor == 0 {
-            panic!("attempt to calculate the remainder with a divisor of zero");
-        }
-
-        if self.value == i64::MIN && divisor == -1 {
-            panic!("attempt to calculate the remainder with overflow");
-        }
-
-        #[cfg(CONFIG_64BIT)]
-        {
-            Self {
-                value: self.as_nanos() % i64::from(divisor),
-            }
-        }
-
-        #[cfg(not(CONFIG_64BIT))]
-        {
-            let mut rem = 0;
-
-            // SAFETY: The checks above exclude a zero divisor and `i64::MIN / -1`.
-            // `&mut rem` provides a valid, aligned, writable pointer for this call.
-            unsafe { bindings::div_s64_rem(self.as_nanos(), divisor, &mut rem) };
-
-            Self {
-                value: i64::from(rem),
-            }
+        Self {
+            value: math::rem_s64(self.as_nanos(), divisor),
         }
     }
 }
-- 
2.34.1