Message-ID: <aFk_3F7Xus2VAVg3@google.com>
Date: Mon, 23 Jun 2025 11:51:56 +0000
From: Alice Ryhl <aliceryhl@...gle.com>
To: "Onur Özkan" <work@...rozkan.dev>
Cc: linux-kernel@...r.kernel.org, rust-for-linux@...r.kernel.org, 
	ojeda@...nel.org, alex.gaynor@...il.com, boqun.feng@...il.com, 
	gary@...yguo.net, lossin@...nel.org, a.hindborg@...nel.org, 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 v5 2/3] implement ww_mutex abstraction for the Rust tree

On Sat, Jun 21, 2025 at 09:44:53PM +0300, Onur Özkan wrote:
> Adds Rust bindings for the kernel's `ww_mutex` infrastructure to enable
> deadlock-free acquisition of multiple related locks.
> 
> The patch abstracts `ww_mutex.h` header and wraps the existing
> C `ww_mutex` with three main types:
>     - `WwClass` for grouping related mutexes
>     - `WwAcquireCtx` for tracking lock acquisition context
>     - `WwMutex<T>` for the actual lock
> 
> Signed-off-by: Onur Özkan <work@...rozkan.dev>
> ---
>  rust/kernel/error.rs              |   1 +
>  rust/kernel/sync/lock.rs          |   1 +
>  rust/kernel/sync/lock/ww_mutex.rs | 421 ++++++++++++++++++++++++++++++
>  3 files changed, 423 insertions(+)
>  create mode 100644 rust/kernel/sync/lock/ww_mutex.rs
> 
> diff --git a/rust/kernel/error.rs b/rust/kernel/error.rs
> index 3dee3139fcd4..28157541e12c 100644
> --- a/rust/kernel/error.rs
> +++ b/rust/kernel/error.rs
> @@ -64,6 +64,7 @@ macro_rules! declare_err {
>      declare_err!(EPIPE, "Broken pipe.");
>      declare_err!(EDOM, "Math argument out of domain of func.");
>      declare_err!(ERANGE, "Math result not representable.");
> +    declare_err!(EDEADLK, "Resource deadlock avoided.");
>      declare_err!(EOVERFLOW, "Value too large for defined data type.");
>      declare_err!(ERESTARTSYS, "Restart the system call.");
>      declare_err!(ERESTARTNOINTR, "System call was interrupted by a signal and will be restarted.");
> diff --git a/rust/kernel/sync/lock.rs b/rust/kernel/sync/lock.rs
> index e82fa5be289c..8824ebc81084 100644
> --- a/rust/kernel/sync/lock.rs
> +++ b/rust/kernel/sync/lock.rs
> @@ -15,6 +15,7 @@
>  
>  pub mod mutex;
>  pub mod spinlock;
> +pub mod ww_mutex;
>  
>  pub(super) mod global;
>  pub use global::{GlobalGuard, GlobalLock, GlobalLockBackend, GlobalLockedBy};
> diff --git a/rust/kernel/sync/lock/ww_mutex.rs b/rust/kernel/sync/lock/ww_mutex.rs
> new file mode 100644
> index 000000000000..dcb23941813c
> --- /dev/null
> +++ b/rust/kernel/sync/lock/ww_mutex.rs
> @@ -0,0 +1,421 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! A kernel Wound/Wait Mutex.
> +//!
> +//! This module provides Rust abstractions for the Linux kernel's `ww_mutex` implementation,
> +//! which provides deadlock avoidance through a wait-wound or wait-die algorithm.
> +//!
> +//! C header: [`include/linux/ww_mutex.h`](srctree/include/linux/ww_mutex.h)
> +//!
> +//! For more information: <https://docs.kernel.org/locking/ww-mutex-design.html>
> +
> +use crate::bindings;
> +use crate::error::to_result;
> +use crate::prelude::*;
> +use crate::types::{NotThreadSafe, Opaque};
> +use core::cell::UnsafeCell;
> +use core::marker::PhantomData;
> +
> +/// Create static [`WwClass`] instances.
> +///
> +/// # Examples
> +///
> +/// ```
> +/// use kernel::{c_str, define_ww_class};
> +///
> +/// define_ww_class!(WOUND_WAIT_GLOBAL_CLASS, wound_wait, c_str!("wound_wait_global_class"));
> +/// define_ww_class!(WAIT_DIE_GLOBAL_CLASS, wait_die, c_str!("wait_die_global_class"));
> +/// ```

should we split this into two macros define_ww_class! and
define_wd_class! to match the C macros?

> +#[macro_export]
> +macro_rules! define_ww_class {
> +    ($name:ident, wound_wait, $class_name:expr) => {
> +        static $name: $crate::sync::lock::ww_mutex::WwClass =
> +            { $crate::sync::lock::ww_mutex::WwClass::new($class_name, false) };
> +    };
> +    ($name:ident, wait_die, $class_name:expr) => {
> +        static $name: $crate::sync::lock::ww_mutex::WwClass =
> +            { $crate::sync::lock::ww_mutex::WwClass::new($class_name, true) };
> +    };
> +}
> +
> +/// A group of mutexes that can participate in deadlock avoidance together.
> +///
> +/// All mutexes that might be acquired together should use the same class.
> +///
> +/// # Examples
> +///
> +/// ```
> +/// use kernel::sync::lock::ww_mutex::WwClass;
> +/// use kernel::c_str;
> +/// use pin_init::stack_pin_init;
> +///
> +/// stack_pin_init!(let _wait_die_class = WwClass::new_wait_die(c_str!("graphics_buffers")));
> +/// stack_pin_init!(let _wound_wait_class = WwClass::new_wound_wait(c_str!("memory_pools")));
> +///
> +/// # Ok::<(), Error>(())
> +/// ```
> +#[pin_data]
> +pub struct WwClass {
> +    #[pin]
> +    inner: Opaque<bindings::ww_class>,
> +}
> +
> +// SAFETY: [`WwClass`] is set up once and never modified. It's fine to share it across threads.
> +unsafe impl Sync for WwClass {}
> +// SAFETY: Doesn't hold anything thread-specific. It's safe to send to other threads.
> +unsafe impl Send for WwClass {}
> +
> +macro_rules! ww_class_init_helper {
> +    ($name:expr, $is_wait_die:expr) => {
> +        Opaque::new(bindings::ww_class {
> +            stamp: bindings::atomic_long_t { counter: 0 },
> +            acquire_name: $name.as_char_ptr(),
> +            mutex_name: $name.as_char_ptr(),
> +            is_wait_die: $is_wait_die as u32,
> +            // TODO: Replace with `bindings::lock_class_key::default()` once stabilized for `const`.
> +            //
> +            // SAFETY: This is always zero-initialized when defined with `DEFINE_WD_CLASS`
> +            // globally on C side.
> +            //
> +            // Ref: <https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/ww_mutex.h?h=v6.16-rc2#n85>
> +            acquire_key: unsafe { core::mem::zeroed() },
> +            // TODO: Replace with `bindings::lock_class_key::default()` once stabilized for `const`.
> +            //
> +            // SAFETY: This is always zero-initialized when defined with `DEFINE_WD_CLASS`
> +            // globally on C side.
> +            //
> +            // Ref: <https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/ww_mutex.h?h=v6.16-rc2#n85>
> +            mutex_key: unsafe { core::mem::zeroed() },
> +        })
> +    };
> +}

You don't need this macro. You can have `new_wait_die` or
`new_wound_wait` call `new` directly and just move the macro into `new`.

> +impl WwClass {
> +    /// Creates a [`WwClass`].
> +    ///
> +    /// It's `pub` only so it can be used by the `define_ww_class!` macro.
> +    ///
> +    /// You should not use this function directly. Use the `define_ww_class!`
> +    /// macro or call [`WwClass::new_wait_die`] or [`WwClass::new_wound_wait`] instead.
> +    pub const fn new(name: &'static CStr, is_wait_die: bool) -> Self {
> +        WwClass {
> +            inner: ww_class_init_helper!(name, is_wait_die),
> +        }
> +    }
> +
> +    /// Creates wait-die [`WwClass`].
> +    pub fn new_wait_die(name: &'static CStr) -> impl PinInit<Self> {
> +        pin_init!(WwClass {
> +            inner: ww_class_init_helper!(name, true),
> +        })
> +    }
> +
> +    /// Creates wound-wait [`WwClass`].
> +    pub fn new_wound_wait(name: &'static CStr) -> impl PinInit<Self> {
> +        pin_init!(WwClass {
> +            inner: ww_class_init_helper!(name, false),
> +        })
> +    }
> +}
> +
> +/// An acquire context is used to group multiple mutex acquisitions together
> +/// for deadlock avoidance. It must be used when acquiring multiple mutexes
> +/// of the same class.
> +///
> +/// # Examples
> +///
> +/// ```
> +/// use kernel::sync::lock::ww_mutex::{WwClass, WwAcquireCtx, WwMutex};
> +/// use kernel::c_str;
> +/// use kernel::alloc::KBox;
> +/// use pin_init::stack_pin_init;
> +///
> +/// stack_pin_init!(let class = WwClass::new_wound_wait(c_str!("my_class")));
> +///
> +/// // Create mutexes.
> +/// stack_pin_init!(let mutex1 = WwMutex::new(1, &class));
> +/// stack_pin_init!(let mutex2 = WwMutex::new(2, &class));
> +///
> +/// // Create acquire context for deadlock avoidance.
> +/// let mut ctx = KBox::pin_init(WwAcquireCtx::new(&class), GFP_KERNEL)?;
> +///
> +/// // Acquire multiple locks safely.
> +/// let guard1 = mutex1.lock(Some(&ctx))?;
> +/// let guard2 = mutex2.lock(Some(&ctx))?;
> +///
> +/// // Mark acquisition phase as complete.
> +/// ctx.as_mut().done();
> +///
> +/// # Ok::<(), Error>(())
> +/// ```
> +#[pin_data(PinnedDrop)]
> +pub struct WwAcquireCtx<'a> {
> +    #[pin]
> +    inner: Opaque<bindings::ww_acquire_ctx>,
> +    _p: PhantomData<&'a WwClass>,
> +}
> +
> +// SAFETY: Used in controlled ways during lock acquisition. No race risk.
> +unsafe impl Sync for WwAcquireCtx<'_> {}
> +// SAFETY: Doesn't rely on thread-local state. Safe to move between threads.
> +unsafe impl Send for WwAcquireCtx<'_> {}
> +
> +impl<'ctx> WwAcquireCtx<'ctx> {
> +    /// Initializes `Self` with calling C side `ww_acquire_init` inside.
> +    pub fn new<'class: 'ctx>(ww_class: &'class WwClass) -> impl PinInit<Self> {

I would rename 'ctx to 'class and get rid of this super-lifetime.

impl<'class> WwAcquireCtx<'class> {
    /// Initializes `Self` with calling C side `ww_acquire_init` inside.
    pub fn new(ww_class: &'class WwClass) -> impl PinInit<Self> {

> +        let raw_ptr = ww_class.inner.get();
> +        pin_init!(WwAcquireCtx {
> +            inner <- Opaque::ffi_init(|slot: *mut bindings::ww_acquire_ctx| {
> +                // SAFETY: The caller guarantees that `ww_class` remains valid.
> +                unsafe { bindings::ww_acquire_init(slot, raw_ptr) }
> +            }),
> +            _p: PhantomData
> +        })
> +    }
> +
> +    /// Marks the end of the acquire phase with C side `ww_acquire_done`.
> +    ///
> +    /// After calling this function, no more mutexes can be acquired with this context.
> +    pub fn done(self: Pin<&mut Self>) {
> +        // SAFETY: The context is pinned and valid.
> +        unsafe { bindings::ww_acquire_done(self.inner.get()) };
> +    }
> +
> +    /// Returns a raw pointer to the inner `ww_acquire_ctx`.
> +    fn as_ptr(&self) -> *mut bindings::ww_acquire_ctx {
> +        self.inner.get()
> +    }
> +}
> +
> +#[pinned_drop]
> +impl PinnedDrop for WwAcquireCtx<'_> {
> +    fn drop(self: Pin<&mut Self>) {
> +        // SAFETY: The context is being dropped and is pinned.
> +        unsafe { bindings::ww_acquire_fini(self.inner.get()) };
> +    }
> +}
> +
> +/// A wound/wait mutex backed with C side `ww_mutex`.
> +///
> +/// This is a mutual exclusion primitive that provides deadlock avoidance when
> +/// acquiring multiple locks of the same class.
> +///
> +/// # Examples
> +///
> +/// ## Basic Usage
> +///
> +/// ```
> +/// use kernel::sync::lock::ww_mutex::{WwClass, WwMutex};
> +/// use kernel::c_str;
> +/// use pin_init::stack_pin_init;
> +///
> +/// stack_pin_init!(let class = WwClass::new_wound_wait(c_str!("buffer_class")));
> +/// stack_pin_init!(let mutex = WwMutex::new(42, &class));
> +///
> +/// // Simple lock without context.
> +/// let guard = mutex.lock(None)?;
> +/// assert_eq!(*guard, 42);
> +///
> +/// # Ok::<(), Error>(())
> +/// ```
> +///
> +/// ## Multiple Locks
> +///
> +/// ```
> +/// use kernel::c_str;
> +/// use kernel::prelude::*;
> +/// use kernel::sync::lock::ww_mutex::{WwClass, WwAcquireCtx, WwMutex};
> +/// use pin_init::stack_pin_init;
> +///
> +/// stack_pin_init!(let class = WwClass::new_wait_die(c_str!("resource_class")));
> +/// stack_pin_init!(let mutex_a = WwMutex::new("Resource A", &class));
> +/// stack_pin_init!(let mutex_b = WwMutex::new("Resource B", &class));
> +///
> +/// let mut ctx = KBox::pin_init(WwAcquireCtx::new(&class), GFP_KERNEL)?;
> +///
> +/// // Try to acquire both locks.
> +/// let guard_a = match mutex_a.lock(Some(&ctx)) {
> +///     Ok(guard) => guard,
> +///     Err(e) if e == EDEADLK => {
> +///         // Deadlock detected, use slow path.
> +///         mutex_a.lock_slow(&ctx)?
> +///     }
> +///     Err(e) => return Err(e),
> +/// };
> +///
> +/// let guard_b = mutex_b.lock(Some(&ctx))?;
> +/// ctx.as_mut().done();
> +///
> +/// # Ok::<(), Error>(())
> +/// ```
> +#[pin_data]
> +pub struct WwMutex<'a, T: ?Sized> {
> +    _p: PhantomData<&'a WwClass>,
> +    #[pin]
> +    mutex: Opaque<bindings::ww_mutex>,
> +    data: UnsafeCell<T>,
> +}
> +
> +// SAFETY: [`WwMutex`] can be shared between threads.
> +unsafe impl<T: ?Sized + Send> Send for WwMutex<'_, T> {}
> +// SAFETY: [`WwMutex`] can be safely accessed from multiple threads concurrently.
> +unsafe impl<T: ?Sized + Sync> Sync for WwMutex<'_, T> {}
> +
> +impl<'ww_class, T> WwMutex<'ww_class, T> {
> +    /// Creates `Self` with calling `ww_mutex_init` inside.
> +    pub fn new(t: T, ww_class: &'ww_class WwClass) -> impl PinInit<Self> {
> +        let raw_ptr = ww_class.inner.get();
> +        pin_init!(WwMutex {
> +            mutex <- Opaque::ffi_init(|slot: *mut bindings::ww_mutex| {
> +                // SAFETY: The caller guarantees that `ww_class` remains valid.
> +                unsafe { bindings::ww_mutex_init(slot, raw_ptr) }
> +            }),
> +            data: UnsafeCell::new(t),
> +            _p: PhantomData,
> +        })
> +    }
> +}
> +
> +impl<T: ?Sized> WwMutex<'_, T> {
> +    /// Locks the mutex with the given acquire context.
> +    pub fn lock<'a>(&'a self, ctx: Option<&WwAcquireCtx<'_>>) -> Result<WwMutexGuard<'a, T>> {
> +        // SAFETY: The mutex is pinned and valid.
> +        let ret = unsafe {
> +            bindings::ww_mutex_lock(
> +                self.mutex.get(),
> +                ctx.map_or(core::ptr::null_mut(), |c| c.as_ptr()),
> +            )
> +        };
> +
> +        to_result(ret)?;
> +
> +        Ok(WwMutexGuard::new(self))
> +    }
> +
> +    /// Locks the mutex with the given acquire context, interruptible.
> +    ///
> +    /// Similar to `lock`, but can be interrupted by signals.
> +    pub fn lock_interruptible<'a>(
> +        &'a self,
> +        ctx: Option<&WwAcquireCtx<'_>>,
> +    ) -> Result<WwMutexGuard<'a, T>> {
> +        // SAFETY: The mutex is pinned and valid.
> +        let ret = unsafe {
> +            bindings::ww_mutex_lock_interruptible(
> +                self.mutex.get(),
> +                ctx.map_or(core::ptr::null_mut(), |c| c.as_ptr()),
> +            )
> +        };
> +
> +        to_result(ret)?;
> +
> +        Ok(WwMutexGuard::new(self))
> +    }
> +
> +    /// Locks the mutex in the slow path after a die case.
> +    ///
> +    /// This should be called after releasing all held mutexes when `lock` returns [`EDEADLK`].
> +    pub fn lock_slow<'a>(&'a self, ctx: &WwAcquireCtx<'_>) -> Result<WwMutexGuard<'a, T>> {
> +        // SAFETY: The mutex is pinned and valid, and we're in the slow path.
> +        unsafe { bindings::ww_mutex_lock_slow(self.mutex.get(), ctx.as_ptr()) };
> +
> +        Ok(WwMutexGuard::new(self))
> +    }
> +
> +    /// Locks the mutex in the slow path after a die case, interruptible.
> +    pub fn lock_slow_interruptible<'a>(
> +        &'a self,
> +        ctx: &WwAcquireCtx<'_>,
> +    ) -> Result<WwMutexGuard<'a, T>> {
> +        // SAFETY: The mutex is pinned and valid, and we are in the slow path.
> +        let ret =
> +            unsafe { bindings::ww_mutex_lock_slow_interruptible(self.mutex.get(), ctx.as_ptr()) };
> +
> +        to_result(ret)?;
> +
> +        Ok(WwMutexGuard::new(self))
> +    }
> +
> +    /// Tries to lock the mutex without blocking.
> +    pub fn try_lock<'a>(&'a self, ctx: Option<&WwAcquireCtx<'_>>) -> Result<WwMutexGuard<'a, T>> {
> +        // SAFETY: The mutex is pinned and valid.
> +        let ret = unsafe {
> +            bindings::ww_mutex_trylock(
> +                self.mutex.get(),
> +                ctx.map_or(core::ptr::null_mut(), |c| c.as_ptr()),
> +            )
> +        };
> +
> +        if ret == 0 {
> +            return Err(EBUSY);
> +        }
> +
> +        to_result(if ret < 0 { ret } else { 0 })?;
> +
> +        Ok(WwMutexGuard::new(self))
> +    }
> +
> +    /// Checks if the mutex is currently locked.
> +    pub fn is_locked(&self) -> bool {
> +        // SAFETY: The mutex is pinned and valid.
> +        unsafe { bindings::ww_mutex_is_locked(self.mutex.get()) }
> +    }
> +
> +    /// Returns a raw pointer to the inner mutex.
> +    fn as_ptr(&self) -> *mut bindings::ww_mutex {
> +        self.mutex.get()
> +    }
> +}
> +
> +/// A guard that provides exclusive access to the data protected
> +/// by a [`WwMutex`].
> +///
> +/// # Invariants
> +///
> +/// The guard holds an exclusive lock on the associated [`WwMutex`]. The lock is held
> +/// for the entire lifetime of this guard and is automatically released when the
> +/// guard is dropped.
> +#[must_use = "the lock unlocks immediately when the guard is unused"]
> +pub struct WwMutexGuard<'a, T: ?Sized> {
> +    mutex: &'a WwMutex<'a, T>,
> +    _not_send: NotThreadSafe,
> +}
> +
> +// SAFETY: [`WwMutexGuard`] can be transferred across thread boundaries if the data can.
> +unsafe impl<T: ?Sized + Send> Send for WwMutexGuard<'_, T> {}
> +
> +// SAFETY: [`WwMutexGuard`] can be shared between threads if the data can.
> +unsafe impl<T: ?Sized + Send + Sync> Sync for WwMutexGuard<'_, T> {}
> +
> +impl<'a, T: ?Sized> WwMutexGuard<'a, T> {
> +    /// Creates a new guard for a locked mutex.
> +    fn new(mutex: &'a WwMutex<'a, T>) -> Self {
> +        Self {
> +            mutex,
> +            _not_send: NotThreadSafe,
> +        }
> +    }
> +}
> +
> +impl<T: ?Sized> core::ops::Deref for WwMutexGuard<'_, T> {
> +    type Target = T;
> +
> +    fn deref(&self) -> &Self::Target {
> +        // SAFETY: We hold the lock, so we have exclusive access.
> +        unsafe { &*self.mutex.data.get() }
> +    }
> +}
> +
> +impl<T: ?Sized> core::ops::DerefMut for WwMutexGuard<'_, T> {
> +    fn deref_mut(&mut self) -> &mut Self::Target {
> +        // SAFETY: We hold the lock, so we have exclusive access.
> +        unsafe { &mut *self.mutex.data.get() }
> +    }
> +}
> +
> +impl<T: ?Sized> Drop for WwMutexGuard<'_, T> {
> +    fn drop(&mut self) {
> +        // SAFETY: We hold the lock and are about to release it.
> +        unsafe { bindings::ww_mutex_unlock(self.mutex.as_ptr()) };
> +    }
> +}
> -- 
> 2.49.0
> 

Powered by blists - more mailing lists