Re: [PATCH v6 6/7] rust: ww_mutex/exec: add high-level API

[Onur <work@onurozkan.dev>]

On Sat, 6 Sep 2025 12:04:34 -0300
Daniel Almeida <daniel.almeida@collabora.com> wrote:

> 
> 
> > On 6 Sep 2025, at 08:13, Onur <work@onurozkan.dev> wrote:
> > 
> > On Fri, 5 Sep 2025 16:42:09 -0300
> > Daniel Almeida <daniel.almeida@collabora.com> wrote:
> > 
> >> Hi Onur,
> >> 
> >>> On 3 Sep 2025, at 10:13, Onur Özkan <work@onurozkan.dev> wrote:
> >>> 
> >>> `ExecContext` is a helper built on top of ww_mutex
> >> 
> >> Again, I wonder what people think about this particular name.
> >> 
> >>> 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.
> >>> 
> >> 
> >> Great, this was exactly what I was looking for! :)
> >> 
> >>> 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();
> >> 
> >> If we move this to the acquire context, then we can do away with
> >> this drop impl.
> >> 
> >>> +    }
> >>> +}
> >>> +
> >>> +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.
> >> 
> >> core::ptr::eq()?
> >> 
> > 
> > I was thinking more of a type-level mechanism to do ensure that.
> 
> Why?
> 

So that wait-wound and wait-die classes don't get mixed up in the
same `ExecContext` by using type validation at compile time.

Of course, `core::ptr::eq()` is still useful/required when the classes
are of the same type but not exactly the same value. Maybe we can do
both.


Thanks,
Onur