[PATCH v4 0/7] rust: Add IO polling

[PATCH v4 0/7] rust: Add IO polling

[FUJITA Tomonori]

polls periodically until a condition is met or a timeout is reached.
By using the function, the 7th patch fixes QT2025 PHY driver to sleep
until the hardware becomes ready.

As a result of the past discussion, this introduces two new types,
Instant and Delta, which represent a specific point in time and a span
of time, respectively.

Unlike the old rust branch, This adds a wrapper for fsleep() instead
of msleep(). fsleep() automatically chooses the best sleep method
based on a duration.

v4:
- rebase on the tip tree's timers/core
- add Instant instead of using Ktime
- remove unused basic methods
- add Delta as_micros_ceil method
- use const fn for Delta from_* methods
- add more comments based on the feedback
- add a safe wrapper for cpu_relax()
- add __might_sleep() macro
v3: https://lore.kernel.org/netdev/20241023.213345.2086786446806395897.fujita.tomonori@gmail.com/
- Update time::Delta methods (use i64 for everything)
- Fix read_poll_timeout to show the proper debug info (file and line)
- Move fsleep to rust/kernel/time/delay.rs
- Round up delta for fsleep
- Access directly ktime_t instead of using ktime APIs
- Add Eq and Ord with PartialEq and PartialOrd
v2: https://lore.kernel.org/lkml/20241005122531.20298-1-fujita.tomonori@gmail.com/
- Introduce time::Delta instead of core::time::Duration
- Add some trait to Ktime for calculating timeout
- Use read_poll_timeout in QT2025 driver instead of using fsleep directly
v1: https://lore.kernel.org/netdev/20241001112512.4861-1-fujita.tomonori@gmail.com/


FUJITA Tomonori (7):
  rust: time: Add PartialEq/Eq/PartialOrd/Ord trait to Ktime
  rust: time: Introduce Delta type
  rust: time: Introduce Instant type
  rust: time: Add wrapper for fsleep function
  MAINTAINERS: rust: Add TIMEKEEPING and TIMER abstractions
  rust: Add read_poll_timeout functions
  net: phy: qt2025: Wait until PHY becomes ready

 MAINTAINERS               |  2 +
 drivers/net/phy/qt2025.rs | 10 ++++-
 rust/helpers/helpers.c    |  2 +
 rust/helpers/kernel.c     | 13 ++++++
 rust/helpers/time.c       |  8 ++++
 rust/kernel/error.rs      |  1 +
 rust/kernel/io.rs         |  5 +++
 rust/kernel/io/poll.rs    | 93 +++++++++++++++++++++++++++++++++++++++
 rust/kernel/lib.rs        |  2 +
 rust/kernel/processor.rs  | 13 ++++++
 rust/kernel/time.rs       | 89 +++++++++++++++++++++++++------------
 rust/kernel/time/delay.rs | 30 +++++++++++++
 12 files changed, 240 insertions(+), 28 deletions(-)
 create mode 100644 rust/helpers/kernel.c
 create mode 100644 rust/helpers/time.c
 create mode 100644 rust/kernel/io.rs
 create mode 100644 rust/kernel/io/poll.rs
 create mode 100644 rust/kernel/processor.rs
 create mode 100644 rust/kernel/time/delay.rs


base-commit: 2e529e637cef39057d9cf199a1ecb915d97ffcd9
-- 
2.43.0

[PATCH v4 1/7] rust: time: Add PartialEq/Eq/PartialOrd/Ord trait to Ktime

[FUJITA Tomonori]

Add PartialEq/Eq/PartialOrd/Ord trait to Ktime so two Ktime instances
can be compared to determine whether a timeout is met or not.

Use the derive implements; we directly touch C's ktime_t rather than
using the C's accessors because more efficient and we already do in
the existing code (Ktime::sub).

Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
---
 rust/kernel/time.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/rust/kernel/time.rs b/rust/kernel/time.rs
index e3bb5e89f88d..4a7c6037c256 100644
--- a/rust/kernel/time.rs
+++ b/rust/kernel/time.rs
@@ -27,7 +27,7 @@ pub fn msecs_to_jiffies(msecs: Msecs) -> Jiffies {
 
 /// A Rust wrapper around a `ktime_t`.
 #[repr(transparent)]
-#[derive(Copy, Clone)]
+#[derive(Copy, Clone, PartialEq, PartialOrd, Eq, Ord)]
 pub struct Ktime {
     inner: bindings::ktime_t,
 }
-- 
2.43.0

[PATCH v4 2/7] rust: time: Introduce Delta type

[FUJITA Tomonori]

Introduce a type representing a span of time. Define our own type
because `core::time::Duration` is large and could panic during
creation.

time::Ktime could be also used for time duration but timestamp and
timedelta are different so better to use a new type.

i64 is used instead of u64 to represent a span of time; some C drivers
uses negative Deltas and i64 is more compatible with Ktime using i64
too (e.g., ktime_[us|ms]_delta() APIs return i64 so we create Delta
object without type conversion.

i64 is used instead of bindings::ktime_t because when the ktime_t
type is used as timestamp, it represents values from 0 to
KTIME_MAX, which different from Delta.

Delta::from_[millis|secs] APIs take i64. When a span of time
overflows, i64::MAX is used.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
---
 rust/kernel/time.rs | 49 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/rust/kernel/time.rs b/rust/kernel/time.rs
index 4a7c6037c256..574e72d3956b 100644
--- a/rust/kernel/time.rs
+++ b/rust/kernel/time.rs
@@ -8,9 +8,15 @@
 //! C header: [`include/linux/jiffies.h`](srctree/include/linux/jiffies.h).
 //! C header: [`include/linux/ktime.h`](srctree/include/linux/ktime.h).
 
+/// The number of nanoseconds per microsecond.
+pub const NSEC_PER_USEC: i64 = bindings::NSEC_PER_USEC as i64;
+
 /// The number of nanoseconds per millisecond.
 pub const NSEC_PER_MSEC: i64 = bindings::NSEC_PER_MSEC as i64;
 
+/// The number of nanoseconds per second.
+pub const NSEC_PER_SEC: i64 = bindings::NSEC_PER_SEC as i64;
+
 /// The time unit of Linux kernel. One jiffy equals (1/HZ) second.
 pub type Jiffies = core::ffi::c_ulong;
 
@@ -81,3 +87,46 @@ fn sub(self, other: Ktime) -> Ktime {
         }
     }
 }
+
+/// A span of time.
+#[derive(Copy, Clone, PartialEq, PartialOrd, Eq, Ord, Debug)]
+pub struct Delta {
+    nanos: i64,
+}
+
+impl Delta {
+    /// Create a new `Delta` from a number of milliseconds.
+    #[inline]
+    pub const fn from_millis(millis: i64) -> Self {
+        Self {
+            nanos: millis.saturating_mul(NSEC_PER_MSEC),
+        }
+    }
+
+    /// Create a new `Delta` from a number of seconds.
+    #[inline]
+    pub const fn from_secs(secs: i64) -> Self {
+        Self {
+            nanos: secs.saturating_mul(NSEC_PER_SEC),
+        }
+    }
+
+    /// Return `true` if the `Detla` spans no time.
+    #[inline]
+    pub fn is_zero(self) -> bool {
+        self.as_nanos() == 0
+    }
+
+    /// Return the number of nanoseconds in the `Delta`.
+    #[inline]
+    pub fn as_nanos(self) -> i64 {
+        self.nanos
+    }
+
+    /// Return the smallest number of microseconds greater than or equal
+    /// to the value in the `Delta`.
+    #[inline]
+    pub fn as_micros_ceil(self) -> i64 {
+        self.as_nanos().saturating_add(NSEC_PER_USEC - 1) / NSEC_PER_USEC
+    }
+}
-- 
2.43.0

[PATCH v4 3/7] rust: time: Introduce Instant type

[FUJITA Tomonori]

Introduce a type representing a specific point in time. We could use
the Ktime type but C's ktime_t is used for both timestamp and
timedelta. To avoid confusion, introduce a new Instant type for
timestamp.

Rename Ktime to Instant and modify their methods for timestamp.

Implement the subtraction operator for Instant:

Delta = Instant A - Instant B

The operation never overflows (Instant ranges from 0 to
`KTIME_MAX`).

Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
---
 rust/kernel/time.rs | 48 +++++++++++++++------------------------------
 1 file changed, 16 insertions(+), 32 deletions(-)

diff --git a/rust/kernel/time.rs b/rust/kernel/time.rs
index 574e72d3956b..3cc1a8a76777 100644
--- a/rust/kernel/time.rs
+++ b/rust/kernel/time.rs
@@ -31,59 +31,43 @@ pub fn msecs_to_jiffies(msecs: Msecs) -> Jiffies {
     unsafe { bindings::__msecs_to_jiffies(msecs) }
 }
 
-/// A Rust wrapper around a `ktime_t`.
+/// A specific point in time.
 #[repr(transparent)]
 #[derive(Copy, Clone, PartialEq, PartialOrd, Eq, Ord)]
-pub struct Ktime {
+pub struct Instant {
+    // Range from 0 to `KTIME_MAX`.
     inner: bindings::ktime_t,
 }
 
-impl Ktime {
-    /// Create a `Ktime` from a raw `ktime_t`.
+impl Instant {
+    /// Create a `Instant` from a raw `ktime_t`.
     #[inline]
-    pub fn from_raw(inner: bindings::ktime_t) -> Self {
+    fn from_raw(inner: bindings::ktime_t) -> Self {
         Self { inner }
     }
 
     /// Get the current time using `CLOCK_MONOTONIC`.
     #[inline]
-    pub fn ktime_get() -> Self {
+    pub fn now() -> Self {
         // SAFETY: It is always safe to call `ktime_get` outside of NMI context.
         Self::from_raw(unsafe { bindings::ktime_get() })
     }
 
-    /// Divide the number of nanoseconds by a compile-time constant.
     #[inline]
-    fn divns_constant<const DIV: i64>(self) -> i64 {
-        self.to_ns() / DIV
-    }
-
-    /// Returns the number of nanoseconds.
-    #[inline]
-    pub fn to_ns(self) -> i64 {
-        self.inner
-    }
-
-    /// Returns the number of milliseconds.
-    #[inline]
-    pub fn to_ms(self) -> i64 {
-        self.divns_constant::<NSEC_PER_MSEC>()
+    /// Return the amount of time elapsed since the `Instant`.
+    pub fn elapsed(&self) -> Delta {
+        Self::now() - *self
     }
 }
 
-/// Returns the number of milliseconds between two ktimes.
-#[inline]
-pub fn ktime_ms_delta(later: Ktime, earlier: Ktime) -> i64 {
-    (later - earlier).to_ms()
-}
-
-impl core::ops::Sub for Ktime {
-    type Output = Ktime;
+impl core::ops::Sub for Instant {
+    type Output = Delta;
 
+    // never overflows
     #[inline]
-    fn sub(self, other: Ktime) -> Ktime {
-        Self {
-            inner: self.inner - other.inner,
+    fn sub(self, other: Instant) -> Delta {
+        Delta {
+            nanos: self.inner - other.inner,
         }
     }
 }
-- 
2.43.0

[PATCH v4 4/7] rust: time: Add wrapper for fsleep function

[FUJITA Tomonori]

Add a wrapper for fsleep, flexible sleep functions in
`include/linux/delay.h` which typically 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.

`sleep` functions including `fsleep` belongs to TIMERS, not
TIMEKEEPING. They are maintained separately. rust/kernel/time.rs is an
abstraction for TIMEKEEPING. To make Rust abstractions match the C
side, add rust/kernel/time/delay.rs for this wrapper.

fsleep() can only be used in a nonatomic context. This requirement is
not checked by these abstractions, but it is intended that klint [1]
or a similar tool will be used to check it in the future.

Link: https://rust-for-linux.com/klint [1]
Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
---
 rust/helpers/helpers.c    |  1 +
 rust/helpers/time.c       |  8 ++++++++
 rust/kernel/time.rs       |  4 +++-
 rust/kernel/time/delay.rs | 30 ++++++++++++++++++++++++++++++
 4 files changed, 42 insertions(+), 1 deletion(-)
 create mode 100644 rust/helpers/time.c
 create mode 100644 rust/kernel/time/delay.rs

diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c
index 30f40149f3a9..c274546bcf78 100644
--- a/rust/helpers/helpers.c
+++ b/rust/helpers/helpers.c
@@ -21,6 +21,7 @@
 #include "slab.c"
 #include "spinlock.c"
 #include "task.c"
+#include "time.c"
 #include "uaccess.c"
 #include "wait.c"
 #include "workqueue.c"
diff --git a/rust/helpers/time.c b/rust/helpers/time.c
new file mode 100644
index 000000000000..7ae64ad8141d
--- /dev/null
+++ b/rust/helpers/time.c
@@ -0,0 +1,8 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/delay.h>
+
+void rust_helper_fsleep(unsigned long usecs)
+{
+	fsleep(usecs);
+}
diff --git a/rust/kernel/time.rs b/rust/kernel/time.rs
index 3cc1a8a76777..cfc31f908710 100644
--- a/rust/kernel/time.rs
+++ b/rust/kernel/time.rs
@@ -2,12 +2,14 @@
 
 //! Time related primitives.
 //!
-//! This module contains the kernel APIs related to time and timers that
+//! This module contains the kernel APIs related to time that
 //! have been ported or wrapped for usage by Rust code in the kernel.
 //!
 //! C header: [`include/linux/jiffies.h`](srctree/include/linux/jiffies.h).
 //! C header: [`include/linux/ktime.h`](srctree/include/linux/ktime.h).
 
+pub mod delay;
+
 /// The number of nanoseconds per microsecond.
 pub const NSEC_PER_USEC: i64 = bindings::NSEC_PER_USEC as i64;
 
diff --git a/rust/kernel/time/delay.rs b/rust/kernel/time/delay.rs
new file mode 100644
index 000000000000..f80f35f50949
--- /dev/null
+++ b/rust/kernel/time/delay.rs
@@ -0,0 +1,30 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Delay and sleep primitives.
+//!
+//! This module contains the kernel APIs related to delay and sleep 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).
+
+use crate::time;
+use core::ffi::c_ulong;
+
+/// Sleeps for a given duration at least.
+///
+/// Equivalent to the kernel's [`fsleep`], flexible sleep function,
+/// which automatically chooses the best sleep method based on a duration.
+///
+/// The function sleeps infinitely (MAX_JIFFY_OFFSET) if `Delta` is negative
+/// or exceedes i32::MAX milliseconds.
+///
+/// This function can only be used in a nonatomic context.
+pub fn fsleep(delta: time::Delta) {
+    // SAFETY: FFI call.
+    unsafe {
+        // Convert the duration to microseconds and round up to preserve
+        // the guarantee; fsleep sleeps for at least the provided duration,
+        // but that it may sleep for longer under some circumstances.
+        bindings::fsleep(delta.as_micros_ceil() as c_ulong)
+    }
+}
-- 
2.43.0

[PATCH v4 5/7] MAINTAINERS: rust: Add TIMEKEEPING and TIMER abstractions

[FUJITA Tomonori]

Add Rust TIMEKEEPING and TIMER abstractions to the maintainers entry
respectively.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
---
 MAINTAINERS | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 2250eb10ece1..6fb56965b4c2 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10171,6 +10171,7 @@ F:	kernel/time/sleep_timeout.c
 F:	kernel/time/timer.c
 F:	kernel/time/timer_list.c
 F:	kernel/time/timer_migration.*
+F:	rust/kernel/time/delay.rs
 F:	tools/testing/selftests/timers/
 
 HIGH-SPEED SCC DRIVER FOR AX.25
@@ -23366,6 +23367,7 @@ F:	kernel/time/timeconv.c
 F:	kernel/time/timecounter.c
 F:	kernel/time/timekeeping*
 F:	kernel/time/time_test.c
+F:	rust/kernel/time.rs
 F:	tools/testing/selftests/timers/
 
 TIPC NETWORK LAYER
-- 
2.43.0

[PATCH v4 6/7] rust: Add read_poll_timeout functions

[FUJITA Tomonori]

Add read_poll_timeout functions which poll periodically until a
condition is met or a timeout is reached.

C's read_poll_timeout (include/linux/iopoll.h) is a complicated macro
and a simple wrapper for Rust doesn't work. So this implements the
same functionality in Rust.

The C version uses usleep_range() while the Rust version uses
fsleep(), which uses the best sleep method so it works with spans that
usleep_range() doesn't work nicely with.

Unlike the C version, __might_sleep() is used instead of might_sleep()
to show proper debug info; the file name and line
number. might_resched() could be added to match what the C version
does but this function works without it.

The sleep_before_read argument isn't supported since there is no user
for now. It's rarely used in the C version.

For the proper debug info, readx_poll_timeout() and __might_sleep()
are implemented as a macro. We could implement them as a normal
function if there is a clean way to get a null-terminated string
without allocation from core::panic::Location::file().

readx_poll_timeout() can only be used in a nonatomic context. This
requirement is not checked by these abstractions, but it is
intended that klint [1] or a similar tool will be used to check it
in the future.

Link: https://rust-for-linux.com/klint [1]
Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
---
 rust/helpers/helpers.c   |  1 +
 rust/helpers/kernel.c    | 13 ++++++
 rust/kernel/error.rs     |  1 +
 rust/kernel/io.rs        |  5 +++
 rust/kernel/io/poll.rs   | 93 ++++++++++++++++++++++++++++++++++++++++
 rust/kernel/lib.rs       |  2 +
 rust/kernel/processor.rs | 13 ++++++
 7 files changed, 128 insertions(+)
 create mode 100644 rust/helpers/kernel.c
 create mode 100644 rust/kernel/io.rs
 create mode 100644 rust/kernel/io/poll.rs
 create mode 100644 rust/kernel/processor.rs

diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c
index c274546bcf78..f9569ff1717e 100644
--- a/rust/helpers/helpers.c
+++ b/rust/helpers/helpers.c
@@ -12,6 +12,7 @@
 #include "build_assert.c"
 #include "build_bug.c"
 #include "err.c"
+#include "kernel.c"
 #include "kunit.c"
 #include "mutex.c"
 #include "page.c"
diff --git a/rust/helpers/kernel.c b/rust/helpers/kernel.c
new file mode 100644
index 000000000000..da847059260b
--- /dev/null
+++ b/rust/helpers/kernel.c
@@ -0,0 +1,13 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/kernel.h>
+
+void rust_helper_cpu_relax(void)
+{
+	cpu_relax();
+}
+
+void rust_helper___might_sleep(const char *file, int line)
+{
+	__might_sleep(file, line);
+}
diff --git a/rust/kernel/error.rs b/rust/kernel/error.rs
index 6f1587a2524e..d571b9587ed6 100644
--- a/rust/kernel/error.rs
+++ b/rust/kernel/error.rs
@@ -58,6 +58,7 @@ macro_rules! declare_err {
     declare_err!(EPIPE, "Broken pipe.");
     declare_err!(EDOM, "Math argument out of domain of func.");
     declare_err!(ERANGE, "Math result not representable.");
+    declare_err!(ETIMEDOUT, "Connection timed out.");
     declare_err!(ERESTARTSYS, "Restart the system call.");
     declare_err!(ERESTARTNOINTR, "System call was interrupted by a signal and will be restarted.");
     declare_err!(ERESTARTNOHAND, "Restart if no handler.");
diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs
new file mode 100644
index 000000000000..033f3c4e4adf
--- /dev/null
+++ b/rust/kernel/io.rs
@@ -0,0 +1,5 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Input and Output.
+
+pub mod poll;
diff --git a/rust/kernel/io/poll.rs b/rust/kernel/io/poll.rs
new file mode 100644
index 000000000000..ef1f38b59fda
--- /dev/null
+++ b/rust/kernel/io/poll.rs
@@ -0,0 +1,93 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! IO polling.
+//!
+//! C header: [`include/linux/iopoll.h`](srctree/include/linux/iopoll.h).
+
+use crate::{
+    error::{code::*, Result},
+    processor::cpu_relax,
+    time::{delay::fsleep, Delta, Instant},
+};
+
+/// Polls periodically until a condition is met or a timeout is reached.
+///
+/// Public but hidden since it should only be used from public macros.
+#[doc(hidden)]
+pub fn read_poll_timeout<Op, Cond, T: Copy>(
+    mut op: Op,
+    cond: Cond,
+    sleep_delta: Delta,
+    timeout_delta: Delta,
+) -> Result<T>
+where
+    Op: FnMut() -> Result<T>,
+    Cond: Fn(T) -> bool,
+{
+    let start = Instant::now();
+    let sleep = !sleep_delta.is_zero();
+    let timeout = !timeout_delta.is_zero();
+
+    let val = loop {
+        let val = op()?;
+        if cond(val) {
+            // Unlike the C version, we immediately return.
+            // We know a condition is met so we don't need to check again.
+            return Ok(val);
+        }
+        if timeout && start.elapsed() > timeout_delta {
+            // Should we return Err(ETIMEDOUT) here instead of call op() again
+            // wihout a sleep between? But we follow the C version. op() could
+            // take some time so might be worth checking again.
+            break op()?;
+        }
+        if sleep {
+            fsleep(sleep_delta);
+        }
+        // fsleep() could be busy-wait loop so we always call cpu_relax().
+        cpu_relax();
+    };
+
+    if cond(val) {
+        Ok(val)
+    } else {
+        Err(ETIMEDOUT)
+    }
+}
+
+/// Print debug information if it's called inside atomic sections.
+#[macro_export]
+macro_rules! __might_sleep {
+    () => {
+        #[cfg(CONFIG_DEBUG_ATOMIC_SLEEP)]
+        // SAFETY: FFI call.
+        unsafe {
+            $crate::bindings::__might_sleep(
+                c_str!(::core::file!()).as_char_ptr(),
+                ::core::line!() as i32,
+            )
+        }
+    };
+}
+
+/// Polls periodically until a condition is met or a timeout is reached.
+///
+/// `op` is called repeatedly until `cond` returns `true` or the timeout is
+///  reached. The return value of `op` is passed to `cond`.
+///
+/// `sleep_delta` is the duration to sleep between calls to `op`.
+/// If `sleep_delta` is less than one microsecond, the function will busy-wait.
+///
+/// `timeout_delta` is the maximum time to wait for `cond` to return `true`.
+///
+/// This macro can only be used in a nonatomic context.
+#[macro_export]
+macro_rules! readx_poll_timeout {
+    ($op:expr, $cond:expr, $sleep_delta:expr, $timeout_delta:expr) => {{
+        if !$sleep_delta.is_zero() {
+            $crate::__might_sleep!();
+        }
+
+        $crate::io::poll::read_poll_timeout($op, $cond, $sleep_delta, $timeout_delta)
+    }};
+}
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index 22a3bfa5a9e9..b775fd1c9be0 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -35,6 +35,7 @@
 #[cfg(CONFIG_RUST_FW_LOADER_ABSTRACTIONS)]
 pub mod firmware;
 pub mod init;
+pub mod io;
 pub mod ioctl;
 #[cfg(CONFIG_KUNIT)]
 pub mod kunit;
@@ -44,6 +45,7 @@
 pub mod page;
 pub mod prelude;
 pub mod print;
+pub mod processor;
 pub mod sizes;
 pub mod rbtree;
 mod static_assert;
diff --git a/rust/kernel/processor.rs b/rust/kernel/processor.rs
new file mode 100644
index 000000000000..eeeff4be84fa
--- /dev/null
+++ b/rust/kernel/processor.rs
@@ -0,0 +1,13 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Processor related primitives.
+//!
+//! C header: [`include/linux/processor.h`](srctree/include/linux/processor.h).
+
+/// Lower CPU power consumption or yield to a hyperthreaded twin processor.
+///
+/// It also happens to serve as a compiler barrier.
+pub fn cpu_relax() {
+    // SAFETY: FFI call.
+    unsafe { bindings::cpu_relax() }
+}
-- 
2.43.0

[PATCH v4 7/7] net: phy: qt2025: Wait until PHY becomes ready

[FUJITA Tomonori]

Wait until a PHY becomes ready in the probe callback by
using readx_poll_timeout function.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
---
 drivers/net/phy/qt2025.rs | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/qt2025.rs b/drivers/net/phy/qt2025.rs
index 28d8981f410b..f7480c19d4cc 100644
--- a/drivers/net/phy/qt2025.rs
+++ b/drivers/net/phy/qt2025.rs
@@ -18,7 +18,9 @@
     DeviceId, Driver,
 };
 use kernel::prelude::*;
+use kernel::readx_poll_timeout;
 use kernel::sizes::{SZ_16K, SZ_8K};
+use kernel::time::Delta;
 
 kernel::module_phy_driver! {
     drivers: [PhyQT2025],
@@ -93,7 +95,13 @@ fn probe(dev: &mut phy::Device) -> Result<()> {
         // The micro-controller will start running from SRAM.
         dev.write(C45::new(Mmd::PCS, 0xe854), 0x0040)?;
 
-        // TODO: sleep here until the hw becomes ready.
+        readx_poll_timeout!(
+            || dev.read(C45::new(Mmd::PCS, 0xd7fd)),
+            |val| val != 0x00 && val != 0x10,
+            Delta::from_millis(50),
+            Delta::from_secs(3)
+        )?;
+
         Ok(())
     }
 
-- 
2.43.0

Re: [PATCH v4 1/7] rust: time: Add PartialEq/Eq/PartialOrd/Ord trait to Ktime

[Trevor Gross]

On Thu, Oct 24, 2024 at 10:34 PM FUJITA Tomonori
<fujita.tomonori@gmail.com> wrote:
>
> Add PartialEq/Eq/PartialOrd/Ord trait to Ktime so two Ktime instances
> can be compared to determine whether a timeout is met or not.
>
> Use the derive implements; we directly touch C's ktime_t rather than
> using the C's accessors because more efficient and we already do in

"because more efficient" -> "because it is more efficient"

> the existing code (Ktime::sub).
>
> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>

Reviewed-by: Trevor Gross <tmgross@umich.edu>

Re: [PATCH v4 3/7] rust: time: Introduce Instant type

[Boqun Feng]

On Fri, Oct 25, 2024 at 12:31:14PM +0900, FUJITA Tomonori wrote:
> Introduce a type representing a specific point in time. We could use
> the Ktime type but C's ktime_t is used for both timestamp and
> timedelta. To avoid confusion, introduce a new Instant type for
> timestamp.
> 
> Rename Ktime to Instant and modify their methods for timestamp.
> 
> Implement the subtraction operator for Instant:
> 
> Delta = Instant A - Instant B
> 
> The operation never overflows (Instant ranges from 0 to
> `KTIME_MAX`).
> 
> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
> ---
>  rust/kernel/time.rs | 48 +++++++++++++++------------------------------
>  1 file changed, 16 insertions(+), 32 deletions(-)
> 
> diff --git a/rust/kernel/time.rs b/rust/kernel/time.rs
> index 574e72d3956b..3cc1a8a76777 100644
> --- a/rust/kernel/time.rs
> +++ b/rust/kernel/time.rs
> @@ -31,59 +31,43 @@ pub fn msecs_to_jiffies(msecs: Msecs) -> Jiffies {
>      unsafe { bindings::__msecs_to_jiffies(msecs) }
>  }
>  
> -/// A Rust wrapper around a `ktime_t`.
> +/// A specific point in time.
>  #[repr(transparent)]
>  #[derive(Copy, Clone, PartialEq, PartialOrd, Eq, Ord)]
> -pub struct Ktime {
> +pub struct Instant {
> +    // Range from 0 to `KTIME_MAX`.
>      inner: bindings::ktime_t,
>  }
>  
> -impl Ktime {
> -    /// Create a `Ktime` from a raw `ktime_t`.
> +impl Instant {
> +    /// Create a `Instant` from a raw `ktime_t`.
>      #[inline]
> -    pub fn from_raw(inner: bindings::ktime_t) -> Self {
> +    fn from_raw(inner: bindings::ktime_t) -> Self {
>          Self { inner }
>      }
>  
>      /// Get the current time using `CLOCK_MONOTONIC`.

I think eventually we want to have the `Instant` generic over
clocksource, i.e. an `Instant<MONOTOMIC>` cannot substract an
`Instant<BOOTTIME>`, but that's something we can do later.

Reviewed-by: Boqun Feng <boqun.feng@gmail.com>

Regards,
Boqun

>      #[inline]
> -    pub fn ktime_get() -> Self {
> +    pub fn now() -> Self {
>          // SAFETY: It is always safe to call `ktime_get` outside of NMI context.
>          Self::from_raw(unsafe { bindings::ktime_get() })
>      }
>  
> -    /// Divide the number of nanoseconds by a compile-time constant.
>      #[inline]
> -    fn divns_constant<const DIV: i64>(self) -> i64 {
> -        self.to_ns() / DIV
> -    }
> -
> -    /// Returns the number of nanoseconds.
> -    #[inline]
> -    pub fn to_ns(self) -> i64 {
> -        self.inner
> -    }
> -
> -    /// Returns the number of milliseconds.
> -    #[inline]
> -    pub fn to_ms(self) -> i64 {
> -        self.divns_constant::<NSEC_PER_MSEC>()
> +    /// Return the amount of time elapsed since the `Instant`.
> +    pub fn elapsed(&self) -> Delta {
> +        Self::now() - *self
>      }
>  }
>  
> -/// Returns the number of milliseconds between two ktimes.
> -#[inline]
> -pub fn ktime_ms_delta(later: Ktime, earlier: Ktime) -> i64 {
> -    (later - earlier).to_ms()
> -}
> -
> -impl core::ops::Sub for Ktime {
> -    type Output = Ktime;
> +impl core::ops::Sub for Instant {
> +    type Output = Delta;
>  
> +    // never overflows
>      #[inline]
> -    fn sub(self, other: Ktime) -> Ktime {
> -        Self {
> -            inner: self.inner - other.inner,
> +    fn sub(self, other: Instant) -> Delta {
> +        Delta {
> +            nanos: self.inner - other.inner,
>          }
>      }
>  }
> -- 
> 2.43.0
> 
> 

Re: [PATCH v4 4/7] rust: time: Add wrapper for fsleep function

[Boqun Feng]

On Fri, Oct 25, 2024 at 12:31:15PM +0900, FUJITA Tomonori wrote:
> Add a wrapper for fsleep, flexible sleep functions in
> `include/linux/delay.h` which typically 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.
> 
> `sleep` functions including `fsleep` belongs to TIMERS, not
> TIMEKEEPING. They are maintained separately. rust/kernel/time.rs is an
> abstraction for TIMEKEEPING. To make Rust abstractions match the C
> side, add rust/kernel/time/delay.rs for this wrapper.
> 
> fsleep() can only be used in a nonatomic context. This requirement is
> not checked by these abstractions, but it is intended that klint [1]
> or a similar tool will be used to check it in the future.
> 
> Link: https://rust-for-linux.com/klint [1]
> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
> ---
>  rust/helpers/helpers.c    |  1 +
>  rust/helpers/time.c       |  8 ++++++++
>  rust/kernel/time.rs       |  4 +++-
>  rust/kernel/time/delay.rs | 30 ++++++++++++++++++++++++++++++
>  4 files changed, 42 insertions(+), 1 deletion(-)
>  create mode 100644 rust/helpers/time.c
>  create mode 100644 rust/kernel/time/delay.rs
> 
> diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c
> index 30f40149f3a9..c274546bcf78 100644
> --- a/rust/helpers/helpers.c
> +++ b/rust/helpers/helpers.c
> @@ -21,6 +21,7 @@
>  #include "slab.c"
>  #include "spinlock.c"
>  #include "task.c"
> +#include "time.c"
>  #include "uaccess.c"
>  #include "wait.c"
>  #include "workqueue.c"
> diff --git a/rust/helpers/time.c b/rust/helpers/time.c
> new file mode 100644
> index 000000000000..7ae64ad8141d
> --- /dev/null
> +++ b/rust/helpers/time.c
> @@ -0,0 +1,8 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <linux/delay.h>
> +
> +void rust_helper_fsleep(unsigned long usecs)
> +{
> +	fsleep(usecs);
> +}
> diff --git a/rust/kernel/time.rs b/rust/kernel/time.rs
> index 3cc1a8a76777..cfc31f908710 100644
> --- a/rust/kernel/time.rs
> +++ b/rust/kernel/time.rs
> @@ -2,12 +2,14 @@
>  
>  //! Time related primitives.
>  //!
> -//! This module contains the kernel APIs related to time and timers that
> +//! This module contains the kernel APIs related to time that
>  //! have been ported or wrapped for usage by Rust code in the kernel.
>  //!
>  //! C header: [`include/linux/jiffies.h`](srctree/include/linux/jiffies.h).
>  //! C header: [`include/linux/ktime.h`](srctree/include/linux/ktime.h).
>  
> +pub mod delay;
> +
>  /// The number of nanoseconds per microsecond.
>  pub const NSEC_PER_USEC: i64 = bindings::NSEC_PER_USEC as i64;
>  
> diff --git a/rust/kernel/time/delay.rs b/rust/kernel/time/delay.rs
> new file mode 100644
> index 000000000000..f80f35f50949
> --- /dev/null
> +++ b/rust/kernel/time/delay.rs
> @@ -0,0 +1,30 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Delay and sleep primitives.
> +//!
> +//! This module contains the kernel APIs related to delay and sleep 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).
> +
> +use crate::time;
> +use core::ffi::c_ulong;
> +
> +/// Sleeps for a given duration at least.
> +///
> +/// Equivalent to the kernel's [`fsleep`], flexible sleep function,
> +/// which automatically chooses the best sleep method based on a duration.
> +///
> +/// The function sleeps infinitely (MAX_JIFFY_OFFSET) if `Delta` is negative
> +/// or exceedes i32::MAX milliseconds.
> +///

I know Miguel has made his suggestion:

	https://lore.kernel.org/rust-for-linux/CANiq72kWqSCSkUk1efZyAi+0ScNTtfALn+wiJY_aoQefu2TNvg@mail.gmail.com/

, but I think what we should really do here is just panic if `Delta` is
negative or exceedes i32::MAX milliseconds, and document clearly that
this function expects `Delta` to be in a certain range, i.e. it's the
user's responsibility to check. Because:

*	You can simply call schedule() with task state set properly to
	"sleep infinitely".

*	Most of the users of fsleep() don't need this "sleep infinitely"
	functionality. Instead, they want to sleep with a reasonable
	short time.

> +/// This function can only be used in a nonatomic context.
> +pub fn fsleep(delta: time::Delta) {
> +    // SAFETY: FFI call.
> +    unsafe {
> +        // Convert the duration to microseconds and round up to preserve
> +        // the guarantee; fsleep sleeps for at least the provided duration,
> +        // but that it may sleep for longer under some circumstances.
> +        bindings::fsleep(delta.as_micros_ceil() as c_ulong)

If delta is 0x10000_0000i64 * 1000_000 (=0xf424000000000i64), which
exceeds i32::MAX milliseconds, the result of `delta.as_micros_ceil() as
c_ulong` is:

*	0 on 32bit
*	0x3e800000000 on 64bit

, if I got my math right. The first is obviously not "sleeps
infinitely".

Continue on 64bit case, in C's fsleep(), 0x3e800000000 will be cast to
"int" (to call msleep()), which results as 0, still not "sleep
infinitely"?

Regards,
Boqun

> +    }
> +}
> -- 
> 2.43.0
> 
> 

Re: [PATCH v4 2/7] rust: time: Introduce Delta type

[Andrew Lunn]

> +    /// Return the number of nanoseconds in the `Delta`.
> +    #[inline]
> +    pub fn as_nanos(self) -> i64 {
> +        self.nanos
> +    }
> +
> +    /// Return the smallest number of microseconds greater than or equal
> +    /// to the value in the `Delta`.
> +    #[inline]
> +    pub fn as_micros_ceil(self) -> i64 {
> +        self.as_nanos().saturating_add(NSEC_PER_USEC - 1) / NSEC_PER_USEC
> +    }

Thanks for dropping all the unsued as_foo helpers, and adding just the
one needed.

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH v4 4/7] rust: time: Add wrapper for fsleep function

[Boqun Feng]

On Fri, Oct 25, 2024 at 03:03:37PM -0700, Boqun Feng wrote:
[...]
> > +/// Sleeps for a given duration at least.
> > +///
> > +/// Equivalent to the kernel's [`fsleep`], flexible sleep function,
> > +/// which automatically chooses the best sleep method based on a duration.
> > +///
> > +/// The function sleeps infinitely (MAX_JIFFY_OFFSET) if `Delta` is negative
> > +/// or exceedes i32::MAX milliseconds.
> > +///
> 
> I know Miguel has made his suggestion:
> 
> 	https://lore.kernel.org/rust-for-linux/CANiq72kWqSCSkUk1efZyAi+0ScNTtfALn+wiJY_aoQefu2TNvg@mail.gmail.com/
> 

"made his suggestion" is probably a wrong statement from me, looking
back the emails, Miguel was simply talking about we should document this
and his assumption that `fsleep()` should inherit the behavior/semantics
of msecs_to_jiffies().

But yeah, I still suggest doing it differently, i.e. panic on negative
or exceeding i32::MAX milliseconds.

Regards,
Boqun

[...]

Re: [PATCH v4 4/7] rust: time: Add wrapper for fsleep function

[FUJITA Tomonori]

On Fri, 25 Oct 2024 15:03:37 -0700
Boqun Feng <boqun.feng@gmail.com> wrote:

>> +/// Sleeps for a given duration at least.
>> +///
>> +/// Equivalent to the kernel's [`fsleep`], flexible sleep function,
>> +/// which automatically chooses the best sleep method based on a duration.
>> +///
>> +/// The function sleeps infinitely (MAX_JIFFY_OFFSET) if `Delta` is negative
>> +/// or exceedes i32::MAX milliseconds.
>> +///
> 
> I know Miguel has made his suggestion:
> 
> 	https://lore.kernel.org/rust-for-linux/CANiq72kWqSCSkUk1efZyAi+0ScNTtfALn+wiJY_aoQefu2TNvg@mail.gmail.com/
> 
> , but I think what we should really do here is just panic if `Delta` is
> negative or exceedes i32::MAX milliseconds, and document clearly that
> this function expects `Delta` to be in a certain range, i.e. it's the
> user's responsibility to check. Because:
> 
> *	You can simply call schedule() with task state set properly to
> 	"sleep infinitely".
> 
> *	Most of the users of fsleep() don't need this "sleep infinitely"
> 	functionality. Instead, they want to sleep with a reasonable
> 	short time.

I agree with the above reasons but I'm not sure about just panic with
a driver's invalid argument.

Can we just return an error instead?

>> +/// This function can only be used in a nonatomic context.
>> +pub fn fsleep(delta: time::Delta) {
>> +    // SAFETY: FFI call.
>> +    unsafe {
>> +        // Convert the duration to microseconds and round up to preserve
>> +        // the guarantee; fsleep sleeps for at least the provided duration,
>> +        // but that it may sleep for longer under some circumstances.
>> +        bindings::fsleep(delta.as_micros_ceil() as c_ulong)
> 
> If delta is 0x10000_0000i64 * 1000_000 (=0xf424000000000i64), which
> exceeds i32::MAX milliseconds, the result of `delta.as_micros_ceil() as
> c_ulong` is:
> 
> *	0 on 32bit
> *	0x3e800000000 on 64bit
> 
> , if I got my math right. The first is obviously not "sleeps
> infinitely".
> 
> Continue on 64bit case, in C's fsleep(), 0x3e800000000 will be cast to
> "int" (to call msleep()), which results as 0, still not "sleep
> infinitely"?

You mean "unsigned int" (to call msleep())?

You are correct that we can't say "the function sleeps infinitely
(MAX_JIFFY_OFFSET) if `Delta` is negative or exceeds i32::MAX
milliseconds.". There are some exceptional ranges.

Considering that Rust-for-Linux might eventually support 32-bit
systems, fsleep's arguments must be less than u32::MAX (usecs).
Additionally, Because of DIV_ROUND_UP (to call msleep()), it must be
less than u32::MAX - 1000. To simplify the expression, the maximum
Delta is u32::MAX / 2 (usecs)? I think that it's long enough for
the users of fsleep().

Re: [PATCH v4 4/7] rust: time: Add wrapper for fsleep function

[Boqun Feng]

On Mon, Oct 28, 2024 at 09:50:30AM +0900, FUJITA Tomonori wrote:
> On Fri, 25 Oct 2024 15:03:37 -0700
> Boqun Feng <boqun.feng@gmail.com> wrote:
> 
> >> +/// Sleeps for a given duration at least.
> >> +///
> >> +/// Equivalent to the kernel's [`fsleep`], flexible sleep function,
> >> +/// which automatically chooses the best sleep method based on a duration.
> >> +///
> >> +/// The function sleeps infinitely (MAX_JIFFY_OFFSET) if `Delta` is negative
> >> +/// or exceedes i32::MAX milliseconds.
> >> +///
> > 
> > I know Miguel has made his suggestion:
> > 
> > 	https://lore.kernel.org/rust-for-linux/CANiq72kWqSCSkUk1efZyAi+0ScNTtfALn+wiJY_aoQefu2TNvg@mail.gmail.com/
> > 
> > , but I think what we should really do here is just panic if `Delta` is
> > negative or exceedes i32::MAX milliseconds, and document clearly that
> > this function expects `Delta` to be in a certain range, i.e. it's the
> > user's responsibility to check. Because:
> > 
> > *	You can simply call schedule() with task state set properly to
> > 	"sleep infinitely".
> > 
> > *	Most of the users of fsleep() don't need this "sleep infinitely"
> > 	functionality. Instead, they want to sleep with a reasonable
> > 	short time.
> 
> I agree with the above reasons but I'm not sure about just panic with
> a driver's invalid argument.
> 

If a driver blindly trusts a user-space input or a value chosen by the
hardware, I would say it's a bug in the driver. So IMO drivers should
check the input of fsleep().

> Can we just return an error instead?
> 

That also works for me, but an immediate question is: do we put
#[must_use] on `fsleep()` to enforce the use of the return value? If
yes, then the normal users would need to explicitly ignore the return
value:

	let _ = fsleep(1sec);

The "let _ =" would be a bit annoying for every user that just uses a
constant duration.

If no, then a user with incorrect input can just skip the return value
check:

	fsleep(duration_based_on_user_input);

Would it be an issue? For example, you put an fsleep() in the loop and
expect it at least sleep for a bit time, however, a craft user input
can cause the sleep never happen, and as a result, turn the loop into a
busy waiting.

All I'm trying to say is that 99% users of fsleep() will just use a
constant and small value, it seems a bit over-doing to me to return an
error just for the <1% users in theory that don't check the input. But
that's not a blocker from me.

> >> +/// This function can only be used in a nonatomic context.
> >> +pub fn fsleep(delta: time::Delta) {
> >> +    // SAFETY: FFI call.
> >> +    unsafe {
> >> +        // Convert the duration to microseconds and round up to preserve
> >> +        // the guarantee; fsleep sleeps for at least the provided duration,
> >> +        // but that it may sleep for longer under some circumstances.
> >> +        bindings::fsleep(delta.as_micros_ceil() as c_ulong)
> > 
> > If delta is 0x10000_0000i64 * 1000_000 (=0xf424000000000i64), which
> > exceeds i32::MAX milliseconds, the result of `delta.as_micros_ceil() as
> > c_ulong` is:
> > 
> > *	0 on 32bit
> > *	0x3e800000000 on 64bit
> > 
> > , if I got my math right. The first is obviously not "sleeps
> > infinitely".
> > 
> > Continue on 64bit case, in C's fsleep(), 0x3e800000000 will be cast to
> > "int" (to call msleep()), which results as 0, still not "sleep
> > infinitely"?
> 
> You mean "unsigned int" (to call msleep())?
> 

Ah, yes.

> You are correct that we can't say "the function sleeps infinitely
> (MAX_JIFFY_OFFSET) if `Delta` is negative or exceeds i32::MAX
> milliseconds.". There are some exceptional ranges.
> 
> Considering that Rust-for-Linux might eventually support 32-bit
> systems, fsleep's arguments must be less than u32::MAX (usecs).
> Additionally, Because of DIV_ROUND_UP (to call msleep()), it must be
> less than u32::MAX - 1000. To simplify the expression, the maximum
> Delta is u32::MAX / 2 (usecs)? I think that it's long enough for
> the users of fsleep().
> 

Agreed. Though I'm attempting to make msleep() takes a `unsigned long`,
but I already have a lot in my plate...

Regards,
Boqun

Re: [PATCH v4 4/7] rust: time: Add wrapper for fsleep function

[FUJITA Tomonori]

On Sun, 27 Oct 2024 21:38:41 -0700
Boqun Feng <boqun.feng@gmail.com> wrote:

> On Mon, Oct 28, 2024 at 09:50:30AM +0900, FUJITA Tomonori wrote:
>> On Fri, 25 Oct 2024 15:03:37 -0700
>> Boqun Feng <boqun.feng@gmail.com> wrote:
>> 
>> >> +/// Sleeps for a given duration at least.
>> >> +///
>> >> +/// Equivalent to the kernel's [`fsleep`], flexible sleep function,
>> >> +/// which automatically chooses the best sleep method based on a duration.
>> >> +///
>> >> +/// The function sleeps infinitely (MAX_JIFFY_OFFSET) if `Delta` is negative
>> >> +/// or exceedes i32::MAX milliseconds.
>> >> +///
>> > 
>> > I know Miguel has made his suggestion:
>> > 
>> > 	https://lore.kernel.org/rust-for-linux/CANiq72kWqSCSkUk1efZyAi+0ScNTtfALn+wiJY_aoQefu2TNvg@mail.gmail.com/
>> > 
>> > , but I think what we should really do here is just panic if `Delta` is
>> > negative or exceedes i32::MAX milliseconds, and document clearly that
>> > this function expects `Delta` to be in a certain range, i.e. it's the
>> > user's responsibility to check. Because:
>> > 
>> > *	You can simply call schedule() with task state set properly to
>> > 	"sleep infinitely".
>> > 
>> > *	Most of the users of fsleep() don't need this "sleep infinitely"
>> > 	functionality. Instead, they want to sleep with a reasonable
>> > 	short time.
>> 
>> I agree with the above reasons but I'm not sure about just panic with
>> a driver's invalid argument.
>> 
> 
> If a driver blindly trusts a user-space input or a value chosen by the
> hardware, I would say it's a bug in the driver. So IMO drivers should
> check the input of fsleep().
> 
>> Can we just return an error instead?
>> 
> 
> That also works for me, but an immediate question is: do we put
> #[must_use] on `fsleep()` to enforce the use of the return value? If
> yes, then the normal users would need to explicitly ignore the return
> value:
> 
> 	let _ = fsleep(1sec);
> 
> The "let _ =" would be a bit annoying for every user that just uses a
> constant duration.

Yeah, but I don't think that we have enough of an excuse here to break
the rule "Do not crash the kernel".

Another possible option is to convert an invalid argument to a safe
value (e.g., the maximum), possibly with WARN_ON_ONCE().

Re: [PATCH v4 4/7] rust: time: Add wrapper for fsleep function

[Thomas Gleixner]

On Tue, Oct 29 2024 at 08:30, FUJITA Tomonori wrote:
> On Sun, 27 Oct 2024 21:38:41 -0700
> Boqun Feng <boqun.feng@gmail.com> wrote:
>> That also works for me, but an immediate question is: do we put
>> #[must_use] on `fsleep()` to enforce the use of the return value? If
>> yes, then the normal users would need to explicitly ignore the return
>> value:
>> 
>> 	let _ = fsleep(1sec);
>> 
>> The "let _ =" would be a bit annoying for every user that just uses a
>> constant duration.
>
> Yeah, but I don't think that we have enough of an excuse here to break
> the rule "Do not crash the kernel".
>
> Another possible option is to convert an invalid argument to a safe
> value (e.g., the maximum), possibly with WARN_ON_ONCE().

That makes sense.

Re: [PATCH v4 4/7] rust: time: Add wrapper for fsleep function

[FUJITA Tomonori]

On Tue, 29 Oct 2024 08:55:50 +0100
Thomas Gleixner <tglx@linutronix.de> wrote:

> On Tue, Oct 29 2024 at 08:30, FUJITA Tomonori wrote:
>> On Sun, 27 Oct 2024 21:38:41 -0700
>> Boqun Feng <boqun.feng@gmail.com> wrote:
>>> That also works for me, but an immediate question is: do we put
>>> #[must_use] on `fsleep()` to enforce the use of the return value? If
>>> yes, then the normal users would need to explicitly ignore the return
>>> value:
>>> 
>>> 	let _ = fsleep(1sec);
>>> 
>>> The "let _ =" would be a bit annoying for every user that just uses a
>>> constant duration.
>>
>> Yeah, but I don't think that we have enough of an excuse here to break
>> the rule "Do not crash the kernel".
>>
>> Another possible option is to convert an invalid argument to a safe
>> value (e.g., the maximum), possibly with WARN_ON_ONCE().
> 
> That makes sense.

Thanks! I'll do the conversion in the next version.