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

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
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20250907113817.768acf3d@nimda.home>
Date: Sun, 7 Sep 2025 11:38:17 +0300
From: Onur <work@...rozkan.dev>
To: Daniel Almeida <daniel.almeida@...labora.com>
Cc: rust-for-linux@...r.kernel.org, linux-kernel@...r.kernel.org,
 lossin@...nel.org, lyude@...hat.com, ojeda@...nel.org,
 alex.gaynor@...il.com, boqun.feng@...il.com, gary@...yguo.net,
 a.hindborg@...nel.org, aliceryhl@...gle.com, tmgross@...ch.edu,
 dakr@...nel.org, peterz@...radead.org, mingo@...hat.com, will@...nel.org,
 longman@...hat.com, felipe_life@...e.com, daniel@...lak.dev,
 bjorn3_gh@...tonmail.com
Subject: Re: [PATCH v6 6/7] rust: ww_mutex/exec: add high-level API

On Sun, 7 Sep 2025 11:20:06 +0300
Onur <work@...rozkan.dev> wrote:

> On Sat, 6 Sep 2025 12:04:34 -0300
> Daniel Almeida <daniel.almeida@...labora.com> wrote:
> 
> > 
> > 
> > > On 6 Sep 2025, at 08:13, Onur <work@...rozkan.dev> wrote:
> > > 
> > > On Fri, 5 Sep 2025 16:42:09 -0300
> > > Daniel Almeida <daniel.almeida@...labora.com> wrote:
> > > 
> > >> Hi Onur,
> > >> 
> > >>> On 3 Sep 2025, at 10:13, Onur Özkan <work@...rozkan.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@...rozkan.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

I will also look into whether it's possible to remove the class from the
mutex and instead derive it from ExecContext and WwAcquireCtx. This
would fix both issues at once in a better way.

 -Onur