[PATCH v1] rust: time: fix as_micros_ceil() to round correctly for negative Delta

[PATCH v1] rust: time: fix as_micros_ceil() to round correctly for negative Delta

[FUJITA Tomonori]

From: FUJITA Tomonori <fujita.tomonori@gmail.com>

The ceiling-division idiom `(n + d - 1) / d` only produces the
correct result when `n` is non-negative.

For example, if n = -1000 (exactly -1us), the old code computed (-1000
+ 999) / 1000 == 0 instead of -1.

For negative n, truncating division already rounds towards positive
infinity, so no bias is needed in that case.

Fixes: fae0cdc12340 ("rust: time: Introduce Delta type")
Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
---
 rust/kernel/time.rs | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/rust/kernel/time.rs b/rust/kernel/time.rs
index 363e93cbb139..b8463823aed9 100644
--- a/rust/kernel/time.rs
+++ b/rust/kernel/time.rs
@@ -441,15 +441,22 @@ pub const fn as_nanos(self) -> i64 {
     /// to the value in the [`Delta`].
     #[inline]
     pub fn as_micros_ceil(self) -> i64 {
+        let n = self.as_nanos();
+        let n = if n >= 0 {
+            n.saturating_add(NSEC_PER_USEC - 1)
+        } else {
+            n
+        };
+
         #[cfg(CONFIG_64BIT)]
         {
-            self.as_nanos().saturating_add(NSEC_PER_USEC - 1) / NSEC_PER_USEC
+            n / NSEC_PER_USEC
         }
 
         #[cfg(not(CONFIG_64BIT))]
         // SAFETY: It is always safe to call `ktime_to_us()` with any value.
         unsafe {
-            bindings::ktime_to_us(self.as_nanos().saturating_add(NSEC_PER_USEC - 1))
+            bindings::ktime_to_us(n)
         }
     }
 

base-commit: 5a81c35c3b18cd59ded56171a6a9f643b92a6759
-- 
2.43.0

Re: [PATCH v1] rust: time: fix as_micros_ceil() to round correctly for negative Delta

[Miguel Ojeda]

On Tue, Jul 14, 2026 at 12:52 AM FUJITA Tomonori <tomo@flapping.org> wrote:
>
> Fixes: fae0cdc12340 ("rust: time: Introduce Delta type")

Should this be Cc: stable@, then? (since this landed in v6.16) Or is
there a reason not to?

By the way, these methods should have `# Examples` showing (and thus
testing) this sort of edge cases. Probably as a separate series? (I
can create a "good first issue", but if you prefer to do it, of
course, that would be great).

Thanks!

Cheers,
Miguel

Re: [PATCH v1] rust: time: fix as_micros_ceil() to round correctly for negative Delta

[FUJITA Tomonori]

On Tue, 14 Jul 2026 11:09:39 +0200
Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote:

> On Tue, Jul 14, 2026 at 12:52 AM FUJITA Tomonori <tomo@flapping.org> wrote:
>>
>> Fixes: fae0cdc12340 ("rust: time: Introduce Delta type")
> 
> Should this be Cc: stable@, then? (since this landed in v6.16) Or is
> there a reason not to?

Yeah, should be.

> By the way, these methods should have `# Examples` showing (and thus
> testing) this sort of edge cases. Probably as a separate series? (I
> can create a "good first issue", but if you prefer to do it, of
> course, that would be great).

I can send a patch but as I recall, the last time I posted a patch
like that, Andreas was opposed to that kind approach?

https://lore.kernel.org/rust-for-linux/20250701001809.496389-1-fujita.tomonori@gmail.com/

Re: [PATCH v1] rust: time: fix as_micros_ceil() to round correctly for negative Delta

[Miguel Ojeda]

On Tue, Jul 14, 2026 at 12:16 PM FUJITA Tomonori <tomo@flapping.org> wrote:
>
> I can send a patch but as I recall, the last time I posted a patch
> like that, Andreas was opposed to that kind approach?
>
> https://lore.kernel.org/rust-for-linux/20250701001809.496389-1-fujita.tomonori@gmail.com/

Ah, that explains why I had the feeling I already mentioned this in
the past... Thanks for linking it! :)

My reading at the end of that thread was that he would open to it as
long as the constants had a name or computed relative to something
else, which is a good idea regardless, and I replied with agreement --
so I don't think Andreas was completely opposed to doing so?

We should anyway be consistent about this treewide. What we have been
doing (or at least my intention) was to document an example of each
possible return path / edge case. That is, we don't want every single
test in examples, but for instance here at the very least I would
expect to see 1) a normal case, 2) a saturating case, 3) the behavior
of that with negatives; but perhaps we don't want to have every single
test we may have about each of those "groups".

Sometimes it can also help for readability to split the groups with a
comment and a newline, e.g. `// Negative numbers.`, especially if we
have several on each "group".

In any case, one way or another, let's please not have those
examples/tests be forgotten... Andreas?

Cheers,
Miguel

Re: [PATCH v1] rust: time: fix as_micros_ceil() to round correctly for negative Delta

[FUJITA Tomonori]

On Tue, 14 Jul 2026 12:38:03 +0200
Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote:

> On Tue, Jul 14, 2026 at 12:16 PM FUJITA Tomonori <tomo@flapping.org> wrote:
>>
>> I can send a patch but as I recall, the last time I posted a patch
>> like that, Andreas was opposed to that kind approach?
>>
>> https://lore.kernel.org/rust-for-linux/20250701001809.496389-1-fujita.tomonori@gmail.com/
> 
> Ah, that explains why I had the feeling I already mentioned this in
> the past... Thanks for linking it! :)
> 
> My reading at the end of that thread was that he would open to it as
> long as the constants had a name or computed relative to something
> else, which is a good idea regardless, and I replied with agreement --
> so I don't think Andreas was completely opposed to doing so?
> 
> We should anyway be consistent about this treewide. What we have been
> doing (or at least my intention) was to document an example of each
> possible return path / edge case. That is, we don't want every single
> test in examples, but for instance here at the very least I would
> expect to see 1) a normal case, 2) a saturating case, 3) the behavior
> of that with negatives; but perhaps we don't want to have every single
> test we may have about each of those "groups".

As I recall, the last time I posted a patch to make them constants,
Gary was opposed?

https://lore.kernel.org/rust-for-linux/20260119233621.3465304-1-fujita.tomonori@gmail.com/


> Sometimes it can also help for readability to split the groups with a
> comment and a newline, e.g. `// Negative numbers.`, especially if we
> have several on each "group".
> 
> In any case, one way or another, let's please not have those
> examples/tests be forgotten... Andreas?

I'm hoping we can actually settle on something this time around, it
would be nice to have a tree-wide policy on how to handle cases like
this.

Re: [PATCH v1] rust: time: fix as_micros_ceil() to round correctly for negative Delta

[Gary Guo]

On Tue Jul 14, 2026 at 11:49 AM BST, FUJITA Tomonori wrote:
> On Tue, 14 Jul 2026 12:38:03 +0200
> Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote:
>
>> On Tue, Jul 14, 2026 at 12:16 PM FUJITA Tomonori <tomo@flapping.org> wrote:
>>>
>>> I can send a patch but as I recall, the last time I posted a patch
>>> like that, Andreas was opposed to that kind approach?
>>>
>>> https://lore.kernel.org/rust-for-linux/20250701001809.496389-1-fujita.tomonori@gmail.com/
>> 
>> Ah, that explains why I had the feeling I already mentioned this in
>> the past... Thanks for linking it! :)
>> 
>> My reading at the end of that thread was that he would open to it as
>> long as the constants had a name or computed relative to something
>> else, which is a good idea regardless, and I replied with agreement --
>> so I don't think Andreas was completely opposed to doing so?
>> 
>> We should anyway be consistent about this treewide. What we have been
>> doing (or at least my intention) was to document an example of each
>> possible return path / edge case. That is, we don't want every single
>> test in examples, but for instance here at the very least I would
>> expect to see 1) a normal case, 2) a saturating case, 3) the behavior
>> of that with negatives; but perhaps we don't want to have every single
>> test we may have about each of those "groups".
>
> As I recall, the last time I posted a patch to make them constants,
> Gary was opposed?
>
> https://lore.kernel.org/rust-for-linux/20260119233621.3465304-1-fujita.tomonori@gmail.com/

I'm okay having test cases for corner cases but they shouldn't be part of the
API.

Best,
Gary

Re: [PATCH v1] rust: time: fix as_micros_ceil() to round correctly for negative Delta

[FUJITA Tomonori]

On Tue, 14 Jul 2026 11:56:58 +0100
"Gary Guo" <gary@garyguo.net> wrote:

> On Tue Jul 14, 2026 at 11:49 AM BST, FUJITA Tomonori wrote:
>> On Tue, 14 Jul 2026 12:38:03 +0200
>> Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote:
>>
>>> On Tue, Jul 14, 2026 at 12:16 PM FUJITA Tomonori <tomo@flapping.org> wrote:
>>>>
>>>> I can send a patch but as I recall, the last time I posted a patch
>>>> like that, Andreas was opposed to that kind approach?
>>>>
>>>> https://lore.kernel.org/rust-for-linux/20250701001809.496389-1-fujita.tomonori@gmail.com/
>>> 
>>> Ah, that explains why I had the feeling I already mentioned this in
>>> the past... Thanks for linking it! :)
>>> 
>>> My reading at the end of that thread was that he would open to it as
>>> long as the constants had a name or computed relative to something
>>> else, which is a good idea regardless, and I replied with agreement --
>>> so I don't think Andreas was completely opposed to doing so?
>>> 
>>> We should anyway be consistent about this treewide. What we have been
>>> doing (or at least my intention) was to document an example of each
>>> possible return path / edge case. That is, we don't want every single
>>> test in examples, but for instance here at the very least I would
>>> expect to see 1) a normal case, 2) a saturating case, 3) the behavior
>>> of that with negatives; but perhaps we don't want to have every single
>>> test we may have about each of those "groups".
>>
>> As I recall, the last time I posted a patch to make them constants,
>> Gary was opposed?
>>
>> https://lore.kernel.org/rust-for-linux/20260119233621.3465304-1-fujita.tomonori@gmail.com/
> 
> I'm okay having test cases for corner cases but they shouldn't be part of the
> API.

If I understand everyone's positions correctly:

- Andreas wanted the docs to avoid raw magic numbers.
- Gary didn't want to grow the public API with constants purely for
  documentation purposes.
- Miguel suggested doctests could show the boundary behavior, with the
  value declared locally inside the example.   

Here's a concreate attempt at from_micros():

diff --git a/rust/kernel/time.rs b/rust/kernel/time.rs
index 363e93cbb139..a60a248a372b 100644
--- a/rust/kernel/time.rs
+++ b/rust/kernel/time.rs
@@ -388,6 +388,24 @@ pub const fn from_nanos(nanos: i64) -> Self {
     /// The `micros` can range from -9_223_372_036_854_775 to 9_223_372_036_854_775.
     /// If `micros` is outside this range, `i64::MIN` is used for negative values,
     /// and `i64::MAX` is used for positive values due to saturation.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// use kernel::time::{Delta, NSEC_PER_USEC};
+    ///
+    /// assert_eq!(Delta::from_micros(1).as_nanos(), 1_000);
+    ///
+    /// // The boundary of the representable range: does not saturate.
+    /// const MAX_MICROS: i64 = 9_223_372_036_854_775;
+    /// assert_eq!(Delta::from_micros(MAX_MICROS).as_nanos(), MAX_MICROS * NSEC_PER_USEC);
+    ///
+    /// // One past the boundary: saturates.
+    /// assert_eq!(Delta::from_micros(MAX_MICROS + 1), Delta::from_nanos(i64::MAX));
+    ///
+    /// // Negative values.
+    /// assert_eq!(Delta::from_micros(-1).as_nanos(), -1_000);
+    /// ```
     #[inline]
     pub const fn from_micros(micros: i64) -> Self {
         Self {


Can we agree on something like this?

Re: [PATCH v1] rust: time: fix as_micros_ceil() to round correctly for negative Delta

[Miguel Ojeda]

On Tue, Jul 14, 2026 at 2:18 PM FUJITA Tomonori <tomo@flapping.org> wrote:
>
> Can we agree on something like this?

Something like that example is what I meant, yes (i.e. the groups with
a comment header, the N and N + 1 thing I discussed with Andreas,
etc.). Thanks!

But timekeeping should decide to what degree `9_223_372_036_854_775`
is meant to be documented, i.e. public, private or a middle ground
where we only mention it to explain the boundary case without the
intention of anyone using it.

Cheers,
Miguel

Re: [PATCH v1] rust: time: fix as_micros_ceil() to round correctly for negative Delta

[Alice Ryhl]

On Tue, Jul 14, 2026 at 02:31:02PM +0200, Miguel Ojeda wrote:
> On Tue, Jul 14, 2026 at 2:18 PM FUJITA Tomonori <tomo@flapping.org> wrote:
> >
> > Can we agree on something like this?
> 
> Something like that example is what I meant, yes (i.e. the groups with
> a comment header, the N and N + 1 thing I discussed with Andreas,
> etc.). Thanks!
> 
> But timekeeping should decide to what degree `9_223_372_036_854_775`
> is meant to be documented, i.e. public, private or a middle ground
> where we only mention it to explain the boundary case without the
> intention of anyone using it.

I mean if the goal is just to explain that values that are too large
saturate, then the example could look like this?

	```rs
	assert_eq!(Delta::from_micros(1_000).as_nanos(), 1_000_000);
	assert_eq!(Delta::from_micros(1).as_nanos(), 1_000);
	assert_eq!(Delta::from_micros(-1).as_nanos(), -1_000);
	
	// If the value is too large, the value saturates to the maximum
	// possible Delta.
	assert_eq!(Delta::from_micros(i64::MAX).as_nanos(), Delta::MAX);
	```

This shows pretty clearly that they saturate without explaining exactly
where the boundary is.

Alice

Re: [PATCH v1] rust: time: fix as_micros_ceil() to round correctly for negative Delta

[Miguel Ojeda]

On Tue, Jul 14, 2026 at 2:57 PM Alice Ryhl <aliceryhl@google.com> wrote:
>
> I mean if the goal is just to explain that values that are too large
> saturate, then the example could look like this?

Yeah, that is what I meant by the 2) group, i.e. I think saturation
should be shown one way or another.

But the boundary case in particular is separate, and a previous patch
showcased the actual boundary (i.e. both N and N + 1). If the docs
mention it (as they do now), then I am OK with the boundary case being
in the example too, even if we may not have a `pub const`, or in
`#[test]`.

Either way, what I really want is to have the basic examples landed
and the fix... we can discuss the boundary (docs, examples, `#[test]`s
and `pub const`s) separately. :)

Thanks!

Cheers,
Miguel

Re: [PATCH v1] rust: time: fix as_micros_ceil() to round correctly for negative Delta

[Andreas Hindborg]

"Miguel Ojeda" <miguel.ojeda.sandonis@gmail.com> writes:

> On Tue, Jul 14, 2026 at 2:57 PM Alice Ryhl <aliceryhl@google.com> wrote:
>>
>> I mean if the goal is just to explain that values that are too large
>> saturate, then the example could look like this?
>
> Yeah, that is what I meant by the 2) group, i.e. I think saturation
> should be shown one way or another.
>
> But the boundary case in particular is separate, and a previous patch
> showcased the actual boundary (i.e. both N and N + 1). If the docs
> mention it (as they do now), then I am OK with the boundary case being
> in the example too, even if we may not have a `pub const`, or in
> `#[test]`.
>
> Either way, what I really want is to have the basic examples landed
> and the fix... we can discuss the boundary (docs, examples, `#[test]`s
> and `pub const`s) separately. :)
>

We can land the fix first and the examples and tests later.

I agree with the suggestions made in this thread about examples and
cases. Let's have the example like Alice suggest and then test boundary
inputs in `#[test]` kunit tests. Define the magic numbers as named
constants local to the test scope, not as part of the exported API.

Best regards,
Andreas Hindborg

Re: [PATCH v1] rust: time: fix as_micros_ceil() to round correctly for negative Delta

[Miguel Ojeda]

On Tue, Jul 14, 2026 at 12:49 PM FUJITA Tomonori <tomo@flapping.org> wrote:
>
> As I recall, the last time I posted a patch to make them constants,
> Gary was opposed?
>
> https://lore.kernel.org/rust-for-linux/20260119233621.3465304-1-fujita.tomonori@gmail.com/

What I meant is to have constants in the example itself rather than
the full number (e.g. if we have two long integer literals N and N +
1, then defining N rather than repeating a very long number), i.e. as
a general principle.

Having said that, if those constants are meant to be kept private,
i.e. if we don't want to expose exactly the edge, then we can just use
another example and/or move that particular edge case to a `#[test]`.

However, we do already expose those numbers in the docs. And so your
patch looks fine to me. Either that, or we drop them altogether from
the docs.

So there are two orthogonal things here:

  - Whether the long constants in `time.rs` in particular should be
private or not. That is up to the maintainers to decide whether they
want to expose that as a documented fact or not. If they should be
private, then we should change the dos and move those cases to
`#[test]`. If they are meant to be public, we should use your `pub
const` patch *and* use them in the added examples.

  - We should have examples and possibly `#[test]`s in these methods,
regardless of whether we make the `9_223_...` constants public or not.

Cheers,
Miguel

Re: [PATCH v1] rust: time: fix as_micros_ceil() to round correctly for negative Delta

[Andreas Hindborg]

"FUJITA Tomonori" <tomo@flapping.org> writes:

> From: FUJITA Tomonori <fujita.tomonori@gmail.com>
>
> The ceiling-division idiom `(n + d - 1) / d` only produces the
> correct result when `n` is non-negative.
>
> For example, if n = -1000 (exactly -1us), the old code computed (-1000
> + 999) / 1000 == 0 instead of -1.
>
> For negative n, truncating division already rounds towards positive
> infinity, so no bias is needed in that case.
>
> Fixes: fae0cdc12340 ("rust: time: Introduce Delta type")
> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>

Acked-by: Andreas Hindborg <a.hindborg@kernel.org>

@Miguel, can you take this with rust fixes?

Best regards,
Andreas Hindborg