Rust for Linux List
 help / color / mirror / Atom feed
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);
> +    }
>  }



  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