Add read_poll_timeout_atomic function which polls periodically until a
condition is met, an error occurs, or the timeout is reached.
The C's read_poll_timeout_atomic (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 delay_before_read argument isn't supported since there is no user
for now. It's rarely used in the C version.
Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
---
rust/kernel/io/poll.rs | 90 +++++++++++++++++++++++++++++++++++++++++-
1 file changed, 89 insertions(+), 1 deletion(-)
diff --git a/rust/kernel/io/poll.rs b/rust/kernel/io/poll.rs
index 7af1934e397a..71c2c0e0d8b4 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, an error occurs,
@@ -102,3 +105,88 @@ pub fn read_poll_timeout<Op, Cond, T>(
cpu_relax();
}
}
+
+/// Polls periodically until a condition is met, an error occurs,
+/// or the 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 `op` returns an error, `cond`
+/// returns `true`, or the timeout specified by `timeout_delta` is
+/// reached.
+///
+/// # 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_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();
+
+ 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 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);
+ left_ns -= delay_ns;
+ }
+
+ cpu_relax();
+ left_ns -= 1;
+ }
+}
--
2.43.0On Thu Aug 21, 2025 at 5:57 AM CEST, FUJITA Tomonori wrote:
> +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();
> +
> + 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 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);
> + left_ns -= delay_ns;
> + }
> +
> + cpu_relax();
> + left_ns -= 1;
How do we know that each iteration costs 1ns? To make it even more obvious, we
don't control the implementation of cond(). Shouldn't we use ktime for this?
On Tue, 26 Aug 2025 16:12:44 +0200
"Danilo Krummrich" <dakr@kernel.org> wrote:
> On Thu Aug 21, 2025 at 5:57 AM CEST, FUJITA Tomonori wrote:
>> +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();
>> +
>> + 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 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);
>> + left_ns -= delay_ns;
>> + }
>> +
>> + cpu_relax();
>> + left_ns -= 1;
>
> How do we know that each iteration costs 1ns? To make it even more obvious, we
> don't control the implementation of cond(). Shouldn't we use ktime for this?
The C version used to use ktime but it has been changed not to:
7349a69cf312 ("iopoll: Do not use timekeeping in read_poll_timeout_atomic()")
https://lore.kernel.org/all/3d2a2f4e553489392d871108797c3be08f88300b.1685692810.git.geert+renesas@glider.be/
I don’t know if the same problem still exists, but I think we should
follow the C implementation. Usually there’s a good reason behind it,
and it has been working so far.
If we want to do it differently in Rust, maybe we should first discuss
the C implementation.
On Wed Aug 27, 2025 at 2:14 AM CEST, FUJITA Tomonori wrote:
> On Tue, 26 Aug 2025 16:12:44 +0200
> "Danilo Krummrich" <dakr@kernel.org> wrote:
>
>> On Thu Aug 21, 2025 at 5:57 AM CEST, FUJITA Tomonori wrote:
>>> +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();
>>> +
>>> + 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 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);
>>> + left_ns -= delay_ns;
>>> + }
>>> +
>>> + cpu_relax();
>>> + left_ns -= 1;
>>
>> How do we know that each iteration costs 1ns? To make it even more obvious, we
>> don't control the implementation of cond(). Shouldn't we use ktime for this?
>
> The C version used to use ktime but it has been changed not to:
>
> 7349a69cf312 ("iopoll: Do not use timekeeping in read_poll_timeout_atomic()")
Ick! That's pretty unfortunate -- no ktime then.
But regardless of that, the current implementation (this and the C one) lack
clarity.
The nanosecond decrement is rather negligible, the real timeout reduction comes
from the delay_delta. Given that, and the fact that we can't use ktime, this
function shouldn't take a raw timeout value, since we can't guarantee the
timeout anyways.
Instead, I think it makes much more sense to provide a retry count as function
argument, such that the user can specify "I want a dealy of 100us, try it 100
times".
This way it is transparent to the caller that the timeout may be significantly
more than 10ms depending on the user's implementation.
As for doing this in C vs Rust: I don't think things have to align in every
implementation detail. If we can improve things on the Rust side from the
get-go, we should not stop ourselves from doing so, just because a similar C
implementation is hard to refactor, due to having a lot of users already.
On Wed Aug 27, 2025 at 11:00 AM CEST, Danilo Krummrich wrote:
> On Wed Aug 27, 2025 at 2:14 AM CEST, FUJITA Tomonori wrote:
>> On Tue, 26 Aug 2025 16:12:44 +0200
>> "Danilo Krummrich" <dakr@kernel.org> wrote:
>>
>>> On Thu Aug 21, 2025 at 5:57 AM CEST, FUJITA Tomonori wrote:
>>>> +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();
>>>> +
>>>> + 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 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);
>>>> + left_ns -= delay_ns;
>>>> + }
>>>> +
>>>> + cpu_relax();
>>>> + left_ns -= 1;
>>>
>>> How do we know that each iteration costs 1ns? To make it even more obvious, we
>>> don't control the implementation of cond(). Shouldn't we use ktime for this?
>>
>> The C version used to use ktime but it has been changed not to:
>>
>> 7349a69cf312 ("iopoll: Do not use timekeeping in read_poll_timeout_atomic()")
>
> Ick! That's pretty unfortunate -- no ktime then.
>
> But regardless of that, the current implementation (this and the C one) lack
> clarity.
>
> The nanosecond decrement is rather negligible, the real timeout reduction comes
> from the delay_delta. Given that, and the fact that we can't use ktime, this
> function shouldn't take a raw timeout value, since we can't guarantee the
> timeout anyways.
Actually, let me put it in other words:
let val = read_poll_timeout_atomic(
|| {
// Fetch the offset to read from from the HW.
let offset = io.read32(0x1000);
// HW needs a break for some odd reason.
udelay(100);
// Read the actual value.
io.try_read32(offset)
},
|val: &u32| *val == HW_READY,
Delta::from_micros(0), // No delay, keep spinning.
Delta::from_millis(10), // Timeout after 10ms.
)?;
Seems like a fairly reasonable usage without knowing the implementation details
of read_poll_timeout_atomic(), right?
Except that if the hardware does not become ready, this will spin for 16.67
*minutes* -- in atomic context. Instead of the 10ms the user would expect.
This would be way less error prone if we do not provide a timeout value, but a
retry count.
> Instead, I think it makes much more sense to provide a retry count as function
> argument, such that the user can specify "I want a dealy of 100us, try it 100
> times".
>
> This way it is transparent to the caller that the timeout may be significantly
> more than 10ms depending on the user's implementation.
>
> As for doing this in C vs Rust: I don't think things have to align in every
> implementation detail. If we can improve things on the Rust side from the
> get-go, we should not stop ourselves from doing so, just because a similar C
> implementation is hard to refactor, due to having a lot of users already.
Hi Danilo,
[…}
>
> Actually, let me put it in other words:
>
> let val = read_poll_timeout_atomic(
> || {
> // Fetch the offset to read from from the HW.
> let offset = io.read32(0x1000);
>
> // HW needs a break for some odd reason.
> udelay(100);
>
> // Read the actual value.
> io.try_read32(offset)
> },
> |val: &u32| *val == HW_READY,
> Delta::from_micros(0), // No delay, keep spinning.
> Delta::from_millis(10), // Timeout after 10ms.
> )?;
>
> Seems like a fairly reasonable usage without knowing the implementation details
> of read_poll_timeout_atomic(), right?
>
> Except that if the hardware does not become ready, this will spin for 16.67
> *minutes* -- in atomic context. Instead of the 10ms the user would expect.
>
> This would be way less error prone if we do not provide a timeout value, but a
> retry count.
>
>> Instead, I think it makes much more sense to provide a retry count as function
>> argument, such that the user can specify "I want a dealy of 100us, try it 100
>> times".
>>
>> This way it is transparent to the caller that the timeout may be significantly
>> more than 10ms depending on the user's implementation.
>>
>> As for doing this in C vs Rust: I don't think things have to align in every
>> implementation detail. If we can improve things on the Rust side from the
>> get-go, we should not stop ourselves from doing so, just because a similar C
>> implementation is hard to refactor, due to having a lot of users already.
I must say I do not follow. Can you expand yet some more on this?
— Daniel
On Wed Aug 27, 2025 at 2:14 PM CEST, Daniel Almeida wrote:
> Hi Danilo,
>
> […}
>
>>
>> Actually, let me put it in other words:
>>
>> let val = read_poll_timeout_atomic(
>> || {
>> // Fetch the offset to read from from the HW.
>> let offset = io.read32(0x1000);
>>
>> // HW needs a break for some odd reason.
>> udelay(100);
>>
>> // Read the actual value.
>> io.try_read32(offset)
>> },
>> |val: &u32| *val == HW_READY,
>> Delta::from_micros(0), // No delay, keep spinning.
>> Delta::from_millis(10), // Timeout after 10ms.
>> )?;
>>
>> Seems like a fairly reasonable usage without knowing the implementation details
>> of read_poll_timeout_atomic(), right?
>>
>> Except that if the hardware does not become ready, this will spin for 16.67
>> *minutes* -- in atomic context. Instead of the 10ms the user would expect.
>>
>> This would be way less error prone if we do not provide a timeout value, but a
>> retry count.
>>
>>> Instead, I think it makes much more sense to provide a retry count as function
>>> argument, such that the user can specify "I want a dealy of 100us, try it 100
>>> times".
>>>
>>> This way it is transparent to the caller that the timeout may be significantly
>>> more than 10ms depending on the user's implementation.
>>>
>>> As for doing this in C vs Rust: I don't think things have to align in every
>>> implementation detail. If we can improve things on the Rust side from the
>>> get-go, we should not stop ourselves from doing so, just because a similar C
>>> implementation is hard to refactor, due to having a lot of users already.
>
> I must say I do not follow. Can you expand yet some more on this?
Sure, but it would help if you could clarify which aspect you want me to expand
on. :)
> On 27 Aug 2025, at 09:19, Danilo Krummrich <dakr@kernel.org> wrote:
>
> On Wed Aug 27, 2025 at 2:14 PM CEST, Daniel Almeida wrote:
>> Hi Danilo,
>>
>> […}
>>
>>>
>>> Actually, let me put it in other words:
>>>
>>> let val = read_poll_timeout_atomic(
>>> || {
>>> // Fetch the offset to read from from the HW.
>>> let offset = io.read32(0x1000);
>>>
>>> // HW needs a break for some odd reason.
>>> udelay(100);
Why would we have a delay here? Can’t this be broken into two calls to
read_poll_timeout_atomic()? That would be equivalent to what you wrote
IIUC.
>>>
>>> // Read the actual value.
>>> io.try_read32(offset)
>>> },
>>> |val: &u32| *val == HW_READY,
>>> Delta::from_micros(0), // No delay, keep spinning.
>>> Delta::from_millis(10), // Timeout after 10ms.
>>> )?;
>>>
>>> Seems like a fairly reasonable usage without knowing the implementation details
>>> of read_poll_timeout_atomic(), right?
>>>
>>> Except that if the hardware does not become ready, this will spin for 16.67
>>> *minutes* -- in atomic context. Instead of the 10ms the user would expect.
This is where you lost me. Where does the 16.67 come from?
>>>
>>> This would be way less error prone if we do not provide a timeout value, but a
>>> retry count.
>>>
>>>> Instead, I think it makes much more sense to provide a retry count as function
>>>> argument, such that the user can specify "I want a dealy of 100us, try it 100
>>>> times".
>>>>
>>>> This way it is transparent to the caller that the timeout may be significantly
>>>> more than 10ms depending on the user's implementation.
>>>>
>>>> As for doing this in C vs Rust: I don't think things have to align in every
>>>> implementation detail. If we can improve things on the Rust side from the
>>>> get-go, we should not stop ourselves from doing so, just because a similar C
>>>> implementation is hard to refactor, due to having a lot of users already.
>>
>> I must say I do not follow. Can you expand yet some more on this?
>
> Sure, but it would help if you could clarify which aspect you want me to expand
> on. :)
On Wed Aug 27, 2025 at 2:22 PM CEST, Daniel Almeida wrote:
>
>
>> On 27 Aug 2025, at 09:19, Danilo Krummrich <dakr@kernel.org> wrote:
>>
>> On Wed Aug 27, 2025 at 2:14 PM CEST, Daniel Almeida wrote:
>>> Hi Danilo,
>>>
>>> […}
>>>
>>>>
>>>> Actually, let me put it in other words:
>>>>
>>>> let val = read_poll_timeout_atomic(
>>>> || {
>>>> // Fetch the offset to read from from the HW.
>>>> let offset = io.read32(0x1000);
>>>>
>>>> // HW needs a break for some odd reason.
>>>> udelay(100);
>
> Why would we have a delay here? Can’t this be broken into two calls to
> read_poll_timeout_atomic()? That would be equivalent to what you wrote
> IIUC.
I'm sure this can somehow be written otherwise as well. But that's not the
point, the point is that this looks like perfectly valid code from a users
perspective.
>>>>
>>>> // Read the actual value.
>>>> io.try_read32(offset)
>>>> },
>>>> |val: &u32| *val == HW_READY,
>>>> Delta::from_micros(0), // No delay, keep spinning.
>>>> Delta::from_millis(10), // Timeout after 10ms.
>>>> )?;
>>>>
>>>> Seems like a fairly reasonable usage without knowing the implementation details
>>>> of read_poll_timeout_atomic(), right?
>>>>
>>>> Except that if the hardware does not become ready, this will spin for 16.67
>>>> *minutes* -- in atomic context. Instead of the 10ms the user would expect.
>
> This is where you lost me. Where does the 16.67 come from?
Ah, I see -- let me explain:
Internally read_poll_timeout_atomic() would convert the timeout (10ms) into ns
(let's call it nanos). Then, it would decrement nanos in every iteration of the
internal loop, based on the (wrong) assumption that every loop takes exactly
1ns.
However, since the user executes udelay(100), which is perfectly valid from the
users perspective, in the Op closure, every loop iteration takes at least 100us
instead.
So, the actual timeout calculates as follows.
Timeout: 10ms = 10.000us = 10.000.000ns
In every iteration this number is decremented by one, hence 10.000.000
iterations.
100us * 10.000.000 iterations = 16.67 minutes
So, the issue really is that we're not measuring time, but the number of
iterations if delay_delta == 0.
As delay_delta grows the relative eror becomes smaller, yet this is far from
sane behavior.
>>>>
>>>> This would be way less error prone if we do not provide a timeout value, but a
>>>> retry count.
>>>>
>>>>> Instead, I think it makes much more sense to provide a retry count as function
>>>>> argument, such that the user can specify "I want a dealy of 100us, try it 100
>>>>> times".
>>>>>
>>>>> This way it is transparent to the caller that the timeout may be significantly
>>>>> more than 10ms depending on the user's implementation.
>>>>>
>>>>> As for doing this in C vs Rust: I don't think things have to align in every
>>>>> implementation detail. If we can improve things on the Rust side from the
>>>>> get-go, we should not stop ourselves from doing so, just because a similar C
>>>>> implementation is hard to refactor, due to having a lot of users already.
>>>
>>> I must say I do not follow. Can you expand yet some more on this?
>>
>> Sure, but it would help if you could clarify which aspect you want me to expand
>> on. :)
> On 26 Aug 2025, at 11:12, Danilo Krummrich <dakr@kernel.org> wrote:
>
> On Thu Aug 21, 2025 at 5:57 AM CEST, FUJITA Tomonori wrote:
>> +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();
>> +
>> + 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 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);
>> + left_ns -= delay_ns;
>> + }
>> +
>> + cpu_relax();
>> + left_ns -= 1;
>
> How do we know that each iteration costs 1ns? To make it even more obvious, we
> don't control the implementation of cond(). Shouldn't we use ktime for this?
I don’t think ktime can be used from an atomic context?
— Daniel
On Tue Aug 26, 2025 at 6:59 PM CEST, Daniel Almeida wrote:
>
>
>> On 26 Aug 2025, at 11:12, Danilo Krummrich <dakr@kernel.org> wrote:
>>
>> On Thu Aug 21, 2025 at 5:57 AM CEST, FUJITA Tomonori wrote:
>>> +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();
>>> +
>>> + 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 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);
>>> + left_ns -= delay_ns;
>>> + }
>>> +
>>> + cpu_relax();
>>> + left_ns -= 1;
>>
>> How do we know that each iteration costs 1ns? To make it even more obvious, we
>> don't control the implementation of cond(). Shouldn't we use ktime for this?
>
> I don’t think ktime can be used from an atomic context?
There's no problem calling things like ktime_get() from atomic context.
Hi Fujita,
> On 21 Aug 2025, at 00:57, FUJITA Tomonori <fujita.tomonori@gmail.com> wrote:
>
> Add read_poll_timeout_atomic function which polls periodically until a
> condition is met, an error occurs, or the timeout is reached.
>
> The C's read_poll_timeout_atomic (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 delay_before_read argument isn't supported since there is no user
> for now. It's rarely used in the C version.
>
> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
> ---
> rust/kernel/io/poll.rs | 90 +++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 89 insertions(+), 1 deletion(-)
>
> diff --git a/rust/kernel/io/poll.rs b/rust/kernel/io/poll.rs
> index 7af1934e397a..71c2c0e0d8b4 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, an error occurs,
> @@ -102,3 +105,88 @@ pub fn read_poll_timeout<Op, Cond, T>(
> cpu_relax();
> }
> }
> +
> +/// Polls periodically until a condition is met, an error occurs,
> +/// or the 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 `op` returns an error, `cond`
> +/// returns `true`, or the timeout specified by `timeout_delta` is
> +/// reached.
> +///
> +/// # 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_atomic};
> +/// use kernel::time::Delta;
> +///
> +/// const HW_READY: u16 = 0x01;
> +///
> +/// fn wait_for_hardware<const SIZE: usize>(io: &Io<SIZE>) -> Result<()> {
Just “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();
> +
> + 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 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);
> + left_ns -= delay_ns;
> + }
> +
> + cpu_relax();
> + left_ns -= 1;
A comment on the line above would be nice.
Also, is timeout_delta == 0 an intended use-case?
> + }
> +}
> --
> 2.43.0
>
>
On Tue, 26 Aug 2025 11:02:18 -0300
Daniel Almeida <daniel.almeida@collabora.com> wrote:
>> +/// Polls periodically until a condition is met, an error occurs,
>> +/// or the 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 `op` returns an error, `cond`
>> +/// returns `true`, or the timeout specified by `timeout_delta` is
>> +/// reached.
>> +///
>> +/// # 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_atomic};
>> +/// use kernel::time::Delta;
>> +///
>> +/// const HW_READY: u16 = 0x01;
>> +///
>> +/// fn wait_for_hardware<const SIZE: usize>(io: &Io<SIZE>) -> Result<()> {
>
> Just “Result”.
Oops, thanks.
I'll update the example for read_poll_timeout() too.
>> +/// 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();
>> +
>> + 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 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);
>> + left_ns -= delay_ns;
>> + }
>> +
>> + cpu_relax();
>> + left_ns -= 1;
>
> A comment on the line above would be nice.
As I wrote in another email, the C version was changed to avoid using
ktime, and that’s when the code above was added. I assume the delay is
considered as 1ns as a compromise because ktime can’t be used.
Maybe this comment should be added to the C version instead?
> Also, is timeout_delta == 0 an intended use-case?
I’m not sure if it’s actually used, but I don’t see any reason to
forbid it.
On Wed, 27 Aug 2025 09:35:59 +0900 (JST)
FUJITA Tomonori <fujita.tomonori@gmail.com> wrote:
>>> +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();
>>> +
>>> + 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 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);
>>> + left_ns -= delay_ns;
>>> + }
>>> +
>>> + cpu_relax();
>>> + left_ns -= 1;
>>
>> A comment on the line above would be nice.
>
> As I wrote in another email, the C version was changed to avoid using
> ktime, and that’s when the code above was added. I assume the delay is
> considered as 1ns as a compromise because ktime can’t be used.
>
> Maybe this comment should be added to the C version instead?
I meant that if we add a comment here, maybe it should be added to the
C version.
© 2016 - 2025 Red Hat, Inc.