* [PATCH v2 0/2] rust: Add read_poll_timeout
@ 2025-08-17 4:47 FUJITA Tomonori
2025-08-17 4:47 ` [PATCH v2 1/2] rust: Add cpu_relax() helper FUJITA Tomonori
2025-08-17 4:47 ` [PATCH v2 2/2] rust: Add read_poll_timeout functions FUJITA Tomonori
0 siblings, 2 replies; 9+ messages in thread
From: FUJITA Tomonori @ 2025-08-17 4:47 UTC (permalink / raw)
To: a.hindborg, alex.gaynor, ojeda
Cc: aliceryhl, anna-maria, bjorn3_gh, boqun.feng, dakr, frederic,
gary, jstultz, linux-kernel, lossin, lyude, rust-for-linux, sboyd,
tglx, tmgross, acourbot, daniel.almeida
Add a helper function to poll periodically until a condition is met or
a timeout is reached.
This patch was previously reviewed as part of another patchset [1] but
was removed to expedite merging into the mainline. Now that all the
features it depends on have been merged into the mainline, it is being
reposted as a new independent patchset.
I put this function kernel/io/poll.rs. This function is not
necessarily related to I/O though. I don't think this function
perfectly matches the existing abstraction. Suggestions for a more
appropriate place are welcome.
After we agree on this patchset, I'll work on
read_poll_timeout_atomic().
v2
- make cpu_relax() inline
- remove the example code to call might_sleep with a lock hold
- move kernel/time/poll.rs to kernel/io/poll.rs
- remove the Option for timeout argument
- avoid the extra variable, sleep.
- update the comment and commit message
- writing closures directly inline in the example code
v1: https://lore.kernel.org/lkml/20250811041039.3231548-1-fujita.tomonori@gmail.com/
The changes since the last posting [2] are
- removed might_sleep() change since it was already merged separately.
- split out cpu_relax() in a separate patch
- make the example code compilable
- update the code to use Clocksource (MONOTONIC)
- call might_sleep() always (even when the function doesn't sleep)
- cosmetic changes to the doc
[1] https://lore.kernel.org/lkml/20250207132623.168854-1-fujita.tomonori@gmail.com/
[2] https://lore.kernel.org/lkml/20250220070611.214262-8-fujita.tomonori@gmail.com/
FUJITA Tomonori (2):
rust: Add cpu_relax() helper
rust: Add read_poll_timeout functions
rust/helpers/helpers.c | 1 +
rust/helpers/processor.c | 8 ++++
rust/kernel/io.rs | 1 +
rust/kernel/io/poll.rs | 96 ++++++++++++++++++++++++++++++++++++++++
rust/kernel/lib.rs | 1 +
rust/kernel/processor.rs | 14 ++++++
6 files changed, 121 insertions(+)
create mode 100644 rust/helpers/processor.c
create mode 100644 rust/kernel/io/poll.rs
create mode 100644 rust/kernel/processor.rs
base-commit: 8f5ae30d69d7543eee0d70083daf4de8fe15d585
--
2.43.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 1/2] rust: Add cpu_relax() helper
2025-08-17 4:47 [PATCH v2 0/2] rust: Add read_poll_timeout FUJITA Tomonori
@ 2025-08-17 4:47 ` FUJITA Tomonori
2025-08-19 18:32 ` Daniel Almeida
2025-08-17 4:47 ` [PATCH v2 2/2] rust: Add read_poll_timeout functions FUJITA Tomonori
1 sibling, 1 reply; 9+ messages in thread
From: FUJITA Tomonori @ 2025-08-17 4:47 UTC (permalink / raw)
To: a.hindborg, alex.gaynor, ojeda
Cc: aliceryhl, anna-maria, bjorn3_gh, boqun.feng, dakr, frederic,
gary, jstultz, linux-kernel, lossin, lyude, rust-for-linux, sboyd,
tglx, tmgross, acourbot, daniel.almeida
Add cpu_relax() helper in preparation for supporting
read_poll_timeout().
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Reviewed-by: Andreas Hindborg <a.hindborg@kernel.org>
Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
---
rust/helpers/helpers.c | 1 +
rust/helpers/processor.c | 8 ++++++++
rust/kernel/lib.rs | 1 +
rust/kernel/processor.rs | 14 ++++++++++++++
4 files changed, 24 insertions(+)
create mode 100644 rust/helpers/processor.c
create mode 100644 rust/kernel/processor.rs
diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c
index 7cf7fe95e41d..04598665e7c8 100644
--- a/rust/helpers/helpers.c
+++ b/rust/helpers/helpers.c
@@ -34,6 +34,7 @@
#include "pid_namespace.c"
#include "platform.c"
#include "poll.c"
+#include "processor.c"
#include "property.c"
#include "rbtree.c"
#include "rcu.c"
diff --git a/rust/helpers/processor.c b/rust/helpers/processor.c
new file mode 100644
index 000000000000..d41355e14d6e
--- /dev/null
+++ b/rust/helpers/processor.c
@@ -0,0 +1,8 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/processor.h>
+
+void rust_helper_cpu_relax(void)
+{
+ cpu_relax();
+}
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index ed53169e795c..c098c47c1817 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -110,6 +110,7 @@
pub mod platform;
pub mod prelude;
pub mod print;
+pub mod processor;
pub mod rbtree;
pub mod regulator;
pub mod revocable;
diff --git a/rust/kernel/processor.rs b/rust/kernel/processor.rs
new file mode 100644
index 000000000000..85b49b3614dd
--- /dev/null
+++ b/rust/kernel/processor.rs
@@ -0,0 +1,14 @@
+// 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.
+#[inline]
+pub fn cpu_relax() {
+ // SAFETY: Always safe to call.
+ unsafe { bindings::cpu_relax() }
+}
--
2.43.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 2/2] rust: Add read_poll_timeout functions
2025-08-17 4:47 [PATCH v2 0/2] rust: Add read_poll_timeout FUJITA Tomonori
2025-08-17 4:47 ` [PATCH v2 1/2] rust: Add cpu_relax() helper FUJITA Tomonori
@ 2025-08-17 4:47 ` FUJITA Tomonori
2025-08-19 0:49 ` Alexandre Courbot
` (2 more replies)
1 sibling, 3 replies; 9+ messages in thread
From: FUJITA Tomonori @ 2025-08-17 4:47 UTC (permalink / raw)
To: a.hindborg, alex.gaynor, ojeda
Cc: aliceryhl, anna-maria, bjorn3_gh, boqun.feng, dakr, frederic,
gary, jstultz, linux-kernel, lossin, lyude, rust-for-linux, sboyd,
tglx, tmgross, acourbot, daniel.almeida, Fiona Behrens
Add read_poll_timeout function which poll periodically until a
condition is met or a timeout is reached.
The 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.
The sleep_before_read argument isn't supported since there is no user
for now. It's rarely used in the C version.
Reviewed-by: Andreas Hindborg <a.hindborg@kernel.org>
Reviewed-by: Fiona Behrens <me@kloenk.dev>
Tested-by: Daniel Almeida <daniel.almeida@collabora.com>
Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
---
rust/kernel/io.rs | 1 +
rust/kernel/io/poll.rs | 96 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 97 insertions(+)
create mode 100644 rust/kernel/io/poll.rs
diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs
index 03b467722b86..ee182b0b5452 100644
--- a/rust/kernel/io.rs
+++ b/rust/kernel/io.rs
@@ -8,6 +8,7 @@
use crate::{bindings, build_assert, ffi::c_void};
pub mod mem;
+pub mod poll;
pub mod resource;
pub use resource::Resource;
diff --git a/rust/kernel/io/poll.rs b/rust/kernel/io/poll.rs
new file mode 100644
index 000000000000..e6325725d5a3
--- /dev/null
+++ b/rust/kernel/io/poll.rs
@@ -0,0 +1,96 @@
+// 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,
+ task::might_sleep,
+ time::{delay::fsleep, Delta, Instant, Monotonic},
+};
+
+/// Polls periodically until a condition is met or a timeout is reached.
+///
+/// The function repeatedly executes the given operation `op` closure and
+/// checks its result using the condition closure `cond`.
+///
+/// If `cond` returns `true`, the function returns successfully with the result of `op`.
+/// Otherwise, it waits for a duration specified by `sleep_delta`
+/// before executing `op` again.
+///
+/// This process continues until either `cond` returns `true` or the timeout,
+/// specified by `timeout_delta`, is reached. If `timeout_delta` is `None`,
+/// polling continues indefinitely until `cond` evaluates to `true` or an error occurs.
+///
+/// This function can only be used in a nonatomic context.
+///
+/// # Examples
+///
+/// ```no_run
+/// use kernel::io::{Io, poll::read_poll_timeout};
+/// use kernel::time::Delta;
+///
+/// const HW_READY: u16 = 0x01;
+///
+/// fn wait_for_hardware<const SIZE: usize>(io: &Io<SIZE>) -> Result<()> {
+/// match read_poll_timeout(
+/// // The `op` closure reads the value of a specific status register.
+/// || io.try_read16(0x1000),
+/// // The `cond` closure takes a reference to the value returned by `op`
+/// // and checks whether the hardware is ready.
+/// |val: &u16| *val == HW_READY,
+/// Delta::from_millis(50),
+/// Delta::from_secs(3),
+/// ) {
+/// Ok(_) => {
+/// // The hardware is ready. The returned value of the `op` closure
+/// // isn't used.
+/// Ok(())
+/// }
+/// Err(e) => Err(e),
+/// }
+/// }
+/// ```
+#[track_caller]
+pub fn read_poll_timeout<Op, Cond, T>(
+ mut op: Op,
+ mut cond: Cond,
+ sleep_delta: Delta,
+ timeout_delta: Delta,
+) -> Result<T>
+where
+ Op: FnMut() -> Result<T>,
+ Cond: FnMut(&T) -> bool,
+{
+ let start: Instant<Monotonic> = Instant::now();
+
+ // Unlike the C version, we always call `might_sleep()` unconditionally,
+ // as conditional calls are error-prone. We clearly separate
+ // `read_poll_timeout()` and `read_poll_timeout_atomic()` to aid
+ // tools like klint.
+ might_sleep();
+
+ loop {
+ let val = op()?;
+ if cond(&val) {
+ // Unlike the C version, we immediately return.
+ // We know the condition is met so we don't need to check again.
+ return Ok(val);
+ }
+
+ if start.elapsed() > timeout_delta {
+ // Unlike the C version, we immediately return.
+ // We have just called `op()` so we don't need to call it again.
+ return Err(ETIMEDOUT);
+ }
+
+ if !sleep_delta.is_zero() {
+ fsleep(sleep_delta);
+ }
+
+ // fsleep() could be busy-wait loop so we always call cpu_relax().
+ cpu_relax();
+ }
+}
--
2.43.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/2] rust: Add read_poll_timeout functions
2025-08-17 4:47 ` [PATCH v2 2/2] rust: Add read_poll_timeout functions FUJITA Tomonori
@ 2025-08-19 0:49 ` Alexandre Courbot
2025-08-20 11:11 ` FUJITA Tomonori
2025-08-19 18:30 ` Daniel Almeida
2025-08-20 10:02 ` Alice Ryhl
2 siblings, 1 reply; 9+ messages in thread
From: Alexandre Courbot @ 2025-08-19 0:49 UTC (permalink / raw)
To: FUJITA Tomonori, a.hindborg, alex.gaynor, ojeda
Cc: aliceryhl, anna-maria, bjorn3_gh, boqun.feng, dakr, frederic,
gary, jstultz, linux-kernel, lossin, lyude, rust-for-linux, sboyd,
tglx, tmgross, daniel.almeida, Fiona Behrens
On Sun Aug 17, 2025 at 1:47 PM JST, FUJITA Tomonori wrote:
> Add read_poll_timeout function which poll periodically until a
> condition is met or a timeout is reached.
>
> The 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.
>
> The sleep_before_read argument isn't supported since there is no user
> for now. It's rarely used in the C version.
>
> Reviewed-by: Andreas Hindborg <a.hindborg@kernel.org>
> Reviewed-by: Fiona Behrens <me@kloenk.dev>
> Tested-by: Daniel Almeida <daniel.almeida@collabora.com>
> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
Tested this with nova-core, and it seems to work fine!
Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>
Tested-by: Alexandre Courbot <acourbot@nvidia.com>
Just one last comment about the documentation below.
<snip>
> +/// Polls periodically until a condition is met or a timeout is reached.
> +///
> +/// The function repeatedly executes the given operation `op` closure and
> +/// checks its result using the condition closure `cond`.
> +///
> +/// If `cond` returns `true`, the function returns successfully with the result of `op`.
> +/// Otherwise, it waits for a duration specified by `sleep_delta`
> +/// before executing `op` again.
> +///
> +/// This process continues until either `cond` returns `true` or the timeout,
> +/// specified by `timeout_delta`, is reached. If `timeout_delta` is `None`,
For precision: "This process continues until either `op` returns and
error, `cond` returns `true`, or the timeout specified by
`timeout_delta` is reached."
> +/// polling continues indefinitely until `cond` evaluates to `true` or an error occurs.
> +///
> +/// This function can only be used in a nonatomic context.
Here I'd add an errors section:
# Errors
If `op` returns an error, then that error is returned directly.
If the timeout specified by `timeout_delta` is reached, then
`Err(ETIMEDOUT)` is returned.
> +///
> +/// # Examples
> +///
> +/// ```no_run
> +/// use kernel::io::{Io, poll::read_poll_timeout};
> +/// use kernel::time::Delta;
> +///
> +/// const HW_READY: u16 = 0x01;
> +///
> +/// fn wait_for_hardware<const SIZE: usize>(io: &Io<SIZE>) -> Result<()> {
> +/// match read_poll_timeout(
> +/// // The `op` closure reads the value of a specific status register.
> +/// || io.try_read16(0x1000),
> +/// // The `cond` closure takes a reference to the value returned by `op`
> +/// // and checks whether the hardware is ready.
> +/// |val: &u16| *val == HW_READY,
> +/// Delta::from_millis(50),
> +/// Delta::from_secs(3),
> +/// ) {
> +/// Ok(_) => {
> +/// // The hardware is ready. The returned value of the `op` closure
> +/// // isn't used.
> +/// Ok(())
> +/// }
> +/// Err(e) => Err(e),
> +/// }
> +/// }
> +/// ```
> +#[track_caller]
> +pub fn read_poll_timeout<Op, Cond, T>(
> + mut op: Op,
> + mut cond: Cond,
> + sleep_delta: Delta,
> + timeout_delta: Delta,
> +) -> Result<T>
> +where
> + Op: FnMut() -> Result<T>,
> + Cond: FnMut(&T) -> bool,
> +{
> + let start: Instant<Monotonic> = Instant::now();
> +
> + // Unlike the C version, we always call `might_sleep()` unconditionally,
> + // as conditional calls are error-prone. We clearly separate
> + // `read_poll_timeout()` and `read_poll_timeout_atomic()` to aid
> + // tools like klint.
> + might_sleep();
> +
> + loop {
> + let val = op()?;
> + if cond(&val) {
> + // Unlike the C version, we immediately return.
> + // We know the condition is met so we don't need to check again.
nit: this comment looks superfluous to me, this is a different
implementation from the C version anyway.
> + return Ok(val);
> + }
> +
> + if start.elapsed() > timeout_delta {
> + // Unlike the C version, we immediately return.
> + // We have just called `op()` so we don't need to call it again.
Same here.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/2] rust: Add read_poll_timeout functions
2025-08-17 4:47 ` [PATCH v2 2/2] rust: Add read_poll_timeout functions FUJITA Tomonori
2025-08-19 0:49 ` Alexandre Courbot
@ 2025-08-19 18:30 ` Daniel Almeida
2025-08-20 6:45 ` FUJITA Tomonori
2025-08-20 10:02 ` Alice Ryhl
2 siblings, 1 reply; 9+ messages in thread
From: Daniel Almeida @ 2025-08-19 18:30 UTC (permalink / raw)
To: FUJITA Tomonori
Cc: a.hindborg, alex.gaynor, ojeda, aliceryhl, anna-maria, bjorn3_gh,
boqun.feng, dakr, frederic, gary, jstultz, linux-kernel, lossin,
lyude, rust-for-linux, sboyd, tglx, tmgross, acourbot,
Fiona Behrens
Hi Fujita,
> On 17 Aug 2025, at 01:47, FUJITA Tomonori <fujita.tomonori@gmail.com> wrote:
>
> Add read_poll_timeout function which poll periodically until a
> condition is met or a timeout is reached.
>
> The 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.
>
> The sleep_before_read argument isn't supported since there is no user
> for now. It's rarely used in the C version.
>
> Reviewed-by: Andreas Hindborg <a.hindborg@kernel.org>
> Reviewed-by: Fiona Behrens <me@kloenk.dev>
> Tested-by: Daniel Almeida <daniel.almeida@collabora.com>
> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
> ---
> rust/kernel/io.rs | 1 +
> rust/kernel/io/poll.rs | 96 ++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 97 insertions(+)
> create mode 100644 rust/kernel/io/poll.rs
>
> diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs
> index 03b467722b86..ee182b0b5452 100644
> --- a/rust/kernel/io.rs
> +++ b/rust/kernel/io.rs
> @@ -8,6 +8,7 @@
> use crate::{bindings, build_assert, ffi::c_void};
>
> pub mod mem;
> +pub mod poll;
> pub mod resource;
>
> pub use resource::Resource;
> diff --git a/rust/kernel/io/poll.rs b/rust/kernel/io/poll.rs
> new file mode 100644
> index 000000000000..e6325725d5a3
> --- /dev/null
> +++ b/rust/kernel/io/poll.rs
> @@ -0,0 +1,96 @@
> +// 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,
> + task::might_sleep,
> + time::{delay::fsleep, Delta, Instant, Monotonic},
> +};
> +
> +/// Polls periodically until a condition is met or a timeout is reached.
> +///
> +/// The function repeatedly executes the given operation `op` closure and
> +/// checks its result using the condition closure `cond`.
> +///
> +/// If `cond` returns `true`, the function returns successfully with the result of `op`.
> +/// Otherwise, it waits for a duration specified by `sleep_delta`
> +/// before executing `op` again.
> +///
> +/// This process continues until either `cond` returns `true` or the timeout,
> +/// specified by `timeout_delta`, is reached. If `timeout_delta` is `None`,
> +/// polling continues indefinitely until `cond` evaluates to `true` or an error occurs.
> +///
> +/// This function can only be used in a nonatomic context.
> +///
> +/// # Examples
> +///
> +/// ```no_run
> +/// use kernel::io::{Io, poll::read_poll_timeout};
> +/// use kernel::time::Delta;
> +///
> +/// const HW_READY: u16 = 0x01;
> +///
> +/// fn wait_for_hardware<const SIZE: usize>(io: &Io<SIZE>) -> Result<()> {
> +/// match read_poll_timeout(
> +/// // The `op` closure reads the value of a specific status register.
> +/// || io.try_read16(0x1000),
> +/// // The `cond` closure takes a reference to the value returned by `op`
> +/// // and checks whether the hardware is ready.
> +/// |val: &u16| *val == HW_READY,
> +/// Delta::from_millis(50),
> +/// Delta::from_secs(3),
> +/// ) {
> +/// Ok(_) => {
> +/// // The hardware is ready. The returned value of the `op` closure
> +/// // isn't used.
> +/// Ok(())
> +/// }
> +/// Err(e) => Err(e),
> +/// }
> +/// }
> +/// ```
> +#[track_caller]
> +pub fn read_poll_timeout<Op, Cond, T>(
> + mut op: Op,
> + mut cond: Cond,
> + sleep_delta: Delta,
> + timeout_delta: Delta,
> +) -> Result<T>
> +where
> + Op: FnMut() -> Result<T>,
> + Cond: FnMut(&T) -> bool,
> +{
> + let start: Instant<Monotonic> = Instant::now();
> +
> + // Unlike the C version, we always call `might_sleep()` unconditionally,
> + // as conditional calls are error-prone. We clearly separate
> + // `read_poll_timeout()` and `read_poll_timeout_atomic()` to aid
> + // tools like klint.
> + might_sleep();
> +
> + loop {
> + let val = op()?;
> + if cond(&val) {
> + // Unlike the C version, we immediately return.
> + // We know the condition is met so we don't need to check again.
> + return Ok(val);
> + }
> +
> + if start.elapsed() > timeout_delta {
> + // Unlike the C version, we immediately return.
> + // We have just called `op()` so we don't need to call it again.
> + return Err(ETIMEDOUT);
> + }
> +
> + if !sleep_delta.is_zero() {
> + fsleep(sleep_delta);
> + }
> +
> + // fsleep() could be busy-wait loop so we always call cpu_relax().
> + cpu_relax();
> + }
> +}
> --
> 2.43.0
>
>
Thanks for working on this. Definitely going to be needed by a lot of drivers.
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
How is the atomic version going to look like? The same, except for
might_sleep() and without the sleep_delta argument?
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 1/2] rust: Add cpu_relax() helper
2025-08-17 4:47 ` [PATCH v2 1/2] rust: Add cpu_relax() helper FUJITA Tomonori
@ 2025-08-19 18:32 ` Daniel Almeida
0 siblings, 0 replies; 9+ messages in thread
From: Daniel Almeida @ 2025-08-19 18:32 UTC (permalink / raw)
To: FUJITA Tomonori
Cc: a.hindborg, alex.gaynor, ojeda, aliceryhl, anna-maria, bjorn3_gh,
boqun.feng, dakr, frederic, gary, jstultz, linux-kernel, lossin,
lyude, rust-for-linux, sboyd, tglx, tmgross, acourbot
> On 17 Aug 2025, at 01:47, FUJITA Tomonori <fujita.tomonori@gmail.com> wrote:
>
> Add cpu_relax() helper in preparation for supporting
> read_poll_timeout().
>
> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
> Reviewed-by: Andreas Hindborg <a.hindborg@kernel.org>
> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
> ---
> rust/helpers/helpers.c | 1 +
> rust/helpers/processor.c | 8 ++++++++
> rust/kernel/lib.rs | 1 +
> rust/kernel/processor.rs | 14 ++++++++++++++
> 4 files changed, 24 insertions(+)
> create mode 100644 rust/helpers/processor.c
> create mode 100644 rust/kernel/processor.rs
>
> diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c
> index 7cf7fe95e41d..04598665e7c8 100644
> --- a/rust/helpers/helpers.c
> +++ b/rust/helpers/helpers.c
> @@ -34,6 +34,7 @@
> #include "pid_namespace.c"
> #include "platform.c"
> #include "poll.c"
> +#include "processor.c"
> #include "property.c"
> #include "rbtree.c"
> #include "rcu.c"
> diff --git a/rust/helpers/processor.c b/rust/helpers/processor.c
> new file mode 100644
> index 000000000000..d41355e14d6e
> --- /dev/null
> +++ b/rust/helpers/processor.c
> @@ -0,0 +1,8 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <linux/processor.h>
> +
> +void rust_helper_cpu_relax(void)
> +{
> + cpu_relax();
> +}
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index ed53169e795c..c098c47c1817 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -110,6 +110,7 @@
> pub mod platform;
> pub mod prelude;
> pub mod print;
> +pub mod processor;
> pub mod rbtree;
> pub mod regulator;
> pub mod revocable;
> diff --git a/rust/kernel/processor.rs b/rust/kernel/processor.rs
> new file mode 100644
> index 000000000000..85b49b3614dd
> --- /dev/null
> +++ b/rust/kernel/processor.rs
> @@ -0,0 +1,14 @@
> +// 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.
> +#[inline]
> +pub fn cpu_relax() {
> + // SAFETY: Always safe to call.
> + unsafe { bindings::cpu_relax() }
> +}
> --
> 2.43.0
>
>
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/2] rust: Add read_poll_timeout functions
2025-08-19 18:30 ` Daniel Almeida
@ 2025-08-20 6:45 ` FUJITA Tomonori
0 siblings, 0 replies; 9+ messages in thread
From: FUJITA Tomonori @ 2025-08-20 6:45 UTC (permalink / raw)
To: daniel.almeida
Cc: fujita.tomonori, a.hindborg, alex.gaynor, ojeda, aliceryhl,
anna-maria, bjorn3_gh, boqun.feng, dakr, frederic, gary, jstultz,
linux-kernel, lossin, lyude, rust-for-linux, sboyd, tglx, tmgross,
acourbot, me
On Tue, 19 Aug 2025 15:30:51 -0300
Daniel Almeida <daniel.almeida@collabora.com> wrote:
> Thanks for working on this. Definitely going to be needed by a lot of drivers.
>
> Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
Thanks!
> How is the atomic version going to look like? The same, except for
> might_sleep() and without the sleep_delta argument?
If we follow the C implementation, it will be different; C's
read_poll_atomic doesn't use ktime to calculate a timeout.
It would look like the following. I think that the read_poll_timeout
patchset is almost complete so I'll send the
read_poll_timeout_atomic() patchset shortly.
diff --git a/rust/kernel/io/poll.rs b/rust/kernel/io/poll.rs
index e6325725d5a3..dc4f1ecdf31f 100644
--- a/rust/kernel/io/poll.rs
+++ b/rust/kernel/io/poll.rs
@@ -8,7 +8,10 @@
error::{code::*, Result},
processor::cpu_relax,
task::might_sleep,
- time::{delay::fsleep, Delta, Instant, Monotonic},
+ time::{
+ delay::{fsleep, udelay},
+ Delta, Instant, Monotonic,
+ },
};
/// Polls periodically until a condition is met or a timeout is reached.
@@ -94,3 +97,86 @@ pub fn read_poll_timeout<Op, Cond, T>(
cpu_relax();
}
}
+
+/// Polls periodically until a condition is met or a timeout is reached.
+///
+/// The function repeatedly executes the given operation `op` closure and
+/// checks its result using the condition closure `cond`.
+///
+/// If `cond` returns `true`, the function returns successfully with the result of `op`.
+/// Otherwise, it performs a busy wait for a duration specified by `delay_delta`
+/// before executing `op` again.
+///
+/// This process continues until either `cond` returns `true` or the timeout,
+/// specified by `timeout_delta`, is reached. If `timeout_delta` is `None`,
+/// polling continues indefinitely until `cond` evaluates to `true` or an error occurs.
+///
+/// # Examples
+///
+/// ```no_run
+/// use kernel::io::{Io, poll::read_poll_timeout_atomic};
+/// use kernel::time::Delta;
+///
+/// const HW_READY: u16 = 0x01;
+///
+/// fn wait_for_hardware<const SIZE: usize>(io: &Io<SIZE>) -> Result<()> {
+/// match read_poll_timeout_atomic(
+/// // The `op` closure reads the value of a specific status register.
+/// || io.try_read16(0x1000),
+/// // The `cond` closure takes a reference to the value returned by `op`
+/// // and checks whether the hardware is ready.
+/// |val: &u16| *val == HW_READY,
+/// Delta::from_micros(50),
+/// Delta::from_micros(300),
+/// ) {
+/// Ok(_) => {
+/// // The hardware is ready. The returned value of the `op` closure
+/// // isn't used.
+/// Ok(())
+/// }
+/// Err(e) => Err(e),
+/// }
+/// }
+/// ```
+pub fn read_poll_timeout_atomic<Op, Cond, T>(
+ mut op: Op,
+ mut cond: Cond,
+ delay_delta: Delta,
+ timeout_delta: Delta,
+) -> Result<T>
+where
+ Op: FnMut() -> Result<T>,
+ Cond: FnMut(&T) -> bool,
+{
+ let mut left_ns = timeout_delta.as_nanos();
+ let delay_ns = delay_delta.as_nanos();
+
+ let timeout_is_zero = timeout_delta.is_zero();
+
+ loop {
+ let val = op()?;
+ if cond(&val) {
+ // Unlike the C version, we immediately return.
+ // We know the condition is met so we don't need to check again.
+ return Ok(val);
+ }
+
+ if !timeout_is_zero && left_ns < 0 {
+ // Unlike the C version, we immediately return.
+ // We have just called `op()` so we don't need to call it again.
+ return Err(ETIMEDOUT);
+ }
+
+ if !delay_delta.is_zero() {
+ udelay(delay_delta);
+ if !timeout_is_zero {
+ left_ns -= delay_ns;
+ }
+ }
+
+ cpu_relax();
+ if !timeout_is_zero {
+ left_ns -= 1;
+ }
+ }
+}
--
2.43.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/2] rust: Add read_poll_timeout functions
2025-08-17 4:47 ` [PATCH v2 2/2] rust: Add read_poll_timeout functions FUJITA Tomonori
2025-08-19 0:49 ` Alexandre Courbot
2025-08-19 18:30 ` Daniel Almeida
@ 2025-08-20 10:02 ` Alice Ryhl
2 siblings, 0 replies; 9+ messages in thread
From: Alice Ryhl @ 2025-08-20 10:02 UTC (permalink / raw)
To: FUJITA Tomonori
Cc: a.hindborg, alex.gaynor, ojeda, anna-maria, bjorn3_gh, boqun.feng,
dakr, frederic, gary, jstultz, linux-kernel, lossin, lyude,
rust-for-linux, sboyd, tglx, tmgross, acourbot, daniel.almeida,
Fiona Behrens
On Sun, Aug 17, 2025 at 01:47:23PM +0900, FUJITA Tomonori wrote:
> Add read_poll_timeout function which poll periodically until a
> condition is met or a timeout is reached.
>
> The 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.
>
> The sleep_before_read argument isn't supported since there is no user
> for now. It's rarely used in the C version.
>
> Reviewed-by: Andreas Hindborg <a.hindborg@kernel.org>
> Reviewed-by: Fiona Behrens <me@kloenk.dev>
> Tested-by: Daniel Almeida <daniel.almeida@collabora.com>
> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/2] rust: Add read_poll_timeout functions
2025-08-19 0:49 ` Alexandre Courbot
@ 2025-08-20 11:11 ` FUJITA Tomonori
0 siblings, 0 replies; 9+ messages in thread
From: FUJITA Tomonori @ 2025-08-20 11:11 UTC (permalink / raw)
To: acourbot
Cc: fujita.tomonori, a.hindborg, alex.gaynor, ojeda, aliceryhl,
anna-maria, bjorn3_gh, boqun.feng, dakr, frederic, gary, jstultz,
linux-kernel, lossin, lyude, rust-for-linux, sboyd, tglx, tmgross,
daniel.almeida, me
On Tue, 19 Aug 2025 09:49:38 +0900
"Alexandre Courbot" <acourbot@nvidia.com> wrote:
> On Sun Aug 17, 2025 at 1:47 PM JST, FUJITA Tomonori wrote:
>> Add read_poll_timeout function which poll periodically until a
>> condition is met or a timeout is reached.
>>
>> The 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.
>>
>> The sleep_before_read argument isn't supported since there is no user
>> for now. It's rarely used in the C version.
>>
>> Reviewed-by: Andreas Hindborg <a.hindborg@kernel.org>
>> Reviewed-by: Fiona Behrens <me@kloenk.dev>
>> Tested-by: Daniel Almeida <daniel.almeida@collabora.com>
>> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
>
> Tested this with nova-core, and it seems to work fine!
>
> Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>
> Tested-by: Alexandre Courbot <acourbot@nvidia.com>
Thanks!
> Just one last comment about the documentation below.
>
> <snip>
>> +/// Polls periodically until a condition is met or a timeout is reached.
>> +///
>> +/// The function repeatedly executes the given operation `op` closure and
>> +/// checks its result using the condition closure `cond`.
>> +///
>> +/// If `cond` returns `true`, the function returns successfully with the result of `op`.
>> +/// Otherwise, it waits for a duration specified by `sleep_delta`
>> +/// before executing `op` again.
>> +///
>> +/// This process continues until either `cond` returns `true` or the timeout,
>> +/// specified by `timeout_delta`, is reached. If `timeout_delta` is `None`,
>
> For precision: "This process continues until either `op` returns and
> error, `cond` returns `true`, or the timeout specified by
> `timeout_delta` is reached."
Indeed, `op` should be mentioned here.
I think it should be "op returns an error", not "op returns and error",
right?
There are three alternatives, so I’d remove "either". The sentence
would be:
This process continues until either `op` returns an error, `cond`
returns `true`, or the timeout specified by `timeout_delta` is
reached.
I just realized that "If `timeout_delta` is `None`," comment is outdated. I'll update.
>> +/// polling continues indefinitely until `cond` evaluates to `true` or an error occurs.
>> +///
>> +/// This function can only be used in a nonatomic context.
>
> Here I'd add an errors section:
>
> # Errors
>
> If `op` returns an error, then that error is returned directly.
>
> If the timeout specified by `timeout_delta` is reached, then
> `Err(ETIMEDOUT)` is returned.
Thanks, looks nice. I'll add it.
>> + loop {
>> + let val = op()?;
>> + if cond(&val) {
>> + // Unlike the C version, we immediately return.
>> + // We know the condition is met so we don't need to check again.
>
> nit: this comment looks superfluous to me, this is a different
> implementation from the C version anyway.
A previous review mentioned that, since this function shares the same
name as the C version, it's good to make the different behavior
explicit. It may be a bit redundant, but I'd prefer to keep it.
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2025-08-20 11:11 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-17 4:47 [PATCH v2 0/2] rust: Add read_poll_timeout FUJITA Tomonori
2025-08-17 4:47 ` [PATCH v2 1/2] rust: Add cpu_relax() helper FUJITA Tomonori
2025-08-19 18:32 ` Daniel Almeida
2025-08-17 4:47 ` [PATCH v2 2/2] rust: Add read_poll_timeout functions FUJITA Tomonori
2025-08-19 0:49 ` Alexandre Courbot
2025-08-20 11:11 ` FUJITA Tomonori
2025-08-19 18:30 ` Daniel Almeida
2025-08-20 6:45 ` FUJITA Tomonori
2025-08-20 10:02 ` Alice Ryhl
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).