From: "Gary Guo" <gary@garyguo.net>
To: "FUJITA Tomonori" <tomo@flapping.org>, <a.hindborg@kernel.org>,
<ojeda@kernel.org>
Cc: <acourbot@nvidia.com>, <aliceryhl@google.com>,
<anna-maria@linutronix.de>, <bjorn3_gh@protonmail.com>,
<boqun@kernel.org>, <dakr@kernel.org>,
<daniel.almeida@collabora.com>, <frederic@kernel.org>,
<gary@garyguo.net>, <jstultz@google.com>, <lossin@kernel.org>,
<lyude@redhat.com>, <sboyd@kernel.org>, <tamird@kernel.org>,
<tglx@kernel.org>, <tmgross@umich.edu>, <work@onurozkan.dev>,
<rust-for-linux@vger.kernel.org>,
"FUJITA Tomonori" <fujita.tomonori@gmail.com>
Subject: Re: [PATCH v1 2/2] rust: time: add Delta::to_jiffies_timeout() for timeout conversion
Date: Tue, 11 Aug 2026 17:44:24 +0100 [thread overview]
Message-ID: <DKM9KW3ARL1K.1JFJOBOB5EDKF@garyguo.net> (raw)
In-Reply-To: <20260811150147.1360102-3-tomo@flapping.org>
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);
> + }
> }
next prev parent reply other threads:[~2026-08-11 16:44 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
2026-08-12 0:05 ` FUJITA Tomonori
2026-08-12 6:06 ` Miguel Ojeda
2026-08-12 11:55 ` Gary Guo
2026-08-12 13:45 ` FUJITA Tomonori
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=DKM9KW3ARL1K.1JFJOBOB5EDKF@garyguo.net \
--to=gary@garyguo.net \
--cc=a.hindborg@kernel.org \
--cc=acourbot@nvidia.com \
--cc=aliceryhl@google.com \
--cc=anna-maria@linutronix.de \
--cc=bjorn3_gh@protonmail.com \
--cc=boqun@kernel.org \
--cc=dakr@kernel.org \
--cc=daniel.almeida@collabora.com \
--cc=frederic@kernel.org \
--cc=fujita.tomonori@gmail.com \
--cc=jstultz@google.com \
--cc=lossin@kernel.org \
--cc=lyude@redhat.com \
--cc=ojeda@kernel.org \
--cc=rust-for-linux@vger.kernel.org \
--cc=sboyd@kernel.org \
--cc=tamird@kernel.org \
--cc=tglx@kernel.org \
--cc=tmgross@umich.edu \
--cc=tomo@flapping.org \
--cc=work@onurozkan.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox