Re: [PATCH net-next v2 4/6] rust: time: add wrapper for fsleep function

[Alice Ryhl]

On Sat, Oct 5, 2024 at 2:26 PM FUJITA Tomonori
<fujita.tomonori@xxxxxxxxx> wrote:
>
> Add a wrapper for fsleep, flexible sleep functions in
> `include/linux/delay.h` which deals with hardware delays.
>
> The kernel supports several `sleep` functions to handle various
> lengths of delay. This adds fsleep, automatically chooses the best
> sleep method based on a duration.
>
> Signed-off-by: FUJITA Tomonori <fujita.tomonori@xxxxxxxxx>
> ---
>  rust/helpers/time.c |  6 ++++++
>  rust/kernel/time.rs | 16 ++++++++++++++++
>  2 files changed, 22 insertions(+)
>
> diff --git a/rust/helpers/time.c b/rust/helpers/time.c
> index 60dee69f4efc..0c85bb06af63 100644
> --- a/rust/helpers/time.c
> +++ b/rust/helpers/time.c
> @@ -1,7 +1,13 @@
>  // SPDX-License-Identifier: GPL-2.0
>
> +#include <linux/delay.h>
>  #include <linux/ktime.h>
>
> +void rust_helper_fsleep(unsigned long usecs)
> +{
> +       fsleep(usecs);
> +}
> +
>  ktime_t rust_helper_ktime_add_ns(const ktime_t kt, const u64 nsec)
>  {
>         return ktime_add_ns(kt, nsec);
> diff --git a/rust/kernel/time.rs b/rust/kernel/time.rs
> index 3e00ad22ed89..5cca9c60f74a 100644
> --- a/rust/kernel/time.rs
> +++ b/rust/kernel/time.rs
> @@ -5,9 +5,12 @@
>  //! This module contains the kernel APIs related to time and timers that
>  //! have been ported or wrapped for usage by Rust code in the kernel.
>  //!
> +//! C header: [`include/linux/delay.h`](srctree/include/linux/delay.h).
>  //! C header: [`include/linux/jiffies.h`](srctree/include/linux/jiffies.h).
>  //! C header: [`include/linux/ktime.h`](srctree/include/linux/ktime.h).
>
> +use core::ffi::c_ulong;
> +
>  /// The number of nanoseconds per microsecond.
>  pub const NSEC_PER_USEC: i64 = bindings::NSEC_PER_USEC as i64;
>
> @@ -178,3 +181,16 @@ fn add(self, delta: Delta) -> Ktime {
>          Ktime::from_raw(t)
>      }
>  }
> +
> +/// Sleeps for a given duration.
> +///
> +/// Equivalent to the kernel's [`fsleep`], flexible sleep function,
> +/// which automatically chooses the best sleep method based on a duration.
> +///
> +/// `Delta` must be longer than one microsecond.
> +///
> +/// This function can only be used in a nonatomic context.
> +pub fn fsleep(delta: Delta) {
> +    // SAFETY: FFI call.
> +    unsafe { bindings::fsleep(delta.as_micros() as c_ulong) }
> +}

This rounds down. Should this round it up to the nearest microsecond
instead? It's generally said that fsleep should sleep for at least the
provided duration, but that it may sleep for longer under some
circumstances. By rounding up, you preserve that guarantee.

Also, the note about always sleeping for "at least" the duration may
be a good fit for the docs here as well.

Alice