[PATCH] rust/time: Add Delta::from_nanos()

Lyude Paul posted 1 patch 2 months, 3 weeks ago
rust/kernel/time.rs | 6 ++++++
1 file changed, 6 insertions(+)
[PATCH] rust/time: Add Delta::from_nanos()
Posted by Lyude Paul 2 months, 3 weeks ago
Since rvkms is going to need to create its own Delta instances, and we
already have functions for creating Delta with every other unit of time.

Signed-off-by: Lyude Paul <lyude@redhat.com>
---
 rust/kernel/time.rs | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/rust/kernel/time.rs b/rust/kernel/time.rs
index 6ea98dfcd0278..2b096e5a61cda 100644
--- a/rust/kernel/time.rs
+++ b/rust/kernel/time.rs
@@ -363,6 +363,12 @@ impl Delta {
     /// A span of time equal to zero.
     pub const ZERO: Self = Self { nanos: 0 };
 
+    /// Create a new [`Delta`] from a number of nanoseconds.
+    #[inline]
+    pub const fn from_nanos(nanos: i64) -> Self {
+        Self { nanos }
+    }
+
     /// Create a new [`Delta`] from a number of microseconds.
     ///
     /// The `micros` can range from -9_223_372_036_854_775 to 9_223_372_036_854_775.

base-commit: 5935461b458463ee51aac8d95c25d7a5e1de8c4d
-- 
2.51.1
Re: [PATCH] rust/time: Add Delta::from_nanos()
Posted by FUJITA Tomonori 2 months, 3 weeks ago
On Fri, 14 Nov 2025 13:42:06 -0500
Lyude Paul <lyude@redhat.com> wrote:

> Since rvkms is going to need to create its own Delta instances, and we
> already have functions for creating Delta with every other unit of time.
> 
> Signed-off-by: Lyude Paul <lyude@redhat.com>
> ---
>  rust/kernel/time.rs | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/rust/kernel/time.rs b/rust/kernel/time.rs
> index 6ea98dfcd0278..2b096e5a61cda 100644
> --- a/rust/kernel/time.rs
> +++ b/rust/kernel/time.rs
> @@ -363,6 +363,12 @@ impl Delta {
>      /// A span of time equal to zero.
>      pub const ZERO: Self = Self { nanos: 0 };
>  
> +    /// Create a new [`Delta`] from a number of nanoseconds.
> +    #[inline]
> +    pub const fn from_nanos(nanos: i64) -> Self {
> +        Self { nanos }
> +    }
> +
>      /// Create a new [`Delta`] from a number of microseconds.
>      ///
>      /// The `micros` can range from -9_223_372_036_854_775 to 9_223_372_036_854_775.

For consistency with the other methods, perhaps we should also mention
the valid nanos range in the comment?
Re: [PATCH] rust/time: Add Delta::from_nanos()
Posted by Andreas Hindborg 2 months, 3 weeks ago
"FUJITA Tomonori" <fujita.tomonori@gmail.com> writes:

> On Fri, 14 Nov 2025 13:42:06 -0500
> Lyude Paul <lyude@redhat.com> wrote:
>
>> Since rvkms is going to need to create its own Delta instances, and we
>> already have functions for creating Delta with every other unit of time.
>>
>> Signed-off-by: Lyude Paul <lyude@redhat.com>
>> ---
>>  rust/kernel/time.rs | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/rust/kernel/time.rs b/rust/kernel/time.rs
>> index 6ea98dfcd0278..2b096e5a61cda 100644
>> --- a/rust/kernel/time.rs
>> +++ b/rust/kernel/time.rs
>> @@ -363,6 +363,12 @@ impl Delta {
>>      /// A span of time equal to zero.
>>      pub const ZERO: Self = Self { nanos: 0 };
>>
>> +    /// Create a new [`Delta`] from a number of nanoseconds.
>> +    #[inline]
>> +    pub const fn from_nanos(nanos: i64) -> Self {
>> +        Self { nanos }
>> +    }
>> +
>>      /// Create a new [`Delta`] from a number of microseconds.
>>      ///
>>      /// The `micros` can range from -9_223_372_036_854_775 to 9_223_372_036_854_775.
>
> For consistency with the other methods, perhaps we should also mention
> the valid nanos range in the comment?

But, please make it a constant instead of an integer literal.


Best regards,
Andreas Hindborg
Re: [PATCH] rust/time: Add Delta::from_nanos()
Posted by FUJITA Tomonori 2 months, 3 weeks ago
On Mon, 17 Nov 2025 12:15:53 +0100
Andreas Hindborg <a.hindborg@kernel.org> wrote:

>>> diff --git a/rust/kernel/time.rs b/rust/kernel/time.rs
>>> index 6ea98dfcd0278..2b096e5a61cda 100644
>>> --- a/rust/kernel/time.rs
>>> +++ b/rust/kernel/time.rs
>>> @@ -363,6 +363,12 @@ impl Delta {
>>>      /// A span of time equal to zero.
>>>      pub const ZERO: Self = Self { nanos: 0 };
>>>
>>> +    /// Create a new [`Delta`] from a number of nanoseconds.
>>> +    #[inline]
>>> +    pub const fn from_nanos(nanos: i64) -> Self {
>>> +        Self { nanos }
>>> +    }
>>> +
>>>      /// Create a new [`Delta`] from a number of microseconds.
>>>      ///
>>>      /// The `micros` can range from -9_223_372_036_854_775 to 9_223_372_036_854_775.
>>
>> For consistency with the other methods, perhaps we should also mention
>> the valid nanos range in the comment?
> 
> But, please make it a constant instead of an integer literal.

Do you mean making the comment of Delta's from_* methods like the
following?

diff --git a/rust/kernel/time.rs b/rust/kernel/time.rs
index 6ea98dfcd027..822746e552fb 100644
--- a/rust/kernel/time.rs
+++ b/rust/kernel/time.rs
@@ -363,9 +363,14 @@ impl Delta {
     /// A span of time equal to zero.
     pub const ZERO: Self = Self { nanos: 0 };
 
+    /// The minimum number of microseconds that can be represented by a [`Delta`].
+    pub const MIN_MICROS: i64 = -9_223_372_036_854_775;
+    /// The maximum number of microseconds that can be represented by a [`Delta`].
+    pub const MAX_MICROS: i64 = 9_223_372_036_854_775;
+
     /// Create a new [`Delta`] from a number of microseconds.
     ///
-    /// The `micros` can range from -9_223_372_036_854_775 to 9_223_372_036_854_775.
+    /// The `micros` can range from [`Delta::MIN_MICROS`] to [`Delta::MAX_MICROS`].
     /// If `micros` is outside this range, `i64::MIN` is used for negative values,
     /// and `i64::MAX` is used for positive values due to saturation.
     #[inline]
Re: [PATCH] rust/time: Add Delta::from_nanos()
Posted by Andreas Hindborg 2 months, 3 weeks ago
"FUJITA Tomonori" <fujita.tomonori@gmail.com> writes:

> On Mon, 17 Nov 2025 12:15:53 +0100
> Andreas Hindborg <a.hindborg@kernel.org> wrote:
>
>>>> diff --git a/rust/kernel/time.rs b/rust/kernel/time.rs
>>>> index 6ea98dfcd0278..2b096e5a61cda 100644
>>>> --- a/rust/kernel/time.rs
>>>> +++ b/rust/kernel/time.rs
>>>> @@ -363,6 +363,12 @@ impl Delta {
>>>>      /// A span of time equal to zero.
>>>>      pub const ZERO: Self = Self { nanos: 0 };
>>>>
>>>> +    /// Create a new [`Delta`] from a number of nanoseconds.
>>>> +    #[inline]
>>>> +    pub const fn from_nanos(nanos: i64) -> Self {
>>>> +        Self { nanos }
>>>> +    }
>>>> +
>>>>      /// Create a new [`Delta`] from a number of microseconds.
>>>>      ///
>>>>      /// The `micros` can range from -9_223_372_036_854_775 to 9_223_372_036_854_775.
>>>
>>> For consistency with the other methods, perhaps we should also mention
>>> the valid nanos range in the comment?
>>
>> But, please make it a constant instead of an integer literal.
>
> Do you mean making the comment of Delta's from_* methods like the
> following?
>
> diff --git a/rust/kernel/time.rs b/rust/kernel/time.rs
> index 6ea98dfcd027..822746e552fb 100644
> --- a/rust/kernel/time.rs
> +++ b/rust/kernel/time.rs
> @@ -363,9 +363,14 @@ impl Delta {
>      /// A span of time equal to zero.
>      pub const ZERO: Self = Self { nanos: 0 };
>
> +    /// The minimum number of microseconds that can be represented by a [`Delta`].
> +    pub const MIN_MICROS: i64 = -9_223_372_036_854_775;
> +    /// The maximum number of microseconds that can be represented by a [`Delta`].
> +    pub const MAX_MICROS: i64 = 9_223_372_036_854_775;
> +
>      /// Create a new [`Delta`] from a number of microseconds.
>      ///
> -    /// The `micros` can range from -9_223_372_036_854_775 to 9_223_372_036_854_775.
> +    /// The `micros` can range from [`Delta::MIN_MICROS`] to [`Delta::MAX_MICROS`].
>      /// If `micros` is outside this range, `i64::MIN` is used for negative values,
>      /// and `i64::MAX` is used for positive values due to saturation.
>      #[inline]

Yes, I would prefer that. The numbers become much more useful like this.


Best regards,
Andreas Hindborg
Re: [PATCH] rust/time: Add Delta::from_nanos()
Posted by Onur Özkan 2 months, 3 weeks ago
On Fri, 14 Nov 2025 13:42:06 -0500
Lyude Paul <lyude@redhat.com> wrote:

> Since rvkms is going to need to create its own Delta instances, and we
> already have functions for creating Delta with every other unit of
> time.
> 
> Signed-off-by: Lyude Paul <lyude@redhat.com>
> ---
>  rust/kernel/time.rs | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/rust/kernel/time.rs b/rust/kernel/time.rs
> index 6ea98dfcd0278..2b096e5a61cda 100644
> --- a/rust/kernel/time.rs
> +++ b/rust/kernel/time.rs
> @@ -363,6 +363,12 @@ impl Delta {
>      /// A span of time equal to zero.
>      pub const ZERO: Self = Self { nanos: 0 };
>  
> +    /// Create a new [`Delta`] from a number of nanoseconds.
> +    #[inline]
> +    pub const fn from_nanos(nanos: i64) -> Self {
> +        Self { nanos }
> +    }
> +

I wonder if it makes sense to remove all the `from_*` functions from
`Delta` and replace them all with something like this:

    pub const fn from_duration(duration: Duration) -> Self {
        Self {
            nanos: duration.as_nanos(),
        }
    }

What do you think?

>      /// Create a new [`Delta`] from a number of microseconds.
>      ///
>      /// The `micros` can range from -9_223_372_036_854_775 to
> 9_223_372_036_854_775.
> 
> base-commit: 5935461b458463ee51aac8d95c25d7a5e1de8c4d
Re: [PATCH] rust/time: Add Delta::from_nanos()
Posted by Miguel Ojeda 2 months, 3 weeks ago
On Sun, Nov 16, 2025 at 7:38 AM Onur Özkan <work@onurozkan.dev> wrote:
>
> I wonder if it makes sense to remove all the `from_*` functions from
> `Delta` and replace them all with something like this:
>
>     pub const fn from_duration(duration: Duration) -> Self {
>         Self {
>             nanos: duration.as_nanos(),
>         }
>     }
>
> What do you think?

`Delta`s can be negative and only store the nanos -- i.e. there are
many `Duration`s that do not fit in a `Delta` (`as_nanos()` above
returns `u128`).

`Delta`s are similar to `Duration` in the sense that they both store a
span of time. It seems reasonable to support similar constructors,
since they perform similar roles and we may not use `Duration`s.

Now, I agree that other types should not start getting things like
nano parameters or `from_nanos` constructors -- they should take
`Delta`s instead.

Cheers,
Miguel
Re: [PATCH] rust/time: Add Delta::from_nanos()
Posted by Onur Özkan 2 months, 3 weeks ago
On Sun, 16 Nov 2025 12:06:41 +0100
Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote:

> On Sun, Nov 16, 2025 at 7:38 AM Onur Özkan <work@onurozkan.dev> wrote:
> >
> > I wonder if it makes sense to remove all the `from_*` functions from
> > `Delta` and replace them all with something like this:
> >
> >     pub const fn from_duration(duration: Duration) -> Self {
> >         Self {
> >             nanos: duration.as_nanos(),
> >         }
> >     }
> >
> > What do you think?
> 
> `Delta`s can be negative and only store the nanos -- i.e. there are
> many `Duration`s that do not fit in a `Delta` (`as_nanos()` above
> returns `u128`).
> 

I didn't know, thanks :)

> `Delta`s are similar to `Duration` in the sense that they both store a
> span of time. It seems reasonable to support similar constructors,
> since they perform similar roles and we may not use `Duration`s.
> 
> Now, I agree that other types should not start getting things like
> nano parameters or `from_nanos` constructors -- they should take
> `Delta`s instead.
> 

Yes, that seems like a good and simple improvement.

Regards,
Onur

> Cheers,
> Miguel