public inbox for netdev@vger.kernel.org 
 help / color / mirror / Atom feed
From: Fiona Behrens <me@kloenk•dev>
To: FUJITA Tomonori <fujita.tomonori@gmail•com>
Cc: linux-kernel@vger•kernel.org,
	 Daniel Almeida <daniel.almeida@collabora•com>,
	 rust-for-linux@vger•kernel.org, netdev@vger•kernel.org,
	 andrew@lunn•ch,  hkallweit1@gmail•com, tmgross@umich•edu,
	 ojeda@kernel•org,  alex.gaynor@gmail•com, gary@garyguo•net,
	 bjorn3_gh@protonmail•com,  benno.lossin@proton•me,
	a.hindborg@samsung•com,  aliceryhl@google•com,
	 anna-maria@linutronix•de, frederic@kernel•org,
	 tglx@linutronix•de,  arnd@arndb•de, jstultz@google•com,
	 sboyd@kernel•org,  mingo@redhat•com, peterz@infradead•org,
	 juri.lelli@redhat•com, vincent.guittot@linaro•org,
	 dietmar.eggemann@arm•com, rostedt@goodmis•org,
	 bsegall@google•com,  mgorman@suse•de, vschneid@redhat•com,
	 tgunders@redhat•com,  david.laight.linux@gmail•com
Subject: Re: [PATCH v11 7/8] rust: Add read_poll_timeout functions
Date: Thu, 20 Feb 2025 16:04:50 +0100	[thread overview]
Message-ID: <878qq064j1.fsf@kloenk.dev> (raw)
In-Reply-To: <20250220070611.214262-8-fujita.tomonori@gmail.com> (FUJITA Tomonori's message of "Thu, 20 Feb 2025 16:06:09 +0900")

FUJITA Tomonori <fujita.tomonori@gmail•com> writes:

> Add read_poll_timeout functions 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.
>
> read_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]
> Tested-by: Daniel Almeida <daniel.almeida@collabora•com>
> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail•com>

Reviewed-by: Fiona Behrens <me@kloenk•dev>

> ---
>  rust/helpers/helpers.c |   1 +
>  rust/helpers/kernel.c  |  18 +++++++
>  rust/kernel/cpu.rs     |  13 +++++
>  rust/kernel/error.rs   |   1 +
>  rust/kernel/io.rs      |   2 +
>  rust/kernel/io/poll.rs | 120 +++++++++++++++++++++++++++++++++++++++++
>  rust/kernel/lib.rs     |   1 +
>  7 files changed, 156 insertions(+)
>  create mode 100644 rust/helpers/kernel.c
>  create mode 100644 rust/kernel/cpu.rs
>  create mode 100644 rust/kernel/io/poll.rs
>
> diff --git a/rust/kernel/io/poll.rs b/rust/kernel/io/poll.rs
> new file mode 100644
> index 000000000000..5977b2082cc6
> --- /dev/null
> +++ b/rust/kernel/io/poll.rs
> @@ -0,0 +1,120 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! IO polling.
> +//!
> +//! C header: [`include/linux/iopoll.h`](srctree/include/linux/iopoll.h).
> +
> +use crate::{
> +    cpu::cpu_relax,
> +    error::{code::*, Result},
> +    time::{delay::fsleep, Delta, Instant},
> +};
> +
> +/// 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.
> +///
> +/// # Examples
> +///
> +/// ```rust,ignore
> +/// fn wait_for_hardware(dev: &mut Device) -> Result<()> {
> +///     // The `op` closure reads the value of a specific status register.
> +///     let op = || -> Result<u16> { dev.read_ready_register() };
> +///
> +///     // The `cond` closure takes a reference to the value returned by `op`
> +///     // and checks whether the hardware is ready.
> +///     let cond = |val: &u16| *val == HW_READY;
> +///
> +///     match read_poll_timeout(op, cond, Delta::from_millis(50), Some(Delta::from_secs(3))) {
> +///         Ok(_) => {
> +///             // The hardware is ready. The returned value of the `op`` closure isn't used.
> +///             Ok(())
> +///         }
> +///         Err(e) => Err(e),
> +///     }
> +/// }
> +/// ```
> +///
> +/// ```rust
> +/// use kernel::io::poll::read_poll_timeout;
> +/// use kernel::time::Delta;
> +/// use kernel::sync::{SpinLock, new_spinlock};
> +///
> +/// let lock = KBox::pin_init(new_spinlock!(()), kernel::alloc::flags::GFP_KERNEL)?;
> +/// let g = lock.lock();
> +/// read_poll_timeout(|| Ok(()), |()| true, Delta::from_micros(42), Some(Delta::from_micros(42)));
> +/// drop(g);
> +///
> +/// # Ok::<(), Error>(())
> +/// ```
> +#[track_caller]
> +pub fn read_poll_timeout<Op, Cond, T>(
> +    mut op: Op,
> +    mut cond: Cond,
> +    sleep_delta: Delta,
> +    timeout_delta: Option<Delta>,

Fun idea I just had, though not sure it is of actuall use (probably not).
Instead of `Option<Delta> we could use `impl Into<Option<Delta>>`,
that enables to use both, so not having to write Some if we have a value.

> +) -> Result<T>
> +where
> +    Op: FnMut() -> Result<T>,
> +    Cond: FnMut(&T) -> bool,
> +{
> +    let start = Instant::now();
> +    let sleep = !sleep_delta.is_zero();
> +
> +    if sleep {
> +        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 let Some(timeout_delta) = timeout_delta {
> +            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 {
> +            fsleep(sleep_delta);
> +        }
> +        // fsleep() could be busy-wait loop so we always call cpu_relax().
> +        cpu_relax();
> +    }
> +}
> +
> +/// Annotation for functions that can sleep.
> +///
> +/// Equivalent to the C side [`might_sleep()`], this function serves as
> +/// a debugging aid and a potential scheduling point.
> +///
> +/// This function can only be used in a nonatomic context.
> +#[track_caller]
> +fn might_sleep() {
> +    #[cfg(CONFIG_DEBUG_ATOMIC_SLEEP)]
> +    {
> +        let loc = core::panic::Location::caller();
> +        // SAFETY: FFI call.
> +        unsafe {
> +            crate::bindings::__might_sleep_precision(
> +                loc.file().as_ptr().cast(),
> +                loc.file().len() as i32,
> +                loc.line() as i32,
> +            )
> +        }
> +    }
> +
> +    // SAFETY: FFI call.
> +    unsafe { crate::bindings::might_resched() }
> +}
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index 496ed32b0911..415c500212dd 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -40,6 +40,7 @@
>  pub mod block;
>  #[doc(hidden)]
>  pub mod build_assert;
> +pub mod cpu;
>  pub mod cred;
>  pub mod device;
>  pub mod device_id;

  reply	other threads:[~2025-02-20 15:04 UTC|newest]

Thread overview: 64+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-02-20  7:06 [PATCH v11 0/8] rust: Add IO polling FUJITA Tomonori
2025-02-20  7:06 ` [PATCH v11 1/8] sched/core: Add __might_sleep_precision() FUJITA Tomonori
2025-02-24  1:40   ` FUJITA Tomonori
2025-02-20  7:06 ` [PATCH v11 2/8] rust: time: Add PartialEq/Eq/PartialOrd/Ord trait to Ktime FUJITA Tomonori
2025-03-22  8:51   ` Andreas Hindborg
2025-02-20  7:06 ` [PATCH v11 3/8] rust: time: Introduce Delta type FUJITA Tomonori
2025-03-22  8:50   ` Andreas Hindborg
2025-02-20  7:06 ` [PATCH v11 4/8] rust: time: Introduce Instant type FUJITA Tomonori
2025-03-22 13:58   ` Andreas Hindborg
2025-04-03  4:40     ` FUJITA Tomonori
2025-04-03 10:41       ` Andreas Hindborg
2025-04-03 12:23         ` FUJITA Tomonori
2025-02-20  7:06 ` [PATCH v11 5/8] rust: time: Add wrapper for fsleep() function FUJITA Tomonori
2025-03-21 22:05   ` Frederic Weisbecker
2025-03-22  1:24     ` FUJITA Tomonori
2025-03-22 14:15       ` Andreas Hindborg
2025-03-22 14:10   ` Andreas Hindborg
2025-02-20  7:06 ` [PATCH v11 6/8] MAINTAINERS: rust: Add new sections for DELAY/SLEEP and TIMEKEEPING API FUJITA Tomonori
2025-03-20 19:05   ` Boqun Feng
2025-03-21 19:18     ` Andreas Hindborg
2025-03-21 20:38       ` Thomas Gleixner
2025-03-21 21:00         ` Boqun Feng
2025-03-22  2:07           ` FUJITA Tomonori
2025-03-22 12:57             ` Boqun Feng
2025-03-22 22:40               ` Andreas Hindborg
2025-03-31 14:03                 ` Boqun Feng
2025-03-31 19:43                   ` Andreas Hindborg
2025-04-03  8:18                     ` FUJITA Tomonori
2025-04-03 10:54                       ` Andreas Hindborg
2025-04-03 12:57                         ` FUJITA Tomonori
2025-04-04 16:40                           ` Boqun Feng
2025-04-02 14:16                   ` FUJITA Tomonori
2025-04-02 16:29                     ` Boqun Feng
2025-04-02 23:03                       ` FUJITA Tomonori
2025-04-03  0:51                         ` Boqun Feng
2025-04-03  3:02                           ` FUJITA Tomonori
2025-04-03  3:17                             ` Boqun Feng
2025-02-20  7:06 ` [PATCH v11 7/8] rust: Add read_poll_timeout functions FUJITA Tomonori
2025-02-20 15:04   ` Fiona Behrens [this message]
2025-02-21 11:20     ` FUJITA Tomonori
2025-03-22 16:02   ` Andreas Hindborg
2025-08-11  1:53     ` FUJITA Tomonori
2025-08-11  9:42       ` Andreas Hindborg
2025-08-14  5:53         ` FUJITA Tomonori
2025-07-28 12:44   ` Daniel Almeida
2025-07-28 12:52     ` FUJITA Tomonori
2025-07-28 12:57       ` Danilo Krummrich
2025-07-28 13:08         ` FUJITA Tomonori
2025-07-28 13:15           ` Danilo Krummrich
2025-07-28 14:13           ` Daniel Almeida
2025-07-28 14:30             ` Danilo Krummrich
2025-07-28 13:13   ` Danilo Krummrich
2025-08-02  1:42     ` FUJITA Tomonori
2025-08-02 11:06       ` Danilo Krummrich
2025-08-05 13:37         ` FUJITA Tomonori
2025-08-05 13:48           ` Danilo Krummrich
2025-08-05 13:53           ` Andrew Lunn
2025-08-05 14:03             ` Danilo Krummrich
2025-08-05 14:01         ` Daniel Almeida
2025-08-05 14:19           ` Danilo Krummrich
2025-02-20  7:06 ` [PATCH v11 8/8] net: phy: qt2025: Wait until PHY becomes ready FUJITA Tomonori
2025-02-27 17:29 ` [PATCH v11 0/8] rust: Add IO polling Daniel Almeida
2025-02-27 23:05   ` FUJITA Tomonori
2025-03-20 19:04 ` Boqun Feng

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=878qq064j1.fsf@kloenk.dev \
    --to=me@kloenk$(echo .)dev \
    --cc=a.hindborg@samsung$(echo .)com \
    --cc=alex.gaynor@gmail$(echo .)com \
    --cc=aliceryhl@google$(echo .)com \
    --cc=andrew@lunn$(echo .)ch \
    --cc=anna-maria@linutronix$(echo .)de \
    --cc=arnd@arndb$(echo .)de \
    --cc=benno.lossin@proton$(echo .)me \
    --cc=bjorn3_gh@protonmail$(echo .)com \
    --cc=bsegall@google$(echo .)com \
    --cc=daniel.almeida@collabora$(echo .)com \
    --cc=david.laight.linux@gmail$(echo .)com \
    --cc=dietmar.eggemann@arm$(echo .)com \
    --cc=frederic@kernel$(echo .)org \
    --cc=fujita.tomonori@gmail$(echo .)com \
    --cc=gary@garyguo$(echo .)net \
    --cc=hkallweit1@gmail$(echo .)com \
    --cc=jstultz@google$(echo .)com \
    --cc=juri.lelli@redhat$(echo .)com \
    --cc=linux-kernel@vger$(echo .)kernel.org \
    --cc=mgorman@suse$(echo .)de \
    --cc=mingo@redhat$(echo .)com \
    --cc=netdev@vger$(echo .)kernel.org \
    --cc=ojeda@kernel$(echo .)org \
    --cc=peterz@infradead$(echo .)org \
    --cc=rostedt@goodmis$(echo .)org \
    --cc=rust-for-linux@vger$(echo .)kernel.org \
    --cc=sboyd@kernel$(echo .)org \
    --cc=tglx@linutronix$(echo .)de \
    --cc=tgunders@redhat$(echo .)com \
    --cc=tmgross@umich$(echo .)edu \
    --cc=vincent.guittot@linaro$(echo .)org \
    --cc=vschneid@redhat$(echo .)com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox