Re: [PATCH v2 2/2] rust: time: Implement basic arithmetic operations for Delta

[Andreas Hindborg <a.hindborg@kernel.org>]

"Benno Lossin" <lossin@kernel.org> writes:

> On Fri Aug 8, 2025 at 11:26 AM CEST, Andreas Hindborg wrote:
>> "Benno Lossin" <lossin@kernel.org> writes:
>>
>>> On Thu Aug 7, 2025 at 9:06 PM CEST, Lyude Paul wrote:
>>>> While rvkms is only going to be using a few of these, since Deltas are
>>>> basically the same as i64 it's easy enough to just implement all of the
>>>> basic arithmetic operations for Delta types.
>>>>
>>>> Keep in mind there's one quirk here - the kernel has no support for
>>>> i64 % i64 on 32 bit platforms, the closest we have is i64 % i32 through
>>>> div_s64_rem(). So, instead of implementing ops::Rem or ops::RemAssign we
>>>> simply provide Delta::rem_nanos().
>>>
>>> We could still provide the trait implementations on CONFIG_64BIT? WDYT?
>>>
>>>> +impl ops::Div for Delta {
>>>> +    type Output = Self;
>>>> +
>>>> +    #[inline]
>>>> +    fn div(self, rhs: Self) -> Self::Output {
>>>> +        #[cfg(CONFIG_64BIT)]
>>>> +        {
>>>
>>> This pattern seems to be rather common in this patchset & in general I
>>> think I've also seen it elsewhere. We should think about adding a
>>> `if_cfg!` macro:
>>>
>>>     Self {
>>>         nanos: if_cfg! {
>>>             if CONFIG_64BIT {
>>>                 self.nanos / rhs.nanos
>>>             } else {
>>>                 unsafe { ... }
>>>             }
>>>         },
>>>     }
>>>
>>
>> Why the need for a macro. `cfg!` is built-in [1]:
>>
>>   if cfg!(CONFIG_64BIT) {
>>      ...
>>   } else {
>>      ...
>>   }
>>
>> The conditional expression should be optimized statically and the dead
>> part should be removed.
>
> AIUI the `self.nanos / rhs.nanos` won't compile on 32 bit platforms.

Ah, of course 👍

Best regards,
Andreas Hindborg