From: Elle Rhumsaa <elle@weathered-steel.dev>
To: "Onur Özkan" <work@onurozkan.dev>
Cc: rust-for-linux@vger.kernel.org, linux-kernel@vger.kernel.org,
lossin@kernel.org, lyude@redhat.com, ojeda@kernel.org,
alex.gaynor@gmail.com, boqun.feng@gmail.com, gary@garyguo.net,
a.hindborg@kernel.org, aliceryhl@google.com, tmgross@umich.edu,
dakr@kernel.org, peterz@infradead.org, mingo@redhat.com,
will@kernel.org, longman@redhat.com, felipe_life@live.com,
daniel@sedlak.dev, bjorn3_gh@protonmail.com,
daniel.almeida@collabora.com
Subject: Re: [PATCH v6 6/7] rust: ww_mutex/exec: add high-level API
Date: Fri, 5 Sep 2025 23:11:01 +0000 [thread overview]
Message-ID: <aLtt6YaPAfGgfZdq@archiso> (raw)
In-Reply-To: <20250903131313.4365-7-work@onurozkan.dev>
On Wed, Sep 03, 2025 at 04:13:12PM +0300, Onur Özkan wrote:
> `ExecContext` is a helper built on top of ww_mutex
> that provides a retrying interface for lock acquisition.
> When `EDEADLK` is hit, it drops all held locks, resets
> the acquire context and retries the given (by the user)
> locking algorithm until it succeeds.
>
> The API keeps track of acquired locks, cleans them up
> automatically and allows data access to the protected
> data through `with_locked()`. The `lock_all()` helper
> allows implementing multi-mutex algorithms in a simpler
> and less error-prone way while keeping the ww_mutex
> semantics.
>
> Signed-off-by: Onur Özkan <work@onurozkan.dev>
> ---
> rust/kernel/sync/lock/ww_mutex.rs | 2 +
> rust/kernel/sync/lock/ww_mutex/exec.rs | 176 +++++++++++++++++++++++++
> 2 files changed, 178 insertions(+)
> create mode 100644 rust/kernel/sync/lock/ww_mutex/exec.rs
>
> diff --git a/rust/kernel/sync/lock/ww_mutex.rs b/rust/kernel/sync/lock/ww_mutex.rs
> index b415d6deae9b..7de6578513e5 100644
> --- a/rust/kernel/sync/lock/ww_mutex.rs
> +++ b/rust/kernel/sync/lock/ww_mutex.rs
> @@ -16,6 +16,8 @@
> use core::cell::UnsafeCell;
> use core::marker::PhantomData;
>
> +pub mod exec;
> +
> /// Create static [`WwClass`] instances.
> ///
> /// # Examples
> diff --git a/rust/kernel/sync/lock/ww_mutex/exec.rs b/rust/kernel/sync/lock/ww_mutex/exec.rs
> new file mode 100644
> index 000000000000..2f1fc540f0b8
> --- /dev/null
> +++ b/rust/kernel/sync/lock/ww_mutex/exec.rs
> @@ -0,0 +1,176 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! A high-level [`WwMutex`] execution helper.
> +//!
> +//! Provides a retrying lock mechanism on top of [`WwMutex`] and [`WwAcquireCtx`].
> +//! It detects [`EDEADLK`] and handles it by rolling back and retrying the
> +//! user-supplied locking algorithm until success.
> +
> +use crate::prelude::*;
> +use crate::sync::lock::ww_mutex::{WwAcquireCtx, WwClass, WwMutex, WwMutexGuard};
> +use core::ptr;
> +
> +/// High-level execution type for ww_mutex.
> +///
> +/// Tracks a series of locks acquired under a common [`WwAcquireCtx`].
> +/// It ensures proper cleanup and retry mechanism on deadlocks and provides
> +/// type-safe access to locked data via [`with_locked`].
> +///
> +/// Typical usage is through [`lock_all`], which retries a user-supplied
> +/// locking algorithm until it succeeds without deadlock.
> +pub struct ExecContext<'a> {
> + class: &'a WwClass,
> + acquire: Pin<KBox<WwAcquireCtx<'a>>>,
> + taken: KVec<WwMutexGuard<'a, ()>>,
> +}
> +
> +impl<'a> Drop for ExecContext<'a> {
> + fn drop(&mut self) {
> + self.release_all_locks();
> + }
> +}
> +
> +impl<'a> ExecContext<'a> {
> + /// Creates a new [`ExecContext`] for the given lock class.
> + ///
> + /// All locks taken through this context must belong to the same class.
> + ///
> + /// TODO: Add some safety mechanism to ensure classes are not different.
> + pub fn new(class: &'a WwClass) -> Result<Self> {
> + Ok(Self {
> + class,
> + acquire: KBox::pin_init(WwAcquireCtx::new(class), GFP_KERNEL)?,
> + taken: KVec::new(),
> + })
> + }
> +
> + /// Attempts to lock a [`WwMutex`] and records the guard.
> + ///
> + /// Returns [`EDEADLK`] if lock ordering would cause a deadlock.
> + pub fn lock<T>(&mut self, mutex: &'a WwMutex<'a, T>) -> Result<()> {
> + let guard = self.acquire.lock(mutex)?;
> + // SAFETY: Type is erased for storage. Actual access uses `with_locked`
> + // which safely casts back.
> + let erased: WwMutexGuard<'a, ()> = unsafe { core::mem::transmute(guard) };
> + self.taken.push(erased, GFP_KERNEL)?;
> +
> + Ok(())
> + }
> +
> + /// Runs `locking_algorithm` until success with retrying on deadlock.
> + ///
> + /// `locking_algorithm` should attempt to acquire all needed locks.
> + /// If [`EDEADLK`] is detected, this function will roll back, reset
> + /// the context and retry automatically.
> + ///
> + /// Once all locks are acquired successfully, `on_all_locks_taken` is
> + /// invoked for exclusive access to the locked values. Afterwards, all
> + /// locks are released.
> + ///
> + /// # Example
> + ///
> + /// ```
> + /// use kernel::alloc::KBox;
> + /// use kernel::c_str;
> + /// use kernel::prelude::*;
> + /// use kernel::sync::Arc;
> + /// use kernel::sync::lock::ww_mutex;
> + /// use pin_init::stack_pin_init;
> + ///
> + /// stack_pin_init!(let class = ww_mutex::WwClass::new_wound_wait(c_str!("lock_all_example")));
> + ///
> + /// let mutex1 = Arc::pin_init(ww_mutex::WwMutex::new(0, &class), GFP_KERNEL)?;
> + /// let mutex2 = Arc::pin_init(ww_mutex::WwMutex::new(0, &class), GFP_KERNEL)?;
> + /// let mut ctx = KBox::pin_init(ww_mutex::exec::ExecContext::new(&class)?, GFP_KERNEL)?;
> + ///
> + /// ctx.lock_all(
> + /// |ctx| {
> + /// // Try to lock both mutexes.
> + /// ctx.lock(&mutex1)?;
> + /// ctx.lock(&mutex2)?;
> + ///
> + /// Ok(())
> + /// },
> + /// |ctx| {
> + /// // Safely mutate both values while holding the locks.
> + /// ctx.with_locked(&mutex1, |v| *v += 1)?;
> + /// ctx.with_locked(&mutex2, |v| *v += 1)?;
> + ///
> + /// Ok(())
> + /// },
> + /// )?;
> + ///
> + /// # Ok::<(), Error>(())
> + /// ```
> + pub fn lock_all<T, Y, Z>(
> + &mut self,
> + mut locking_algorithm: T,
> + mut on_all_locks_taken: Y,
> + ) -> Result<Z>
> + where
> + T: FnMut(&mut ExecContext<'a>) -> Result<()>,
> + Y: FnMut(&mut ExecContext<'a>) -> Result<Z>,
> + {
> + loop {
> + match locking_algorithm(self) {
> + Ok(()) => {
> + // All locks in `locking_algorithm` succeeded.
> + // The user can now safely use them in `on_all_locks_taken`.
> + let res = on_all_locks_taken(self);
> + self.release_all_locks();
> +
> + return res;
> + }
> + Err(e) if e == EDEADLK => {
> + // Deadlock detected, retry from scratch.
> + self.cleanup_on_deadlock()?;
> + continue;
> + }
> + Err(e) => {
> + return Err(e);
> + }
> + }
> + }
> + }
> +
> + /// Executes `f` with a mutable reference to the data behind `mutex`.
> + ///
> + /// Fails with [`EINVAL`] if the mutex was not locked in this context.
> + pub fn with_locked<T, Y>(
> + &mut self,
> + mutex: &'a WwMutex<'a, T>,
> + f: impl FnOnce(&mut T) -> Y,
> + ) -> Result<Y> {
> + // Find the matching guard.
> + for guard in &mut self.taken {
> + if mutex.as_ptr() == guard.mutex.as_ptr() {
> + // SAFETY: We know this guard belongs to `mutex` and holds the lock.
> + let typed = unsafe { &mut *ptr::from_mut(guard).cast::<WwMutexGuard<'a, T>>() };
> + return Ok(f(&mut **typed));
> + }
> + }
> +
> + // `mutex` isn't locked in this `ExecContext`.
> + Err(EINVAL)
> + }
This can be refactored using a more idiomatic functional style:
diff --git a/rust/kernel/sync/lock/ww_mutex/exec.rs b/rust/kernel/sync/lock/ww_mutex/exec.rs
index 543c5218232a..8fdfb11104b9 100644
--- a/rust/kernel/sync/lock/ww_mutex/exec.rs
+++ b/rust/kernel/sync/lock/ww_mutex/exec.rs
@@ -142,16 +142,17 @@ pub fn with_locked<T, Y>(
f: impl FnOnce(&mut T) -> Y,
) -> Result<Y> {
// Find the matching guard.
- for guard in &mut self.taken {
- if mutex.as_ptr() == guard.mutex.as_ptr() {
+ self.taken
+ .iter_mut()
+ .find_map(|guard| if mutex.as_ptr() == guard.mutex.as_ptr() {
// SAFETY: We know this guard belongs to `mutex` and holds the lock.
- let typed = unsafe { &mut *ptr::from_mut(guard).cast::<WwMutexGuard<'a, T>>() };
- return Ok(f(&mut **typed));
- }
- }
-
- // `mutex` isn't locked in this `ExecContext`.
- Err(EINVAL)
+ Some(unsafe { &mut *ptr::from_mut(guard).cast::<WwMutexGuard<'a, T>>() })
+ } else {
+ None
+ })
+ .map(|typed| f(&mut **typed))
+ // `mutex` isn't locked in this `ExecContext`.
+ .ok_or(EINVAL)
}
/// Releases all currently held locks in this context.
> +
> + /// Releases all currently held locks in this context.
> + ///
> + /// It is intended to be used for internal implementation only.
> + fn release_all_locks(&mut self) {
> + self.taken.clear();
> + }
> +
> + /// Resets this context after a deadlock detection.
> + ///
> + /// Drops all held locks and reinitializes the [`WwAcquireCtx`].
> + ///
> + /// It is intended to be used for internal implementation only.
> + fn cleanup_on_deadlock(&mut self) -> Result {
> + self.release_all_locks();
> + // Re-init fresh `WwAcquireCtx`.
> + self.acquire = KBox::pin_init(WwAcquireCtx::new(self.class), GFP_KERNEL)?;
> +
> + Ok(())
> + }
> +}
> --
> 2.50.0
>
>
Reviewed-by: Elle Rhumsaa <elle@weathered-steel.dev>
next prev parent reply other threads:[~2025-09-05 23:11 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-09-03 13:13 [PATCH v6 0/7] rust: add `ww_mutex` support Onur Özkan
2025-09-03 13:13 ` [PATCH v6 1/7] rust: add C wrappers for ww_mutex inline functions Onur Özkan
2025-09-03 13:46 ` Daniel Almeida
2025-09-03 13:13 ` [PATCH v6 2/7] rust: implement `WwClass` for ww_mutex support Onur Özkan
2025-09-03 16:06 ` Boqun Feng
2025-09-04 8:23 ` Onur Özkan
2025-09-03 13:13 ` [PATCH v6 3/7] rust: implement `WwMutex`, `WwAcquireCtx` and `WwMutexGuard` Onur Özkan
2025-09-05 18:49 ` Daniel Almeida
2025-09-05 19:03 ` Daniel Almeida
2025-09-06 11:38 ` Onur
2025-10-22 10:47 ` Onur Özkan
2025-09-06 11:35 ` Onur
2025-09-03 13:13 ` [PATCH v6 4/7] add KUnit coverage on Rust ww_mutex implementation Onur Özkan
2025-09-05 19:04 ` Daniel Almeida
2025-09-03 13:13 ` [PATCH v6 5/7] rust: ww_mutex: add context-free locking functions Onur Özkan
2025-09-05 19:14 ` Daniel Almeida
2025-09-06 11:20 ` Onur
2025-10-21 13:31 ` Daniel Almeida
2025-10-21 13:20 ` Onur Özkan
2025-09-03 13:13 ` [PATCH v6 6/7] rust: ww_mutex/exec: add high-level API Onur Özkan
2025-09-05 19:42 ` Daniel Almeida
2025-09-06 11:13 ` Onur
2025-09-06 15:04 ` Daniel Almeida
2025-09-07 8:20 ` Onur
2025-09-07 8:38 ` Onur
2025-10-21 19:36 ` Onur Özkan
2025-10-21 13:24 ` Onur Özkan
2025-10-21 14:04 ` Onur Özkan
2025-09-05 23:11 ` Elle Rhumsaa [this message]
2025-09-06 11:47 ` Onur Özkan
2025-09-03 13:13 ` [PATCH v6 7/7] add KUnit coverage on ww_mutex/exec implementation Onur Özkan
2025-09-05 23:12 ` Elle Rhumsaa
2025-10-16 19:47 ` [PATCH v6 0/7] rust: add `ww_mutex` support Lyude Paul
2025-10-17 5:03 ` Onur Özkan
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=aLtt6YaPAfGgfZdq@archiso \
--to=elle@weathered-steel.dev \
--cc=a.hindborg@kernel.org \
--cc=alex.gaynor@gmail.com \
--cc=aliceryhl@google.com \
--cc=bjorn3_gh@protonmail.com \
--cc=boqun.feng@gmail.com \
--cc=dakr@kernel.org \
--cc=daniel.almeida@collabora.com \
--cc=daniel@sedlak.dev \
--cc=felipe_life@live.com \
--cc=gary@garyguo.net \
--cc=linux-kernel@vger.kernel.org \
--cc=longman@redhat.com \
--cc=lossin@kernel.org \
--cc=lyude@redhat.com \
--cc=mingo@redhat.com \
--cc=ojeda@kernel.org \
--cc=peterz@infradead.org \
--cc=rust-for-linux@vger.kernel.org \
--cc=tmgross@umich.edu \
--cc=will@kernel.org \
--cc=work@onurozkan.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.