Add udelay() function, inserts a delay based on microseconds with busy
waiting, in preparation for supporting read_poll_count_atomic().

Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
---
 rust/helpers/time.c       |  5 +++++
 rust/kernel/time/delay.rs | 37 +++++++++++++++++++++++++++++++++++++
 2 files changed, 42 insertions(+)

diff --git a/rust/helpers/time.c b/rust/helpers/time.c
index a318e9fa4408..67a36ccc3ec4 100644
--- a/rust/helpers/time.c
+++ b/rust/helpers/time.c
@@ -33,3 +33,8 @@ s64 rust_helper_ktime_to_ms(const ktime_t kt)
 {
 	return ktime_to_ms(kt);
 }
+
+void rust_helper_udelay(unsigned long usec)
+{
+	udelay(usec);
+}
diff --git a/rust/kernel/time/delay.rs b/rust/kernel/time/delay.rs
index eb8838da62bc..fb7c15dfe186 100644
--- a/rust/kernel/time/delay.rs
+++ b/rust/kernel/time/delay.rs
@@ -47,3 +47,40 @@ pub fn fsleep(delta: Delta) {
         bindings::fsleep(delta.as_micros_ceil() as c_ulong)
     }
 }
+
+/// Inserts a delay based on microseconds with busy waiting.
+///
+/// Equivalent to the C side [`udelay()`], which delays in microseconds.
+///
+/// `delta` must be within `[0, `MAX_UDELAY_MS`]` in milliseconds;
+/// otherwise, it is erroneous behavior. That is, it is considered a bug to
+/// call this function with an out-of-range value, in which case the function
+/// will insert a delay for at least the maximum value in the range and
+/// may warn in the future.
+///
+/// The behavior above differs from the C side [`udelay()`] for which out-of-range
+/// values could lead to an overflow and unexpected behavior.
+///
+/// [`udelay()`]: https://docs.kernel.org/timers/delay_sleep_functions.html#c.udelay
+pub fn udelay(delta: Delta) {
+    const MAX_UDELAY_DELTA: Delta = Delta::from_millis(bindings::MAX_UDELAY_MS as i64);
+
+    let delta = if (Delta::ZERO..=MAX_UDELAY_DELTA).contains(&delta) {
+        delta
+    } else {
+        // TODO: Add WARN_ONCE() when it's supported.
+        MAX_UDELAY_DELTA
+    };
+
+    // SAFETY: It is always safe to call `udelay()` with any duration.
+    // Note that the kernel is compiled with `-fno-strict-overflow`
+    // so any out-of-range value could lead to unexpected behavior
+    // but won't lead to undefined behavior.
+    unsafe {
+        // Convert the duration to microseconds and round up to preserve
+        // the guarantee; `udelay()` inserts a delay for at least
+        // the provided duration, but that it may delay for longer
+        // under some circumstances.
+        bindings::udelay(delta.as_micros_ceil() as c_ulong)
+    }
+}
-- 
2.43.0

On Tue Oct 21, 2025 at 9:11 AM CEST, FUJITA Tomonori wrote:
> Add udelay() function, inserts a delay based on microseconds with busy
> waiting, in preparation for supporting read_poll_count_atomic().
>
> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
> ---
>  rust/helpers/time.c       |  5 +++++
>  rust/kernel/time/delay.rs | 37 +++++++++++++++++++++++++++++++++++++
>  2 files changed, 42 insertions(+)
>
> diff --git a/rust/helpers/time.c b/rust/helpers/time.c
> index a318e9fa4408..67a36ccc3ec4 100644
> --- a/rust/helpers/time.c
> +++ b/rust/helpers/time.c
> @@ -33,3 +33,8 @@ s64 rust_helper_ktime_to_ms(const ktime_t kt)
>  {
>  	return ktime_to_ms(kt);
>  }
> +
> +void rust_helper_udelay(unsigned long usec)
> +{
> +	udelay(usec);
> +}
> diff --git a/rust/kernel/time/delay.rs b/rust/kernel/time/delay.rs
> index eb8838da62bc..fb7c15dfe186 100644
> --- a/rust/kernel/time/delay.rs
> +++ b/rust/kernel/time/delay.rs
> @@ -47,3 +47,40 @@ pub fn fsleep(delta: Delta) {
>          bindings::fsleep(delta.as_micros_ceil() as c_ulong)
>      }
>  }
> +
> +/// Inserts a delay based on microseconds with busy waiting.
> +///
> +/// Equivalent to the C side [`udelay()`], which delays in microseconds.
> +///
> +/// `delta` must be within `[0, `MAX_UDELAY_MS`]` in milliseconds;
> +/// otherwise, it is erroneous behavior. That is, it is considered a bug to
> +/// call this function with an out-of-range value, in which case the function
> +/// will insert a delay for at least the maximum value in the range and
> +/// may warn in the future.

I think clamping the given value to Delta::ZERO..=MAX_UDELAY_DELTA is the
correct thing to do and it should be mentioned by the documentation of the
function (as you do already).

However, if we want to consider it an error if an out of range value is given,
we should just return a Result. (A simple out of range condition, that can be
handled by the user easily shouldn't result into a potential WARN()).

Additionally, we can also have an infallible version of udelay() that evaluates
the delta at build time.

Another alternative would be to just clamp the value and call it a day (i.e. do
not consider out of range values to be an error).

I don't mind one or the other, but I'm against adding a WARN() for something
that can be handled with a Result easily *and* is already handled gracefully.

> +///
> +/// The behavior above differs from the C side [`udelay()`] for which out-of-range
> +/// values could lead to an overflow and unexpected behavior.
> +///
> +/// [`udelay()`]: https://docs.kernel.org/timers/delay_sleep_functions.html#c.udelay
> +pub fn udelay(delta: Delta) {
> +    const MAX_UDELAY_DELTA: Delta = Delta::from_millis(bindings::MAX_UDELAY_MS as i64);
> +
> +    let delta = if (Delta::ZERO..=MAX_UDELAY_DELTA).contains(&delta) {
> +        delta
> +    } else {
> +        // TODO: Add WARN_ONCE() when it's supported.
> +        MAX_UDELAY_DELTA
> +    };
> +
> +    // SAFETY: It is always safe to call `udelay()` with any duration.
> +    // Note that the kernel is compiled with `-fno-strict-overflow`
> +    // so any out-of-range value could lead to unexpected behavior
> +    // but won't lead to undefined behavior.
> +    unsafe {
> +        // Convert the duration to microseconds and round up to preserve
> +        // the guarantee; `udelay()` inserts a delay for at least
> +        // the provided duration, but that it may delay for longer
> +        // under some circumstances.
> +        bindings::udelay(delta.as_micros_ceil() as c_ulong)
> +    }
> +}
> -- 
> 2.43.0

On Tue, Oct 21, 2025 at 2:08 PM Danilo Krummrich <dakr@kernel.org> wrote:
>
> However, if we want to consider it an error if an out of range value is given,
> we should just return a Result. (A simple out of range condition, that can be
> handled by the user easily shouldn't result into a potential WARN()).

I think most callers that will hit this are just buggy, and C is EB
(or worse, UB) as well here, so `Result` isn't great.

I agree that a compile-time one is best, since many (most?) cases will
be constants anyway, so we should definitely have that and avoid
calling the runtime one as much as possible.

Now, for runtime values, since random drivers will call this with
possibly computed values based on who knows what, a warn once may be
too much. A debug assert instead would be less risky if it makes
people more comfortable.

Cheers,
Miguel

On 10/21/25 4:39 PM, Miguel Ojeda wrote:
> Now, for runtime values, since random drivers will call this with
> possibly computed values based on who knows what, a warn once may be
> too much. A debug assert instead would be less risky if it makes
> people more comfortable.

Exactly, also consider that MAX_UDELAY_MS depends on the architecture and HZ.

Given that we'd have a WARN() for any value passed that is > MAX_UDELAY_MS, and
given that WARN() is considered a vulnerability if hit (Greg, please correct me
if that's wrong), this would literally become a vulnerability generator. :)

On Tue, Oct 21, 2025 at 4:46 PM Danilo Krummrich <dakr@kernel.org> wrote:
>
> Given that we'd have a WARN() for any value passed that is > MAX_UDELAY_MS, and
> given that WARN() is considered a vulnerability if hit (Greg, please correct me
> if that's wrong), this would literally become a vulnerability generator. :)

Yes, but only if hit in a way that can be triggered by an attacker,
e.g. user actions, not in general.

i.e. that is why I was saying that someone could end up calculating a
delay value based on something e.g. user controlled, and then gets an
edge case wrong, and hits the `WARN()`, which is the "bad case" and a
CVE would be assigned.

A `pr_warn_once!` plus `debug_assert!` (or similar) should be a fine
way for having EB in functions, which is a useful concept without
leaving it UB liek in C nor going with a full `Result`. I can document
that combo.

Cheers,
Miguel

On Tue Oct 21, 2025 at 4:58 PM CEST, Miguel Ojeda wrote:
> On Tue, Oct 21, 2025 at 4:46 PM Danilo Krummrich <dakr@kernel.org> wrote:
>>
>> Given that we'd have a WARN() for any value passed that is > MAX_UDELAY_MS, and
>> given that WARN() is considered a vulnerability if hit (Greg, please correct me
>> if that's wrong), this would literally become a vulnerability generator. :)
>
> Yes, but only if hit in a way that can be triggered by an attacker,
> e.g. user actions, not in general.

Sure, that's what I assumed.

> i.e. that is why I was saying that someone could end up calculating a
> delay value based on something e.g. user controlled, and then gets an
> edge case wrong, and hits the `WARN()`, which is the "bad case" and a
> CVE would be assigned.
>
> A `pr_warn_once!` plus `debug_assert!` (or similar) should be a fine
> way for having EB in functions, which is a useful concept without
> leaving it UB liek in C nor going with a full `Result`. I can document
> that combo.

I'm not even sure we want that necessarily. I'd probably go for just documenting
that the value will be clamped to 0 <= value <= MAX_UDELAY_MS plus an internal
pr_debug!().

This way the function can also explicitly used in cases where the driver isn't
sure whether the value is in range and use it without duplicating the clamp
logic that the function already does internally anyways.

On Tue, Oct 21, 2025 at 5:09 PM Danilo Krummrich <dakr@kernel.org> wrote:
>
> I'm not even sure we want that necessarily. I'd probably go for just documenting
> that the value will be clamped to 0 <= value <= MAX_UDELAY_MS plus an internal
> pr_debug!().
>
> This way the function can also explicitly used in cases where the driver isn't
> sure whether the value is in range and use it without duplicating the clamp
> logic that the function already does internally anyways.

That would mean we cannot catch bugs in the common case where I would
expect callers to know what the delay range is.

i.e. if they aren't sure what the value is, then I would prefer they
clamp it explicitly on the callee side (or we provide an explicitly
clamped version if it is a common case, but it seems to me runtime
values are already the minority).

i.e. EB is useful because it allows us to catch bugs. If we remove the
EB, then it means things are more ambiguous and thus bugs cannot be
easily be caught anymore because one can actually rely on the
behavior.

Cheers,
Miguel

On Tue Oct 21, 2025 at 5:13 PM CEST, Miguel Ojeda wrote:
> i.e. if they aren't sure what the value is, then I would prefer they
> clamp it explicitly on the callee side (or we provide an explicitly
> clamped version if it is a common case, but it seems to me runtime
> values are already the minority).

Absolutely! Especially given the context udelay() is introduced
(read_poll_timeout_atomic()), the compile time checked version is what we really
want.

Maybe we should even defer a runtime checked / clamped version until it is
actually needed.

On Tue, 21 Oct 2025 17:20:41 +0200
"Danilo Krummrich" <dakr@kernel.org> wrote:

> On Tue Oct 21, 2025 at 5:13 PM CEST, Miguel Ojeda wrote:
>> i.e. if they aren't sure what the value is, then I would prefer they
>> clamp it explicitly on the callee side (or we provide an explicitly
>> clamped version if it is a common case, but it seems to me runtime
>> values are already the minority).
> 
> Absolutely! Especially given the context udelay() is introduced
> (read_poll_timeout_atomic()), the compile time checked version is what we really
> want.
> 
> Maybe we should even defer a runtime checked / clamped version until it is
> actually needed.

Then perhaps something like this?

#[inline(always)]
pub fn udelay(delta: Delta) {
    build_assert!(
        delta.as_nanos() >= 0 && delta.as_nanos() <= i64::from(bindings::MAX_UDELAY_MS) * 1_000_000
    );

    // SAFETY: It is always safe to call `udelay()` with any duration.
    // Note that the kernel is compiled with `-fno-strict-overflow`
    // so any out-of-range value could lead to unexpected behavior
    // but won't lead to undefined behavior.
    unsafe {
        // Convert the duration to microseconds and round up to preserve
        // the guarantee; `udelay()` inserts a delay for at least
        // the provided duration, but that it may delay for longer
        // under some circumstances.
        bindings::udelay(delta.as_micros_ceil() as c_ulong)
    }
}

On Wed, Oct 22, 2025 at 07:32:30PM +0900, FUJITA Tomonori wrote:
> On Tue, 21 Oct 2025 17:20:41 +0200
> "Danilo Krummrich" <dakr@kernel.org> wrote:
> 
> > On Tue Oct 21, 2025 at 5:13 PM CEST, Miguel Ojeda wrote:
> >> i.e. if they aren't sure what the value is, then I would prefer they
> >> clamp it explicitly on the callee side (or we provide an explicitly
> >> clamped version if it is a common case, but it seems to me runtime
> >> values are already the minority).
> > 
> > Absolutely! Especially given the context udelay() is introduced
> > (read_poll_timeout_atomic()), the compile time checked version is what we really
> > want.
> > 
> > Maybe we should even defer a runtime checked / clamped version until it is
> > actually needed.
> 
> Then perhaps something like this?
> 
> #[inline(always)]
> pub fn udelay(delta: Delta) {
>     build_assert!(
>         delta.as_nanos() >= 0 && delta.as_nanos() <= i64::from(bindings::MAX_UDELAY_MS) * 1_000_000
>     );

This is a bad idea. Using build_assert! assert for range checks works
poorly, as we found for register index bounds checks.

If you really want to check it at compile-time, you'll need a wrapper
type around Delta that can only be constructed with delays in the right
range.

Alice

"Alice Ryhl" <aliceryhl@google.com> writes:

> On Wed, Oct 22, 2025 at 07:32:30PM +0900, FUJITA Tomonori wrote:
>> On Tue, 21 Oct 2025 17:20:41 +0200
>> "Danilo Krummrich" <dakr@kernel.org> wrote:
>>
>> > On Tue Oct 21, 2025 at 5:13 PM CEST, Miguel Ojeda wrote:
>> >> i.e. if they aren't sure what the value is, then I would prefer they
>> >> clamp it explicitly on the callee side (or we provide an explicitly
>> >> clamped version if it is a common case, but it seems to me runtime
>> >> values are already the minority).
>> >
>> > Absolutely! Especially given the context udelay() is introduced
>> > (read_poll_timeout_atomic()), the compile time checked version is what we really
>> > want.
>> >
>> > Maybe we should even defer a runtime checked / clamped version until it is
>> > actually needed.
>>
>> Then perhaps something like this?
>>
>> #[inline(always)]
>> pub fn udelay(delta: Delta) {
>>     build_assert!(
>>         delta.as_nanos() >= 0 && delta.as_nanos() <= i64::from(bindings::MAX_UDELAY_MS) * 1_000_000
>>     );
>
> This is a bad idea. Using build_assert! assert for range checks works
> poorly, as we found for register index bounds checks.

What was the issue you encountered here?


Best regards,
Andreas Hindborg

On Fri, Oct 24, 2025 at 10:20:56AM +0200, Andreas Hindborg wrote:
> "Alice Ryhl" <aliceryhl@google.com> writes:
> 
> > On Wed, Oct 22, 2025 at 07:32:30PM +0900, FUJITA Tomonori wrote:
> >> On Tue, 21 Oct 2025 17:20:41 +0200
> >> "Danilo Krummrich" <dakr@kernel.org> wrote:
> >>
> >> > On Tue Oct 21, 2025 at 5:13 PM CEST, Miguel Ojeda wrote:
> >> >> i.e. if they aren't sure what the value is, then I would prefer they
> >> >> clamp it explicitly on the callee side (or we provide an explicitly
> >> >> clamped version if it is a common case, but it seems to me runtime
> >> >> values are already the minority).
> >> >
> >> > Absolutely! Especially given the context udelay() is introduced
> >> > (read_poll_timeout_atomic()), the compile time checked version is what we really
> >> > want.
> >> >
> >> > Maybe we should even defer a runtime checked / clamped version until it is
> >> > actually needed.
> >>
> >> Then perhaps something like this?
> >>
> >> #[inline(always)]
> >> pub fn udelay(delta: Delta) {
> >>     build_assert!(
> >>         delta.as_nanos() >= 0 && delta.as_nanos() <= i64::from(bindings::MAX_UDELAY_MS) * 1_000_000
> >>     );
> >
> > This is a bad idea. Using build_assert! assert for range checks works
> > poorly, as we found for register index bounds checks.
> 
> What was the issue you encountered here?

Basically, the problem is that it's too unreliable. The code does not
build unless the compiler can optimize out the build_error() call.
For range checks, seemingly unrelated code changes turn out to affect
these optimizations and break the code.

To make matters worse, the error message when a build_assert!() fails is
terrible. But even if it wasn't, the optimization issue is enough for me
to say we should not use it for range checks.

There have already been at least two instances where someone wasted a
lot of time and had to ask for help trying to debug a failing
build_assert!() used for bounds checks.

Alice

On Fri, Oct 24, 2025 at 11:27 AM Alice Ryhl <aliceryhl@google.com> wrote:
>
> For range checks, seemingly unrelated code changes turn out to affect
> these optimizations and break the code.

Most of these calls use constants, so in those cases it would be fine
(and otherwise it is really an issue on the optimizer).

But, yes, using `build_assert!` on the "normal" version of `udelay()`
will eventually surprise someone, because someone out there will start
using it with runtime values that happen to work and that later may
not when code gets shuffled around, especially given it shares the
name with C.

So for functions that do `build_assert!` on parameters we may want at
least a suffix with a particular word (e.g. `_const`) or similar, so
that it is clear calling them may have issues if not "obviously
constant for the optimizer", leaving the "normal" name for the runtime
one or the const generics one etc.

Here, I would suggest we do what we did for `fsleep()` and likely move
both to `debug_assert!` plus `pr_warn!` (and likely `pr_warn_once!`
when supported).

Hopefully we will get soon enough const generics that are flexible
enough -- but passing primitives seems bad here, we want the `Delta`.
So we may want the `udelay_const()` here still.

It pains me a bit that the common case would have the longer name, but
such is life.

Cheers,
Miguel

On Fri, 24 Oct 2025 21:05:19 +0200
Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote:

> On Fri, Oct 24, 2025 at 11:27 AM Alice Ryhl <aliceryhl@google.com> wrote:
>>
>> For range checks, seemingly unrelated code changes turn out to affect
>> these optimizations and break the code.
> 
> Most of these calls use constants, so in those cases it would be fine
> (and otherwise it is really an issue on the optimizer).
> 
> But, yes, using `build_assert!` on the "normal" version of `udelay()`
> will eventually surprise someone, because someone out there will start
> using it with runtime values that happen to work and that later may
> not when code gets shuffled around, especially given it shares the
> name with C.
> 
> So for functions that do `build_assert!` on parameters we may want at
> least a suffix with a particular word (e.g. `_const`) or similar, so
> that it is clear calling them may have issues if not "obviously
> constant for the optimizer", leaving the "normal" name for the runtime
> one or the const generics one etc.
> 
> Here, I would suggest we do what we did for `fsleep()` and likely move
> both to `debug_assert!` plus `pr_warn!` (and likely `pr_warn_once!`
> when supported).

I've sent v3 with debug_assert! added.

Is anyone currently working on pr_*_once? If I remember correctly,
there were a few proposals in the past, but they didn’t reach a
conclusion on how to implement it and never got merged.

On Sun, Oct 26, 2025 at 2:11 PM FUJITA Tomonori
<fujita.tomonori@gmail.com> wrote:
>
> I've sent v3 with debug_assert! added.
>
> Is anyone currently working on pr_*_once? If I remember correctly,
> there were a few proposals in the past, but they didn’t reach a
> conclusion on how to implement it and never got merged.

Thanks Tomo -- I think the latest was:

    https://lore.kernel.org/rust-for-linux/20241126-pr_once_macros-v4-0-410b8ca9643e@tuta.io/

Cheers,
Miguel

On Wed, 22 Oct 2025 14:11:53 +0000
Alice Ryhl <aliceryhl@google.com> wrote:

> On Wed, Oct 22, 2025 at 07:32:30PM +0900, FUJITA Tomonori wrote:
>> On Tue, 21 Oct 2025 17:20:41 +0200
>> "Danilo Krummrich" <dakr@kernel.org> wrote:
>> 
>> > On Tue Oct 21, 2025 at 5:13 PM CEST, Miguel Ojeda wrote:
>> >> i.e. if they aren't sure what the value is, then I would prefer they
>> >> clamp it explicitly on the callee side (or we provide an explicitly
>> >> clamped version if it is a common case, but it seems to me runtime
>> >> values are already the minority).
>> > 
>> > Absolutely! Especially given the context udelay() is introduced
>> > (read_poll_timeout_atomic()), the compile time checked version is what we really
>> > want.
>> > 
>> > Maybe we should even defer a runtime checked / clamped version until it is
>> > actually needed.
>> 
>> Then perhaps something like this?
>> 
>> #[inline(always)]
>> pub fn udelay(delta: Delta) {
>>     build_assert!(
>>         delta.as_nanos() >= 0 && delta.as_nanos() <= i64::from(bindings::MAX_UDELAY_MS) * 1_000_000
>>     );
> 
> This is a bad idea. Using build_assert! assert for range checks works
> poorly, as we found for register index bounds checks.

Oh, I didn’t know about that. Do you have a pointer or some details I
could look at?


> If you really want to check it at compile-time, you'll need a wrapper
> type around Delta that can only be constructed with delays in the right
> range.

You meant that introducing a new type like UdelayDelta, right?

read_poll_timeout() and read_poll_timeout_atomic() use different Delta
types... I'm not sure it's a good idea.

Danilo and Miguel, any ideas for other ways we could do the
compile-time check?

"FUJITA Tomonori" <fujita.tomonori@gmail.com> writes:

> On Wed, 22 Oct 2025 14:11:53 +0000
> Alice Ryhl <aliceryhl@google.com> wrote:
>
>> On Wed, Oct 22, 2025 at 07:32:30PM +0900, FUJITA Tomonori wrote:
>>> On Tue, 21 Oct 2025 17:20:41 +0200
>>> "Danilo Krummrich" <dakr@kernel.org> wrote:
>>>
>>> > On Tue Oct 21, 2025 at 5:13 PM CEST, Miguel Ojeda wrote:
>>> >> i.e. if they aren't sure what the value is, then I would prefer they
>>> >> clamp it explicitly on the callee side (or we provide an explicitly
>>> >> clamped version if it is a common case, but it seems to me runtime
>>> >> values are already the minority).
>>> >
>>> > Absolutely! Especially given the context udelay() is introduced
>>> > (read_poll_timeout_atomic()), the compile time checked version is what we really
>>> > want.
>>> >
>>> > Maybe we should even defer a runtime checked / clamped version until it is
>>> > actually needed.
>>>
>>> Then perhaps something like this?
>>>
>>> #[inline(always)]
>>> pub fn udelay(delta: Delta) {
>>>     build_assert!(
>>>         delta.as_nanos() >= 0 && delta.as_nanos() <= i64::from(bindings::MAX_UDELAY_MS) * 1_000_000
>>>     );
>>
>> This is a bad idea. Using build_assert! assert for range checks works
>> poorly, as we found for register index bounds checks.
>
> Oh, I didn’t know about that. Do you have a pointer or some details I
> could look at?
>
>
>> If you really want to check it at compile-time, you'll need a wrapper
>> type around Delta that can only be constructed with delays in the right
>> range.
>
> You meant that introducing a new type like UdelayDelta, right?
>
> read_poll_timeout() and read_poll_timeout_atomic() use different Delta
> types... I'm not sure it's a good idea.

I would assume we keep this type private and only construct it in
`udelay`. @Alice, could you give a pointer on this approach?


Best regards,
Andreas Hindborg