* [PATCH v1 0/2] Add Delta::to_jiffies_timeout() with tests @ 2026-08-11 15:01 FUJITA Tomonori 2026-08-11 15:01 ` [PATCH v1 1/2] rust: time: add example and KUnit test for Delta::as_millis_ceil() FUJITA Tomonori 2026-08-11 15:01 ` [PATCH v1 2/2] rust: time: add Delta::to_jiffies_timeout() for timeout conversion FUJITA Tomonori 0 siblings, 2 replies; 5+ messages in thread From: FUJITA Tomonori @ 2026-08-11 15:01 UTC (permalink / raw) To: a.hindborg, ojeda Cc: acourbot, aliceryhl, anna-maria, bjorn3_gh, boqun, dakr, daniel.almeida, frederic, gary, jstultz, lossin, lyude, sboyd, tamird, tglx, tmgross, work, rust-for-linux, FUJITA Tomonori From: FUJITA Tomonori <fujita.tomonori@gmail.com> The first patch is new. It adds an example and a KUnit test for Delta::as_millis_ceil(), which Miguel asked for [1]. The second patch was previously posted as patch 4 of [2]; patches 1-3 of that series have been applied. It has been reworked to fix the bug Miguel reported [1]: on 32 bit with HZ=1000, to_jiffies_timeout() could return a value larger than MAX_JIFFY_OFFSET. Both patches follow the format agreed in [3]: the examples show the behaviour, while the boundary inputs live in `#[test]` KUnit tests, with the magic numbers as named constants local to the test module rather than part of the exported API. [1] https://lore.kernel.org/rust-for-linux/CANiq72kHP1s7bGr4=xfJGR8KY=aUpjOB4OwLEG5E=TKULXg5Rg@mail.gmail.com/ [2] https://lore.kernel.org/rust-for-linux/20260808062839.1159990-1-tomo@flapping.org/ [3] https://lore.kernel.org/rust-for-linux/874ihxgxy0.fsf@kernel.org/ FUJITA Tomonori (2): rust: time: add example and KUnit test for Delta::as_millis_ceil() rust: time: add Delta::to_jiffies_timeout() for timeout conversion rust/kernel/Kconfig.test | 10 ++++ rust/kernel/time.rs | 108 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 118 insertions(+) base-commit: 8fe5e5f62bdb9660999449a4b5eaebcc37d7f842 -- 2.43.0 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v1 1/2] rust: time: add example and KUnit test for Delta::as_millis_ceil() 2026-08-11 15:01 [PATCH v1 0/2] Add Delta::to_jiffies_timeout() with tests FUJITA Tomonori @ 2026-08-11 15:01 ` FUJITA Tomonori 2026-08-11 15:01 ` [PATCH v1 2/2] rust: time: add Delta::to_jiffies_timeout() for timeout conversion FUJITA Tomonori 1 sibling, 0 replies; 5+ messages in thread From: FUJITA Tomonori @ 2026-08-11 15:01 UTC (permalink / raw) To: a.hindborg, ojeda Cc: acourbot, aliceryhl, anna-maria, bjorn3_gh, boqun, dakr, daniel.almeida, frederic, gary, jstultz, lossin, lyude, sboyd, tamird, tglx, tmgross, work, rust-for-linux, FUJITA Tomonori From: FUJITA Tomonori <fujita.tomonori@gmail.com> Add an example showing that the value is rounded towards positive infinity, for both positive and negative spans. as_millis_ceil() uses the same idiom as as_micros_ceil(), which dropped the rounding bias near i64::MAX before commit ec90dfcf05f0 ("rust: time: fix as_micros_ceil() rounding near i64::MAX"), so add a KUnit test for the i64::MAX and i64::MIN extremes. Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com> --- rust/kernel/Kconfig.test | 10 ++++++++++ rust/kernel/time.rs | 42 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+) diff --git a/rust/kernel/Kconfig.test b/rust/kernel/Kconfig.test index e6a5c7a795f0..0087749995d2 100644 --- a/rust/kernel/Kconfig.test +++ b/rust/kernel/Kconfig.test @@ -83,4 +83,14 @@ config RUST_BITFIELD_KUNIT_TEST If unsure, say N. +config RUST_TIME_KUNIT_TEST + bool "KUnit tests for the Rust time API" if !KUNIT_ALL_TESTS + default KUNIT_ALL_TESTS + help + This option enables KUnit tests for the Rust time API. + These are only for development and testing, not for regular + kernel use cases. + + If unsure, say N. + endif diff --git a/rust/kernel/time.rs b/rust/kernel/time.rs index 6c0a5e8090d0..b0b43ad3aa47 100644 --- a/rust/kernel/time.rs +++ b/rust/kernel/time.rs @@ -530,6 +530,21 @@ pub fn as_millis(self) -> i64 { /// Return the smallest number of milliseconds greater than or equal /// to the value in the [`Delta`]. + /// + /// # Examples + /// + /// ``` + /// use kernel::time::Delta; + /// + /// // Whole milliseconds are returned as-is. + /// assert_eq!(Delta::from_millis(2).as_millis_ceil(), 2); + /// assert_eq!(Delta::from_nanos(1_000_000).as_millis_ceil(), 1); + /// assert_eq!(Delta::from_nanos(-1_000_000).as_millis_ceil(), -1); + /// + /// // Anything else is rounded towards positive infinity. + /// assert_eq!(Delta::from_nanos(1_000_001).as_millis_ceil(), 2); + /// assert_eq!(Delta::from_nanos(-1_000_001).as_millis_ceil(), -1); + /// ``` #[inline] pub fn as_millis_ceil(self) -> i64 { // Only positive values need to be rounded up: truncating division already @@ -580,3 +595,30 @@ pub fn rem_nanos(self, dividend: i32) -> Self { } } } + +#[cfg(CONFIG_RUST_TIME_KUNIT_TEST)] +#[macros::kunit_tests(rust_kernel_time)] +mod tests { + use super::*; + + /// `i64::MAX` nanoseconds in milliseconds, rounded towards positive infinity. + const MAX_MILLIS_CEIL: i64 = 9_223_372_036_855; + + /// `i64::MIN` nanoseconds in milliseconds, rounded towards positive infinity. + const MIN_MILLIS_CEIL: i64 = -9_223_372_036_854; + + #[test] + fn as_millis_ceil_extremes() { + // The rounding bias must survive near `i64::MAX`. + assert_eq!( + Delta::from_nanos(i64::MAX).as_millis_ceil(), + MAX_MILLIS_CEIL + ); + + // No bias is applied to negative values, so `i64::MIN` cannot overflow. + assert_eq!( + Delta::from_nanos(i64::MIN).as_millis_ceil(), + MIN_MILLIS_CEIL + ); + } +} -- 2.43.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v1 2/2] rust: time: add Delta::to_jiffies_timeout() for timeout conversion 2026-08-11 15:01 [PATCH v1 0/2] Add Delta::to_jiffies_timeout() with tests FUJITA Tomonori 2026-08-11 15:01 ` [PATCH v1 1/2] rust: time: add example and KUnit test for Delta::as_millis_ceil() FUJITA Tomonori @ 2026-08-11 15:01 ` FUJITA Tomonori 2026-08-11 16:44 ` Gary Guo 1 sibling, 1 reply; 5+ messages in thread From: FUJITA Tomonori @ 2026-08-11 15:01 UTC (permalink / raw) To: a.hindborg, ojeda Cc: acourbot, aliceryhl, anna-maria, bjorn3_gh, boqun, dakr, daniel.almeida, frederic, gary, jstultz, lossin, lyude, sboyd, tamird, tglx, tmgross, work, rust-for-linux, FUJITA Tomonori From: FUJITA Tomonori <fujita.tomonori@gmail.com> Add Delta<Nsec>::to_jiffies_timeout() conversion. Unless the result saturates, the value is rounded up, so the resulting timeout is never shorter than the requested span. The result saturates at zero jiffies for a negative span, i.e. an immediate timeout, and at the kernel's MAX_JIFFY_OFFSET "wait forever" value for a span that is too large. Reviewed-by: Gary Guo <gary@garyguo.net> Reviewed-by: Andreas Hindborg <a.hindborg@kernel.org> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com> --- rust/kernel/time.rs | 66 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/rust/kernel/time.rs b/rust/kernel/time.rs index b0b43ad3aa47..4ec1748c2602 100644 --- a/rust/kernel/time.rs +++ b/rust/kernel/time.rs @@ -39,6 +39,10 @@ /// The number of nanoseconds per second. pub const NSEC_PER_SEC: i64 = bindings::NSEC_PER_SEC as i64; +/// The C side `MAX_JIFFY_OFFSET`, i.e. `((LONG_MAX >> 1) - 1)`, which the kernel +/// treats as an infinite timeout. +const MAX_JIFFY_OFFSET: isize = (isize::MAX >> 1) - 1; + /// The time unit of Linux kernel. One jiffy equals (1/HZ) second. pub type Jiffies = crate::ffi::c_ulong; @@ -569,6 +573,51 @@ pub fn as_millis_ceil(self) -> i64 { } } + /// Convert this span to a [`Delta<Jiffy>`] suitable for use as a timeout. + /// + /// Unless the result saturates, the value is rounded up to the next whole + /// jiffy, so the resulting timeout is never shorter than `self` (as + /// [`msecs_to_jiffies()`] does). + /// + /// The result saturates at zero jiffies for a negative span, i.e. an + /// immediate timeout, and at the kernel's [`MAX_JIFFY_OFFSET`] for a span + /// that is too large, which the C side treats as an infinite timeout. + /// + /// # Examples + /// + /// ``` + /// use kernel::time::Delta; + /// + /// // A negative span is an immediate timeout. + /// assert_eq!(Delta::from_millis(-1).to_jiffies_timeout().as_jiffies(), 0); + /// + /// // A span shorter than a jiffy still waits, i.e. the timeout is never + /// // shorter than the span. + /// assert!(Delta::from_nanos(1).to_jiffies_timeout().as_jiffies() >= 1); + /// ``` + /// + /// [`msecs_to_jiffies()`]: srctree/include/linux/jiffies.h + /// [`MAX_JIFFY_OFFSET`]: srctree/include/linux/jiffies.h + #[inline] + pub fn to_jiffies_timeout(self) -> Delta<Jiffy> { + let msecs = self.as_millis_ceil(); + + // CAST: `msecs` is clamped to `0..=c_uint::MAX`, so it is non-negative and + // fits in `c_uint`. + let msecs = msecs.clamp(0, i64::from(crate::ffi::c_uint::MAX)) as crate::ffi::c_uint; + + // SAFETY: `__msecs_to_jiffies()` is always safe to call. + let jiffies = unsafe { bindings::__msecs_to_jiffies(msecs) }; + + // `__msecs_to_jiffies()` only saturates when its argument is negative as an + // `int`: with `HZ=1000` it returns `msecs` as-is, which exceeds + // `MAX_JIFFY_OFFSET` on 32 bit. + let jiffies = jiffies.min(MAX_JIFFY_OFFSET as crate::ffi::c_ulong); + + // CAST: `jiffies` is clamped to `MAX_JIFFY_OFFSET`, which is `<= isize::MAX`. + Delta::<Jiffy>::from_jiffies(jiffies as isize) + } + /// 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 @@ -621,4 +670,21 @@ fn as_millis_ceil_extremes() { MIN_MILLIS_CEIL ); } + + #[test] + fn to_jiffies_timeout_saturates() { + // `__msecs_to_jiffies()` does not clamp its result for every `HZ` configuration, + // e.g. with `HZ=1000` it returns the millisecond value as-is, which exceeds + // `MAX_JIFFY_OFFSET` on 32 bit. + let max = Delta::from_millis(i64::from(i32::MAX)).to_jiffies_timeout(); + assert!(max.as_jiffies() <= MAX_JIFFY_OFFSET); + + // An overlong span is an infinite timeout. + let overlong = Delta::from_nanos(i64::MAX).to_jiffies_timeout(); + assert_eq!(overlong.as_jiffies(), MAX_JIFFY_OFFSET); + + // A negative span is an immediate timeout, however long it is. + let negative = Delta::from_nanos(i64::MIN).to_jiffies_timeout(); + assert_eq!(negative.as_jiffies(), 0); + } } -- 2.43.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v1 2/2] rust: time: add Delta::to_jiffies_timeout() for timeout conversion 2026-08-11 15:01 ` [PATCH v1 2/2] rust: time: add Delta::to_jiffies_timeout() for timeout conversion FUJITA Tomonori @ 2026-08-11 16:44 ` Gary Guo 2026-08-12 0:05 ` FUJITA Tomonori 0 siblings, 1 reply; 5+ messages in thread From: Gary Guo @ 2026-08-11 16:44 UTC (permalink / raw) To: FUJITA Tomonori, a.hindborg, ojeda Cc: acourbot, aliceryhl, anna-maria, bjorn3_gh, boqun, dakr, daniel.almeida, frederic, gary, jstultz, lossin, lyude, sboyd, tamird, tglx, tmgross, work, rust-for-linux, FUJITA Tomonori On Tue Aug 11, 2026 at 4:01 PM BST, FUJITA Tomonori wrote: > From: FUJITA Tomonori <fujita.tomonori@gmail.com> > > Add Delta<Nsec>::to_jiffies_timeout() conversion. Unless the result > saturates, the value is rounded up, so the resulting timeout is never > shorter than the requested span. > > The result saturates at zero jiffies for a negative span, i.e. an > immediate timeout, and at the kernel's MAX_JIFFY_OFFSET "wait forever" > value for a span that is too large. > > Reviewed-by: Gary Guo <gary@garyguo.net> You should drop old review tags given this has been changed non trivially. > Reviewed-by: Andreas Hindborg <a.hindborg@kernel.org> > Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com> > --- > rust/kernel/time.rs | 66 +++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 66 insertions(+) > > diff --git a/rust/kernel/time.rs b/rust/kernel/time.rs > index b0b43ad3aa47..4ec1748c2602 100644 > --- a/rust/kernel/time.rs > +++ b/rust/kernel/time.rs > @@ -39,6 +39,10 @@ > /// The number of nanoseconds per second. > pub const NSEC_PER_SEC: i64 = bindings::NSEC_PER_SEC as i64; > > +/// The C side `MAX_JIFFY_OFFSET`, i.e. `((LONG_MAX >> 1) - 1)`, which the kernel > +/// treats as an infinite timeout. > +const MAX_JIFFY_OFFSET: isize = (isize::MAX >> 1) - 1; > + > /// The time unit of Linux kernel. One jiffy equals (1/HZ) second. > pub type Jiffies = crate::ffi::c_ulong; > > @@ -569,6 +573,51 @@ pub fn as_millis_ceil(self) -> i64 { > } > } > > + /// Convert this span to a [`Delta<Jiffy>`] suitable for use as a timeout. > + /// > + /// Unless the result saturates, the value is rounded up to the next whole > + /// jiffy, so the resulting timeout is never shorter than `self` (as > + /// [`msecs_to_jiffies()`] does). > + /// > + /// The result saturates at zero jiffies for a negative span, i.e. an > + /// immediate timeout, and at the kernel's [`MAX_JIFFY_OFFSET`] for a span > + /// that is too large, which the C side treats as an infinite timeout. > + /// > + /// # Examples > + /// > + /// ``` > + /// use kernel::time::Delta; > + /// > + /// // A negative span is an immediate timeout. > + /// assert_eq!(Delta::from_millis(-1).to_jiffies_timeout().as_jiffies(), 0); > + /// > + /// // A span shorter than a jiffy still waits, i.e. the timeout is never > + /// // shorter than the span. > + /// assert!(Delta::from_nanos(1).to_jiffies_timeout().as_jiffies() >= 1); > + /// ``` > + /// > + /// [`msecs_to_jiffies()`]: srctree/include/linux/jiffies.h > + /// [`MAX_JIFFY_OFFSET`]: srctree/include/linux/jiffies.h > + #[inline] > + pub fn to_jiffies_timeout(self) -> Delta<Jiffy> { > + let msecs = self.as_millis_ceil(); > + > + // CAST: `msecs` is clamped to `0..=c_uint::MAX`, so it is non-negative and > + // fits in `c_uint`. > + let msecs = msecs.clamp(0, i64::from(crate::ffi::c_uint::MAX)) as crate::ffi::c_uint; > + > + // SAFETY: `__msecs_to_jiffies()` is always safe to call. > + let jiffies = unsafe { bindings::__msecs_to_jiffies(msecs) }; As I mentioned in previous 2 versions, I don't think __msecs_to_jiffies should be used for this. You're doing two rounding and saturation operations here. If you do nsecs_to_jiffies64 and then clamp, you wouldn't run into any of these boundary conditions. Best, Gary > + > + // `__msecs_to_jiffies()` only saturates when its argument is negative as an > + // `int`: with `HZ=1000` it returns `msecs` as-is, which exceeds > + // `MAX_JIFFY_OFFSET` on 32 bit. > + let jiffies = jiffies.min(MAX_JIFFY_OFFSET as crate::ffi::c_ulong); > + > + // CAST: `jiffies` is clamped to `MAX_JIFFY_OFFSET`, which is `<= isize::MAX`. > + Delta::<Jiffy>::from_jiffies(jiffies as isize) > + } > + > /// 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 > @@ -621,4 +670,21 @@ fn as_millis_ceil_extremes() { > MIN_MILLIS_CEIL > ); > } > + > + #[test] > + fn to_jiffies_timeout_saturates() { > + // `__msecs_to_jiffies()` does not clamp its result for every `HZ` configuration, > + // e.g. with `HZ=1000` it returns the millisecond value as-is, which exceeds > + // `MAX_JIFFY_OFFSET` on 32 bit. > + let max = Delta::from_millis(i64::from(i32::MAX)).to_jiffies_timeout(); > + assert!(max.as_jiffies() <= MAX_JIFFY_OFFSET); > + > + // An overlong span is an infinite timeout. > + let overlong = Delta::from_nanos(i64::MAX).to_jiffies_timeout(); > + assert_eq!(overlong.as_jiffies(), MAX_JIFFY_OFFSET); > + > + // A negative span is an immediate timeout, however long it is. > + let negative = Delta::from_nanos(i64::MIN).to_jiffies_timeout(); > + assert_eq!(negative.as_jiffies(), 0); > + } > } ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v1 2/2] rust: time: add Delta::to_jiffies_timeout() for timeout conversion 2026-08-11 16:44 ` Gary Guo @ 2026-08-12 0:05 ` FUJITA Tomonori 0 siblings, 0 replies; 5+ messages in thread From: FUJITA Tomonori @ 2026-08-12 0:05 UTC (permalink / raw) To: gary, ojeda Cc: tomo, a.hindborg, acourbot, aliceryhl, anna-maria, bjorn3_gh, boqun, dakr, daniel.almeida, frederic, jstultz, lossin, lyude, sboyd, tamird, tglx, tmgross, work, rust-for-linux, fujita.tomonori On Tue, 11 Aug 2026 17:44:24 +0100 "Gary Guo" <gary@garyguo.net> wrote: > On Tue Aug 11, 2026 at 4:01 PM BST, FUJITA Tomonori wrote: >> From: FUJITA Tomonori <fujita.tomonori@gmail.com> >> >> Add Delta<Nsec>::to_jiffies_timeout() conversion. Unless the result >> saturates, the value is rounded up, so the resulting timeout is never >> shorter than the requested span. >> >> The result saturates at zero jiffies for a negative span, i.e. an >> immediate timeout, and at the kernel's MAX_JIFFY_OFFSET "wait forever" >> value for a span that is too large. >> >> Reviewed-by: Gary Guo <gary@garyguo.net> > > You should drop old review tags given this has been changed non trivially. Sorry about that. >> + #[inline] >> + pub fn to_jiffies_timeout(self) -> Delta<Jiffy> { >> + let msecs = self.as_millis_ceil(); >> + >> + // CAST: `msecs` is clamped to `0..=c_uint::MAX`, so it is non-negative and >> + // fits in `c_uint`. >> + let msecs = msecs.clamp(0, i64::from(crate::ffi::c_uint::MAX)) as crate::ffi::c_uint; >> + >> + // SAFETY: `__msecs_to_jiffies()` is always safe to call. >> + let jiffies = unsafe { bindings::__msecs_to_jiffies(msecs) }; > > As I mentioned in previous 2 versions, I don't think __msecs_to_jiffies should > be used for this. You're doing two rounding and saturation operations here. Fair enough. > If you do nsecs_to_jiffies64 and then clamp, you wouldn't run into any of these > boundary conditions. I think nsecs_to_jiffies64() has boundary conditions of its own that the Rust side has to take care of: u64 nsecs_to_jiffies64(u64 n) { #if (NSEC_PER_SEC % HZ) == 0 /* Common case, HZ = 100, 128, 200, 250, 256, 500, 512, 1000 etc. */ return div_u64(n, NSEC_PER_SEC / HZ); #elif (HZ % 512) == 0 /* overflow after 292 years if HZ = 1024 */ return div_u64(n * HZ / 512, NSEC_PER_SEC / 512); #else /* * Generic case - optimized for cases where HZ is a multiple of 3. * overflow after 64.99 years, exact for HZ = 60, 72, 90, 120 etc. */ return div_u64(n * 9, (9ull * NSEC_PER_SEC + HZ / 2) / HZ); #endif } In the generic case (e.g. CONFIG_HZ_300), n * 9 can overflow and return a small value so the Rust side has to clamp the Delta before the call. The boundaries move rather than go away. I agree the double rounding should go, but doing the ceiling on the Rust side will need a few rounds of review. Miguel, you suggested landing the minimum set, e.g. the first 2 or first 4 patches. Patches 1-3 are already in, so that leaves patch 4. Would you like to take this version, which clamps the result and has a KUnit test that fails on arm with HZ=1000 without the clamp, or should I hold it for early next cycle? Either is fine with me. ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-08-12 0:05 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-08-11 15:01 [PATCH v1 0/2] Add Delta::to_jiffies_timeout() with tests FUJITA Tomonori 2026-08-11 15:01 ` [PATCH v1 1/2] rust: time: add example and KUnit test for Delta::as_millis_ceil() FUJITA Tomonori 2026-08-11 15:01 ` [PATCH v1 2/2] rust: time: add Delta::to_jiffies_timeout() for timeout conversion FUJITA Tomonori 2026-08-11 16:44 ` Gary Guo 2026-08-12 0:05 ` FUJITA Tomonori
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.