mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 1/3 v4] rust: add C wrappers for `ww_mutex` inline functions
@ 2025-06-21 15:21 Onur Özkan
  2025-06-21 15:21 ` [PATCH 2/3 v4] implement ww_mutex abstraction for the Rust tree Onur Özkan
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Onur Özkan @ 2025-06-21 15:21 UTC (permalink / raw)
  To: linux-kernel, rust-for-linux
  Cc: ojeda, alex.gaynor, boqun.feng, gary, lossin, a.hindborg,
	aliceryhl, tmgross, dakr, peterz, mingo, will, longman,
	felipe_life, daniel, bjorn3_gh, Onur Özkan

Some of the kernel's `ww_mutex` functions are implemented as
`static inline`, so they are inaccessible from Rust as bindgen
can't generate code on them. This patch provides C function wrappers
around these inline implementations, so bindgen can see them and generate
the corresponding Rust code.

Signed-off-by: Onur Özkan <work@onurozkan.dev>
---
 rust/helpers/helpers.c  |  1 +
 rust/helpers/ww_mutex.c | 39 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 40 insertions(+)
 create mode 100644 rust/helpers/ww_mutex.c

diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c
index 0f1b5d115985..fd071cbe20a1 100644
--- a/rust/helpers/helpers.c
+++ b/rust/helpers/helpers.c
@@ -44,3 +44,4 @@
 #include "wait.c"
 #include "workqueue.c"
 #include "xarray.c"
+#include "ww_mutex.c"
diff --git a/rust/helpers/ww_mutex.c b/rust/helpers/ww_mutex.c
new file mode 100644
index 000000000000..61a487653394
--- /dev/null
+++ b/rust/helpers/ww_mutex.c
@@ -0,0 +1,39 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/ww_mutex.h>
+
+void rust_helper_ww_mutex_init(struct ww_mutex *lock, struct ww_class *ww_class)
+{
+	ww_mutex_init(lock, ww_class);
+}
+
+void rust_helper_ww_acquire_init(struct ww_acquire_ctx *ctx, struct ww_class *ww_class)
+{
+	ww_acquire_init(ctx, ww_class);
+}
+
+void rust_helper_ww_acquire_done(struct ww_acquire_ctx *ctx)
+{
+	ww_acquire_done(ctx);
+}
+
+void rust_helper_ww_acquire_fini(struct ww_acquire_ctx *ctx)
+{
+	ww_acquire_fini(ctx);
+}
+
+void rust_helper_ww_mutex_lock_slow(struct ww_mutex *lock, struct ww_acquire_ctx *ctx)
+{
+	ww_mutex_lock_slow(lock, ctx);
+}
+
+int rust_helper_ww_mutex_lock_slow_interruptible(struct ww_mutex *lock, struct ww_acquire_ctx *ctx)
+{
+	return ww_mutex_lock_slow_interruptible(lock, ctx);
+}
+
+bool rust_helper_ww_mutex_is_locked(struct ww_mutex *lock)
+{
+	return ww_mutex_is_locked(lock);
+}
+
-- 
2.49.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 2/3 v4] implement ww_mutex abstraction for the Rust tree
  2025-06-21 15:21 [PATCH 1/3 v4] rust: add C wrappers for `ww_mutex` inline functions Onur Özkan
@ 2025-06-21 15:21 ` Onur Özkan
  2025-06-21 16:15   ` Miguel Ojeda
  2025-06-21 15:21 ` [PATCH 3/3 v4] add KUnit coverage on Rust `ww_mutex` implementation Onur Özkan
  2025-06-21 16:07 ` [PATCH 1/3 v4] rust: add C wrappers for `ww_mutex` inline functions Miguel Ojeda
  2 siblings, 1 reply; 6+ messages in thread
From: Onur Özkan @ 2025-06-21 15:21 UTC (permalink / raw)
  To: linux-kernel, rust-for-linux
  Cc: ojeda, alex.gaynor, boqun.feng, gary, lossin, a.hindborg,
	aliceryhl, tmgross, dakr, peterz, mingo, will, longman,
	felipe_life, daniel, bjorn3_gh, Onur Özkan, thatslyude

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

Link: https://rust-for-linux.zulipchat.com/#narrow/channel/291566-Library/topic/Writing.20up.20wrappers.20for.20ww_mutex.3F/with/524269974
Suggested-by: thatslyude@gmail.com
Signed-off-by: Onur Özkan <work@onurozkan.dev>
---
 rust/kernel/error.rs              |   1 +
 rust/kernel/sync/lock.rs          |   1 +
 rust/kernel/sync/lock/ww_mutex.rs | 419 ++++++++++++++++++++++++++++++
 3 files changed, 421 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..94d8014b236b 100644
--- a/rust/kernel/error.rs
+++ b/rust/kernel/error.rs
@@ -84,6 +84,7 @@ macro_rules! declare_err {
     declare_err!(EIOCBQUEUED, "iocb queued, will get completion event.");
     declare_err!(ERECALLCONFLICT, "Conflict with recalled state.");
     declare_err!(ENOGRACE, "NFS file lock reclaim refused.");
+    declare_err!(EDEADLK, "Resource deadlock avoided.");
 }
 
 /// Generic integer kernel error.
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..0f77e03304e4
--- /dev/null
+++ b/rust/kernel/sync/lock/ww_mutex.rs
@@ -0,0 +1,419 @@
+// 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.
+
+use crate::error::{to_result, Result};
+use crate::prelude::EBUSY;
+use crate::types::{NotThreadSafe, Opaque};
+use crate::{bindings, str::CStr};
+use core::marker::PhantomData;
+use core::{cell::UnsafeCell, pin::Pin};
+use pin_init::{pin_data, pin_init, pinned_drop, PinInit};
+
+/// Create static `WwClass` instances.
+///
+/// # Examples
+///
+/// ```
+/// use kernel::c_str;
+/// use kernel::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"));
+/// ```
+#[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) };
+    };
+}
+
+/// Represents 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://github.com/torvalds/linux/blob/master/include/linux/ww_mutex.h#L85-L89
+            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://github.com/torvalds/linux/blob/master/include/linux/ww_mutex.h#L85-L89
+            mutex_key: unsafe { core::mem::zeroed() },
+        })
+    };
+}
+
+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 pin_init::stack_pin_init;
+/// use kernel::alloc::KBox;
+///
+/// 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).unwrap();
+///
+/// // Acquire multiple locks safely
+/// let guard1 = mutex1.lock(Some(&ctx)).unwrap();
+/// let guard2 = mutex2.lock(Some(&ctx)).unwrap();
+///
+/// // 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> {
+        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).unwrap();
+/// assert_eq!(*guard, 42);
+///
+/// # Ok::<(), Error>(())
+/// ```
+///
+/// ## Multiple Locks
+///
+/// ```
+/// use kernel::sync::lock::ww_mutex::{WwClass, WwAcquireCtx, WwMutex};
+/// use kernel::alloc::KBox;
+/// use kernel::c_str;
+/// use kernel::error::code::EDEADLK;
+/// 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).unwrap();
+///
+/// // 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).unwrap()
+///     }
+///     Err(e) => return Err(e),
+/// };
+///
+/// let guard_b = mutex_b.lock(Some(&ctx)).unwrap();
+/// 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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 3/3 v4] add KUnit coverage on Rust `ww_mutex` implementation
  2025-06-21 15:21 [PATCH 1/3 v4] rust: add C wrappers for `ww_mutex` inline functions Onur Özkan
  2025-06-21 15:21 ` [PATCH 2/3 v4] implement ww_mutex abstraction for the Rust tree Onur Özkan
@ 2025-06-21 15:21 ` Onur Özkan
  2025-06-21 16:15   ` Miguel Ojeda
  2025-06-21 16:07 ` [PATCH 1/3 v4] rust: add C wrappers for `ww_mutex` inline functions Miguel Ojeda
  2 siblings, 1 reply; 6+ messages in thread
From: Onur Özkan @ 2025-06-21 15:21 UTC (permalink / raw)
  To: linux-kernel, rust-for-linux
  Cc: ojeda, alex.gaynor, boqun.feng, gary, lossin, a.hindborg,
	aliceryhl, tmgross, dakr, peterz, mingo, will, longman,
	felipe_life, daniel, bjorn3_gh, Onur Özkan

Adds coverage around the core `ww_mutex` functionality

Signed-off-by: Onur Özkan <work@onurozkan.dev>
---
 rust/kernel/sync/lock/ww_mutex.rs | 116 ++++++++++++++++++++++++++++++
 1 file changed, 116 insertions(+)

diff --git a/rust/kernel/sync/lock/ww_mutex.rs b/rust/kernel/sync/lock/ww_mutex.rs
index 0f77e03304e4..30c6068f315d 100644
--- a/rust/kernel/sync/lock/ww_mutex.rs
+++ b/rust/kernel/sync/lock/ww_mutex.rs
@@ -11,6 +11,7 @@
 use crate::{bindings, str::CStr};
 use core::marker::PhantomData;
 use core::{cell::UnsafeCell, pin::Pin};
+use macros::kunit_tests;
 use pin_init::{pin_data, pin_init, pinned_drop, PinInit};
 
 /// Create static `WwClass` instances.
@@ -417,3 +418,118 @@ fn drop(&mut self) {
         unsafe { bindings::ww_mutex_unlock(self.mutex.as_ptr()) };
     }
 }
+
+#[kunit_tests(rust_kernel_ww_mutex)]
+mod tests {
+    use crate::alloc::KBox;
+    use crate::c_str;
+    use crate::prelude::*;
+    use pin_init::stack_pin_init;
+
+    use super::*;
+
+    // A simple coverage on `define_ww_class` macro.
+    define_ww_class!(TEST_WOUND_WAIT_CLASS, wound_wait, c_str!("test_wound_wait"));
+    define_ww_class!(TEST_WAIT_DIE_CLASS, wait_die, c_str!("test_wait_die"));
+
+    #[test]
+    fn test_ww_mutex_basic_lock_unlock() {
+        stack_pin_init!(let class = WwClass::new_wound_wait(c_str!("test_mutex_class")));
+
+        stack_pin_init!(let mutex = WwMutex::new(42, &class));
+
+        // Lock without context
+        let guard = mutex.lock(None).unwrap();
+        assert_eq!(*guard, 42);
+
+        // Drop the lock
+        drop(guard);
+
+        // Lock it again
+        let mut guard = mutex.lock(None).unwrap();
+        *guard = 100;
+        assert_eq!(*guard, 100);
+    }
+
+    #[test]
+    fn test_ww_mutex_trylock() {
+        stack_pin_init!(let class = WwClass::new_wound_wait(c_str!("trylock_class")));
+
+        stack_pin_init!(let mutex = WwMutex::new(123, &class));
+
+        // trylock on unlocked mutex should succeed
+        let guard = mutex.try_lock(None).unwrap();
+        assert_eq!(*guard, 123);
+        drop(guard);
+
+        // lock it first
+        let _guard1 = mutex.lock(None).unwrap();
+
+        // trylock should fail with EBUSY when already locked
+        let result = mutex.try_lock(None);
+        match result {
+            Err(e) => assert_eq!(e, EBUSY),
+            Ok(_) => panic!("Expected `EBUSY` but got success"),
+        }
+    }
+
+    #[test]
+    fn test_ww_mutex_is_locked() {
+        stack_pin_init!(let class = WwClass::new_wait_die(c_str!("locked_check_class")));
+
+        stack_pin_init!(let mutex = WwMutex::new("hello", &class));
+
+        // should not be locked initially
+        assert!(!mutex.is_locked());
+
+        let guard = mutex.lock(None).unwrap();
+        assert!(mutex.is_locked());
+
+        drop(guard);
+        assert!(!mutex.is_locked());
+    }
+
+    #[test]
+    fn test_ww_acquire_context() {
+        stack_pin_init!(let class = WwClass::new_wound_wait(c_str!("ctx_class")));
+
+        stack_pin_init!(let mutex1 = WwMutex::new(1, &class));
+        stack_pin_init!(let mutex2 = WwMutex::new(2, &class));
+
+        let mut ctx = KBox::pin_init(WwAcquireCtx::new(&class), GFP_KERNEL).unwrap();
+
+        // acquire multiple mutexes with same context
+        let guard1 = mutex1.lock(Some(&ctx)).unwrap();
+        let guard2 = mutex2.lock(Some(&ctx)).unwrap();
+
+        assert_eq!(*guard1, 1);
+        assert_eq!(*guard2, 2);
+
+        ctx.as_mut().done();
+
+        // we shouldn't be able to lock once it's `done`.
+        assert!(mutex1.lock(Some(&ctx)).is_err());
+        assert!(mutex2.lock(Some(&ctx)).is_err());
+    }
+
+    #[test]
+    fn test_with_global_classes() {
+        stack_pin_init!(let wound_wait_mutex = WwMutex::new(100, &TEST_WOUND_WAIT_CLASS));
+        stack_pin_init!(let wait_die_mutex = WwMutex::new(200, &TEST_WAIT_DIE_CLASS));
+
+        let ww_guard = wound_wait_mutex.lock(None).unwrap();
+        let wd_guard = wait_die_mutex.lock(None).unwrap();
+
+        assert_eq!(*ww_guard, 100);
+        assert_eq!(*wd_guard, 200);
+
+        assert!(wound_wait_mutex.is_locked());
+        assert!(wait_die_mutex.is_locked());
+
+        drop(ww_guard);
+        drop(wd_guard);
+
+        assert!(!wound_wait_mutex.is_locked());
+        assert!(!wait_die_mutex.is_locked());
+    }
+}
-- 
2.49.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/3 v4] rust: add C wrappers for `ww_mutex` inline functions
  2025-06-21 15:21 [PATCH 1/3 v4] rust: add C wrappers for `ww_mutex` inline functions Onur Özkan
  2025-06-21 15:21 ` [PATCH 2/3 v4] implement ww_mutex abstraction for the Rust tree Onur Özkan
  2025-06-21 15:21 ` [PATCH 3/3 v4] add KUnit coverage on Rust `ww_mutex` implementation Onur Özkan
@ 2025-06-21 16:07 ` Miguel Ojeda
  2 siblings, 0 replies; 6+ messages in thread
From: Miguel Ojeda @ 2025-06-21 16:07 UTC (permalink / raw)
  To: Onur Özkan
  Cc: linux-kernel, rust-for-linux, ojeda, alex.gaynor, boqun.feng,
	gary, lossin, a.hindborg, aliceryhl, tmgross, dakr, peterz,
	mingo, will, longman, felipe_life, daniel, bjorn3_gh

On Sat, Jun 21, 2025 at 5:22 PM Onur Özkan <work@onurozkan.dev> wrote:
>
>  #include "workqueue.c"
>  #include "xarray.c"
> +#include "ww_mutex.c"

Please keep the list sorted.

Also, a couple nits on the series:

  - The title would normally be ... vN 1/M ..., rather than the other
way around. You can make Git generate it for you with `git
format-patch -vN`.

  - Typically you want to give a changelog, either per-patch (ideal,
especially for non-trivial patches that change a lot) after the first
`---` line, or per-series (in the cover letter, which is also
generally good to have for multi-patch series).

Thanks!

Cheers,
Miguel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/3 v4] implement ww_mutex abstraction for the Rust tree
  2025-06-21 15:21 ` [PATCH 2/3 v4] implement ww_mutex abstraction for the Rust tree Onur Özkan
@ 2025-06-21 16:15   ` Miguel Ojeda
  0 siblings, 0 replies; 6+ messages in thread
From: Miguel Ojeda @ 2025-06-21 16:15 UTC (permalink / raw)
  To: Onur Özkan
  Cc: linux-kernel, rust-for-linux, ojeda, alex.gaynor, boqun.feng,
	gary, lossin, a.hindborg, aliceryhl, tmgross, dakr, peterz,
	mingo, will, longman, felipe_life, daniel, bjorn3_gh, thatslyude

Hi Onur,

A few docs-related nits, for these and future patches (some apply
several times).

On Sat, Jun 21, 2025 at 5:22 PM Onur Özkan <work@onurozkan.dev> wrote:
>
> Suggested-by: thatslyude@gmail.com

Please use the usual format for names/emails, like in the other tags.
(Link also typically goes after Suggested-by to link to the
suggestion).

> +    declare_err!(EDEADLK, "Resource deadlock avoided.");

This should go after `ERANGE`.

> +//! 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.

Please link the C header like other modules do (with the `srctree/` syntax).

In addition, if there are good C docs, you can link those, e.g.

    https://docs.kernel.org/locking/ww-mutex-design.html

> +use crate::prelude::EBUSY;

Please import the entire prelude, rather than a subset -- that is its
goal after all.

In turn, that means you can drop other imports.

> +/// Represents a group of mutexes that can participate in deadlock avoidance together.
> +/// All mutexes that might be acquired together should use the same class.

The first paragraph is a title ("short description"), which is used in
lists of Rust items, so it is best to keep it short. So I would
recommend moving the second sentence to a new paragraph.

I would also probably suggest dropping the "Represents", i.e. you can
just start with "A group ...", since it is an item.

> +            // Ref: https://github.com/torvalds/linux/blob/master/include/linux/ww_mutex.h#L85-L89

These line numbers will likely go stale, and we typically don't link like this.

If you feel like you should link to particular lines, then I can
suggest git.kernel.org/linus/ instead and a tag to keep it stable,
e.g.:

    https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/ww_mutex.h?h=v6.16-rc2#n85

Also, please use <> to wrap links.

> +    /// 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.

Please use intra-doc links wherever they may work, e.g. [`WwClass`].

> +/// // Create mutexes

Period at the end.

> +/// let mut ctx = KBox::pin_init(WwAcquireCtx::new(&class), GFP_KERNEL).unwrap();

Please avoid `unwrap()` and other panics in examples (and in KUnit
tests too -- I will mention it there). `assert*!` is fine, though, if
you want to show that something is a particular value (that is
relevant for the example) etc.

More generally, the idea is to try to write code in examples like we
would in normal kernel code.

> /// use kernel::error::code::EDEADLK;

Some of these should be already available from the prelude, which gets
imported automatically, i.e. please check if you can reduce the number
of imports in the examples.

Thanks!

Cheers,
Miguel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 3/3 v4] add KUnit coverage on Rust `ww_mutex` implementation
  2025-06-21 15:21 ` [PATCH 3/3 v4] add KUnit coverage on Rust `ww_mutex` implementation Onur Özkan
@ 2025-06-21 16:15   ` Miguel Ojeda
  0 siblings, 0 replies; 6+ messages in thread
From: Miguel Ojeda @ 2025-06-21 16:15 UTC (permalink / raw)
  To: Onur Özkan
  Cc: linux-kernel, rust-for-linux, ojeda, alex.gaynor, boqun.feng,
	gary, lossin, a.hindborg, aliceryhl, tmgross, dakr, peterz,
	mingo, will, longman, felipe_life, daniel, bjorn3_gh

On Sat, Jun 21, 2025 at 5:22 PM Onur Özkan <work@onurozkan.dev> wrote:
>
> +        let guard = mutex.lock(None).unwrap();

Like in examples in docs, please try to avoid panicking if possible.
`assert*!s` are fine, though -- they are mapped especially and do not
panic.

KUnit tests now support `-> Result` too, so you can use that too,
which is also handy to write code more kernel like, e.g. to replace
things that the test is not really concerned about but that should
generally not fail like the memory allocation `unwrap()` that you have
below.

Thanks!

Cheers,
Miguel

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2025-06-21 16:15 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-06-21 15:21 [PATCH 1/3 v4] rust: add C wrappers for `ww_mutex` inline functions Onur Özkan
2025-06-21 15:21 ` [PATCH 2/3 v4] implement ww_mutex abstraction for the Rust tree Onur Özkan
2025-06-21 16:15   ` Miguel Ojeda
2025-06-21 15:21 ` [PATCH 3/3 v4] add KUnit coverage on Rust `ww_mutex` implementation Onur Özkan
2025-06-21 16:15   ` Miguel Ojeda
2025-06-21 16:07 ` [PATCH 1/3 v4] rust: add C wrappers for `ww_mutex` inline functions Miguel Ojeda

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®