Re: [PATCH v1 2/2] rust: time: add Delta::to_jiffies_timeout() for timeout conversion

On Tue, 11 Aug 2026 17:44:24 +0100
"Gary Guo" <[email protected]> wrote:

> On Tue Aug 11, 2026 at 4:01 PM BST, FUJITA Tomonori wrote:
>> From: FUJITA Tomonori <[email protected]>
>>
>> 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 <[email protected]>
> 
> 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.