Re: [PATCH v3 2/2] rust: use Delta and a Jiffies newtype for timeouts and delays

On Mon, 20 Jul 2026 13:04:40 +0100
"Gary Guo" <[email protected]> wrote:

>>> I wonder if we should have time types being generic over units. So you can have
>>> `Delta<Nsec>` and `Delta<Jiffy>` and `Instant<Nsec>`, `Instant<Jiffy>`, with the
>>> generic default being set to `Nsec`.
>>> 
>>>     Delta::new(42) // Delta<Nsec>
>>>     Delta::new_jiffies(42) // Delta<Jiffy>
>>> 
>>> Thoughts?
>>
>> I think making Delta generic over the time unit makes sense; Delta
>> <Nsec> and Delta<Jiffy>.
>>
>> However, I don't think making Instant generic over the time unit is a
>> good idea, even though it clearly is for Delta.
>>
>> Instant is already generic over ClockSource, and jiffies is not a
>> clock source: it has no clockid_t, it is read via get_jiffies_64()
>> rather than ktime_get(), and it cannot be armed through hrtimer. That
>> leaves two ways to force a jiffies Instant, both looks wrong:
>>
>> a) Add a second unit parameter, Instant<C, Unit>. But then the type
>> admits meaningless combinations - there is no Instant<Monotonic,
>> Jiffy> - and jiffies still has no ClockSource to put in the C slot, so
>> (a) really collapses into (b).
> 
> We could split the `ClockSource` trait to be a generic `ClockSource` that
> supports everything and a `HrClockSource` that provides ID.

After John's point, the current ClockSource trait is exactly C's
clockid_t; ID is the clockid_t and ktime_get*() is that clockid's
accessor returning ktime_t. So it should be renamed to ClockId.

Your split; a general readable-source abstraction for Instant,
separate from ClockId, looks like a reasonable direction to revisit
when there's a user for it. Let's finish Delta<Jiffy>/Delta<Nsec>
first, since we already have in-tree users for those.


>> b) Make jiffies a fake ClockSource. But ClockSource::ID is passed
>> straight to hrtimer_setup(), so a fabricated clockid_t would make
>> HrTimer over jiffies type-check even though there is no corresponding
>> C operation.
>>
>> This matches the C world: for deltas, jiffy and nsec interconvert
>> (nsecs_to_jiffies() and friends), which is exactly what Delta<Unit>
>> with conversions models. But the jiffies counter and ktime_get()
>> values are never mixed - there isn't even an API to compare or convert
>> between them - so there is no unit-generic notion of an instant to
>> represent. A jiffies point in time, should be its own concrete type
>> rather than a specialization of Instant.
> 
> Well, `Instant` never inter-converts with anything, so a `Instant<Jiffies>`
> would be fine too. That said, we can delay making the change until we have a
> user that needs to get jiffies count. So far it seems that people just use it as
> a delta only.

Fair enough.

>>>> -/// The millisecond time unit.
>>>> -pub type Msecs = crate::ffi::c_uint;
>>>> +impl Jiffies {
>>>> +    /// A jiffies value of zero.
>>>> +    pub const ZERO: Self = Self(0);
>>>>  
>>>> -/// Converts milliseconds to jiffies.
>>>> -#[inline]
>>>> -pub fn msecs_to_jiffies(msecs: Msecs) -> Jiffies {
>>>> -    // SAFETY: The `__msecs_to_jiffies` function is always safe to call no
>>>> -    // matter what the argument is.
>>>> -    unsafe { bindings::__msecs_to_jiffies(msecs) }
>>>> +    /// Create a new [`Jiffies`] from the C side's `jiffies` value.
>>>> +    #[inline]
>>>> +    pub const fn new(jiffies: crate::ffi::c_ulong) -> Self {
>>> 
>>> Given that this is a public API not just for binding code, I think it's better
>>> to use `usize`.
>>
>> Good point, I'll use usize.
>>
>>
>>> This function can be `pub`.
>>> 
>>>> +    #[inline]
>>>> +    pub(crate) const fn as_raw(self) -> crate::ffi::c_ulong {
>>>> +        self.0
>>>> +    }
>>>> +
>>
>> I'd prefer to keep this pub(crate) until there's an in-tree user,
>> following the usual practice of not exposing public API without a
>> user. Also, if we do make it pub, it should return usize rather than
>> ffi::c_ulong, to stay consistent with the constructor change above. Do
>> you have a specific use case in mind that needs it public?
> 
> I think knowing how many jiffies are there in a `Delta<Jiffy>` is a reasonable
> need while getting a raw pointer type from other abstractions are more
> suspicious for drivers.

Makes sense. I'll make it a public as_jiffies(self) -> isize then,
instead of exposing as_raw(). Note that isize since Delta<Jiffy> is a
signed span, consistent with Delta<Nsec>.