Add udelay() function, inserts a delay based on microseconds with busy
waiting, in preparation for supporting read_poll_timeout_atomic().
Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
---
rust/helpers/time.c | 5 +++++
rust/kernel/time/delay.rs | 34 ++++++++++++++++++++++++++++++++++
2 files changed, 39 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..baae3238d419 100644
--- a/rust/kernel/time/delay.rs
+++ b/rust/kernel/time/delay.rs
@@ -47,3 +47,37 @@ 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.
+ 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
Hi Fujita, > On 21 Aug 2025, at 00:57, FUJITA Tomonori <fujita.tomonori@gmail.com> wrote: > > Add udelay() function, inserts a delay based on microseconds with busy > waiting, in preparation for supporting read_poll_timeout_atomic(). > > Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com> > --- > rust/helpers/time.c | 5 +++++ > rust/kernel/time/delay.rs | 34 ++++++++++++++++++++++++++++++++++ > 2 files changed, 39 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..baae3238d419 100644 > --- a/rust/kernel/time/delay.rs > +++ b/rust/kernel/time/delay.rs > @@ -47,3 +47,37 @@ 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); We should perhaps add a build_assert here to make sure this cast is always valid? > + > + 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. > + 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 > > With the change you suggested for the safety comment in udelay: Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
On Tue, 26 Aug 2025 09:44:27 -0300 Daniel Almeida <daniel.almeida@collabora.com> wrote: >> +/// 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); > > We should perhaps add a build_assert here to make sure this cast is always valid? Are you concerned that bindings::MAX_UDELAY_MS might exceed i64::MAX? If so, such a value seems unrealistic as a delay. While bindings::MAX_UDELAY_MS is architecture-dependent, its maximum is currently 10, so I don’t think this will ever become an issue in the future. If really necessary, we could add something like the followings? build_assert!(i128::from(bindings::MAX_UDELAY_MS) < i64::MAX.into()); >> + >> + 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. >> + 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 >> >> > > With the change you suggested for the safety comment in udelay: > > Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com> As Miguel wrote, any duration is safe from Rust’s perspective, and it won’t invoke UB on the C side either. So I'm not sure the above safety comment needs to be changed. Let’s ask for Andreas’s opinion.
"FUJITA Tomonori" <fujita.tomonori@gmail.com> writes: > Add udelay() function, inserts a delay based on microseconds with busy > waiting, in preparation for supporting read_poll_timeout_atomic(). > > Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com> > --- > rust/helpers/time.c | 5 +++++ > rust/kernel/time/delay.rs | 34 ++++++++++++++++++++++++++++++++++ > 2 files changed, 39 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..baae3238d419 100644 > --- a/rust/kernel/time/delay.rs > +++ b/rust/kernel/time/delay.rs > @@ -47,3 +47,37 @@ 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. Function documentation says it is overflow and unexpected behavior to call `udelay` with out of range value, but above comment says any duration is safe. Which is it? Best regards, Andreas Hindborg
On Tue, 26 Aug 2025 11:09:12 +0200 Andreas Hindborg <a.hindborg@kernel.org> wrote: >> +/// 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. > > Function documentation says it is overflow and unexpected behavior to > call `udelay` with out of range value, but above comment says any > duration is safe. Which is it? This can lead to an unexpected delay duration, but it's safe in Rust’s sense of safety? If not, how about the followings? // SAFETY: `delta` is clamped to the range [0, MAX_UDELAY_DELTA], // so calling `udelay()` with it is always safe.
On Tue, Aug 26, 2025 at 1:59 PM FUJITA Tomonori <fujita.tomonori@gmail.com> wrote: > > This can lead to an unexpected delay duration, but it's safe in Rust’s > sense of safety? If it is just unexpected behavior, then yeah. Perhaps Andreas is referring to C overflow UB? If that is the case, then in the kernel it is actually defined due to `-fno-strict-overflow`. Cheers, Miguel
"Miguel Ojeda" <miguel.ojeda.sandonis@gmail.com> writes: > On Tue, Aug 26, 2025 at 1:59 PM FUJITA Tomonori > <fujita.tomonori@gmail.com> wrote: >> >> This can lead to an unexpected delay duration, but it's safe in Rust’s >> sense of safety? > > If it is just unexpected behavior, then yeah. > > Perhaps Andreas is referring to C overflow UB? If that is the case, > then in the kernel it is actually defined due to > `-fno-strict-overflow`. OK, cool Then I would suggest that we just add a small note in the docs about the C behavior that even though passing an invalid value is considered a bug, it will not cause UB or memory unsafety. Best regards, Andreas Hindborg
© 2016 - 2025 Red Hat, Inc.