From: FUJITA Tomonori <tomo@flapping.org>
To: gary@garyguo.net, ojeda@kernel.org
Cc: tomo@flapping.org, a.hindborg@kernel.org, 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,
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@gmail.com
Subject: Re: [PATCH v1 2/2] rust: time: add Delta::to_jiffies_timeout() for timeout conversion
Date: Wed, 12 Aug 2026 09:05:16 +0900 (JST) [thread overview]
Message-ID: <20260812.090516.1638662871785456326.tomo@flapping.org> (raw)
In-Reply-To: <DKM9KW3ARL1K.1JFJOBOB5EDKF@garyguo.net>
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.
next prev parent reply other threads:[~2026-08-12 0:05 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
2026-08-12 0:05 ` FUJITA Tomonori [this message]
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=20260812.090516.1638662871785456326.tomo@flapping.org \
--to=tomo@flapping.org \
--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=gary@garyguo.net \
--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=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