[PATCH v15 0/6] rust: Add IO polling

[PATCH v15 0/6] rust: Add IO polling

[FUJITA Tomonori]

Add two new types, Instant and Delta, which represent a specific point
in time and a span of time, respectively, with Rust version of
fsleep().

I dropped patches related with read_poll_timeout() in this version,
which we haven't reached agreement on yet. There are other potential
uses for the Instant and Delta types, so it's better to upstream them
first. Note that I haven't changed the subject to avoid confusion.

Unlike the old rust time branch, this adds a wrapper for fsleep()
instead of msleep(). fsleep() automatically chooses the best sleep
method based on a duration.

v15
- fix the comment and commit message on Ktime
v14: https://lore.kernel.org/lkml/20250422135336.194579-1-fujita.tomonori@gmail.com/
- rebased on timekeeping-next
- add Ktime type temporarily to hrtimer
v13: https://lore.kernel.org/lkml/20250413104310.162045-1-fujita.tomonori@gmail.com/
- fix typo in MAINTAINERS file
v12: https://lore.kernel.org/lkml/20250406013445.124688-1-fujita.tomonori@gmail.com/
- drop #1, #6, and #7 patches, which we haven't reached agreement on yet
- adjust hrtimer code to use Instance for the removal of Ktime
v11: https://lore.kernel.org/lkml/20250220070611.214262-1-fujita.tomonori@gmail.com/
- use file_len arg name in __might_resched_precision() instead of len for clarity
- remove unnecessary strlen in __might_resched(); just use a large value for the precision
- add more doc and example for read_poll_timeout()
- fix read_poll_timeout() to call __might_sleep() only with CONFIG_DEBUG_ATOMIC_SLEEP enabled
- call might_sleep() instead of __might_sleep() in read_poll_timeout() to match the C version
- Add new sections for the abstractions in MAINTAINERS instead of adding rust files to the existing sections
v10: https://lore.kernel.org/lkml/20250207132623.168854-1-fujita.tomonori@gmail.com/
- rebased on rust-next
- use Option type for timeout argument for read_poll_timeout()
- remove obsoleted comment on read_poll_timeout()
v9: https://lore.kernel.org/lkml/20250125101854.112261-1-fujita.tomonori@gmail.com/
- make the might_sleep() changes into as a separate patch
- add as_millis() method to Delta for Binder driver
- make Delta's as_*() methods const (useful in some use cases)
- add Delta::ZERO const; used in fsleep()
- fix typos
- use intra-doc links
- place the #[inline] marker before the documentation
- remove Instant's from_raw() method
- add Invariants to Instant type
- improve Delta's methods documents
- fix fsleep() SAFETY comment
- improve fsleep() documents
- lift T:Copy restriction in read_poll_timeout()
- use MutFn for Cond in read_poll_timeout() instead of Fn
- fix might_sleep() call in read_poll_timeout()
- simplify read_poll_timeout() logic
v8: https://lore.kernel.org/lkml/20250116044100.80679-1-fujita.tomonori@gmail.com/
- fix compile warnings
v7: https://lore.kernel.org/lkml/20241220061853.2782878-1-fujita.tomonori@gmail.com/
- rebased on rust-next
- use crate::ffi instead of core::ffi
v6: https://lore.kernel.org/lkml/20241114070234.116329-1-fujita.tomonori@gmail.com/
- use super::Delta in delay.rs
- improve the comments
- add Delta's is_negative() method
- rename processor.rs to cpu.rs for cpu_relax()
- add __might_sleep_precision() taking pointer to a string with the length
- implement read_poll_timeout as normal function instead of macro
v5: https://lore.kernel.org/lkml/20241101010121.69221-1-fujita.tomonori@gmail.com/
- set the range of Delta for fsleep function
- update comments
v4: https://lore.kernel.org/lkml/20241025033118.44452-1-fujita.tomonori@gmail.com/
- rebase on the tip tree's timers/core
- add Instant instead of using Ktime
- remove unused basic methods
- add Delta as_micros_ceil method
- use const fn for Delta from_* methods
- add more comments based on the feedback
- add a safe wrapper for cpu_relax()
- add __might_sleep() macro
v3: https://lore.kernel.org/lkml/20241016035214.2229-1-fujita.tomonori@gmail.com/
- Update time::Delta methods (use i64 for everything)
- Fix read_poll_timeout to show the proper debug info (file and line)
- Move fsleep to rust/kernel/time/delay.rs
- Round up delta for fsleep
- Access directly ktime_t instead of using ktime APIs
- Add Eq and Ord with PartialEq and PartialOrd
v2: https://lore.kernel.org/lkml/20241005122531.20298-1-fujita.tomonori@gmail.com/
- Introduce time::Delta instead of core::time::Duration
- Add some trait to Ktime for calculating timeout
- Use read_poll_timeout in QT2025 driver instead of using fsleep directly
v1: https://lore.kernel.org/netdev/20241001112512.4861-1-fujita.tomonori@gmail.com/

FUJITA Tomonori (6):
  rust: hrtimer: Add Ktime temporarily
  rust: time: Add PartialEq/Eq/PartialOrd/Ord trait to Ktime
  rust: time: Introduce Delta type
  rust: time: Introduce Instant type
  rust: time: Add wrapper for fsleep() function
  MAINTAINERS: rust: Add a new section for all of the time stuff

 MAINTAINERS                         |  11 +-
 rust/helpers/helpers.c              |   1 +
 rust/helpers/time.c                 |   8 ++
 rust/kernel/time.rs                 | 168 +++++++++++++++++++++-------
 rust/kernel/time/delay.rs           |  49 ++++++++
 rust/kernel/time/hrtimer.rs         |  18 ++-
 rust/kernel/time/hrtimer/arc.rs     |   2 +-
 rust/kernel/time/hrtimer/pin.rs     |   2 +-
 rust/kernel/time/hrtimer/pin_mut.rs |   4 +-
 rust/kernel/time/hrtimer/tbox.rs    |   2 +-
 10 files changed, 216 insertions(+), 49 deletions(-)
 create mode 100644 rust/helpers/time.c
 create mode 100644 rust/kernel/time/delay.rs


base-commit: 8ffd015db85fea3e15a77027fda6c02ced4d2444
-- 
2.43.0

[PATCH v15 1/6] rust: hrtimer: Add Ktime temporarily

[FUJITA Tomonori]

Add Ktime temporarily until hrtimer is refactored to use Instant and
Delta types.

Reviewed-by: Andreas Hindborg <a.hindborg@kernel.org>
Reviewed-by: Boqun Feng <boqun.feng@gmail.com>
Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
---
 rust/kernel/time/hrtimer.rs         | 18 +++++++++++++++++-
 rust/kernel/time/hrtimer/arc.rs     |  2 +-
 rust/kernel/time/hrtimer/pin.rs     |  2 +-
 rust/kernel/time/hrtimer/pin_mut.rs |  4 ++--
 rust/kernel/time/hrtimer/tbox.rs    |  2 +-
 5 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/rust/kernel/time/hrtimer.rs b/rust/kernel/time/hrtimer.rs
index ce53f8579d18..17824aa0c0f3 100644
--- a/rust/kernel/time/hrtimer.rs
+++ b/rust/kernel/time/hrtimer.rs
@@ -68,10 +68,26 @@
 //! `start` operation.
 
 use super::ClockId;
-use crate::{prelude::*, time::Ktime, types::Opaque};
+use crate::{prelude::*, types::Opaque};
 use core::marker::PhantomData;
 use pin_init::PinInit;
 
+/// A Rust wrapper around a `ktime_t`.
+// NOTE: Ktime is going to be removed when hrtimer is converted to Instant/Delta.
+#[repr(transparent)]
+#[derive(Copy, Clone, PartialEq, PartialOrd, Eq, Ord)]
+pub struct Ktime {
+    inner: bindings::ktime_t,
+}
+
+impl Ktime {
+    /// Returns the number of nanoseconds.
+    #[inline]
+    pub fn to_ns(self) -> i64 {
+        self.inner
+    }
+}
+
 /// A timer backed by a C `struct hrtimer`.
 ///
 /// # Invariants
diff --git a/rust/kernel/time/hrtimer/arc.rs b/rust/kernel/time/hrtimer/arc.rs
index 4a984d85b4a1..ccf1e66e5b2d 100644
--- a/rust/kernel/time/hrtimer/arc.rs
+++ b/rust/kernel/time/hrtimer/arc.rs
@@ -5,10 +5,10 @@
 use super::HrTimerCallback;
 use super::HrTimerHandle;
 use super::HrTimerPointer;
+use super::Ktime;
 use super::RawHrTimerCallback;
 use crate::sync::Arc;
 use crate::sync::ArcBorrow;
-use crate::time::Ktime;
 
 /// A handle for an `Arc<HasHrTimer<T>>` returned by a call to
 /// [`HrTimerPointer::start`].
diff --git a/rust/kernel/time/hrtimer/pin.rs b/rust/kernel/time/hrtimer/pin.rs
index f760db265c7b..293ca9cf058c 100644
--- a/rust/kernel/time/hrtimer/pin.rs
+++ b/rust/kernel/time/hrtimer/pin.rs
@@ -4,9 +4,9 @@
 use super::HrTimer;
 use super::HrTimerCallback;
 use super::HrTimerHandle;
+use super::Ktime;
 use super::RawHrTimerCallback;
 use super::UnsafeHrTimerPointer;
-use crate::time::Ktime;
 use core::pin::Pin;
 
 /// A handle for a `Pin<&HasHrTimer>`. When the handle exists, the timer might be
diff --git a/rust/kernel/time/hrtimer/pin_mut.rs b/rust/kernel/time/hrtimer/pin_mut.rs
index 90c0351d62e4..6033572d35ad 100644
--- a/rust/kernel/time/hrtimer/pin_mut.rs
+++ b/rust/kernel/time/hrtimer/pin_mut.rs
@@ -1,9 +1,9 @@
 // SPDX-License-Identifier: GPL-2.0
 
 use super::{
-    HasHrTimer, HrTimer, HrTimerCallback, HrTimerHandle, RawHrTimerCallback, UnsafeHrTimerPointer,
+    HasHrTimer, HrTimer, HrTimerCallback, HrTimerHandle, Ktime, RawHrTimerCallback,
+    UnsafeHrTimerPointer,
 };
-use crate::time::Ktime;
 use core::{marker::PhantomData, pin::Pin, ptr::NonNull};
 
 /// A handle for a `Pin<&mut HasHrTimer>`. When the handle exists, the timer might
diff --git a/rust/kernel/time/hrtimer/tbox.rs b/rust/kernel/time/hrtimer/tbox.rs
index 2071cae07234..29526a5da203 100644
--- a/rust/kernel/time/hrtimer/tbox.rs
+++ b/rust/kernel/time/hrtimer/tbox.rs
@@ -5,9 +5,9 @@
 use super::HrTimerCallback;
 use super::HrTimerHandle;
 use super::HrTimerPointer;
+use super::Ktime;
 use super::RawHrTimerCallback;
 use crate::prelude::*;
-use crate::time::Ktime;
 use core::ptr::NonNull;
 
 /// A handle for a [`Box<HasHrTimer<T>>`] returned by a call to
-- 
2.43.0

[PATCH v15 2/6] rust: time: Add PartialEq/Eq/PartialOrd/Ord trait to Ktime

[FUJITA Tomonori]

Add PartialEq/Eq/PartialOrd/Ord trait to Ktime so two Ktime instances
can be compared to determine whether a timeout is met or not.

Use the derive implements; we directly touch C's ktime_t rather than
using the C's accessors because it is more efficient and we already do
in the existing code (Ktime::sub).

Reviewed-by: Trevor Gross <tmgross@umich.edu>
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Reviewed-by: Gary Guo <gary@garyguo.net>
Reviewed-by: Fiona Behrens <me@kloenk.dev>
Tested-by: Daniel Almeida <daniel.almeida@collabora.com>
Reviewed-by: Andreas Hindborg <a.hindborg@kernel.org>
Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
---
 rust/kernel/time.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/rust/kernel/time.rs b/rust/kernel/time.rs
index f509cb0eb71e..9d57e8a5552a 100644
--- a/rust/kernel/time.rs
+++ b/rust/kernel/time.rs
@@ -29,7 +29,7 @@ pub fn msecs_to_jiffies(msecs: Msecs) -> Jiffies {
 
 /// A Rust wrapper around a `ktime_t`.
 #[repr(transparent)]
-#[derive(Copy, Clone)]
+#[derive(Copy, Clone, PartialEq, PartialOrd, Eq, Ord)]
 pub struct Ktime {
     inner: bindings::ktime_t,
 }
-- 
2.43.0

[PATCH v15 3/6] rust: time: Introduce Delta type

[FUJITA Tomonori]

Introduce a type representing a span of time. Define our own type
because `core::time::Duration` is large and could panic during
creation.

time::Ktime could be also used for time duration but timestamp and
timedelta are different so better to use a new type.

i64 is used instead of u64 to represent a span of time; some C drivers
uses negative Deltas and i64 is more compatible with Ktime using i64
too (e.g., ktime_[us|ms]_delta() APIs return i64 so we create Delta
object without type conversion.

i64 is used instead of bindings::ktime_t because when the ktime_t
type is used as timestamp, it represents values from 0 to
KTIME_MAX, which is different from Delta.

as_millis() method isn't used in this patchset. It's planned to be
used in Binder driver.

Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Reviewed-by: Gary Guo <gary@garyguo.net>
Reviewed-by: Fiona Behrens <me@kloenk.dev>
Tested-by: Daniel Almeida <daniel.almeida@collabora.com>
Reviewed-by: Andreas Hindborg <a.hindborg@kernel.org>
Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
---
 rust/kernel/time.rs | 88 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 88 insertions(+)

diff --git a/rust/kernel/time.rs b/rust/kernel/time.rs
index 9d57e8a5552a..e00b9a853e6a 100644
--- a/rust/kernel/time.rs
+++ b/rust/kernel/time.rs
@@ -10,9 +10,15 @@
 
 pub mod hrtimer;
 
+/// The number of nanoseconds per microsecond.
+pub const NSEC_PER_USEC: i64 = bindings::NSEC_PER_USEC as i64;
+
 /// The number of nanoseconds per millisecond.
 pub const NSEC_PER_MSEC: i64 = bindings::NSEC_PER_MSEC as i64;
 
+/// The number of nanoseconds per second.
+pub const NSEC_PER_SEC: i64 = bindings::NSEC_PER_SEC as i64;
+
 /// The time unit of Linux kernel. One jiffy equals (1/HZ) second.
 pub type Jiffies = crate::ffi::c_ulong;
 
@@ -149,3 +155,85 @@ fn into_c(self) -> bindings::clockid_t {
         self as bindings::clockid_t
     }
 }
+
+/// A span of time.
+///
+/// This struct represents a span of time, with its value stored as nanoseconds.
+/// The value can represent any valid i64 value, including negative, zero, and
+/// positive numbers.
+#[derive(Copy, Clone, PartialEq, PartialOrd, Eq, Ord, Debug)]
+pub struct Delta {
+    nanos: i64,
+}
+
+impl Delta {
+    /// A span of time equal to zero.
+    pub const ZERO: Self = Self { nanos: 0 };
+
+    /// Create a new [`Delta`] from a number of microseconds.
+    ///
+    /// The `micros` can range from -9_223_372_036_854_775 to 9_223_372_036_854_775.
+    /// If `micros` is outside this range, `i64::MIN` is used for negative values,
+    /// and `i64::MAX` is used for positive values due to saturation.
+    #[inline]
+    pub const fn from_micros(micros: i64) -> Self {
+        Self {
+            nanos: micros.saturating_mul(NSEC_PER_USEC),
+        }
+    }
+
+    /// Create a new [`Delta`] from a number of milliseconds.
+    ///
+    /// The `millis` can range from -9_223_372_036_854 to 9_223_372_036_854.
+    /// If `millis` is outside this range, `i64::MIN` is used for negative values,
+    /// and `i64::MAX` is used for positive values due to saturation.
+    #[inline]
+    pub const fn from_millis(millis: i64) -> Self {
+        Self {
+            nanos: millis.saturating_mul(NSEC_PER_MSEC),
+        }
+    }
+
+    /// Create a new [`Delta`] from a number of seconds.
+    ///
+    /// The `secs` can range from -9_223_372_036 to 9_223_372_036.
+    /// If `secs` is outside this range, `i64::MIN` is used for negative values,
+    /// and `i64::MAX` is used for positive values due to saturation.
+    #[inline]
+    pub const fn from_secs(secs: i64) -> Self {
+        Self {
+            nanos: secs.saturating_mul(NSEC_PER_SEC),
+        }
+    }
+
+    /// Return `true` if the [`Delta`] spans no time.
+    #[inline]
+    pub fn is_zero(self) -> bool {
+        self.as_nanos() == 0
+    }
+
+    /// Return `true` if the [`Delta`] spans a negative amount of time.
+    #[inline]
+    pub fn is_negative(self) -> bool {
+        self.as_nanos() < 0
+    }
+
+    /// Return the number of nanoseconds in the [`Delta`].
+    #[inline]
+    pub const fn as_nanos(self) -> i64 {
+        self.nanos
+    }
+
+    /// Return the smallest number of microseconds greater than or equal
+    /// to the value in the [`Delta`].
+    #[inline]
+    pub const fn as_micros_ceil(self) -> i64 {
+        self.as_nanos().saturating_add(NSEC_PER_USEC - 1) / NSEC_PER_USEC
+    }
+
+    /// Return the number of milliseconds in the [`Delta`].
+    #[inline]
+    pub const fn as_millis(self) -> i64 {
+        self.as_nanos() / NSEC_PER_MSEC
+    }
+}
-- 
2.43.0

[PATCH v15 4/6] rust: time: Introduce Instant type

[FUJITA Tomonori]

Introduce a type representing a specific point in time. We could use
the Ktime type but C's ktime_t is used for both timestamp and
timedelta. To avoid confusion, introduce a new Instant type for
timestamp.

Rename Ktime to Instant and modify their methods for timestamp.

Implement the subtraction operator for Instant:

Delta = Instant A - Instant B

Reviewed-by: Boqun Feng <boqun.feng@gmail.com>
Reviewed-by: Gary Guo <gary@garyguo.net>
Reviewed-by: Fiona Behrens <me@kloenk.dev>
Tested-by: Daniel Almeida <daniel.almeida@collabora.com>
Reviewed-by: Andreas Hindborg <a.hindborg@kernel.org>
Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
---
 rust/kernel/time.rs | 77 +++++++++++++++++++++++----------------------
 1 file changed, 39 insertions(+), 38 deletions(-)

diff --git a/rust/kernel/time.rs b/rust/kernel/time.rs
index e00b9a853e6a..a8089a98da9e 100644
--- a/rust/kernel/time.rs
+++ b/rust/kernel/time.rs
@@ -5,6 +5,22 @@
 //! This module contains the kernel APIs related to time and timers that
 //! have been ported or wrapped for usage by Rust code in the kernel.
 //!
+//! There are two types in this module:
+//!
+//! - The [`Instant`] type represents a specific point in time.
+//! - The [`Delta`] type represents a span of time.
+//!
+//! Note that the C side uses `ktime_t` type to represent both. However, timestamp
+//! and timedelta are different. To avoid confusion, we use two different types.
+//!
+//! A [`Instant`] object can be created by calling the [`Instant::now()`] function.
+//! It represents a point in time at which the object was created.
+//! By calling the [`Instant::elapsed()`] method, a [`Delta`] object representing
+//! the elapsed time can be created. The [`Delta`] object can also be created
+//! by subtracting two [`Instant`] objects.
+//!
+//! A [`Delta`] type supports methods to retrieve the duration in various units.
+//!
 //! C header: [`include/linux/jiffies.h`](srctree/include/linux/jiffies.h).
 //! C header: [`include/linux/ktime.h`](srctree/include/linux/ktime.h).
 
@@ -33,59 +49,44 @@ pub fn msecs_to_jiffies(msecs: Msecs) -> Jiffies {
     unsafe { bindings::__msecs_to_jiffies(msecs) }
 }
 
-/// A Rust wrapper around a `ktime_t`.
+/// A specific point in time.
+///
+/// # Invariants
+///
+/// The `inner` value is in the range from 0 to `KTIME_MAX`.
 #[repr(transparent)]
 #[derive(Copy, Clone, PartialEq, PartialOrd, Eq, Ord)]
-pub struct Ktime {
+pub struct Instant {
     inner: bindings::ktime_t,
 }
 
-impl Ktime {
-    /// Create a `Ktime` from a raw `ktime_t`.
-    #[inline]
-    pub fn from_raw(inner: bindings::ktime_t) -> Self {
-        Self { inner }
-    }
-
+impl Instant {
     /// Get the current time using `CLOCK_MONOTONIC`.
     #[inline]
-    pub fn ktime_get() -> Self {
-        // SAFETY: It is always safe to call `ktime_get` outside of NMI context.
-        Self::from_raw(unsafe { bindings::ktime_get() })
-    }
-
-    /// Divide the number of nanoseconds by a compile-time constant.
-    #[inline]
-    fn divns_constant<const DIV: i64>(self) -> i64 {
-        self.to_ns() / DIV
-    }
-
-    /// Returns the number of nanoseconds.
-    #[inline]
-    pub fn to_ns(self) -> i64 {
-        self.inner
+    pub fn now() -> Self {
+        // INVARIANT: The `ktime_get()` function returns a value in the range
+        // from 0 to `KTIME_MAX`.
+        Self {
+            // SAFETY: It is always safe to call `ktime_get()` outside of NMI context.
+            inner: unsafe { bindings::ktime_get() },
+        }
     }
 
-    /// Returns the number of milliseconds.
+    /// Return the amount of time elapsed since the [`Instant`].
     #[inline]
-    pub fn to_ms(self) -> i64 {
-        self.divns_constant::<NSEC_PER_MSEC>()
+    pub fn elapsed(&self) -> Delta {
+        Self::now() - *self
     }
 }
 
-/// Returns the number of milliseconds between two ktimes.
-#[inline]
-pub fn ktime_ms_delta(later: Ktime, earlier: Ktime) -> i64 {
-    (later - earlier).to_ms()
-}
-
-impl core::ops::Sub for Ktime {
-    type Output = Ktime;
+impl core::ops::Sub for Instant {
+    type Output = Delta;
 
+    // By the type invariant, it never overflows.
     #[inline]
-    fn sub(self, other: Ktime) -> Ktime {
-        Self {
-            inner: self.inner - other.inner,
+    fn sub(self, other: Instant) -> Delta {
+        Delta {
+            nanos: self.inner - other.inner,
         }
     }
 }
-- 
2.43.0

[PATCH v15 5/6] rust: time: Add wrapper for fsleep() function

[FUJITA Tomonori]

Add a wrapper for fsleep(), flexible sleep functions in
include/linux/delay.h which typically deals with hardware delays.

The kernel supports several sleep functions to handle various lengths
of delay. This adds fsleep(), automatically chooses the best sleep
method based on a duration.

sleep functions including fsleep() belongs to TIMERS, not
TIMEKEEPING. They are maintained separately. rust/kernel/time.rs is an
abstraction for TIMEKEEPING. To make Rust abstractions match the C
side, add rust/kernel/time/delay.rs for this wrapper.

fsleep() can only be used in a nonatomic context. This requirement is
not checked by these abstractions, but it is intended that klint [1]
or a similar tool will be used to check it in the future.

Link: https://rust-for-linux.com/klint [1]
Reviewed-by: Gary Guo <gary@garyguo.net>
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Reviewed-by: Fiona Behrens <me@kloenk.dev>
Tested-by: Daniel Almeida <daniel.almeida@collabora.com>
Reviewed-by: Andreas Hindborg <a.hindborg@kernel.org>
Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
---
 rust/helpers/helpers.c    |  1 +
 rust/helpers/time.c       |  8 +++++++
 rust/kernel/time.rs       |  1 +
 rust/kernel/time/delay.rs | 49 +++++++++++++++++++++++++++++++++++++++
 4 files changed, 59 insertions(+)
 create mode 100644 rust/helpers/time.c
 create mode 100644 rust/kernel/time/delay.rs

diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c
index e1c21eba9b15..48143cdd26b3 100644
--- a/rust/helpers/helpers.c
+++ b/rust/helpers/helpers.c
@@ -33,6 +33,7 @@
 #include "spinlock.c"
 #include "sync.c"
 #include "task.c"
+#include "time.c"
 #include "uaccess.c"
 #include "vmalloc.c"
 #include "wait.c"
diff --git a/rust/helpers/time.c b/rust/helpers/time.c
new file mode 100644
index 000000000000..7ae64ad8141d
--- /dev/null
+++ b/rust/helpers/time.c
@@ -0,0 +1,8 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/delay.h>
+
+void rust_helper_fsleep(unsigned long usecs)
+{
+	fsleep(usecs);
+}
diff --git a/rust/kernel/time.rs b/rust/kernel/time.rs
index a8089a98da9e..863385905029 100644
--- a/rust/kernel/time.rs
+++ b/rust/kernel/time.rs
@@ -24,6 +24,7 @@
 //! C header: [`include/linux/jiffies.h`](srctree/include/linux/jiffies.h).
 //! C header: [`include/linux/ktime.h`](srctree/include/linux/ktime.h).
 
+pub mod delay;
 pub mod hrtimer;
 
 /// The number of nanoseconds per microsecond.
diff --git a/rust/kernel/time/delay.rs b/rust/kernel/time/delay.rs
new file mode 100644
index 000000000000..02b8731433c7
--- /dev/null
+++ b/rust/kernel/time/delay.rs
@@ -0,0 +1,49 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Delay and sleep primitives.
+//!
+//! This module contains the kernel APIs related to delay and sleep that
+//! have been ported or wrapped for usage by Rust code in the kernel.
+//!
+//! C header: [`include/linux/delay.h`](srctree/include/linux/delay.h).
+
+use super::Delta;
+use crate::ffi::c_ulong;
+
+/// Sleeps for a given duration at least.
+///
+/// Equivalent to the C side [`fsleep()`], flexible sleep function,
+/// which automatically chooses the best sleep method based on a duration.
+///
+/// `delta` must be within `[0, i32::MAX]` microseconds;
+/// otherwise, it is erroneous behavior. That is, it is considered a bug
+/// to call this function with an out-of-range value, in which case the function
+/// will sleep for at least the maximum value in the range and may warn
+/// in the future.
+///
+/// The behavior above differs from the C side [`fsleep()`] for which out-of-range
+/// values mean "infinite timeout" instead.
+///
+/// This function can only be used in a nonatomic context.
+///
+/// [`fsleep`]: https://docs.kernel.org/timers/delay_sleep_functions.html#c.fsleep
+pub fn fsleep(delta: Delta) {
+    // The maximum value is set to `i32::MAX` microseconds to prevent integer
+    // overflow inside fsleep, which could lead to unintentional infinite sleep.
+    const MAX_DELTA: Delta = Delta::from_micros(i32::MAX as i64);
+
+    let delta = if (Delta::ZERO..=MAX_DELTA).contains(&delta) {
+        delta
+    } else {
+        // TODO: Add WARN_ONCE() when it's supported.
+        MAX_DELTA
+    };
+
+    // SAFETY: It is always safe to call `fsleep()` with any duration.
+    unsafe {
+        // Convert the duration to microseconds and round up to preserve
+        // the guarantee; `fsleep()` sleeps for at least the provided duration,
+        // but that it may sleep for longer under some circumstances.
+        bindings::fsleep(delta.as_micros_ceil() as c_ulong)
+    }
+}
-- 
2.43.0

[PATCH v15 6/6] MAINTAINERS: rust: Add a new section for all of the time stuff

[FUJITA Tomonori]

Add a new section for all of the time stuff to MAINTAINERS file, with
the existing hrtimer entry fold.

Acked-by: John Stultz <jstultz@google.com>
Reviewed-by: Boqun Feng <boqun.feng@gmail.com>
Reviewed-by: Andreas Hindborg <a.hindborg@kernel.org>
Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
---
 MAINTAINERS | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index c59316109e3f..3072c0f8ec0e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10585,20 +10585,23 @@ F:	kernel/time/timer_list.c
 F:	kernel/time/timer_migration.*
 F:	tools/testing/selftests/timers/
 
-HIGH-RESOLUTION TIMERS [RUST]
+DELAY, SLEEP, TIMEKEEPING, TIMERS [RUST]
 M:	Andreas Hindborg <a.hindborg@kernel.org>
 R:	Boqun Feng <boqun.feng@gmail.com>
+R:	FUJITA Tomonori <fujita.tomonori@gmail.com>
 R:	Frederic Weisbecker <frederic@kernel.org>
 R:	Lyude Paul <lyude@redhat.com>
 R:	Thomas Gleixner <tglx@linutronix.de>
 R:	Anna-Maria Behnsen <anna-maria@linutronix.de>
+R:	John Stultz <jstultz@google.com>
+R:	Stephen Boyd <sboyd@kernel.org>
 L:	rust-for-linux@vger.kernel.org
 S:	Supported
 W:	https://rust-for-linux.com
 B:	https://github.com/Rust-for-Linux/linux/issues
-T:	git https://github.com/Rust-for-Linux/linux.git hrtimer-next
-F:	rust/kernel/time/hrtimer.rs
-F:	rust/kernel/time/hrtimer/
+T:	git https://github.com/Rust-for-Linux/linux.git rust-timekeeping-next
+F:	rust/kernel/time.rs
+F:	rust/kernel/time/
 
 HIGH-SPEED SCC DRIVER FOR AX.25
 L:	linux-hams@vger.kernel.org
-- 
2.43.0

Re: [PATCH v15 5/6] rust: time: Add wrapper for fsleep() function

[Andreas Hindborg]

Hi Tomonori,

"FUJITA Tomonori" <fujita.tomonori@gmail.com> writes:

> Add a wrapper for fsleep(), flexible sleep functions in
> include/linux/delay.h which typically deals with hardware delays.
>
> The kernel supports several sleep functions to handle various lengths
> of delay. This adds fsleep(), automatically chooses the best sleep
> method based on a duration.
>
> sleep functions including fsleep() belongs to TIMERS, not
> TIMEKEEPING. They are maintained separately. rust/kernel/time.rs is an
> abstraction for TIMEKEEPING. To make Rust abstractions match the C
> side, add rust/kernel/time/delay.rs for this wrapper.
>
> fsleep() can only be used in a nonatomic context. This requirement is
> not checked by these abstractions, but it is intended that klint [1]
> or a similar tool will be used to check it in the future.

I get an error when building this patch for arm32:

  + kernel-make -j 96 O=/home/aeh/src/linux-rust/test-build-arm-1.78.0 vmlinux modules
  ld.lld: error: undefined symbol: __aeabi_uldivmod
  >>> referenced by kernel.df165ca450b1fd1-cgu.0
  >>>               rust/kernel.o:(kernel::time::delay::fsleep) in archive vmlinux.a
  >>> did you mean: __aeabi_uidivmod
  >>> defined in: vmlinux.a(arch/arm/lib/lib1funcs.o)

Looks like a division function of some sort is not defined. Can you
reproduce?


Best regards,
Andreas Hindborg

Re: [PATCH v15 5/6] rust: time: Add wrapper for fsleep() function

[FUJITA Tomonori]

On Mon, 28 Apr 2025 20:16:47 +0200
Andreas Hindborg <a.hindborg@kernel.org> wrote:

> Hi Tomonori,
> 
> "FUJITA Tomonori" <fujita.tomonori@gmail.com> writes:
> 
>> Add a wrapper for fsleep(), flexible sleep functions in
>> include/linux/delay.h which typically deals with hardware delays.
>>
>> The kernel supports several sleep functions to handle various lengths
>> of delay. This adds fsleep(), automatically chooses the best sleep
>> method based on a duration.
>>
>> sleep functions including fsleep() belongs to TIMERS, not
>> TIMEKEEPING. They are maintained separately. rust/kernel/time.rs is an
>> abstraction for TIMEKEEPING. To make Rust abstractions match the C
>> side, add rust/kernel/time/delay.rs for this wrapper.
>>
>> fsleep() can only be used in a nonatomic context. This requirement is
>> not checked by these abstractions, but it is intended that klint [1]
>> or a similar tool will be used to check it in the future.
> 
> I get an error when building this patch for arm32:
> 
>   + kernel-make -j 96 O=/home/aeh/src/linux-rust/test-build-arm-1.78.0 vmlinux modules
>   ld.lld: error: undefined symbol: __aeabi_uldivmod
>   >>> referenced by kernel.df165ca450b1fd1-cgu.0
>   >>>               rust/kernel.o:(kernel::time::delay::fsleep) in archive vmlinux.a
>   >>> did you mean: __aeabi_uidivmod
>   >>> defined in: vmlinux.a(arch/arm/lib/lib1funcs.o)
> 
> Looks like a division function of some sort is not defined. Can you
> reproduce?

Ah, 64-bit integer division on 32-bit architectures.

I think that the DRM QR driver has the same problem:

https://lore.kernel.org/rust-for-linux/CANiq72ke45eOwckMhWHvmwxc03dxr4rnxxKvx+HvWdBLopZfrQ@mail.gmail.com/

It appears that there is still no consensus on how to resolve it. CC
the participants in the above thread.

I think that we can drop this patch and better to focus on Instant and
Delta types in this merge window.

With the patch below, this issue could be resolved like the C side,
but I'm not sure whether we can reach a consensus quickly.

diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c
index 48143cdd26b3..c44d45960eb1 100644
--- a/rust/helpers/helpers.c
+++ b/rust/helpers/helpers.c
@@ -19,6 +19,7 @@
 #include "io.c"
 #include "jump_label.c"
 #include "kunit.c"
+#include "math64.c"
 #include "mutex.c"
 #include "page.c"
 #include "platform.c"
diff --git a/rust/helpers/math64.c b/rust/helpers/math64.c
new file mode 100644
index 000000000000..f94708cf8fcb
--- /dev/null
+++ b/rust/helpers/math64.c
@@ -0,0 +1,8 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/math64.h>
+
+s64 rust_helper_div64_s64(s64 dividend, s64 divisor)
+{
+	return div64_s64(dividend, divisor);
+}
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index de07aadd1ff5..d272e0b0b05d 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -60,6 +60,7 @@
 #[cfg(CONFIG_KUNIT)]
 pub mod kunit;
 pub mod list;
+pub mod math64;
 pub mod miscdevice;
 #[cfg(CONFIG_NET)]
 pub mod net;
diff --git a/rust/kernel/math64.rs b/rust/kernel/math64.rs
new file mode 100644
index 000000000000..523e47911859
--- /dev/null
+++ b/rust/kernel/math64.rs
@@ -0,0 +1,12 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! 64-bit integer arithmetic helpers.
+//!
+//! C header: [`include/linux/math64.h`](srctree/include/linux/math64.h)
+
+/// Divide a signed 64-bit integer by another signed 64-bit integer.
+#[inline]
+pub fn div64_s64(dividend: i64, divisor: i64) -> i64 {
+    // SAFETY: Calling `div64_s64()` is safe as long as `divisor` is non-zero.
+    unsafe { bindings::div64_s64(dividend, divisor) }
+}
diff --git a/rust/kernel/time.rs b/rust/kernel/time.rs
index 863385905029..7b5255893929 100644
--- a/rust/kernel/time.rs
+++ b/rust/kernel/time.rs
@@ -24,6 +24,8 @@
 //! C header: [`include/linux/jiffies.h`](srctree/include/linux/jiffies.h).
 //! C header: [`include/linux/ktime.h`](srctree/include/linux/ktime.h).
 
+use crate::math64;
+
 pub mod delay;
 pub mod hrtimer;
 
@@ -229,13 +231,16 @@ pub const fn as_nanos(self) -> i64 {
     /// Return the smallest number of microseconds greater than or equal
     /// to the value in the [`Delta`].
     #[inline]
-    pub const fn as_micros_ceil(self) -> i64 {
-        self.as_nanos().saturating_add(NSEC_PER_USEC - 1) / NSEC_PER_USEC
+    pub fn as_micros_ceil(self) -> i64 {
+        math64::div64_s64(
+            self.as_nanos().saturating_add(NSEC_PER_USEC - 1),
+            NSEC_PER_USEC,
+        )
     }
 
     /// Return the number of milliseconds in the [`Delta`].
     #[inline]
-    pub const fn as_millis(self) -> i64 {
-        self.as_nanos() / NSEC_PER_MSEC
+    pub fn as_millis(self) -> i64 {
+        math64::div64_s64(self.as_nanos(), NSEC_PER_MSEC)
     }
 }

base-commit: da37ddd3f607897d039d82e6621671c3f7baa886
-- 
2.43.0

Re: [PATCH v15 5/6] rust: time: Add wrapper for fsleep() function

[Christian Schrefl]

On 29.04.25 3:17 PM, FUJITA Tomonori wrote:
> On Mon, 28 Apr 2025 20:16:47 +0200
> Andreas Hindborg <a.hindborg@kernel.org> wrote:
> 
>> Hi Tomonori,
>>
>> "FUJITA Tomonori" <fujita.tomonori@gmail.com> writes:
>>
>>> Add a wrapper for fsleep(), flexible sleep functions in
>>> include/linux/delay.h which typically deals with hardware delays.
>>>
>>> The kernel supports several sleep functions to handle various lengths
>>> of delay. This adds fsleep(), automatically chooses the best sleep
>>> method based on a duration.
>>>
>>> sleep functions including fsleep() belongs to TIMERS, not
>>> TIMEKEEPING. They are maintained separately. rust/kernel/time.rs is an
>>> abstraction for TIMEKEEPING. To make Rust abstractions match the C
>>> side, add rust/kernel/time/delay.rs for this wrapper.
>>>
>>> fsleep() can only be used in a nonatomic context. This requirement is
>>> not checked by these abstractions, but it is intended that klint [1]
>>> or a similar tool will be used to check it in the future.
>>
>> I get an error when building this patch for arm32:
>>
>>   + kernel-make -j 96 O=/home/aeh/src/linux-rust/test-build-arm-1.78.0 vmlinux modules
>>   ld.lld: error: undefined symbol: __aeabi_uldivmod
>>   >>> referenced by kernel.df165ca450b1fd1-cgu.0
>>   >>>               rust/kernel.o:(kernel::time::delay::fsleep) in archive vmlinux.a
>>   >>> did you mean: __aeabi_uidivmod
>>   >>> defined in: vmlinux.a(arch/arm/lib/lib1funcs.o)
>>
>> Looks like a division function of some sort is not defined. Can you
>> reproduce?
> 
> Ah, 64-bit integer division on 32-bit architectures.
> 
> I think that the DRM QR driver has the same problem:
> 
> https://lore.kernel.org/rust-for-linux/CANiq72ke45eOwckMhWHvmwxc03dxr4rnxxKvx+HvWdBLopZfrQ@mail.gmail.com/
> 
> It appears that there is still no consensus on how to resolve it. CC
> the participants in the above thread.

From what I remember from the thread is that generally 64 bit divisions
should be avoided (like the solution for DRM).

> I think that we can drop this patch and better to focus on Instant and
> Delta types in this merge window.
> 
> With the patch below, this issue could be resolved like the C side,
> but I'm not sure whether we can reach a consensus quickly.

I think adding rust bindings for this is fine (and most likely needed),
for cases where it is required.

> 
> diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c
> index 48143cdd26b3..c44d45960eb1 100644
> --- a/rust/helpers/helpers.c
> +++ b/rust/helpers/helpers.c
> @@ -19,6 +19,7 @@
>  #include "io.c"
>  #include "jump_label.c"
>  #include "kunit.c"
> +#include "math64.c"
>  #include "mutex.c"
>  #include "page.c"
>  #include "platform.c"
> diff --git a/rust/helpers/math64.c b/rust/helpers/math64.c
> new file mode 100644
> index 000000000000..f94708cf8fcb
> --- /dev/null
> +++ b/rust/helpers/math64.c
> @@ -0,0 +1,8 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <linux/math64.h>
> +
> +s64 rust_helper_div64_s64(s64 dividend, s64 divisor)
> +{
> +	return div64_s64(dividend, divisor);
> +}
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index de07aadd1ff5..d272e0b0b05d 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -60,6 +60,7 @@
>  #[cfg(CONFIG_KUNIT)]
>  pub mod kunit;
>  pub mod list;
> +pub mod math64;
>  pub mod miscdevice;
>  #[cfg(CONFIG_NET)]
>  pub mod net;
> diff --git a/rust/kernel/math64.rs b/rust/kernel/math64.rs
> new file mode 100644
> index 000000000000..523e47911859
> --- /dev/null
> +++ b/rust/kernel/math64.rs
> @@ -0,0 +1,12 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! 64-bit integer arithmetic helpers.
> +//!
> +//! C header: [`include/linux/math64.h`](srctree/include/linux/math64.h)
> +
> +/// Divide a signed 64-bit integer by another signed 64-bit integer.
> +#[inline]
> +pub fn div64_s64(dividend: i64, divisor: i64) -> i64 {
> +    // SAFETY: Calling `div64_s64()` is safe as long as `divisor` is non-zero.
The safety comment is not valid, nowhere is it guaranteed divisor is non-zero.

There's three solutions I can think of:
* Mark this function as `unsafe` and give the responsibility of checking
  this to the caller,
* return a `Result` with a division by zero error type or
* change the type of divisor to `NonZeroI64` [0].

Probably the best way is to use `NonZeroI64` since that way
it's statically guaranteed.

In that case it would also make sense to change `NSEC_PER_USEC` to be `NonZeroI64`.


Link: https://doc.rust-lang.org/nightly/core/num/type.NonZeroI64.html [0]
> +    unsafe { bindings::div64_s64(dividend, divisor) }

Is `s64` just a typedef for `int64_t` and if so this true for every
architecture? (I don't know the C side very well).

If not there might need to be some kind of conversion to make sure
they are passed correctly.

> +}
> diff --git a/rust/kernel/time.rs b/rust/kernel/time.rs
> index 863385905029..7b5255893929 100644
> --- a/rust/kernel/time.rs
> +++ b/rust/kernel/time.rs
> @@ -24,6 +24,8 @@
>  //! C header: [`include/linux/jiffies.h`](srctree/include/linux/jiffies.h).
>  //! C header: [`include/linux/ktime.h`](srctree/include/linux/ktime.h).
>  
> +use crate::math64;
> +
>  pub mod delay;
>  pub mod hrtimer;
>  
> @@ -229,13 +231,16 @@ pub const fn as_nanos(self) -> i64 {
>      /// Return the smallest number of microseconds greater than or equal
>      /// to the value in the [`Delta`].
>      #[inline]
> -    pub const fn as_micros_ceil(self) -> i64 {
> -        self.as_nanos().saturating_add(NSEC_PER_USEC - 1) / NSEC_PER_USEC
> +    pub fn as_micros_ceil(self) -> i64 {
> +        math64::div64_s64(
It would make sense to change `NSEC_PER_USEC` to be `NonZeroI64`.

> +            self.as_nanos().saturating_add(NSEC_PER_USEC - 1),
> +            NSEC_PER_USEC,
> +        )
>      }
>  
>      /// Return the number of milliseconds in the [`Delta`].
>      #[inline]
> -    pub const fn as_millis(self) -> i64 {
> -        self.as_nanos() / NSEC_PER_MSEC
> +    pub fn as_millis(self) -> i64 {
> +        math64::div64_s64(self.as_nanos(), NSEC_PER_MSEC)
>      }
>  }
> 
> base-commit: da37ddd3f607897d039d82e6621671c3f7baa886

Cheers
Christian

Re: [PATCH v15 5/6] rust: time: Add wrapper for fsleep() function

[Russell King (Oracle)]

On Tue, Apr 29, 2025 at 04:16:01PM +0200, Christian Schrefl wrote:
> On 29.04.25 3:17 PM, FUJITA Tomonori wrote:
> > On Mon, 28 Apr 2025 20:16:47 +0200
> > Andreas Hindborg <a.hindborg@kernel.org> wrote:
> > 
> >> Hi Tomonori,
> >>
> >> "FUJITA Tomonori" <fujita.tomonori@gmail.com> writes:
> >>
> >>> Add a wrapper for fsleep(), flexible sleep functions in
> >>> include/linux/delay.h which typically deals with hardware delays.
> >>>
> >>> The kernel supports several sleep functions to handle various lengths
> >>> of delay. This adds fsleep(), automatically chooses the best sleep
> >>> method based on a duration.
> >>>
> >>> sleep functions including fsleep() belongs to TIMERS, not
> >>> TIMEKEEPING. They are maintained separately. rust/kernel/time.rs is an
> >>> abstraction for TIMEKEEPING. To make Rust abstractions match the C
> >>> side, add rust/kernel/time/delay.rs for this wrapper.
> >>>
> >>> fsleep() can only be used in a nonatomic context. This requirement is
> >>> not checked by these abstractions, but it is intended that klint [1]
> >>> or a similar tool will be used to check it in the future.
> >>
> >> I get an error when building this patch for arm32:
> >>
> >>   + kernel-make -j 96 O=/home/aeh/src/linux-rust/test-build-arm-1.78.0 vmlinux modules
> >>   ld.lld: error: undefined symbol: __aeabi_uldivmod
> >>   >>> referenced by kernel.df165ca450b1fd1-cgu.0
> >>   >>>               rust/kernel.o:(kernel::time::delay::fsleep) in archive vmlinux.a
> >>   >>> did you mean: __aeabi_uidivmod
> >>   >>> defined in: vmlinux.a(arch/arm/lib/lib1funcs.o)
> >>
> >> Looks like a division function of some sort is not defined. Can you
> >> reproduce?
> > 
> > Ah, 64-bit integer division on 32-bit architectures.
> > 
> > I think that the DRM QR driver has the same problem:
> > 
> > https://lore.kernel.org/rust-for-linux/CANiq72ke45eOwckMhWHvmwxc03dxr4rnxxKvx+HvWdBLopZfrQ@mail.gmail.com/
> > 
> > It appears that there is still no consensus on how to resolve it. CC
> > the participants in the above thread.
> 
> From what I remember from the thread is that generally 64 bit divisions
> should be avoided (like the solution for DRM).
> 
> > I think that we can drop this patch and better to focus on Instant and
> > Delta types in this merge window.
> > 
> > With the patch below, this issue could be resolved like the C side,
> > but I'm not sure whether we can reach a consensus quickly.
> 
> I think adding rust bindings for this is fine (and most likely needed),
> for cases where it is required.
> 
> > 
> > diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c
> > index 48143cdd26b3..c44d45960eb1 100644
> > --- a/rust/helpers/helpers.c
> > +++ b/rust/helpers/helpers.c
> > @@ -19,6 +19,7 @@
> >  #include "io.c"
> >  #include "jump_label.c"
> >  #include "kunit.c"
> > +#include "math64.c"
> >  #include "mutex.c"
> >  #include "page.c"
> >  #include "platform.c"
> > diff --git a/rust/helpers/math64.c b/rust/helpers/math64.c
> > new file mode 100644
> > index 000000000000..f94708cf8fcb
> > --- /dev/null
> > +++ b/rust/helpers/math64.c
> > @@ -0,0 +1,8 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +#include <linux/math64.h>
> > +
> > +s64 rust_helper_div64_s64(s64 dividend, s64 divisor)
> > +{
> > +	return div64_s64(dividend, divisor);
> > +}
> > diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> > index de07aadd1ff5..d272e0b0b05d 100644
> > --- a/rust/kernel/lib.rs
> > +++ b/rust/kernel/lib.rs
> > @@ -60,6 +60,7 @@
> >  #[cfg(CONFIG_KUNIT)]
> >  pub mod kunit;
> >  pub mod list;
> > +pub mod math64;
> >  pub mod miscdevice;
> >  #[cfg(CONFIG_NET)]
> >  pub mod net;
> > diff --git a/rust/kernel/math64.rs b/rust/kernel/math64.rs
> > new file mode 100644
> > index 000000000000..523e47911859
> > --- /dev/null
> > +++ b/rust/kernel/math64.rs
> > @@ -0,0 +1,12 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +//! 64-bit integer arithmetic helpers.
> > +//!
> > +//! C header: [`include/linux/math64.h`](srctree/include/linux/math64.h)
> > +
> > +/// Divide a signed 64-bit integer by another signed 64-bit integer.
> > +#[inline]
> > +pub fn div64_s64(dividend: i64, divisor: i64) -> i64 {
> > +    // SAFETY: Calling `div64_s64()` is safe as long as `divisor` is non-zero.
> The safety comment is not valid, nowhere is it guaranteed divisor is non-zero.
> 
> There's three solutions I can think of:
> * Mark this function as `unsafe` and give the responsibility of checking
>   this to the caller,
> * return a `Result` with a division by zero error type or
> * change the type of divisor to `NonZeroI64` [0].
> 
> Probably the best way is to use `NonZeroI64` since that way
> it's statically guaranteed.
> 
> In that case it would also make sense to change `NSEC_PER_USEC` to be `NonZeroI64`.
> 
> 
> Link: https://doc.rust-lang.org/nightly/core/num/type.NonZeroI64.html [0]
> > +    unsafe { bindings::div64_s64(dividend, divisor) }
> 
> Is `s64` just a typedef for `int64_t` and if so this true for every
> architecture? (I don't know the C side very well).
> 
> If not there might need to be some kind of conversion to make sure
> they are passed correctly.
> 
> > +}
> > diff --git a/rust/kernel/time.rs b/rust/kernel/time.rs
> > index 863385905029..7b5255893929 100644
> > --- a/rust/kernel/time.rs
> > +++ b/rust/kernel/time.rs
> > @@ -24,6 +24,8 @@
> >  //! C header: [`include/linux/jiffies.h`](srctree/include/linux/jiffies.h).
> >  //! C header: [`include/linux/ktime.h`](srctree/include/linux/ktime.h).
> >  
> > +use crate::math64;
> > +
> >  pub mod delay;
> >  pub mod hrtimer;
> >  
> > @@ -229,13 +231,16 @@ pub const fn as_nanos(self) -> i64 {
> >      /// Return the smallest number of microseconds greater than or equal
> >      /// to the value in the [`Delta`].
> >      #[inline]
> > -    pub const fn as_micros_ceil(self) -> i64 {
> > -        self.as_nanos().saturating_add(NSEC_PER_USEC - 1) / NSEC_PER_USEC
> > +    pub fn as_micros_ceil(self) -> i64 {
> > +        math64::div64_s64(
> It would make sense to change `NSEC_PER_USEC` to be `NonZeroI64`.
> 
> > +            self.as_nanos().saturating_add(NSEC_PER_USEC - 1),
> > +            NSEC_PER_USEC,
> > +        )
> >      }
> >  
> >      /// Return the number of milliseconds in the [`Delta`].
> >      #[inline]
> > -    pub const fn as_millis(self) -> i64 {
> > -        self.as_nanos() / NSEC_PER_MSEC
> > +    pub fn as_millis(self) -> i64 {
> > +        math64::div64_s64(self.as_nanos(), NSEC_PER_MSEC)

There is no way this should ever be an expensive 64-bit by 64-bit
division. Think about value of the divisor here - there's 1000us
in 1ms, and 1000ns in 1us, so this has the value of 1000000,
which is 20 bits.

So for a 32-bit architecture, trying to do a 64-bit by 64-bit
division is expensive, and 32-bit architectures don't implement
this as a general rule because of this - most times you do not
have a 64-bit divisor, but something much smaller, making a
64-bit by 32-bit division more suitable. That is a lot faster on
32-bit architectures.

So, I think more thought is needed to be put into this by Rust
folk, rather than blindly forcing everything to be 64-bit by
64-bit even when the divisor is 20-bit.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH v15 5/6] rust: time: Add wrapper for fsleep() function

[Miguel Ojeda]

On Tue, Apr 29, 2025 at 3:17 PM FUJITA Tomonori
<fujita.tomonori@gmail.com> wrote:
>
> Ah, 64-bit integer division on 32-bit architectures.
>
> I think that the DRM QR driver has the same problem:
>
> https://lore.kernel.org/rust-for-linux/CANiq72ke45eOwckMhWHvmwxc03dxr4rnxxKvx+HvWdBLopZfrQ@mail.gmail.com/

Yeah.

> It appears that there is still no consensus on how to resolve it. CC
> the participants in the above thread.
>
> I think that we can drop this patch and better to focus on Instant and
> Delta types in this merge window.
>
> With the patch below, this issue could be resolved like the C side,
> but I'm not sure whether we can reach a consensus quickly.

I think using the C ones is fine for the moment, but up to what arm
and others think.

This one is also a constant, so something simpler may be better (and
it is also a power of 10 divisor, so the other suggestions on that
thread would apply too).

> +/// Divide a signed 64-bit integer by another signed 64-bit integer.

Perhaps an example wouldn't hurt. And if `unsafe` or fallible is
picked, then the example allows to showcase (and test) what happens in
the zero divisor case that Christian points out.

By the way, apart from that case, we should also consider the min/-1 case.

We may want an assert under `CONFIG_RUST_OVERFLOW_CHECKS=y`, too.

And it wouldn't hurt to test a few other boundary values, with the new
`#[test]` support.

Thanks!

Cheers,
Miguel

Re: [PATCH v15 5/6] rust: time: Add wrapper for fsleep() function

[Arnd Bergmann]

On Tue, Apr 29, 2025, at 15:17, FUJITA Tomonori wrote:
> On Mon, 28 Apr 2025 20:16:47 +0200 Andreas Hindborg <a.hindborg@kernel.org> wrote:
>      /// Return the number of milliseconds in the [`Delta`].
>      #[inline]
> -    pub const fn as_millis(self) -> i64 {
> -        self.as_nanos() / NSEC_PER_MSEC
> +    pub fn as_millis(self) -> i64 {
> +        math64::div64_s64(self.as_nanos(), NSEC_PER_MSEC)
>      }
>  }

I think simply calling ktime_to_ms()/ktime_to_us() should result
in reasonably efficient code, since the C version is able to
convert the constant divisor into a multiply/shift operation.

div64_s64() itself is never optimized for constant arguments since
the C version is not inline, if any driver needs those, this is
better done open-coded so hopefully it gets caught in review
when this is called in a fast path.

       Arnd

Re: [PATCH v15 5/6] rust: time: Add wrapper for fsleep() function

[Boqun Feng]

On Tue, Apr 29, 2025, at 8:51 AM, Arnd Bergmann wrote:
> On Tue, Apr 29, 2025, at 15:17, FUJITA Tomonori wrote:
>> On Mon, 28 Apr 2025 20:16:47 +0200 Andreas Hindborg <a.hindborg@kernel.org> wrote:
>>      /// Return the number of milliseconds in the [`Delta`].
>>      #[inline]
>> -    pub const fn as_millis(self) -> i64 {
>> -        self.as_nanos() / NSEC_PER_MSEC
>> +    pub fn as_millis(self) -> i64 {
>> +        math64::div64_s64(self.as_nanos(), NSEC_PER_MSEC)
>>      }
>>  }
>
> I think simply calling ktime_to_ms()/ktime_to_us() should result
> in reasonably efficient code, since the C version is able to
> convert the constant divisor into a multiply/shift operation.
>

Well, before we jump into this, I would like to understand why
this is not optimized with multiply/shift operations on arm in
Rust code. Ideally all the dividing constants cases should not
need to call a C function.

Regards,
Boqun

> div64_s64() itself is never optimized for constant arguments since
> the C version is not inline, if any driver needs those, this is
> better done open-coded so hopefully it gets caught in review
> when this is called in a fast path.
>
>        Arnd

Re: [PATCH v15 5/6] rust: time: Add wrapper for fsleep() function

[Arnd Bergmann]

On Tue, Apr 29, 2025, at 18:03, Boqun Feng wrote:
> On Tue, Apr 29, 2025, at 8:51 AM, Arnd Bergmann wrote:
>> On Tue, Apr 29, 2025, at 15:17, FUJITA Tomonori wrote:
>>> On Mon, 28 Apr 2025 20:16:47 +0200 Andreas Hindborg <a.hindborg@kernel.org> wrote:
>>>      /// Return the number of milliseconds in the [`Delta`].
>>>      #[inline]
>>> -    pub const fn as_millis(self) -> i64 {
>>> -        self.as_nanos() / NSEC_PER_MSEC
>>> +    pub fn as_millis(self) -> i64 {
>>> +        math64::div64_s64(self.as_nanos(), NSEC_PER_MSEC)
>>>      }
>>>  }
>>
>> I think simply calling ktime_to_ms()/ktime_to_us() should result
>> in reasonably efficient code, since the C version is able to
>> convert the constant divisor into a multiply/shift operation.
>>
>
> Well, before we jump into this, I would like to understand why
> this is not optimized with multiply/shift operations on arm in
> Rust code. Ideally all the dividing constants cases should not
> need to call a C function.

I think it's just because nobody has rewritten the
macros from include/asm-generic/div64.h into rust code.

The compiler could do the same transformation, but they
generally just fall back to calling a libgcc function.

     Arnd

Re: [PATCH v15 5/6] rust: time: Add wrapper for fsleep() function

[Boqun Feng]

On Tue, Apr 29, 2025 at 06:11:02PM +0200, Arnd Bergmann wrote:
> On Tue, Apr 29, 2025, at 18:03, Boqun Feng wrote:
> > On Tue, Apr 29, 2025, at 8:51 AM, Arnd Bergmann wrote:
> >> On Tue, Apr 29, 2025, at 15:17, FUJITA Tomonori wrote:
> >>> On Mon, 28 Apr 2025 20:16:47 +0200 Andreas Hindborg <a.hindborg@kernel.org> wrote:
> >>>      /// Return the number of milliseconds in the [`Delta`].
> >>>      #[inline]
> >>> -    pub const fn as_millis(self) -> i64 {
> >>> -        self.as_nanos() / NSEC_PER_MSEC
> >>> +    pub fn as_millis(self) -> i64 {
> >>> +        math64::div64_s64(self.as_nanos(), NSEC_PER_MSEC)
> >>>      }
> >>>  }
> >>
> >> I think simply calling ktime_to_ms()/ktime_to_us() should result
> >> in reasonably efficient code, since the C version is able to
> >> convert the constant divisor into a multiply/shift operation.
> >>
> >
> > Well, before we jump into this, I would like to understand why
> > this is not optimized with multiply/shift operations on arm in
> > Rust code. Ideally all the dividing constants cases should not
> > need to call a C function.
> 
> I think it's just because nobody has rewritten the
> macros from include/asm-generic/div64.h into rust code.
> 
> The compiler could do the same transformation, but they
> generally just fall back to calling a libgcc function.
> 

FWIW, I found this:

	https://github.com/llvm/llvm-project/issues/63731

seems a WIP though.

Would it make sense if we rely on compiler optimization when it's
avaiable (for x86_64, arm64, riscv, etc), and only call ktime_to_ms() if
not? The downside of calling ktime_to_ms() are:

* it's a call function, and cannot be inlined with LTO or INLINE_HELPER:

	https://lore.kernel.org/all/20250319205141.3528424-1-gary@garyguo.net/

* it doesn't provide the overflow checking even if
  CONFIG_RUST_OVERFLOW_CHECKS=y

Thoughts?

Regards,
Boqun

>      Arnd

Re: [PATCH v15 5/6] rust: time: Add wrapper for fsleep() function

[Arnd Bergmann]

On Tue, Apr 29, 2025, at 19:15, Boqun Feng wrote:
> On Tue, Apr 29, 2025 at 06:11:02PM +0200, Arnd Bergmann wrote:
>> On Tue, Apr 29, 2025, at 18:03, Boqun Feng wrote:
>
> Would it make sense if we rely on compiler optimization when it's
> avaiable (for x86_64, arm64, riscv, etc), and only call ktime_to_ms() if
> not? The downside of calling ktime_to_ms() are:
>
> * it's a call function, and cannot be inlined with LTO or INLINE_HELPER:
>
> 	https://lore.kernel.org/all/20250319205141.3528424-1-gary@garyguo.net/
>
> * it doesn't provide the overflow checking even if
>   CONFIG_RUST_OVERFLOW_CHECKS=y
>
> Thoughts?

The function call overhead is tiny compared to replacing a 64-bit
division with a constant mult/shift.

What is the possible overflow that can happen here? For a constant
division at least there is no chance of divide-by-zero. Do you mean
truncating to 32 bit?

     Arnd

Re: [PATCH v15 5/6] rust: time: Add wrapper for fsleep() function

[Boqun Feng]

On Tue, Apr 29, 2025 at 08:33:47PM +0200, Arnd Bergmann wrote:
> On Tue, Apr 29, 2025, at 19:15, Boqun Feng wrote:
> > On Tue, Apr 29, 2025 at 06:11:02PM +0200, Arnd Bergmann wrote:
> >> On Tue, Apr 29, 2025, at 18:03, Boqun Feng wrote:
> >
> > Would it make sense if we rely on compiler optimization when it's
> > avaiable (for x86_64, arm64, riscv, etc), and only call ktime_to_ms() if
> > not? The downside of calling ktime_to_ms() are:
> >
> > * it's a call function, and cannot be inlined with LTO or INLINE_HELPER:
> >
> > 	https://lore.kernel.org/all/20250319205141.3528424-1-gary@garyguo.net/
> >
> > * it doesn't provide the overflow checking even if
> >   CONFIG_RUST_OVERFLOW_CHECKS=y
> >
> > Thoughts?
> 
> The function call overhead is tiny compared to replacing a 64-bit
> division with a constant mult/shift.
> 

Just to be clear, are you essientially saying that even in C,
ktime_to_ms() is not worth inlining? Because the call overhead is tiny
compared to the function own cost?

My impression is that on x86 at least, function call is 10+ cycles, and
multiply is 3 cycles, so I would think that ktime_to_ms() itself is at
most 10 cycles. Maybe I'm out of date of the modern micro-architecture?

> What is the possible overflow that can happen here? For a constant
> division at least there is no chance of divide-by-zero. Do you mean
> truncating to 32 bit?
> 

I was referring the last part of Miguel's email:

	https://lore.kernel.org/rust-for-linux/CANiq72mMRpY4NC4_8v_wDpq6Z3qs99Y8gXd-7XL_3Bed58gkJg@mail.gmail.com/

Regards,
Boqun

>      Arnd

Re: [PATCH v15 5/6] rust: time: Add wrapper for fsleep() function

[Boqun Feng]

On Tue, Apr 29, 2025 at 12:14:04PM -0700, Boqun Feng wrote:
> On Tue, Apr 29, 2025 at 08:33:47PM +0200, Arnd Bergmann wrote:
> > On Tue, Apr 29, 2025, at 19:15, Boqun Feng wrote:
> > > On Tue, Apr 29, 2025 at 06:11:02PM +0200, Arnd Bergmann wrote:
> > >> On Tue, Apr 29, 2025, at 18:03, Boqun Feng wrote:
> > >
> > > Would it make sense if we rely on compiler optimization when it's
> > > avaiable (for x86_64, arm64, riscv, etc), and only call ktime_to_ms() if

In case I wasn't clear, nowadays Rust compiler supports optimizating
constant division into multi/shift on x86_64, arm64, riscv already, the
optimization is only not availabe for arm32. (It's actually an
optimization provided by LLVM I think)

Regards,
Boqun

> > > not? The downside of calling ktime_to_ms() are:
> > >
> > > * it's a call function, and cannot be inlined with LTO or INLINE_HELPER:
> > >
> > > 	https://lore.kernel.org/all/20250319205141.3528424-1-gary@garyguo.net/
> > >
> > > * it doesn't provide the overflow checking even if
> > >   CONFIG_RUST_OVERFLOW_CHECKS=y
> > >
> > > Thoughts?
> > 
> > The function call overhead is tiny compared to replacing a 64-bit
> > division with a constant mult/shift.
> > 
> 
> Just to be clear, are you essientially saying that even in C,
> ktime_to_ms() is not worth inlining? Because the call overhead is tiny
> compared to the function own cost?
> 
> My impression is that on x86 at least, function call is 10+ cycles, and
> multiply is 3 cycles, so I would think that ktime_to_ms() itself is at
> most 10 cycles. Maybe I'm out of date of the modern micro-architecture?
> 
> > What is the possible overflow that can happen here? For a constant
> > division at least there is no chance of divide-by-zero. Do you mean
> > truncating to 32 bit?
> > 
> 
> I was referring the last part of Miguel's email:
> 
> 	https://lore.kernel.org/rust-for-linux/CANiq72mMRpY4NC4_8v_wDpq6Z3qs99Y8gXd-7XL_3Bed58gkJg@mail.gmail.com/
> 
> Regards,
> Boqun
> 
> >      Arnd

Re: [PATCH v15 0/6] rust: Add IO polling

[Andreas Hindborg]

On Thu, 24 Apr 2025 04:28:50 +0900, FUJITA Tomonori wrote:
> Add two new types, Instant and Delta, which represent a specific point
> in time and a span of time, respectively, with Rust version of
> fsleep().
> 
> I dropped patches related with read_poll_timeout() in this version,
> which we haven't reached agreement on yet. There are other potential
> uses for the Instant and Delta types, so it's better to upstream them
> first. Note that I haven't changed the subject to avoid confusion.
> 
> [...]

Applied, thanks!

[1/6] rust: hrtimer: Add Ktime temporarily
      commit: 1116f0c5ff3385658ceb8ae2c5c4cb05bd7836d7
[2/6] rust: time: Add PartialEq/Eq/PartialOrd/Ord trait to Ktime
      commit: 3caad57d29b5f64fa41cff0b12cc5d9144dacb04
[3/6] rust: time: Introduce Delta type
      commit: fae0cdc12340ce402a4681dba0f357b05d167d00
[4/6] rust: time: Introduce Instant type
      commit: ddc671506458849c1a1c882208bbffed033e770c
[5/6] Dropped
[6/6] MAINTAINERS: rust: Add a new section for all of the time stuff
      commit: 679185904972421c570a1c337a8266835045012d
      [ Changed T: entry branch to `timekeeping-next` - Andreas ]

Best regards,
-- 
Andreas Hindborg <a.hindborg@kernel.org>

Re: [PATCH v15 5/6] rust: time: Add wrapper for fsleep() function

[FUJITA Tomonori]

On Tue, 29 Apr 2025 16:35:09 +0200
Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote:

>> It appears that there is still no consensus on how to resolve it. CC
>> the participants in the above thread.
>>
>> I think that we can drop this patch and better to focus on Instant and
>> Delta types in this merge window.
>>
>> With the patch below, this issue could be resolved like the C side,
>> but I'm not sure whether we can reach a consensus quickly.
> 
> I think using the C ones is fine for the moment, but up to what arm
> and others think.

I don't think anyone would disagree with the rule Russell mentioned
that expensive 64-bit by 64-bit division should be avoided unless
absolutely necessary so we should go with function div_s64() instead
of function div64_s64().

The DRM QR driver was already fixed by avoiding 64-bit division, so
for now, the only place where we still need to solve this issue is the
time abstraction. So, it seems like Arnd's suggestion to simply call
ktime_to_ms() or ktime_to_us() is the right way to go.

> This one is also a constant, so something simpler may be better (and
> it is also a power of 10 divisor, so the other suggestions on that
> thread would apply too).

One downside of calling the C's functions is that the as_micros/millis
methods can no longer be const fn (or is there a way to make that
work?). We could implement the method Paolo suggested from Hacker's
Delight, 2nd edition in Rust and keep using const fn, but I'm not sure
if it's really worth it.

Any thoughts?

diff --git a/rust/kernel/time.rs b/rust/kernel/time.rs
index a8089a98da9e..daf9e5925e47 100644
--- a/rust/kernel/time.rs
+++ b/rust/kernel/time.rs
@@ -229,12 +229,116 @@ pub const fn as_nanos(self) -> i64 {
     /// to the value in the [`Delta`].
     #[inline]
     pub const fn as_micros_ceil(self) -> i64 {
-        self.as_nanos().saturating_add(NSEC_PER_USEC - 1) / NSEC_PER_USEC
+        const NSEC_PER_USEC_EXP: u32 = 3;
+
+        div10::<NSEC_PER_USEC_EXP>(self.as_nanos().saturating_add(NSEC_PER_USEC - 1))
     }
 
     /// Return the number of milliseconds in the [`Delta`].
     #[inline]
     pub const fn as_millis(self) -> i64 {
-        self.as_nanos() / NSEC_PER_MSEC
+        const NSEC_PER_MSEC_EXP: u32 = 6;
+
+        div10::<NSEC_PER_MSEC_EXP>(self.as_nanos())
+    }
+}
+
+/// Precomputed magic constants for division by powers of 10.
+///
+/// Each entry corresponds to dividing a number by `10^exp`, where `exp` ranges from 0 to 18.
+/// These constants were computed using the algorithm from Hacker's Delight, 2nd edition.
+struct MagicMul {
+    mult: u64,
+    shift: u32,
+}
+
+const DIV10: [MagicMul; 19] = [
+    MagicMul {
+        mult: 0x1,
+        shift: 0,
+    },
+    MagicMul {
+        mult: 0x6666666666666667,
+        shift: 66,
+    },
+    MagicMul {
+        mult: 0xA3D70A3D70A3D70B,
+        shift: 70,
+    },
+    MagicMul {
+        mult: 0x20C49BA5E353F7CF,
+        shift: 71,
+    },
+    MagicMul {
+        mult: 0x346DC5D63886594B,
+        shift: 75,
+    },
+    MagicMul {
+        mult: 0x29F16B11C6D1E109,
+        shift: 78,
+    },
+    MagicMul {
+        mult: 0x431BDE82D7B634DB,
+        shift: 82,
+    },
+    MagicMul {
+        mult: 0xD6BF94D5E57A42BD,
+        shift: 87,
+    },
+    MagicMul {
+        mult: 0x55E63B88C230E77F,
+        shift: 89,
+    },
+    MagicMul {
+        mult: 0x112E0BE826D694B3,
+        shift: 90,
+    },
+    MagicMul {
+        mult: 0x036F9BFB3AF7B757,
+        shift: 91,
+    },
+    MagicMul {
+        mult: 0x00AFEBFF0BCB24AB,
+        shift: 92,
+    },
+    MagicMul {
+        mult: 0x232F33025BD42233,
+        shift: 101,
+    },
+    MagicMul {
+        mult: 0x384B84D092ED0385,
+        shift: 105,
+    },
+    MagicMul {
+        mult: 0x0B424DC35095CD81,
+        shift: 106,
+    },
+    MagicMul {
+        mult: 0x480EBE7B9D58566D,
+        shift: 112,
+    },
+    MagicMul {
+        mult: 0x39A5652FB1137857,
+        shift: 115,
+    },
+    MagicMul {
+        mult: 0x5C3BD5191B525A25,
+        shift: 119,
+    },
+    MagicMul {
+        mult: 0x12725DD1D243ABA1,
+        shift: 120,
+    },
+];
+
+const fn div10<const EXP: u32>(val: i64) -> i64 {
+    crate::build_assert!(EXP <= 18);
+    let MagicMul { mult, shift } = DIV10[EXP as usize];
+    let abs_val = val.wrapping_abs() as u64;
+    let ret = ((abs_val as u128 * mult as u128) >> shift) as u64;
+    if val < 0 {
+        -(ret as i64)
+    } else {
+        ret as i64
     }
 }

Re: [PATCH v15 5/6] rust: time: Add wrapper for fsleep() function

[Boqun Feng]

On Wed, Apr 30, 2025 at 10:51:31PM +0900, FUJITA Tomonori wrote:
> On Tue, 29 Apr 2025 16:35:09 +0200
> Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote:
> 
> >> It appears that there is still no consensus on how to resolve it. CC
> >> the participants in the above thread.
> >>
> >> I think that we can drop this patch and better to focus on Instant and
> >> Delta types in this merge window.
> >>
> >> With the patch below, this issue could be resolved like the C side,
> >> but I'm not sure whether we can reach a consensus quickly.
> > 
> > I think using the C ones is fine for the moment, but up to what arm
> > and others think.
> 
> I don't think anyone would disagree with the rule Russell mentioned
> that expensive 64-bit by 64-bit division should be avoided unless
> absolutely necessary so we should go with function div_s64() instead
> of function div64_s64().
> 
> The DRM QR driver was already fixed by avoiding 64-bit division, so
> for now, the only place where we still need to solve this issue is the
> time abstraction. So, it seems like Arnd's suggestion to simply call
> ktime_to_ms() or ktime_to_us() is the right way to go.
> 
> > This one is also a constant, so something simpler may be better (and
> > it is also a power of 10 divisor, so the other suggestions on that
> > thread would apply too).
> 
> One downside of calling the C's functions is that the as_micros/millis
> methods can no longer be const fn (or is there a way to make that
> work?). We could implement the method Paolo suggested from Hacker's
> Delight, 2nd edition in Rust and keep using const fn, but I'm not sure
> if it's really worth it.
> 
> Any thoughts?
> 

For ARM, where the constant division optimization (into to mult/shift)
is not available, I'm OK with anything you and others come out with
(calling a C function, implementing our own optimization, etc.) For
other architectures where the compilers can do the right thing, I
suggest we use the compiler optimization and don't re-invent the wheel.
For example, in your div10() solution below, we can have a generic
version which just uses a normal division for x86, arm64, riscv, etc,
and an ARM specific version.

Btw, the const fn point is a good one, thanks for bringing that up.

Regards,
Boqun

> diff --git a/rust/kernel/time.rs b/rust/kernel/time.rs
> index a8089a98da9e..daf9e5925e47 100644
> --- a/rust/kernel/time.rs
> +++ b/rust/kernel/time.rs
> @@ -229,12 +229,116 @@ pub const fn as_nanos(self) -> i64 {
>      /// to the value in the [`Delta`].
>      #[inline]
>      pub const fn as_micros_ceil(self) -> i64 {
> -        self.as_nanos().saturating_add(NSEC_PER_USEC - 1) / NSEC_PER_USEC
> +        const NSEC_PER_USEC_EXP: u32 = 3;
> +
> +        div10::<NSEC_PER_USEC_EXP>(self.as_nanos().saturating_add(NSEC_PER_USEC - 1))
>      }
>  
>      /// Return the number of milliseconds in the [`Delta`].
>      #[inline]
>      pub const fn as_millis(self) -> i64 {
> -        self.as_nanos() / NSEC_PER_MSEC
> +        const NSEC_PER_MSEC_EXP: u32 = 6;
> +
> +        div10::<NSEC_PER_MSEC_EXP>(self.as_nanos())
> +    }
> +}
> +
> +/// Precomputed magic constants for division by powers of 10.
> +///
> +/// Each entry corresponds to dividing a number by `10^exp`, where `exp` ranges from 0 to 18.
> +/// These constants were computed using the algorithm from Hacker's Delight, 2nd edition.
> +struct MagicMul {
> +    mult: u64,
> +    shift: u32,
> +}
> +
> +const DIV10: [MagicMul; 19] = [
> +    MagicMul {
> +        mult: 0x1,
> +        shift: 0,
> +    },
> +    MagicMul {
> +        mult: 0x6666666666666667,
> +        shift: 66,
> +    },
> +    MagicMul {
> +        mult: 0xA3D70A3D70A3D70B,
> +        shift: 70,
> +    },
> +    MagicMul {
> +        mult: 0x20C49BA5E353F7CF,
> +        shift: 71,
> +    },
> +    MagicMul {
> +        mult: 0x346DC5D63886594B,
> +        shift: 75,
> +    },
> +    MagicMul {
> +        mult: 0x29F16B11C6D1E109,
> +        shift: 78,
> +    },
> +    MagicMul {
> +        mult: 0x431BDE82D7B634DB,
> +        shift: 82,
> +    },
> +    MagicMul {
> +        mult: 0xD6BF94D5E57A42BD,
> +        shift: 87,
> +    },
> +    MagicMul {
> +        mult: 0x55E63B88C230E77F,
> +        shift: 89,
> +    },
> +    MagicMul {
> +        mult: 0x112E0BE826D694B3,
> +        shift: 90,
> +    },
> +    MagicMul {
> +        mult: 0x036F9BFB3AF7B757,
> +        shift: 91,
> +    },
> +    MagicMul {
> +        mult: 0x00AFEBFF0BCB24AB,
> +        shift: 92,
> +    },
> +    MagicMul {
> +        mult: 0x232F33025BD42233,
> +        shift: 101,
> +    },
> +    MagicMul {
> +        mult: 0x384B84D092ED0385,
> +        shift: 105,
> +    },
> +    MagicMul {
> +        mult: 0x0B424DC35095CD81,
> +        shift: 106,
> +    },
> +    MagicMul {
> +        mult: 0x480EBE7B9D58566D,
> +        shift: 112,
> +    },
> +    MagicMul {
> +        mult: 0x39A5652FB1137857,
> +        shift: 115,
> +    },
> +    MagicMul {
> +        mult: 0x5C3BD5191B525A25,
> +        shift: 119,
> +    },
> +    MagicMul {
> +        mult: 0x12725DD1D243ABA1,
> +        shift: 120,
> +    },
> +];
> +
> +const fn div10<const EXP: u32>(val: i64) -> i64 {
> +    crate::build_assert!(EXP <= 18);
> +    let MagicMul { mult, shift } = DIV10[EXP as usize];
> +    let abs_val = val.wrapping_abs() as u64;
> +    let ret = ((abs_val as u128 * mult as u128) >> shift) as u64;
> +    if val < 0 {
> +        -(ret as i64)
> +    } else {
> +        ret as i64
>      }
>  }

Re: [PATCH v15 5/6] rust: time: Add wrapper for fsleep() function

[Christian Schrefl]

On 30.04.25 3:51 PM, FUJITA Tomonori wrote:
> On Tue, 29 Apr 2025 16:35:09 +0200
> Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote:
> 
>>> It appears that there is still no consensus on how to resolve it. CC
>>> the participants in the above thread.
>>>
>>> I think that we can drop this patch and better to focus on Instant and
>>> Delta types in this merge window.
>>>
>>> With the patch below, this issue could be resolved like the C side,
>>> but I'm not sure whether we can reach a consensus quickly.
>>
>> I think using the C ones is fine for the moment, but up to what arm
>> and others think.
> 
> I don't think anyone would disagree with the rule Russell mentioned
> that expensive 64-bit by 64-bit division should be avoided unless
> absolutely necessary so we should go with function div_s64() instead
> of function div64_s64().
> 
> The DRM QR driver was already fixed by avoiding 64-bit division, so
> for now, the only place where we still need to solve this issue is the
> time abstraction. So, it seems like Arnd's suggestion to simply call
> ktime_to_ms() or ktime_to_us() is the right way to go.
> 
>> This one is also a constant, so something simpler may be better (and
>> it is also a power of 10 divisor, so the other suggestions on that
>> thread would apply too).
> 
> One downside of calling the C's functions is that the as_micros/millis
> methods can no longer be const fn (or is there a way to make that
> work?).

It would theoretically be possible to use the unstable `const_eval_select`
to use the C implementation at runtume and just divide at compile time.
I don't think that we want to do this (or that it should be done in any
serious project should do this) but it would be technically possible.

I'm also not sure how/if const-eval would handle the 64 by 64 bit division.

> We could implement the method Paolo suggested from Hacker's
> Delight, 2nd edition in Rust and keep using const fn, but I'm not sure
> if it's really worth it.
> 
> Any thoughts?

`const fn` would be nice, but if it is not currently needed and would
complicate the implementation we should probably keep it non-const
until someone needs it to be const.

> 
> diff --git a/rust/kernel/time.rs b/rust/kernel/time.rs
> index a8089a98da9e..daf9e5925e47 100644
> --- a/rust/kernel/time.rs
> +++ b/rust/kernel/time.rs
> @@ -229,12 +229,116 @@ pub const fn as_nanos(self) -> i64 {
>      /// to the value in the [`Delta`].
>      #[inline]
>      pub const fn as_micros_ceil(self) -> i64 {
> -        self.as_nanos().saturating_add(NSEC_PER_USEC - 1) / NSEC_PER_USEC
> +        const NSEC_PER_USEC_EXP: u32 = 3;
> +
> +        div10::<NSEC_PER_USEC_EXP>(self.as_nanos().saturating_add(NSEC_PER_USEC - 1))
>      }
>  
>      /// Return the number of milliseconds in the [`Delta`].
>      #[inline]
>      pub const fn as_millis(self) -> i64 {
> -        self.as_nanos() / NSEC_PER_MSEC
> +        const NSEC_PER_MSEC_EXP: u32 = 6;
> +
> +        div10::<NSEC_PER_MSEC_EXP>(self.as_nanos())
> +    }
> +}
> +
> +/// Precomputed magic constants for division by powers of 10.
> +///
> +/// Each entry corresponds to dividing a number by `10^exp`, where `exp` ranges from 0 to 18.
> +/// These constants were computed using the algorithm from Hacker's Delight, 2nd edition.
> +struct MagicMul {
> +    mult: u64,
> +    shift: u32,
> +}
> +
> +const DIV10: [MagicMul; 19] = [
> +    MagicMul {
> +        mult: 0x1,
> +        shift: 0,
> +    },
> +    MagicMul {
> +        mult: 0x6666666666666667,
> +        shift: 66,
> +    },
> +    MagicMul {
> +        mult: 0xA3D70A3D70A3D70B,
> +        shift: 70,
> +    },
> +    MagicMul {
> +        mult: 0x20C49BA5E353F7CF,
> +        shift: 71,
> +    },
> +    MagicMul {
> +        mult: 0x346DC5D63886594B,
> +        shift: 75,
> +    },
> +    MagicMul {
> +        mult: 0x29F16B11C6D1E109,
> +        shift: 78,
> +    },
> +    MagicMul {
> +        mult: 0x431BDE82D7B634DB,
> +        shift: 82,
> +    },
> +    MagicMul {
> +        mult: 0xD6BF94D5E57A42BD,
> +        shift: 87,
> +    },
> +    MagicMul {
> +        mult: 0x55E63B88C230E77F,
> +        shift: 89,
> +    },
> +    MagicMul {
> +        mult: 0x112E0BE826D694B3,
> +        shift: 90,
> +    },
> +    MagicMul {
> +        mult: 0x036F9BFB3AF7B757,
> +        shift: 91,
> +    },
> +    MagicMul {
> +        mult: 0x00AFEBFF0BCB24AB,
> +        shift: 92,
> +    },
> +    MagicMul {
> +        mult: 0x232F33025BD42233,
> +        shift: 101,
> +    },
> +    MagicMul {
> +        mult: 0x384B84D092ED0385,
> +        shift: 105,
> +    },
> +    MagicMul {
> +        mult: 0x0B424DC35095CD81,
> +        shift: 106,
> +    },
> +    MagicMul {
> +        mult: 0x480EBE7B9D58566D,
> +        shift: 112,
> +    },
> +    MagicMul {
> +        mult: 0x39A5652FB1137857,
> +        shift: 115,
> +    },
> +    MagicMul {
> +        mult: 0x5C3BD5191B525A25,
> +        shift: 119,
> +    },
> +    MagicMul {
> +        mult: 0x12725DD1D243ABA1,
> +        shift: 120,
> +    },
> +];
> +
> +const fn div10<const EXP: u32>(val: i64) -> i64 {
> +    crate::build_assert!(EXP <= 18);
> +    let MagicMul { mult, shift } = DIV10[EXP as usize];
> +    let abs_val = val.wrapping_abs() as u64;
> +    let ret = ((abs_val as u128 * mult as u128) >> shift) as u64;
> +    if val < 0 {
> +        -(ret as i64)
> +    } else {
> +        ret as i64
>      }
>  }