| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <878qq064j1.fsf@kloenk.dev>
Date: Thu, 20 Feb 2025 16:04:50 +0100
From: Fiona Behrens <me@...enk.dev>
To: FUJITA Tomonori <fujita.tomonori@...il.com>
Cc: linux-kernel@...r.kernel.org, Daniel Almeida
<daniel.almeida@...labora.com>, rust-for-linux@...r.kernel.org,
netdev@...r.kernel.org, andrew@...n.ch, hkallweit1@...il.com,
tmgross@...ch.edu, ojeda@...nel.org, alex.gaynor@...il.com,
gary@...yguo.net, bjorn3_gh@...tonmail.com, benno.lossin@...ton.me,
a.hindborg@...sung.com, aliceryhl@...gle.com, anna-maria@...utronix.de,
frederic@...nel.org, tglx@...utronix.de, arnd@...db.de,
jstultz@...gle.com, sboyd@...nel.org, mingo@...hat.com,
peterz@...radead.org, juri.lelli@...hat.com,
vincent.guittot@...aro.org, dietmar.eggemann@....com,
rostedt@...dmis.org, bsegall@...gle.com, mgorman@...e.de,
vschneid@...hat.com, tgunders@...hat.com, david.laight.linux@...il.com
Subject: Re: [PATCH v11 7/8] rust: Add read_poll_timeout functions
FUJITA Tomonori <fujita.tomonori@...il.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@...labora.com>
> Signed-off-by: FUJITA Tomonori <fujita.tomonori@...il.com>
Reviewed-by: Fiona Behrens <me@...enk.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;
Powered by blists - more mailing lists