[PATCH] rust: time: add debug assertions for out-of-range input in fsleep()

[PATCH] rust: time: add debug assertions for out-of-range input in fsleep()

[Sang-Heon Jeon]

fsleep() documents out-of-range input as a bug but does not check
it, unlike udelay(). Add `debug_assert!` calls to catch it in
debug builds.

Signed-off-by: Sang-Heon Jeon <ekffu200098@gmail.com>
---
Hello,

I found this small inconsistency while reading the code. The same
check was applied to udelay() [1] but does not seem to have been
extended to fsleep(). Please let me know if I have misunderstood
anything.

[1] https://lore.kernel.org/all/20251103112958.2961517-2-fujita.tomonori@gmail.com

Best Regards,
Sang-Heon Jeon
---
 rust/kernel/time/delay.rs | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/rust/kernel/time/delay.rs b/rust/kernel/time/delay.rs
index b5b1b42797a0..1e699fe647ff 100644
--- a/rust/kernel/time/delay.rs
+++ b/rust/kernel/time/delay.rs
@@ -32,6 +32,9 @@ pub fn fsleep(delta: Delta) {
     // overflow inside fsleep, which could lead to unintentional infinite sleep.
     const MAX_DELTA: Delta = Delta::from_micros(i32::MAX as i64);
 
+    debug_assert!(delta.as_nanos() >= 0);
+    debug_assert!(delta <= MAX_DELTA);
+
     let delta = if (Delta::ZERO..=MAX_DELTA).contains(&delta) {
         delta
     } else {
-- 
2.43.0

Re: [PATCH] rust: time: add debug assertions for out-of-range input in fsleep()

[Onur Özkan]

On Sat, 09 May 2026 02:38:27 +0900
Sang-Heon Jeon <ekffu200098@gmail.com> wrote:

> fsleep() documents out-of-range input as a bug but does not check
> it, unlike udelay(). Add `debug_assert!` calls to catch it in
> debug builds.
> 
> Signed-off-by: Sang-Heon Jeon <ekffu200098@gmail.com>
> ---
> Hello,
> 
> I found this small inconsistency while reading the code. The same
> check was applied to udelay() [1] but does not seem to have been
> extended to fsleep(). Please let me know if I have misunderstood
> anything.
> 
> [1] https://lore.kernel.org/all/20251103112958.2961517-2-fujita.tomonori@gmail.com
> 
> Best Regards,
> Sang-Heon Jeon
> ---
>  rust/kernel/time/delay.rs | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/rust/kernel/time/delay.rs b/rust/kernel/time/delay.rs
> index b5b1b42797a0..1e699fe647ff 100644
> --- a/rust/kernel/time/delay.rs
> +++ b/rust/kernel/time/delay.rs
> @@ -32,6 +32,9 @@ pub fn fsleep(delta: Delta) {
>      // overflow inside fsleep, which could lead to unintentional infinite sleep.
>      const MAX_DELTA: Delta = Delta::from_micros(i32::MAX as i64);
>  
> +    debug_assert!(delta.as_nanos() >= 0);
> +    debug_assert!(delta <= MAX_DELTA);
> +
>      let delta = if (Delta::ZERO..=MAX_DELTA).contains(&delta) {
>          delta
>      } else {
> -- 
> 2.43.0
> 

Reviewed-by: Onur Özkan <work@onurozkan.dev>

Re: [PATCH] rust: time: add debug assertions for out-of-range input in fsleep()

[Andreas Hindborg]

"Sang-Heon Jeon" <ekffu200098@gmail.com> writes:

> fsleep() documents out-of-range input as a bug but does not check
> it, unlike udelay(). Add `debug_assert!` calls to catch it in
> debug builds.
>
> Signed-off-by: Sang-Heon Jeon <ekffu200098@gmail.com>
> ---
> Hello,
>
> I found this small inconsistency while reading the code. The same
> check was applied to udelay() [1] but does not seem to have been
> extended to fsleep(). Please let me know if I have misunderstood
> anything.
>
> [1] https://lore.kernel.org/all/20251103112958.2961517-2-fujita.tomonori@gmail.com
>
> Best Regards,
> Sang-Heon Jeon

Looks good to me. Could you add the warn_once and remove the todo as
well? And add the warn_once to udelay as well, so both functions have
same shape.

Thanks,
Andreas

Re: [PATCH] rust: time: add debug assertions for out-of-range input in fsleep()

[Gary Guo]

On Sat May 9, 2026 at 9:08 AM BST, Andreas Hindborg wrote:
> "Sang-Heon Jeon" <ekffu200098@gmail.com> writes:
>
>> fsleep() documents out-of-range input as a bug but does not check
>> it, unlike udelay(). Add `debug_assert!` calls to catch it in
>> debug builds.
>>
>> Signed-off-by: Sang-Heon Jeon <ekffu200098@gmail.com>
>> ---
>> Hello,
>>
>> I found this small inconsistency while reading the code. The same
>> check was applied to udelay() [1] but does not seem to have been
>> extended to fsleep(). Please let me know if I have misunderstood
>> anything.
>>
>> [1] https://lore.kernel.org/all/20251103112958.2961517-2-fujita.tomonori@gmail.com
>>
>> Best Regards,
>> Sang-Heon Jeon
>
> Looks good to me. Could you add the warn_once and remove the todo as
> well? And add the warn_once to udelay as well, so both functions have
> same shape.

If `warn_once` is added then the debug assert would be unnecessary.

Best,
Gary

Re: [PATCH] rust: time: add debug assertions for out-of-range input in fsleep()

[Andreas Hindborg]

"Gary Guo" <gary@garyguo.net> writes:

> On Sat May 9, 2026 at 9:08 AM BST, Andreas Hindborg wrote:
>> "Sang-Heon Jeon" <ekffu200098@gmail.com> writes:
>>
>>> fsleep() documents out-of-range input as a bug but does not check
>>> it, unlike udelay(). Add `debug_assert!` calls to catch it in
>>> debug builds.
>>>
>>> Signed-off-by: Sang-Heon Jeon <ekffu200098@gmail.com>
>>> ---
>>> Hello,
>>>
>>> I found this small inconsistency while reading the code. The same
>>> check was applied to udelay() [1] but does not seem to have been
>>> extended to fsleep(). Please let me know if I have misunderstood
>>> anything.
>>>
>>> [1] https://lore.kernel.org/all/20251103112958.2961517-2-fujita.tomonori@gmail.com
>>>
>>> Best Regards,
>>> Sang-Heon Jeon
>>
>> Looks good to me. Could you add the warn_once and remove the todo as
>> well? And add the warn_once to udelay as well, so both functions have
>> same shape.
>
> If `warn_once` is added then the debug assert would be unnecessary.

It is different mechanisms in different situations, right?
`pr_warn_once` prints a warning once (and a stack trace depending on
kernel configuration?). It would be enabled in production environments,
and it allows the thread to continue executing after triggering.

`debug_assert` panics when debug assertions are enabled, otherwise is
compiled out.

I don't think one makes the other unnecessary.


Best regards,
Andreas Hindborg

Re: [PATCH] rust: time: add debug assertions for out-of-range input in fsleep()

[Gary Guo]

On Sat May 9, 2026 at 3:36 PM BST, Andreas Hindborg wrote:
> "Gary Guo" <gary@garyguo.net> writes:
>
>> On Sat May 9, 2026 at 9:08 AM BST, Andreas Hindborg wrote:
>>> "Sang-Heon Jeon" <ekffu200098@gmail.com> writes:
>>>
>>>> fsleep() documents out-of-range input as a bug but does not check
>>>> it, unlike udelay(). Add `debug_assert!` calls to catch it in
>>>> debug builds.
>>>>
>>>> Signed-off-by: Sang-Heon Jeon <ekffu200098@gmail.com>
>>>> ---
>>>> Hello,
>>>>
>>>> I found this small inconsistency while reading the code. The same
>>>> check was applied to udelay() [1] but does not seem to have been
>>>> extended to fsleep(). Please let me know if I have misunderstood
>>>> anything.
>>>>
>>>> [1] https://lore.kernel.org/all/20251103112958.2961517-2-fujita.tomonori@gmail.com
>>>>
>>>> Best Regards,
>>>> Sang-Heon Jeon
>>>
>>> Looks good to me. Could you add the warn_once and remove the todo as
>>> well? And add the warn_once to udelay as well, so both functions have
>>> same shape.
>>
>> If `warn_once` is added then the debug assert would be unnecessary.
>
> It is different mechanisms in different situations, right?
> `pr_warn_once` prints a warning once (and a stack trace depending on
> kernel configuration?). It would be enabled in production environments,
> and it allows the thread to continue executing after triggering.

The existing comment and your message says `warn_once`, not `pr_warn_once`.

>
> `debug_assert` panics when debug assertions are enabled, otherwise is
> compiled out.
>
> I don't think one makes the other unnecessary.

I don't think there should be two checks of the same thing. Even if we want
different behaviour depending on if debug assertions are enabled or not, it
should be a single macro that switches behaviour based on that config.

Best,
Gary

Re: [PATCH] rust: time: add debug assertions for out-of-range input in fsleep()

[Andreas Hindborg]

"Gary Guo" <gary@garyguo.net> writes:

> On Sat May 9, 2026 at 3:36 PM BST, Andreas Hindborg wrote:
>> "Gary Guo" <gary@garyguo.net> writes:
>>
>>> On Sat May 9, 2026 at 9:08 AM BST, Andreas Hindborg wrote:
>>>> "Sang-Heon Jeon" <ekffu200098@gmail.com> writes:
>>>>
>>>>> fsleep() documents out-of-range input as a bug but does not check
>>>>> it, unlike udelay(). Add `debug_assert!` calls to catch it in
>>>>> debug builds.
>>>>>
>>>>> Signed-off-by: Sang-Heon Jeon <ekffu200098@gmail.com>
>>>>> ---
>>>>> Hello,
>>>>>
>>>>> I found this small inconsistency while reading the code. The same
>>>>> check was applied to udelay() [1] but does not seem to have been
>>>>> extended to fsleep(). Please let me know if I have misunderstood
>>>>> anything.
>>>>>
>>>>> [1] https://lore.kernel.org/all/20251103112958.2961517-2-fujita.tomonori@gmail.com
>>>>>
>>>>> Best Regards,
>>>>> Sang-Heon Jeon
>>>>
>>>> Looks good to me. Could you add the warn_once and remove the todo as
>>>> well? And add the warn_once to udelay as well, so both functions have
>>>> same shape.
>>>
>>> If `warn_once` is added then the debug assert would be unnecessary.
>>
>> It is different mechanisms in different situations, right?
>> `pr_warn_once` prints a warning once (and a stack trace depending on
>> kernel configuration?). It would be enabled in production environments,
>> and it allows the thread to continue executing after triggering.
>
> The existing comment and your message says `warn_once`, not `pr_warn_once`.

I actually mixed these up, thanks for pointing that out. We do not have
`warn_once!` yet.

>
>>
>> `debug_assert` panics when debug assertions are enabled, otherwise is
>> compiled out.
>>
>> I don't think one makes the other unnecessary.
>
> I don't think there should be two checks of the same thing. Even if we want
> different behaviour depending on if debug assertions are enabled or not, it
> should be a single macro that switches behaviour based on that config.

Apparently this was discussed in start of April [1] (I somehow missed
it), and as far as I can tell, consensus is that we do not use
`WARN_ONCE`. Instead we use `debug_assert` with `pr_warn_once`. That
sounds reasonable to me.

This can be a combined macro or open coded, either way is fine with me.

I think we should add the `debug_assert` as suggested by this patch, add
the `pr_warn_once` in the not taken branch, and remove the TODO about
`WARN_ONCE`. We can bike-shed a combined macro on the side.

Sounds reasonable @Gary?

Best regards,
Andreas Hindborg


[1] https://lore.kernel.org/r/20260226120848.82891-5-adarshdas950@gmail.com