Covers the entire low-level locking API (lock, try_lock,
slow path, interruptible variants) and integration with
kernel bindings.
Signed-off-by: Onur Özkan <work@onurozkan.dev>
---
rust/kernel/sync/lock/ww_mutex.rs | 442 ++++++++++++++++++
rust/kernel/sync/lock/ww_mutex/acquire_ctx.rs | 172 +++++++
2 files changed, 614 insertions(+)
create mode 100644 rust/kernel/sync/lock/ww_mutex/acquire_ctx.rs
diff --git a/rust/kernel/sync/lock/ww_mutex.rs b/rust/kernel/sync/lock/ww_mutex.rs
index 727c51cc73af..00e25872a042 100644
--- a/rust/kernel/sync/lock/ww_mutex.rs
+++ b/rust/kernel/sync/lock/ww_mutex.rs
@@ -1,7 +1,449 @@
// SPDX-License-Identifier: GPL-2.0
//! Rust abstractions for the kernel's wound-wait locking primitives.
+//!
+//! It is designed to avoid deadlocks when locking multiple [`Mutex`]es
+//! that belong to the same [`Class`]. Each lock acquisition uses an
+//! [`AcquireCtx`] to track ordering and ensure forward progress.
+//!
+//! It is recommended to use [`LockSet`] as it provides safe high-level
+//! interface that automatically handles deadlocks, retries and context
+//! management.
+use crate::error::to_result;
+use crate::prelude::*;
+use crate::types::{NotThreadSafe, Opaque};
+use crate::{bindings, container_of};
+
+use core::cell::UnsafeCell;
+use core::marker::PhantomData;
+
+pub use acquire_ctx::AcquireCtx;
pub use class::Class;
+pub use lock_set::LockSet;
+mod acquire_ctx;
mod class;
+mod lock_set;
+
+/// A wound-wait (ww) mutex that is powered with deadlock avoidance
+/// when acquiring multiple locks of the same [`Class`].
+///
+/// Each mutex belongs to a [`Class`], which the wound-wait algorithm
+/// uses to figure out the order of acquisition and prevent deadlocks.
+///
+/// # Examples
+///
+/// ```
+/// use kernel::c_str;
+/// use kernel::sync::Arc;
+/// use kernel::sync::lock::ww_mutex::{AcquireCtx, Class, Mutex};
+/// use pin_init::stack_pin_init;
+///
+/// stack_pin_init!(let class = Class::new_wound_wait(c_str!("some_class")));
+/// let mutex = Arc::pin_init(Mutex::new(42, &class), GFP_KERNEL)?;
+///
+/// let ctx = KBox::pin_init(AcquireCtx::new(&class), GFP_KERNEL)?;
+///
+/// let guard = ctx.lock(&mutex)?;
+/// assert_eq!(*guard, 42);
+///
+/// # Ok::<(), Error>(())
+/// ```
+#[pin_data]
+#[repr(C)]
+pub struct Mutex<'a, T: ?Sized> {
+ _p: PhantomData<&'a Class>,
+ #[pin]
+ inner: Opaque<bindings::ww_mutex>,
+ data: UnsafeCell<T>,
+}
+
+// SAFETY: `Mutex` can be sent to another thread if the protected
+// data `T` can be.
+unsafe impl<T: ?Sized + Send> Send for Mutex<'_, T> {}
+
+// SAFETY: `Mutex` can be shared across threads if the protected
+// data `T` can be.
+unsafe impl<T: ?Sized + Send + Sync> Sync for Mutex<'_, T> {}
+
+impl<'class, T> Mutex<'class, T> {
+ /// Initializes [`Mutex`] with the given `data` and [`Class`].
+ pub fn new(data: T, class: &'class Class) -> impl PinInit<Self> {
+ let class_ptr = class.inner.get();
+ pin_init!(Mutex {
+ inner <- Opaque::ffi_init(|slot: *mut bindings::ww_mutex| {
+ // SAFETY: `class` is valid for the lifetime `'class` captured by `Self`.
+ unsafe { bindings::ww_mutex_init(slot, class_ptr) }
+ }),
+ data: UnsafeCell::new(data),
+ _p: PhantomData
+ })
+ }
+}
+
+impl<'class> Mutex<'class, ()> {
+ /// Creates a [`Mutex`] from a raw pointer.
+ ///
+ /// This function is intended for interoperability with C code.
+ ///
+ /// # Safety
+ ///
+ /// The caller must ensure that `ptr` is a valid pointer to a `ww_mutex`
+ /// and that it remains valid for the lifetime `'a`.
+ pub unsafe fn from_raw<'a>(ptr: *mut bindings::ww_mutex) -> &'a Self {
+ // SAFETY: By the safety contract, the caller guarantees that `ptr`
+ // points to a valid `ww_mutex` which is the `inner` field of `Mutex`
+ // and remains valid for the lifetime `'a`.
+ //
+ // Because [`Mutex`] is `#[repr(C)]`, the `inner` field sits at a
+ // stable offset that `container_of!` can safely rely on.
+ unsafe { &*container_of!(Opaque::cast_from(ptr), Self, inner) }
+ }
+}
+
+impl<'class, T: ?Sized> Mutex<'class, T> {
+ /// Checks if this [`Mutex`] is currently locked.
+ ///
+ /// The returned value is racy as another thread can acquire
+ /// or release the lock immediately after this call returns.
+ pub fn is_locked(&self) -> bool {
+ // SAFETY: It's safe to call `ww_mutex_is_locked` on
+ // a valid mutex.
+ unsafe { bindings::ww_mutex_is_locked(self.inner.get()) }
+ }
+
+ /// Locks this [`Mutex`] without [`AcquireCtx`].
+ pub fn lock(&self) -> Result<MutexGuard<'_, T>> {
+ lock_common(self, None, LockKind::Regular)
+ }
+
+ /// Similar to [`Self::lock`], but can be interrupted by signals.
+ pub fn lock_interruptible(&self) -> Result<MutexGuard<'_, T>> {
+ lock_common(self, None, LockKind::Interruptible)
+ }
+
+ /// Locks this [`Mutex`] without [`AcquireCtx`] using the slow path.
+ ///
+ /// This function should be used when [`Self::lock`] fails (typically due
+ /// to a potential deadlock).
+ pub fn lock_slow(&self) -> Result<MutexGuard<'_, T>> {
+ lock_common(self, None, LockKind::Slow)
+ }
+
+ /// Similar to [`Self::lock_slow`], but can be interrupted by signals.
+ pub fn lock_slow_interruptible(&self) -> Result<MutexGuard<'_, T>> {
+ lock_common(self, None, LockKind::SlowInterruptible)
+ }
+
+ /// Tries to lock this [`Mutex`] with no [`AcquireCtx`] and without blocking.
+ ///
+ /// Unlike [`Self::lock`], no deadlock handling is performed.
+ pub fn try_lock(&self) -> Result<MutexGuard<'_, T>> {
+ lock_common(self, None, LockKind::Try)
+ }
+}
+
+/// A guard that provides exclusive access to the data protected
+/// by a [`Mutex`].
+///
+/// # Invariants
+///
+/// The guard holds an exclusive lock on the associated [`Mutex`]. 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 MutexGuard<'a, T: ?Sized> {
+ mutex: &'a Mutex<'a, T>,
+ _not_send: NotThreadSafe,
+}
+
+// SAFETY: `MutexGuard` can be shared between threads if the data can.
+unsafe impl<T: ?Sized + Sync> Sync for MutexGuard<'_, T> {}
+
+impl<'a, T: ?Sized> MutexGuard<'a, T> {
+ /// Creates a new guard for the given [`Mutex`].
+ fn new(mutex: &'a Mutex<'a, T>) -> Self {
+ Self {
+ mutex,
+ _not_send: NotThreadSafe,
+ }
+ }
+}
+
+impl<'a> MutexGuard<'a, ()> {
+ /// Creates a [`MutexGuard`] from a raw pointer.
+ ///
+ /// This function is intended for interoperability with C code.
+ ///
+ /// # Safety
+ ///
+ /// The caller must ensure that `ptr` is a valid pointer to a `ww_mutex`
+ /// and that it remains valid for the lifetime `'a`.
+ pub unsafe fn from_raw<'b>(ptr: *mut bindings::ww_mutex) -> MutexGuard<'b, ()> {
+ // SAFETY: By the safety contract, the caller guarantees that `ptr`
+ // points to a valid `ww_mutex` which is the `mutex` field of `Mutex`
+ // and remains valid for the lifetime `'a`.
+ let mutex = unsafe { Mutex::from_raw(ptr) };
+
+ MutexGuard::new(mutex)
+ }
+}
+
+impl<T: ?Sized> core::ops::Deref for MutexGuard<'_, 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 + Unpin> core::ops::DerefMut for MutexGuard<'_, 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 MutexGuard<'_, T> {
+ fn drop(&mut self) {
+ // SAFETY: We hold the lock and are about to release it.
+ unsafe { bindings::ww_mutex_unlock(self.mutex.inner.get()) };
+ }
+}
+
+/// Locking kinds used by [`lock_common`] to unify the internal
+/// locking logic.
+///
+/// It's best not to expose this type (and [`lock_common`]) to the
+/// kernel, as it allows internal API changes without worrying
+/// about breaking external compatibility.
+#[derive(Copy, Clone, Debug)]
+enum LockKind {
+ /// Blocks until lock is acquired.
+ Regular,
+ /// Blocks but can be interrupted by signals.
+ Interruptible,
+ /// Used in slow path after deadlock detection.
+ Slow,
+ /// Slow path but interruptible.
+ SlowInterruptible,
+ /// Does not block, returns immediately if busy.
+ Try,
+}
+
+/// Internal helper that unifies the different locking kinds.
+///
+/// Returns [`EINVAL`] if the [`Mutex`] has a different [`Class`].
+fn lock_common<'a, T: ?Sized>(
+ mutex: &'a Mutex<'a, T>,
+ ctx: Option<&AcquireCtx<'_>>,
+ kind: LockKind,
+) -> Result<MutexGuard<'a, T>> {
+ let mutex_ptr = mutex.inner.get();
+
+ let ctx_ptr = match ctx {
+ Some(acquire_ctx) => {
+ let ctx_ptr = acquire_ctx.inner.get();
+
+ // SAFETY: `ctx_ptr` is a valid pointer for the entire
+ // lifetime of `ctx`.
+ let ctx_class = unsafe { (*ctx_ptr).ww_class };
+
+ // SAFETY: `mutex_ptr` is a valid pointer for the entire
+ // lifetime of `mutex`.
+ let mutex_class = unsafe { (*mutex_ptr).ww_class };
+
+ // `ctx` and `mutex` must use the same class.
+ if ctx_class != mutex_class {
+ return Err(EINVAL);
+ }
+
+ ctx_ptr
+ }
+ None => core::ptr::null_mut(),
+ };
+
+ match kind {
+ LockKind::Regular => {
+ // SAFETY: `Mutex` is always pinned. If `AcquireCtx` is `Some`, it is pinned,
+ // if `None`, it is set to `core::ptr::null_mut()`. Both cases are safe.
+ let ret = unsafe { bindings::ww_mutex_lock(mutex_ptr, ctx_ptr) };
+
+ to_result(ret)?;
+ }
+ LockKind::Interruptible => {
+ // SAFETY: `Mutex` is always pinned. If `AcquireCtx` is `Some`, it is pinned,
+ // if `None`, it is set to `core::ptr::null_mut()`. Both cases are safe.
+ let ret = unsafe { bindings::ww_mutex_lock_interruptible(mutex_ptr, ctx_ptr) };
+
+ to_result(ret)?;
+ }
+ LockKind::Slow => {
+ // SAFETY: `Mutex` is always pinned. If `AcquireCtx` is `Some`, it is pinned,
+ // if `None`, it is set to `core::ptr::null_mut()`. Both cases are safe.
+ unsafe { bindings::ww_mutex_lock_slow(mutex_ptr, ctx_ptr) };
+ }
+ LockKind::SlowInterruptible => {
+ // SAFETY: `Mutex` is always pinned. If `AcquireCtx` is `Some`, it is pinned,
+ // if `None`, it is set to `core::ptr::null_mut()`. Both cases are safe.
+ let ret = unsafe { bindings::ww_mutex_lock_slow_interruptible(mutex_ptr, ctx_ptr) };
+
+ to_result(ret)?;
+ }
+ LockKind::Try => {
+ // SAFETY: `Mutex` is always pinned. If `AcquireCtx` is `Some`, it is pinned,
+ // if `None`, it is set to `core::ptr::null_mut()`. Both cases are safe.
+ let ret = unsafe { bindings::ww_mutex_trylock(mutex_ptr, ctx_ptr) };
+
+ if ret == 0 {
+ return Err(EBUSY);
+ } else {
+ to_result(ret)?;
+ }
+ }
+ };
+
+ Ok(MutexGuard::new(mutex))
+}
+
+#[kunit_tests(rust_kernel_ww_mutex)]
+mod tests {
+ use crate::prelude::*;
+ use crate::sync::Arc;
+ use crate::{c_str, define_class};
+ use pin_init::stack_pin_init;
+
+ use super::*;
+
+ // A simple coverage on `define_class` macro.
+ define_class!(TEST_WOUND_WAIT_CLASS, wound_wait, c_str!("test"));
+ define_class!(TEST_WAIT_DIE_CLASS, wait_die, c_str!("test"));
+
+ #[test]
+ fn test_ww_mutex_basic_lock_unlock() -> Result {
+ stack_pin_init!(let class = Class::new_wound_wait(c_str!("test")));
+
+ let mutex = Arc::pin_init(Mutex::new(42, &class), GFP_KERNEL)?;
+
+ let ctx = KBox::pin_init(AcquireCtx::new(&class), GFP_KERNEL)?;
+
+ let guard = ctx.lock(&mutex)?;
+ assert_eq!(*guard, 42);
+
+ // Drop the lock.
+ drop(guard);
+
+ let mut guard = ctx.lock(&mutex)?;
+ *guard = 100;
+ assert_eq!(*guard, 100);
+
+ Ok(())
+ }
+
+ #[test]
+ fn test_ww_mutex_trylock() -> Result {
+ stack_pin_init!(let class = Class::new_wound_wait(c_str!("test")));
+
+ let mutex = Arc::pin_init(Mutex::new(123, &class), GFP_KERNEL)?;
+
+ let ctx = KBox::pin_init(AcquireCtx::new(&class), GFP_KERNEL)?;
+
+ // `try_lock` on unlocked mutex should succeed.
+ let guard = ctx.try_lock(&mutex)?;
+ assert_eq!(*guard, 123);
+
+ // Now it should fail immediately as it's already locked.
+ assert!(ctx.try_lock(&mutex).is_err());
+
+ Ok(())
+ }
+
+ #[test]
+ fn test_ww_mutex_is_locked() -> Result {
+ stack_pin_init!(let class = Class::new_wait_die(c_str!("test")));
+
+ let mutex = Arc::pin_init(Mutex::new("hello", &class), GFP_KERNEL)?;
+
+ let ctx = KBox::pin_init(AcquireCtx::new(&class), GFP_KERNEL)?;
+
+ // Should not be locked initially.
+ assert!(!mutex.is_locked());
+
+ let guard = ctx.lock(&mutex)?;
+ assert!(mutex.is_locked());
+
+ drop(guard);
+ assert!(!mutex.is_locked());
+
+ Ok(())
+ }
+
+ #[test]
+ fn test_ww_acquire_context_done() -> Result {
+ stack_pin_init!(let class = Class::new_wound_wait(c_str!("test")));
+
+ let mutex1 = Arc::pin_init(Mutex::new(1, &class), GFP_KERNEL)?;
+ let mutex2 = Arc::pin_init(Mutex::new(2, &class), GFP_KERNEL)?;
+
+ let ctx = KBox::pin_init(AcquireCtx::new(&class), GFP_KERNEL)?;
+
+ // Acquire multiple mutexes with the same context.
+ let guard1 = ctx.lock(&mutex1)?;
+ let guard2 = ctx.lock(&mutex2)?;
+
+ assert_eq!(*guard1, 1);
+ assert_eq!(*guard2, 2);
+
+ // SAFETY: It's called exactly once here and nowhere else.
+ unsafe { ctx.done() };
+
+ // We shouldn't be able to lock once it's `done`.
+ assert!(ctx.lock(&mutex1).is_err());
+ assert!(ctx.lock(&mutex2).is_err());
+
+ Ok(())
+ }
+
+ #[test]
+ fn test_with_global_classes() -> Result {
+ let mutex1 = Arc::pin_init(Mutex::new(100, &TEST_WOUND_WAIT_CLASS), GFP_KERNEL)?;
+ let mutex2 = Arc::pin_init(Mutex::new(200, &TEST_WAIT_DIE_CLASS), GFP_KERNEL)?;
+
+ let ww_ctx = KBox::pin_init(AcquireCtx::new(&TEST_WOUND_WAIT_CLASS), GFP_KERNEL)?;
+ let wd_ctx = KBox::pin_init(AcquireCtx::new(&TEST_WAIT_DIE_CLASS), GFP_KERNEL)?;
+
+ let ww_guard = ww_ctx.lock(&mutex1)?;
+ let wd_guard = wd_ctx.lock(&mutex2)?;
+
+ assert_eq!(*ww_guard, 100);
+ assert_eq!(*wd_guard, 200);
+
+ assert!(mutex1.is_locked());
+ assert!(mutex2.is_locked());
+
+ drop(ww_guard);
+ drop(wd_guard);
+
+ assert!(!mutex1.is_locked());
+ assert!(!mutex2.is_locked());
+
+ Ok(())
+ }
+
+ #[test]
+ fn test_mutex_without_ctx() -> Result {
+ let mutex = Arc::pin_init(Mutex::new(100, &TEST_WOUND_WAIT_CLASS), GFP_KERNEL)?;
+ let guard = mutex.lock()?;
+
+ assert_eq!(*guard, 100);
+ assert!(mutex.is_locked());
+
+ drop(guard);
+
+ assert!(!mutex.is_locked());
+
+ Ok(())
+ }
+}
diff --git a/rust/kernel/sync/lock/ww_mutex/acquire_ctx.rs b/rust/kernel/sync/lock/ww_mutex/acquire_ctx.rs
new file mode 100644
index 000000000000..141300e849eb
--- /dev/null
+++ b/rust/kernel/sync/lock/ww_mutex/acquire_ctx.rs
@@ -0,0 +1,172 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Provides [`AcquireCtx`] for managing multiple wound/wait
+//! mutexes from the same [`Class`].
+
+use crate::bindings;
+use crate::prelude::*;
+use crate::types::Opaque;
+
+use core::marker::PhantomData;
+
+use super::{lock_common, Class, LockKind, Mutex, MutexGuard};
+
+/// Groups multiple [`Mutex`]es for deadlock avoidance when acquired
+/// with the same [`Class`].
+///
+/// # Examples
+///
+/// ```
+/// use kernel::sync::lock::ww_mutex::{Class, AcquireCtx, Mutex};
+/// use kernel::c_str;
+/// use kernel::sync::Arc;
+/// use pin_init::stack_pin_init;
+///
+/// stack_pin_init!(let class = Class::new_wound_wait(c_str!("demo")));
+///
+/// // Create mutexes.
+/// let mutex1 = Arc::pin_init(Mutex::new(1, &class), GFP_KERNEL)?;
+/// let mutex2 = Arc::pin_init(Mutex::new(2, &class), GFP_KERNEL)?;
+///
+/// // Create acquire context for deadlock avoidance.
+/// let ctx = KBox::pin_init(AcquireCtx::new(&class), GFP_KERNEL)?;
+///
+/// let guard1 = ctx.lock(&mutex1)?;
+/// let guard2 = ctx.lock(&mutex2)?;
+///
+/// // Mark acquisition phase as complete.
+/// // SAFETY: It's called exactly once here and nowhere else.
+/// unsafe { ctx.done() };
+///
+/// # Ok::<(), Error>(())
+/// ```
+#[pin_data(PinnedDrop)]
+#[repr(transparent)]
+pub struct AcquireCtx<'a> {
+ #[pin]
+ pub(super) inner: Opaque<bindings::ww_acquire_ctx>,
+ _p: PhantomData<&'a Class>,
+}
+
+impl<'class> AcquireCtx<'class> {
+ /// Initializes a new [`AcquireCtx`] with the given [`Class`].
+ pub fn new(class: &'class Class) -> impl PinInit<Self> {
+ let class_ptr = class.inner.get();
+ pin_init!(AcquireCtx {
+ inner <- Opaque::ffi_init(|slot: *mut bindings::ww_acquire_ctx| {
+ // SAFETY: `class` is valid for the lifetime `'class` captured
+ // by `AcquireCtx`.
+ unsafe { bindings::ww_acquire_init(slot, class_ptr) }
+ }),
+ _p: PhantomData
+ })
+ }
+
+ /// Creates a [`AcquireCtx`] from a raw pointer.
+ ///
+ /// This function is intended for interoperability with C code.
+ ///
+ /// # Safety
+ ///
+ /// The caller must ensure that `ptr` is a valid pointer to the `inner` field
+ /// of [`AcquireCtx`] and that it remains valid for the lifetime `'a`.
+ pub unsafe fn from_raw<'a>(ptr: *mut bindings::ww_acquire_ctx) -> &'a Self {
+ // SAFETY: By the safety contract, `ptr` is valid to construct `AcquireCtx`.
+ unsafe { &*ptr.cast() }
+ }
+
+ /// Marks the end of the acquire phase.
+ ///
+ /// Calling this function is optional. It is just useful to document
+ /// the code and clearly designated the acquire phase from actually
+ /// using the locked data structures.
+ ///
+ /// After calling this function, no more mutexes can be acquired with
+ /// this context.
+ ///
+ /// # Safety
+ ///
+ /// The caller must ensure that this function is called only once
+ /// and after calling it, no further mutexes are acquired using
+ /// this context.
+ pub unsafe fn done(&self) {
+ // SAFETY: By the safety contract, the caller guarantees that this
+ // function is called only once.
+ unsafe { bindings::ww_acquire_done(self.inner.get()) };
+ }
+
+ /// Re-initializes the [`AcquireCtx`].
+ ///
+ /// Must be called after releasing all locks when [`EDEADLK`] occurs.
+ ///
+ /// # Safety
+ ///
+ /// The caller must ensure no locks are held in this [`AcquireCtx`].
+ pub unsafe fn reinit(self: Pin<&mut Self>) {
+ let ctx = self.inner.get();
+
+ // SAFETY: `ww_class` is always a valid pointer in properly initialized
+ // `AcquireCtx`.
+ let class_ptr = unsafe { (*ctx).ww_class };
+
+ // SAFETY:
+ // - Lifetime of any guard (which hold an immutable borrow of `self`) cannot overlap
+ // with the execution of this function. This enforces that all locks acquired via
+ // this context have been released.
+ //
+ // - `ctx` is guaranteed to be initialized because `ww_acquire_fini`
+ // can only be called from the `Drop` implementation.
+ //
+ // - `ww_acquire_fini` is safe to call on an initialized context.
+ unsafe { bindings::ww_acquire_fini(ctx) };
+
+ // SAFETY: `ww_acquire_init` is safe to call with valid pointers
+ // to initialize an uninitialized context.
+ unsafe { bindings::ww_acquire_init(ctx, class_ptr) };
+ }
+
+ /// Locks the given [`Mutex`] on this [`AcquireCtx`].
+ pub fn lock<'a, T>(&'a self, mutex: &'a Mutex<'a, T>) -> Result<MutexGuard<'a, T>> {
+ lock_common(mutex, Some(self), LockKind::Regular)
+ }
+
+ /// Similar to [`Self::lock`], but can be interrupted by signals.
+ pub fn lock_interruptible<'a, T>(
+ &'a self,
+ mutex: &'a Mutex<'a, T>,
+ ) -> Result<MutexGuard<'a, T>> {
+ lock_common(mutex, Some(self), LockKind::Interruptible)
+ }
+
+ /// Locks the given [`Mutex`] on this [`AcquireCtx`] using the slow path.
+ ///
+ /// This function should be used when [`Self::lock`] fails (typically due
+ /// to a potential deadlock).
+ pub fn lock_slow<'a, T>(&'a self, mutex: &'a Mutex<'a, T>) -> Result<MutexGuard<'a, T>> {
+ lock_common(mutex, Some(self), LockKind::Slow)
+ }
+
+ /// Similar to [`Self::lock_slow`], but can be interrupted by signals.
+ pub fn lock_slow_interruptible<'a, T>(
+ &'a self,
+ mutex: &'a Mutex<'a, T>,
+ ) -> Result<MutexGuard<'a, T>> {
+ lock_common(mutex, Some(self), LockKind::SlowInterruptible)
+ }
+
+ /// Tries to lock the [`Mutex`] on this [`AcquireCtx`] without blocking.
+ ///
+ /// Unlike [`Self::lock`], no deadlock handling is performed.
+ pub fn try_lock<'a, T>(&'a self, mutex: &'a Mutex<'a, T>) -> Result<MutexGuard<'a, T>> {
+ lock_common(mutex, Some(self), LockKind::Try)
+ }
+}
+
+#[pinned_drop]
+impl PinnedDrop for AcquireCtx<'_> {
+ fn drop(self: Pin<&mut Self>) {
+ // SAFETY: Given the lifetime bounds we know no locks are held,
+ // so calling `ww_acquire_fini` is safe.
+ unsafe { bindings::ww_acquire_fini(self.inner.get()) };
+ }
+}
--
2.51.2
On Mon, Dec 01, 2025 at 01:28:54PM +0300, Onur Özkan wrote:
> Covers the entire low-level locking API (lock, try_lock,
> slow path, interruptible variants) and integration with
> kernel bindings.
>
> Signed-off-by: Onur Özkan <work@onurozkan.dev>
> +impl<'class> Mutex<'class, ()> {
> + /// Creates a [`Mutex`] from a raw pointer.
> + ///
> + /// This function is intended for interoperability with C code.
> + ///
> + /// # Safety
> + ///
> + /// The caller must ensure that `ptr` is a valid pointer to a `ww_mutex`
> + /// and that it remains valid for the lifetime `'a`.
> + pub unsafe fn from_raw<'a>(ptr: *mut bindings::ww_mutex) -> &'a Self {
Should also require that the class is valid for the duration of 'class.
> +/// Internal helper that unifies the different locking kinds.
> +///
> +/// Returns [`EINVAL`] if the [`Mutex`] has a different [`Class`].
> +fn lock_common<'a, T: ?Sized>(
> + mutex: &'a Mutex<'a, T>,
> + ctx: Option<&AcquireCtx<'_>>,
> + kind: LockKind,
> +) -> Result<MutexGuard<'a, T>> {
> + let mutex_ptr = mutex.inner.get();
> +
> + let ctx_ptr = match ctx {
> + Some(acquire_ctx) => {
> + let ctx_ptr = acquire_ctx.inner.get();
> +
> + // SAFETY: `ctx_ptr` is a valid pointer for the entire
> + // lifetime of `ctx`.
> + let ctx_class = unsafe { (*ctx_ptr).ww_class };
> +
> + // SAFETY: `mutex_ptr` is a valid pointer for the entire
> + // lifetime of `mutex`.
> + let mutex_class = unsafe { (*mutex_ptr).ww_class };
> +
> + // `ctx` and `mutex` must use the same class.
> + if ctx_class != mutex_class {
> + return Err(EINVAL);
> + }
Hmm, this originates from the previous conversation:
https://lore.kernel.org/all/20251124184928.30b8bbaf@nimda/
>>> + /// // SAFETY: Both `lock_set` and `mutex1` uses the
>>> same class.
>>> + /// unsafe { lock_set.lock(&mutex1)? };
>>> + ///
>>> + /// // SAFETY: Both `lock_set` and `mutex2` uses the
>>> same class.
>>> + /// unsafe { lock_set.lock(&mutex2)? };
>>
>> I wonder if there's some way we can get rid of the safety contract
>> here and verify this at compile time, it would be a shame if every
>> single lock invocation needed to be unsafe.
>>
>
> Yeah :(. We could get rid of them easily by keeping the class that was
> passed to the constructor functions but that becomes a problem for the
> from_raw implementations.
>
> I think the best solution would be to expose ww_class type from
> ww_acquire_ctx and ww_mutex unconditionally (right now it depends on
> DEBUG_WW_MUTEXES). That way we can just access the class and verify
> that the mutex and acquire_ctx classes match.
>
> What do you think? I can submit a patch for the C-side implementation.
> It should be straightforward and shouldn't have any runtime impact.
I think there is a better solution. We can create a different type for
every single class, like how rust/kernel/sync/lock/global.rs creates a
different type for every single mutex. Then, you know that the classes
are the same since the class is part of the type.
Alice
> On 3 Dec 2025, at 10:26, Alice Ryhl <aliceryhl@google.com> wrote:
>
> On Mon, Dec 01, 2025 at 01:28:54PM +0300, Onur Özkan wrote:
>> Covers the entire low-level locking API (lock, try_lock,
>> slow path, interruptible variants) and integration with
>> kernel bindings.
>>
>> Signed-off-by: Onur Özkan <work@onurozkan.dev>
>
>> +impl<'class> Mutex<'class, ()> {
>> + /// Creates a [`Mutex`] from a raw pointer.
>> + ///
>> + /// This function is intended for interoperability with C code.
>> + ///
>> + /// # Safety
>> + ///
>> + /// The caller must ensure that `ptr` is a valid pointer to a `ww_mutex`
>> + /// and that it remains valid for the lifetime `'a`.
>> + pub unsafe fn from_raw<'a>(ptr: *mut bindings::ww_mutex) -> &'a Self {
>
> Should also require that the class is valid for the duration of 'class.
>
>> +/// Internal helper that unifies the different locking kinds.
>> +///
>> +/// Returns [`EINVAL`] if the [`Mutex`] has a different [`Class`].
>> +fn lock_common<'a, T: ?Sized>(
>> + mutex: &'a Mutex<'a, T>,
>> + ctx: Option<&AcquireCtx<'_>>,
>> + kind: LockKind,
>> +) -> Result<MutexGuard<'a, T>> {
>> + let mutex_ptr = mutex.inner.get();
>> +
>> + let ctx_ptr = match ctx {
>> + Some(acquire_ctx) => {
>> + let ctx_ptr = acquire_ctx.inner.get();
>> +
>> + // SAFETY: `ctx_ptr` is a valid pointer for the entire
>> + // lifetime of `ctx`.
>> + let ctx_class = unsafe { (*ctx_ptr).ww_class };
>> +
>> + // SAFETY: `mutex_ptr` is a valid pointer for the entire
>> + // lifetime of `mutex`.
>> + let mutex_class = unsafe { (*mutex_ptr).ww_class };
>> +
>> + // `ctx` and `mutex` must use the same class.
>> + if ctx_class != mutex_class {
>> + return Err(EINVAL);
>> + }
>
> Hmm, this originates from the previous conversation:
>
> https://lore.kernel.org/all/20251124184928.30b8bbaf@nimda/
>>>> + /// // SAFETY: Both `lock_set` and `mutex1` uses the
>>>> same class.
>>>> + /// unsafe { lock_set.lock(&mutex1)? };
>>>> + ///
>>>> + /// // SAFETY: Both `lock_set` and `mutex2` uses the
>>>> same class.
>>>> + /// unsafe { lock_set.lock(&mutex2)? };
>>>
>>> I wonder if there's some way we can get rid of the safety contract
>>> here and verify this at compile time, it would be a shame if every
>>> single lock invocation needed to be unsafe.
>>>
>>
>> Yeah :(. We could get rid of them easily by keeping the class that was
>> passed to the constructor functions but that becomes a problem for the
>> from_raw implementations.
>>
>> I think the best solution would be to expose ww_class type from
>> ww_acquire_ctx and ww_mutex unconditionally (right now it depends on
>> DEBUG_WW_MUTEXES). That way we can just access the class and verify
>> that the mutex and acquire_ctx classes match.
>>
>> What do you think? I can submit a patch for the C-side implementation.
>> It should be straightforward and shouldn't have any runtime impact.
>
> I think there is a better solution. We can create a different type for
> every single class, like how rust/kernel/sync/lock/global.rs creates a
> different type for every single mutex. Then, you know that the classes
> are the same since the class is part of the type.
I don’t think this would work with the from_raw() functions. What class
would you assign then? I think this is precisely what sparked the current
solution.
>
> Alice
On Wed, Dec 03, 2025 at 02:23:14PM -0300, Daniel Almeida wrote: > > > > On 3 Dec 2025, at 10:26, Alice Ryhl <aliceryhl@google.com> wrote: > > > > On Mon, Dec 01, 2025 at 01:28:54PM +0300, Onur Özkan wrote: > >> Yeah :(. We could get rid of them easily by keeping the class that was > >> passed to the constructor functions but that becomes a problem for the > >> from_raw implementations. > >> > >> I think the best solution would be to expose ww_class type from > >> ww_acquire_ctx and ww_mutex unconditionally (right now it depends on > >> DEBUG_WW_MUTEXES). That way we can just access the class and verify > >> that the mutex and acquire_ctx classes match. > >> > >> What do you think? I can submit a patch for the C-side implementation. > >> It should be straightforward and shouldn't have any runtime impact. > > > > I think there is a better solution. We can create a different type for > > every single class, like how rust/kernel/sync/lock/global.rs creates a > > different type for every single mutex. Then, you know that the classes > > are the same since the class is part of the type. > > I don’t think this would work with the from_raw() functions. What class > would you assign then? I think this is precisely what sparked the current > solution. There can be a way to create a type for a C-defined class, and from_raw() can require that you don't use the same Rust type for different C classes. Alice
On Thu, 4 Dec 2025 09:07:18 +0000 Alice Ryhl <aliceryhl@google.com> wrote: > On Wed, Dec 03, 2025 at 02:23:14PM -0300, Daniel Almeida wrote: > > > > > > > On 3 Dec 2025, at 10:26, Alice Ryhl <aliceryhl@google.com> wrote: > > > > > > On Mon, Dec 01, 2025 at 01:28:54PM +0300, Onur Özkan wrote: > > >> Yeah :(. We could get rid of them easily by keeping the class > > >> that was passed to the constructor functions but that becomes a > > >> problem for the from_raw implementations. > > >> > > >> I think the best solution would be to expose ww_class type from > > >> ww_acquire_ctx and ww_mutex unconditionally (right now it > > >> depends on DEBUG_WW_MUTEXES). That way we can just access the > > >> class and verify that the mutex and acquire_ctx classes match. > > >> > > >> What do you think? I can submit a patch for the C-side > > >> implementation. It should be straightforward and shouldn't have > > >> any runtime impact. > > > > > > I think there is a better solution. We can create a different > > > type for every single class, like how > > > rust/kernel/sync/lock/global.rs creates a different type for > > > every single mutex. Then, you know that the classes are the same > > > since the class is part of the type. > > > > I don’t think this would work with the from_raw() functions. What > > class would you assign then? I think this is precisely what sparked > > the current solution. > > There can be a way to create a type for a C-defined class, and > from_raw() can require that you don't use the same Rust type for > different C classes. > Do you think this is a better alternative? IMO it doesn't seem worth it for what it's doing. Current approach adds less complexity and is easier to maintain. It's not just helping from_raw functions, the class validation is being much simpler without having to deal with storing class references or creating new types. I am holding off the next version because we don't have a clear consensus on this. - Onur > Alice
> On 15 Dec 2025, at 06:10, Onur Özkan <work@onurozkan.dev> wrote: > > On Thu, 4 Dec 2025 09:07:18 +0000 > Alice Ryhl <aliceryhl@google.com> wrote: > >> On Wed, Dec 03, 2025 at 02:23:14PM -0300, Daniel Almeida wrote: >>> >>> >>>> On 3 Dec 2025, at 10:26, Alice Ryhl <aliceryhl@google.com> wrote: >>>> >>>> On Mon, Dec 01, 2025 at 01:28:54PM +0300, Onur Özkan wrote: >>>>> Yeah :(. We could get rid of them easily by keeping the class >>>>> that was passed to the constructor functions but that becomes a >>>>> problem for the from_raw implementations. >>>>> >>>>> I think the best solution would be to expose ww_class type from >>>>> ww_acquire_ctx and ww_mutex unconditionally (right now it >>>>> depends on DEBUG_WW_MUTEXES). That way we can just access the >>>>> class and verify that the mutex and acquire_ctx classes match. >>>>> >>>>> What do you think? I can submit a patch for the C-side >>>>> implementation. It should be straightforward and shouldn't have >>>>> any runtime impact. >>>> >>>> I think there is a better solution. We can create a different >>>> type for every single class, like how >>>> rust/kernel/sync/lock/global.rs creates a different type for >>>> every single mutex. Then, you know that the classes are the same >>>> since the class is part of the type. >>> >>> I don’t think this would work with the from_raw() functions. What >>> class would you assign then? I think this is precisely what sparked >>> the current solution. >> >> There can be a way to create a type for a C-defined class, and >> from_raw() can require that you don't use the same Rust type for >> different C classes. >> > > Do you think this is a better alternative? IMO it doesn't seem worth > it for what it's doing. Current approach adds less complexity and is > easier to maintain. It's not just helping from_raw functions, the class > validation is being much simpler without having to deal with storing > class references or creating new types. > > I am holding off the next version because we don't have a clear > consensus on this. > > - Onur > >> Alice I am frankly not sure whether what Alice suggested is an improvement either. Having the class stored seems like a much simpler solution to the problem. > Then when you call from_raw(), you call > > Mutex::<T, MY_C_CLASS>::from_raw(ptr_to_mutex) That’s what we are trying to avoid in the first place. It does not really matter that from_raw() is unsafe, because this is not really about UB, but rather about trying to avoid broken code if possible. IMHO, if we can check this, then I propose that we do. Hence why we are storing the class. — Daniel
> On 4 Dec 2025, at 03:07, Alice Ryhl <aliceryhl@google.com> wrote: > > On Wed, Dec 03, 2025 at 02:23:14PM -0300, Daniel Almeida wrote: >> >> >>> On 3 Dec 2025, at 10:26, Alice Ryhl <aliceryhl@google.com> wrote: >>> >>> On Mon, Dec 01, 2025 at 01:28:54PM +0300, Onur Özkan wrote: >>>> Yeah :(. We could get rid of them easily by keeping the class that was >>>> passed to the constructor functions but that becomes a problem for the >>>> from_raw implementations. >>>> >>>> I think the best solution would be to expose ww_class type from >>>> ww_acquire_ctx and ww_mutex unconditionally (right now it depends on >>>> DEBUG_WW_MUTEXES). That way we can just access the class and verify >>>> that the mutex and acquire_ctx classes match. >>>> >>>> What do you think? I can submit a patch for the C-side implementation. >>>> It should be straightforward and shouldn't have any runtime impact. >>> >>> I think there is a better solution. We can create a different type for >>> every single class, like how rust/kernel/sync/lock/global.rs creates a >>> different type for every single mutex. Then, you know that the classes >>> are the same since the class is part of the type. >> >> I don’t think this would work with the from_raw() functions. What class >> would you assign then? I think this is precisely what sparked the current >> solution. > > There can be a way to create a type for a C-defined class, and That’s the problem, if we don’t have patch 2, we don’t know the class. What you’re suggesting seems unimplementable to me at first. Otherwise, can you expand some more? — Daniel > from_raw() can require that you don't use the same Rust type for > different C classes. > > Alice
On Thu, Dec 04, 2025 at 07:26:25AM -0600, Daniel Almeida wrote: > > > > On 4 Dec 2025, at 03:07, Alice Ryhl <aliceryhl@google.com> wrote: > > > > On Wed, Dec 03, 2025 at 02:23:14PM -0300, Daniel Almeida wrote: > >> > >> > >>> On 3 Dec 2025, at 10:26, Alice Ryhl <aliceryhl@google.com> wrote: > >>> > >>> On Mon, Dec 01, 2025 at 01:28:54PM +0300, Onur Özkan wrote: > >>>> Yeah :(. We could get rid of them easily by keeping the class that was > >>>> passed to the constructor functions but that becomes a problem for the > >>>> from_raw implementations. > >>>> > >>>> I think the best solution would be to expose ww_class type from > >>>> ww_acquire_ctx and ww_mutex unconditionally (right now it depends on > >>>> DEBUG_WW_MUTEXES). That way we can just access the class and verify > >>>> that the mutex and acquire_ctx classes match. > >>>> > >>>> What do you think? I can submit a patch for the C-side implementation. > >>>> It should be straightforward and shouldn't have any runtime impact. > >>> > >>> I think there is a better solution. We can create a different type for > >>> every single class, like how rust/kernel/sync/lock/global.rs creates a > >>> different type for every single mutex. Then, you know that the classes > >>> are the same since the class is part of the type. > >> > >> I don’t think this would work with the from_raw() functions. What class > >> would you assign then? I think this is precisely what sparked the current > >> solution. > > > > There can be a way to create a type for a C-defined class, and > > That’s the problem, if we don’t have patch 2, we don’t know the class. > > What you’re suggesting seems unimplementable to me at first. Otherwise, can > you expand some more? For each class defined by C code, you invoke a macro: ww_class_from_c_code(MY_C_CLASS, bindings::my_c_class); Then when you call from_raw(), you call Mutex::<T, MY_C_CLASS>::from_raw(ptr_to_mutex) There is no need for a check, because from_raw() is unsafe. Alice
On Wed, 3 Dec 2025 13:26:23 +0000
Alice Ryhl <aliceryhl@google.com> wrote:
> On Mon, Dec 01, 2025 at 01:28:54PM +0300, Onur Özkan wrote:
> > Covers the entire low-level locking API (lock, try_lock,
> > slow path, interruptible variants) and integration with
> > kernel bindings.
> >
> > Signed-off-by: Onur Özkan <work@onurozkan.dev>
>
> > +impl<'class> Mutex<'class, ()> {
> > + /// Creates a [`Mutex`] from a raw pointer.
> > + ///
> > + /// This function is intended for interoperability with C code.
> > + ///
> > + /// # Safety
> > + ///
> > + /// The caller must ensure that `ptr` is a valid pointer to a
> > `ww_mutex`
> > + /// and that it remains valid for the lifetime `'a`.
> > + pub unsafe fn from_raw<'a>(ptr: *mut bindings::ww_mutex) ->
> > &'a Self {
>
> Should also require that the class is valid for the duration of
> 'class.
>
> > +/// Internal helper that unifies the different locking kinds.
> > +///
> > +/// Returns [`EINVAL`] if the [`Mutex`] has a different [`Class`].
> > +fn lock_common<'a, T: ?Sized>(
> > + mutex: &'a Mutex<'a, T>,
> > + ctx: Option<&AcquireCtx<'_>>,
> > + kind: LockKind,
> > +) -> Result<MutexGuard<'a, T>> {
> > + let mutex_ptr = mutex.inner.get();
> > +
> > + let ctx_ptr = match ctx {
> > + Some(acquire_ctx) => {
> > + let ctx_ptr = acquire_ctx.inner.get();
> > +
> > + // SAFETY: `ctx_ptr` is a valid pointer for the entire
> > + // lifetime of `ctx`.
> > + let ctx_class = unsafe { (*ctx_ptr).ww_class };
> > +
> > + // SAFETY: `mutex_ptr` is a valid pointer for the
> > entire
> > + // lifetime of `mutex`.
> > + let mutex_class = unsafe { (*mutex_ptr).ww_class };
> > +
> > + // `ctx` and `mutex` must use the same class.
> > + if ctx_class != mutex_class {
> > + return Err(EINVAL);
> > + }
>
> Hmm, this originates from the previous conversation:
>
> https://lore.kernel.org/all/20251124184928.30b8bbaf@nimda/
> >>> + /// // SAFETY: Both `lock_set` and `mutex1` uses the
> >>> same class.
> >>> + /// unsafe { lock_set.lock(&mutex1)? };
> >>> + ///
> >>> + /// // SAFETY: Both `lock_set` and `mutex2` uses the
> >>> same class.
> >>> + /// unsafe { lock_set.lock(&mutex2)? };
> >>
> >> I wonder if there's some way we can get rid of the safety contract
> >> here and verify this at compile time, it would be a shame if every
> >> single lock invocation needed to be unsafe.
> >>
> >
> > Yeah :(. We could get rid of them easily by keeping the class that
> > was passed to the constructor functions but that becomes a problem
> > for the from_raw implementations.
> >
> > I think the best solution would be to expose ww_class type from
> > ww_acquire_ctx and ww_mutex unconditionally (right now it depends on
> > DEBUG_WW_MUTEXES). That way we can just access the class and verify
> > that the mutex and acquire_ctx classes match.
> >
> > What do you think? I can submit a patch for the C-side
> > implementation. It should be straightforward and shouldn't have any
> > runtime impact.
>
> I think there is a better solution. We can create a different type for
> every single class, like how rust/kernel/sync/lock/global.rs creates a
> different type for every single mutex. Then, you know that the classes
> are the same since the class is part of the type.
>
> Alice
You can have same types but different memory addresses and that would
break the ww_mutex logic we are trying to solve.
-Onur
On Wed, Dec 03, 2025 at 07:02:30PM +0300, Onur Özkan wrote:
> On Wed, 3 Dec 2025 13:26:23 +0000
> Alice Ryhl <aliceryhl@google.com> wrote:
>
> > On Mon, Dec 01, 2025 at 01:28:54PM +0300, Onur Özkan wrote:
> > > +/// Internal helper that unifies the different locking kinds.
> > > +///
> > > +/// Returns [`EINVAL`] if the [`Mutex`] has a different [`Class`].
> > > +fn lock_common<'a, T: ?Sized>(
> > > + mutex: &'a Mutex<'a, T>,
> > > + ctx: Option<&AcquireCtx<'_>>,
> > > + kind: LockKind,
> > > +) -> Result<MutexGuard<'a, T>> {
> > > + let mutex_ptr = mutex.inner.get();
> > > +
> > > + let ctx_ptr = match ctx {
> > > + Some(acquire_ctx) => {
> > > + let ctx_ptr = acquire_ctx.inner.get();
> > > +
> > > + // SAFETY: `ctx_ptr` is a valid pointer for the entire
> > > + // lifetime of `ctx`.
> > > + let ctx_class = unsafe { (*ctx_ptr).ww_class };
> > > +
> > > + // SAFETY: `mutex_ptr` is a valid pointer for the
> > > entire
> > > + // lifetime of `mutex`.
> > > + let mutex_class = unsafe { (*mutex_ptr).ww_class };
> > > +
> > > + // `ctx` and `mutex` must use the same class.
> > > + if ctx_class != mutex_class {
> > > + return Err(EINVAL);
> > > + }
> >
> > Hmm, this originates from the previous conversation:
> >
> > https://lore.kernel.org/all/20251124184928.30b8bbaf@nimda/
> > >>> + /// // SAFETY: Both `lock_set` and `mutex1` uses the
> > >>> same class.
> > >>> + /// unsafe { lock_set.lock(&mutex1)? };
> > >>> + ///
> > >>> + /// // SAFETY: Both `lock_set` and `mutex2` uses the
> > >>> same class.
> > >>> + /// unsafe { lock_set.lock(&mutex2)? };
> > >>
> > >> I wonder if there's some way we can get rid of the safety contract
> > >> here and verify this at compile time, it would be a shame if every
> > >> single lock invocation needed to be unsafe.
> > >>
> > >
> > > Yeah :(. We could get rid of them easily by keeping the class that
> > > was passed to the constructor functions but that becomes a problem
> > > for the from_raw implementations.
> > >
> > > I think the best solution would be to expose ww_class type from
> > > ww_acquire_ctx and ww_mutex unconditionally (right now it depends on
> > > DEBUG_WW_MUTEXES). That way we can just access the class and verify
> > > that the mutex and acquire_ctx classes match.
> > >
> > > What do you think? I can submit a patch for the C-side
> > > implementation. It should be straightforward and shouldn't have any
> > > runtime impact.
> >
> > I think there is a better solution. We can create a different type for
> > every single class, like how rust/kernel/sync/lock/global.rs creates a
> > different type for every single mutex. Then, you know that the classes
> > are the same since the class is part of the type.
>
> You can have same types but different memory addresses and that would
> break the ww_mutex logic we are trying to solve.
The entire idea behind rust/kernel/sync/lock/global.rs is one type per
memory address. Can you elaborate on the difficult case?
Alice
Hi Onur,
> On 1 Dec 2025, at 07:28, Onur Özkan <work@onurozkan.dev> wrote:
>
> Covers the entire low-level locking API (lock, try_lock,
> slow path, interruptible variants) and integration with
> kernel bindings.
>
> Signed-off-by: Onur Özkan <work@onurozkan.dev>
> ---
> rust/kernel/sync/lock/ww_mutex.rs | 442 ++++++++++++++++++
> rust/kernel/sync/lock/ww_mutex/acquire_ctx.rs | 172 +++++++
> 2 files changed, 614 insertions(+)
> create mode 100644 rust/kernel/sync/lock/ww_mutex/acquire_ctx.rs
>
> diff --git a/rust/kernel/sync/lock/ww_mutex.rs b/rust/kernel/sync/lock/ww_mutex.rs
> index 727c51cc73af..00e25872a042 100644
> --- a/rust/kernel/sync/lock/ww_mutex.rs
> +++ b/rust/kernel/sync/lock/ww_mutex.rs
> @@ -1,7 +1,449 @@
> // SPDX-License-Identifier: GPL-2.0
>
> //! Rust abstractions for the kernel's wound-wait locking primitives.
> +//!
> +//! It is designed to avoid deadlocks when locking multiple [`Mutex`]es
> +//! that belong to the same [`Class`]. Each lock acquisition uses an
> +//! [`AcquireCtx`] to track ordering and ensure forward progress.
> +//!
> +//! It is recommended to use [`LockSet`] as it provides safe high-level
> +//! interface that automatically handles deadlocks, retries and context
> +//! management.
This will break the docs, since LockSet is introduced in the next commit.
Perhaps add this last paragraph in the next commit?
>
> +use crate::error::to_result;
> +use crate::prelude::*;
> +use crate::types::{NotThreadSafe, Opaque};
> +use crate::{bindings, container_of};
> +
> +use core::cell::UnsafeCell;
> +use core::marker::PhantomData;
> +
> +pub use acquire_ctx::AcquireCtx;
> pub use class::Class;
> +pub use lock_set::LockSet;
>
> +mod acquire_ctx;
> mod class;
> +mod lock_set;
> +
> +/// A wound-wait (ww) mutex that is powered with deadlock avoidance
> +/// when acquiring multiple locks of the same [`Class`].
> +///
> +/// Each mutex belongs to a [`Class`], which the wound-wait algorithm
> +/// uses to figure out the order of acquisition and prevent deadlocks.
> +///
> +/// # Examples
> +///
> +/// ```
> +/// use kernel::c_str;
> +/// use kernel::sync::Arc;
> +/// use kernel::sync::lock::ww_mutex::{AcquireCtx, Class, Mutex};
> +/// use pin_init::stack_pin_init;
> +///
> +/// stack_pin_init!(let class = Class::new_wound_wait(c_str!("some_class")));
> +/// let mutex = Arc::pin_init(Mutex::new(42, &class), GFP_KERNEL)?;
> +///
> +/// let ctx = KBox::pin_init(AcquireCtx::new(&class), GFP_KERNEL)?;
> +///
> +/// let guard = ctx.lock(&mutex)?;
> +/// assert_eq!(*guard, 42);
> +///
> +/// # Ok::<(), Error>(())
> +/// ```
> +#[pin_data]
> +#[repr(C)]
> +pub struct Mutex<'a, T: ?Sized> {
> + _p: PhantomData<&'a Class>,
nit: can you keep this last? Let’s not start a #[repr(C)] struct with a ZST if we can help it.
> + #[pin]
> + inner: Opaque<bindings::ww_mutex>,
> + data: UnsafeCell<T>,
> +}
> +
> +// SAFETY: `Mutex` can be sent to another thread if the protected
> +// data `T` can be.
> +unsafe impl<T: ?Sized + Send> Send for Mutex<'_, T> {}
> +
> +// SAFETY: `Mutex` can be shared across threads if the protected
> +// data `T` can be.
> +unsafe impl<T: ?Sized + Send + Sync> Sync for Mutex<'_, T> {}
> +
Foo and impl Foo together, please.
> +impl<'class, T> Mutex<'class, T> {
> + /// Initializes [`Mutex`] with the given `data` and [`Class`].
> + pub fn new(data: T, class: &'class Class) -> impl PinInit<Self> {
> + let class_ptr = class.inner.get();
> + pin_init!(Mutex {
> + inner <- Opaque::ffi_init(|slot: *mut bindings::ww_mutex| {
> + // SAFETY: `class` is valid for the lifetime `'class` captured by `Self`.
> + unsafe { bindings::ww_mutex_init(slot, class_ptr) }
> + }),
> + data: UnsafeCell::new(data),
> + _p: PhantomData
> + })
> + }
> +}
> +
> +impl<'class> Mutex<'class, ()> {
> + /// Creates a [`Mutex`] from a raw pointer.
> + ///
> + /// This function is intended for interoperability with C code.
> + ///
> + /// # Safety
> + ///
> + /// The caller must ensure that `ptr` is a valid pointer to a `ww_mutex`
> + /// and that it remains valid for the lifetime `'a`.
> + pub unsafe fn from_raw<'a>(ptr: *mut bindings::ww_mutex) -> &'a Self {
> + // SAFETY: By the safety contract, the caller guarantees that `ptr`
> + // points to a valid `ww_mutex` which is the `inner` field of `Mutex`
> + // and remains valid for the lifetime `'a`.
> + //
> + // Because [`Mutex`] is `#[repr(C)]`, the `inner` field sits at a
> + // stable offset that `container_of!` can safely rely on.
> + unsafe { &*container_of!(Opaque::cast_from(ptr), Self, inner) }
> + }
> +}
nit: can you move this implementation to be after the fully generic one?
> +
> +impl<'class, T: ?Sized> Mutex<'class, T> {
> + /// Checks if this [`Mutex`] is currently locked.
> + ///
> + /// The returned value is racy as another thread can acquire
> + /// or release the lock immediately after this call returns.
Then why have this? Apparently there’s only a handful of users in the entire kernel?
> + pub fn is_locked(&self) -> bool {
> + // SAFETY: It's safe to call `ww_mutex_is_locked` on
> + // a valid mutex.
> + unsafe { bindings::ww_mutex_is_locked(self.inner.get()) }
> + }
> +
> + /// Locks this [`Mutex`] without [`AcquireCtx`].
> + pub fn lock(&self) -> Result<MutexGuard<'_, T>> {
> + lock_common(self, None, LockKind::Regular)
> + }
> +
> + /// Similar to [`Self::lock`], but can be interrupted by signals.
> + pub fn lock_interruptible(&self) -> Result<MutexGuard<'_, T>> {
> + lock_common(self, None, LockKind::Interruptible)
> + }
> +
> + /// Locks this [`Mutex`] without [`AcquireCtx`] using the slow path.
> + ///
> + /// This function should be used when [`Self::lock`] fails (typically due
> + /// to a potential deadlock).
> + pub fn lock_slow(&self) -> Result<MutexGuard<'_, T>> {
> + lock_common(self, None, LockKind::Slow)
> + }
> +
> + /// Similar to [`Self::lock_slow`], but can be interrupted by signals.
> + pub fn lock_slow_interruptible(&self) -> Result<MutexGuard<'_, T>> {
> + lock_common(self, None, LockKind::SlowInterruptible)
> + }
> +
> + /// Tries to lock this [`Mutex`] with no [`AcquireCtx`] and without blocking.
> + ///
> + /// Unlike [`Self::lock`], no deadlock handling is performed.
> + pub fn try_lock(&self) -> Result<MutexGuard<'_, T>> {
> + lock_common(self, None, LockKind::Try)
> + }
> +}
> +
> +/// A guard that provides exclusive access to the data protected
> +/// by a [`Mutex`].
> +///
> +/// # Invariants
> +///
> +/// The guard holds an exclusive lock on the associated [`Mutex`]. 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 MutexGuard<'a, T: ?Sized> {
> + mutex: &'a Mutex<'a, T>,
> + _not_send: NotThreadSafe,
> +}
> +
> +// SAFETY: `MutexGuard` can be shared between threads if the data can.
> +unsafe impl<T: ?Sized + Sync> Sync for MutexGuard<'_, T> {}
> +
> +impl<'a, T: ?Sized> MutexGuard<'a, T> {
> + /// Creates a new guard for the given [`Mutex`].
> + fn new(mutex: &'a Mutex<'a, T>) -> Self {
> + Self {
> + mutex,
> + _not_send: NotThreadSafe,
> + }
> + }
> +}
> +
> +impl<'a> MutexGuard<'a, ()> {
> + /// Creates a [`MutexGuard`] from a raw pointer.
> + ///
> + /// This function is intended for interoperability with C code.
> + ///
> + /// # Safety
> + ///
> + /// The caller must ensure that `ptr` is a valid pointer to a `ww_mutex`
> + /// and that it remains valid for the lifetime `'a`.
The caller must ensure that the ww_mutex is indeed locked, as an invariant of
Guards is that they’re acquired by locking the underlying synchronization
primitive.
> + pub unsafe fn from_raw<'b>(ptr: *mut bindings::ww_mutex) -> MutexGuard<'b, ()> {
> + // SAFETY: By the safety contract, the caller guarantees that `ptr`
> + // points to a valid `ww_mutex` which is the `mutex` field of `Mutex`
> + // and remains valid for the lifetime `'a`.
> + let mutex = unsafe { Mutex::from_raw(ptr) };
> +
> + MutexGuard::new(mutex)
> + }
> +}
> +
> +impl<T: ?Sized> core::ops::Deref for MutexGuard<'_, T> {
> + type Target = T;
> +
> + fn deref(&self) -> &Self::Target {
> + // SAFETY: We hold the lock, so we have exclusive access.
Not if you call from_raw() on an unlocked ww_mutex.
> + unsafe { &*self.mutex.data.get() }
> + }
> +}
> +
> +impl<T: ?Sized + Unpin> core::ops::DerefMut for MutexGuard<'_, T> {
> + fn deref_mut(&mut self) -> &mut Self::Target {
> + // SAFETY: We hold the lock, so we have exclusive access.
Same here.
> + unsafe { &mut *self.mutex.data.get() }
> + }
> +}
> +
> +impl<T: ?Sized> Drop for MutexGuard<'_, T> {
> + fn drop(&mut self) {
> + // SAFETY: We hold the lock and are about to release it.
> + unsafe { bindings::ww_mutex_unlock(self.mutex.inner.get()) };
Same here. Also, unlocking mutex that was not locked in the first place would
possibly have some unintended consequences.
> + }
> +}
> +
> +/// Locking kinds used by [`lock_common`] to unify the internal
> +/// locking logic.
> +///
> +/// It's best not to expose this type (and [`lock_common`]) to the
> +/// kernel, as it allows internal API changes without worrying
> +/// about breaking external compatibility.
> +#[derive(Copy, Clone, Debug)]
> +enum LockKind {
> + /// Blocks until lock is acquired.
> + Regular,
> + /// Blocks but can be interrupted by signals.
> + Interruptible,
> + /// Used in slow path after deadlock detection.
> + Slow,
> + /// Slow path but interruptible.
> + SlowInterruptible,
> + /// Does not block, returns immediately if busy.
> + Try,
> +}
> +
> +/// Internal helper that unifies the different locking kinds.
> +///
> +/// Returns [`EINVAL`] if the [`Mutex`] has a different [`Class`].
> +fn lock_common<'a, T: ?Sized>(
> + mutex: &'a Mutex<'a, T>,
> + ctx: Option<&AcquireCtx<'_>>,
> + kind: LockKind,
> +) -> Result<MutexGuard<'a, T>> {
> + let mutex_ptr = mutex.inner.get();
> +
> + let ctx_ptr = match ctx {
> + Some(acquire_ctx) => {
> + let ctx_ptr = acquire_ctx.inner.get();
> +
> + // SAFETY: `ctx_ptr` is a valid pointer for the entire
> + // lifetime of `ctx`.
> + let ctx_class = unsafe { (*ctx_ptr).ww_class };
> +
> + // SAFETY: `mutex_ptr` is a valid pointer for the entire
> + // lifetime of `mutex`.
> + let mutex_class = unsafe { (*mutex_ptr).ww_class };
> +
> + // `ctx` and `mutex` must use the same class.
> + if ctx_class != mutex_class {
IMHO, this is indeed better!
> + return Err(EINVAL);
> + }
> +
> + ctx_ptr
> + }
> + None => core::ptr::null_mut(),
> + };
> +
> + match kind {
> + LockKind::Regular => {
> + // SAFETY: `Mutex` is always pinned. If `AcquireCtx` is `Some`, it is pinned,
> + // if `None`, it is set to `core::ptr::null_mut()`. Both cases are safe.
> + let ret = unsafe { bindings::ww_mutex_lock(mutex_ptr, ctx_ptr) };
> +
> + to_result(ret)?;
> + }
> + LockKind::Interruptible => {
> + // SAFETY: `Mutex` is always pinned. If `AcquireCtx` is `Some`, it is pinned,
> + // if `None`, it is set to `core::ptr::null_mut()`. Both cases are safe.
> + let ret = unsafe { bindings::ww_mutex_lock_interruptible(mutex_ptr, ctx_ptr) };
> +
> + to_result(ret)?;
> + }
> + LockKind::Slow => {
> + // SAFETY: `Mutex` is always pinned. If `AcquireCtx` is `Some`, it is pinned,
> + // if `None`, it is set to `core::ptr::null_mut()`. Both cases are safe.
> + unsafe { bindings::ww_mutex_lock_slow(mutex_ptr, ctx_ptr) };
> + }
> + LockKind::SlowInterruptible => {
> + // SAFETY: `Mutex` is always pinned. If `AcquireCtx` is `Some`, it is pinned,
> + // if `None`, it is set to `core::ptr::null_mut()`. Both cases are safe.
> + let ret = unsafe { bindings::ww_mutex_lock_slow_interruptible(mutex_ptr, ctx_ptr) };
> +
> + to_result(ret)?;
> + }
> + LockKind::Try => {
> + // SAFETY: `Mutex` is always pinned. If `AcquireCtx` is `Some`, it is pinned,
> + // if `None`, it is set to `core::ptr::null_mut()`. Both cases are safe.
> + let ret = unsafe { bindings::ww_mutex_trylock(mutex_ptr, ctx_ptr) };
> +
> + if ret == 0 {
> + return Err(EBUSY);
> + } else {
> + to_result(ret)?;
> + }
> + }
> + };
> +
> + Ok(MutexGuard::new(mutex))
> +}
> +
> +#[kunit_tests(rust_kernel_ww_mutex)]
> +mod tests {
> + use crate::prelude::*;
> + use crate::sync::Arc;
> + use crate::{c_str, define_class};
> + use pin_init::stack_pin_init;
> +
> + use super::*;
> +
> + // A simple coverage on `define_class` macro.
> + define_class!(TEST_WOUND_WAIT_CLASS, wound_wait, c_str!("test"));
> + define_class!(TEST_WAIT_DIE_CLASS, wait_die, c_str!("test"));
> +
> + #[test]
> + fn test_ww_mutex_basic_lock_unlock() -> Result {
> + stack_pin_init!(let class = Class::new_wound_wait(c_str!("test")));
> +
> + let mutex = Arc::pin_init(Mutex::new(42, &class), GFP_KERNEL)?;
> +
> + let ctx = KBox::pin_init(AcquireCtx::new(&class), GFP_KERNEL)?;
> +
> + let guard = ctx.lock(&mutex)?;
> + assert_eq!(*guard, 42);
> +
> + // Drop the lock.
> + drop(guard);
> +
> + let mut guard = ctx.lock(&mutex)?;
> + *guard = 100;
> + assert_eq!(*guard, 100);
> +
> + Ok(())
> + }
> +
> + #[test]
> + fn test_ww_mutex_trylock() -> Result {
> + stack_pin_init!(let class = Class::new_wound_wait(c_str!("test")));
> +
> + let mutex = Arc::pin_init(Mutex::new(123, &class), GFP_KERNEL)?;
> +
> + let ctx = KBox::pin_init(AcquireCtx::new(&class), GFP_KERNEL)?;
> +
> + // `try_lock` on unlocked mutex should succeed.
> + let guard = ctx.try_lock(&mutex)?;
> + assert_eq!(*guard, 123);
> +
> + // Now it should fail immediately as it's already locked.
> + assert!(ctx.try_lock(&mutex).is_err());
> +
> + Ok(())
> + }
> +
> + #[test]
> + fn test_ww_mutex_is_locked() -> Result {
> + stack_pin_init!(let class = Class::new_wait_die(c_str!("test")));
> +
> + let mutex = Arc::pin_init(Mutex::new("hello", &class), GFP_KERNEL)?;
> +
> + let ctx = KBox::pin_init(AcquireCtx::new(&class), GFP_KERNEL)?;
> +
> + // Should not be locked initially.
> + assert!(!mutex.is_locked());
> +
> + let guard = ctx.lock(&mutex)?;
> + assert!(mutex.is_locked());
> +
> + drop(guard);
> + assert!(!mutex.is_locked());
> +
> + Ok(())
> + }
> +
> + #[test]
> + fn test_ww_acquire_context_done() -> Result {
> + stack_pin_init!(let class = Class::new_wound_wait(c_str!("test")));
> +
> + let mutex1 = Arc::pin_init(Mutex::new(1, &class), GFP_KERNEL)?;
> + let mutex2 = Arc::pin_init(Mutex::new(2, &class), GFP_KERNEL)?;
> +
> + let ctx = KBox::pin_init(AcquireCtx::new(&class), GFP_KERNEL)?;
> +
> + // Acquire multiple mutexes with the same context.
> + let guard1 = ctx.lock(&mutex1)?;
> + let guard2 = ctx.lock(&mutex2)?;
> +
> + assert_eq!(*guard1, 1);
> + assert_eq!(*guard2, 2);
> +
> + // SAFETY: It's called exactly once here and nowhere else.
> + unsafe { ctx.done() };
> +
> + // We shouldn't be able to lock once it's `done`.
> + assert!(ctx.lock(&mutex1).is_err());
> + assert!(ctx.lock(&mutex2).is_err());
> +
> + Ok(())
> + }
> +
> + #[test]
> + fn test_with_global_classes() -> Result {
> + let mutex1 = Arc::pin_init(Mutex::new(100, &TEST_WOUND_WAIT_CLASS), GFP_KERNEL)?;
> + let mutex2 = Arc::pin_init(Mutex::new(200, &TEST_WAIT_DIE_CLASS), GFP_KERNEL)?;
> +
> + let ww_ctx = KBox::pin_init(AcquireCtx::new(&TEST_WOUND_WAIT_CLASS), GFP_KERNEL)?;
> + let wd_ctx = KBox::pin_init(AcquireCtx::new(&TEST_WAIT_DIE_CLASS), GFP_KERNEL)?;
> +
> + let ww_guard = ww_ctx.lock(&mutex1)?;
> + let wd_guard = wd_ctx.lock(&mutex2)?;
> +
> + assert_eq!(*ww_guard, 100);
> + assert_eq!(*wd_guard, 200);
> +
> + assert!(mutex1.is_locked());
> + assert!(mutex2.is_locked());
> +
> + drop(ww_guard);
> + drop(wd_guard);
> +
> + assert!(!mutex1.is_locked());
> + assert!(!mutex2.is_locked());
> +
> + Ok(())
> + }
> +
> + #[test]
> + fn test_mutex_without_ctx() -> Result {
> + let mutex = Arc::pin_init(Mutex::new(100, &TEST_WOUND_WAIT_CLASS), GFP_KERNEL)?;
> + let guard = mutex.lock()?;
> +
> + assert_eq!(*guard, 100);
> + assert!(mutex.is_locked());
> +
> + drop(guard);
> +
> + assert!(!mutex.is_locked());
> +
> + Ok(())
> + }
> +}
> diff --git a/rust/kernel/sync/lock/ww_mutex/acquire_ctx.rs b/rust/kernel/sync/lock/ww_mutex/acquire_ctx.rs
> new file mode 100644
> index 000000000000..141300e849eb
> --- /dev/null
> +++ b/rust/kernel/sync/lock/ww_mutex/acquire_ctx.rs
> @@ -0,0 +1,172 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Provides [`AcquireCtx`] for managing multiple wound/wait
> +//! mutexes from the same [`Class`].
> +
> +use crate::bindings;
> +use crate::prelude::*;
> +use crate::types::Opaque;
> +
> +use core::marker::PhantomData;
> +
> +use super::{lock_common, Class, LockKind, Mutex, MutexGuard};
> +
> +/// Groups multiple [`Mutex`]es for deadlock avoidance when acquired
> +/// with the same [`Class`].
> +///
> +/// # Examples
> +///
> +/// ```
> +/// use kernel::sync::lock::ww_mutex::{Class, AcquireCtx, Mutex};
> +/// use kernel::c_str;
> +/// use kernel::sync::Arc;
> +/// use pin_init::stack_pin_init;
> +///
> +/// stack_pin_init!(let class = Class::new_wound_wait(c_str!("demo")));
> +///
> +/// // Create mutexes.
> +/// let mutex1 = Arc::pin_init(Mutex::new(1, &class), GFP_KERNEL)?;
> +/// let mutex2 = Arc::pin_init(Mutex::new(2, &class), GFP_KERNEL)?;
> +///
> +/// // Create acquire context for deadlock avoidance.
> +/// let ctx = KBox::pin_init(AcquireCtx::new(&class), GFP_KERNEL)?;
> +///
> +/// let guard1 = ctx.lock(&mutex1)?;
> +/// let guard2 = ctx.lock(&mutex2)?;
> +///
> +/// // Mark acquisition phase as complete.
> +/// // SAFETY: It's called exactly once here and nowhere else.
> +/// unsafe { ctx.done() };
> +///
> +/// # Ok::<(), Error>(())
> +/// ```
> +#[pin_data(PinnedDrop)]
> +#[repr(transparent)]
> +pub struct AcquireCtx<'a> {
> + #[pin]
> + pub(super) inner: Opaque<bindings::ww_acquire_ctx>,
> + _p: PhantomData<&'a Class>,
> +}
> +
> +impl<'class> AcquireCtx<'class> {
> + /// Initializes a new [`AcquireCtx`] with the given [`Class`].
> + pub fn new(class: &'class Class) -> impl PinInit<Self> {
> + let class_ptr = class.inner.get();
> + pin_init!(AcquireCtx {
> + inner <- Opaque::ffi_init(|slot: *mut bindings::ww_acquire_ctx| {
> + // SAFETY: `class` is valid for the lifetime `'class` captured
> + // by `AcquireCtx`.
> + unsafe { bindings::ww_acquire_init(slot, class_ptr) }
> + }),
> + _p: PhantomData
> + })
> + }
> +
> + /// Creates a [`AcquireCtx`] from a raw pointer.
> + ///
> + /// This function is intended for interoperability with C code.
> + ///
> + /// # Safety
> + ///
> + /// The caller must ensure that `ptr` is a valid pointer to the `inner` field
> + /// of [`AcquireCtx`] and that it remains valid for the lifetime `'a`.
> + pub unsafe fn from_raw<'a>(ptr: *mut bindings::ww_acquire_ctx) -> &'a Self {
> + // SAFETY: By the safety contract, `ptr` is valid to construct `AcquireCtx`.
> + unsafe { &*ptr.cast() }
> + }
> +
> + /// Marks the end of the acquire phase.
> + ///
> + /// Calling this function is optional. It is just useful to document
> + /// the code and clearly designated the acquire phase from actually
> + /// using the locked data structures.
> + ///
> + /// After calling this function, no more mutexes can be acquired with
> + /// this context.
> + ///
> + /// # Safety
> + ///
> + /// The caller must ensure that this function is called only once
> + /// and after calling it, no further mutexes are acquired using
> + /// this context.
> + pub unsafe fn done(&self) {
> + // SAFETY: By the safety contract, the caller guarantees that this
> + // function is called only once.
> + unsafe { bindings::ww_acquire_done(self.inner.get()) };
> + }
> +
> + /// Re-initializes the [`AcquireCtx`].
> + ///
> + /// Must be called after releasing all locks when [`EDEADLK`] occurs.
> + ///
> + /// # Safety
> + ///
> + /// The caller must ensure no locks are held in this [`AcquireCtx`].
> + pub unsafe fn reinit(self: Pin<&mut Self>) {
> + let ctx = self.inner.get();
> +
> + // SAFETY: `ww_class` is always a valid pointer in properly initialized
> + // `AcquireCtx`.
> + let class_ptr = unsafe { (*ctx).ww_class };
> +
> + // SAFETY:
> + // - Lifetime of any guard (which hold an immutable borrow of `self`) cannot overlap
> + // with the execution of this function. This enforces that all locks acquired via
> + // this context have been released.
> + //
> + // - `ctx` is guaranteed to be initialized because `ww_acquire_fini`
> + // can only be called from the `Drop` implementation.
> + //
> + // - `ww_acquire_fini` is safe to call on an initialized context.
> + unsafe { bindings::ww_acquire_fini(ctx) };
> +
> + // SAFETY: `ww_acquire_init` is safe to call with valid pointers
> + // to initialize an uninitialized context.
> + unsafe { bindings::ww_acquire_init(ctx, class_ptr) };
> + }
> +
> + /// Locks the given [`Mutex`] on this [`AcquireCtx`].
> + pub fn lock<'a, T>(&'a self, mutex: &'a Mutex<'a, T>) -> Result<MutexGuard<'a, T>> {
> + lock_common(mutex, Some(self), LockKind::Regular)
> + }
> +
> + /// Similar to [`Self::lock`], but can be interrupted by signals.
> + pub fn lock_interruptible<'a, T>(
> + &'a self,
> + mutex: &'a Mutex<'a, T>,
> + ) -> Result<MutexGuard<'a, T>> {
> + lock_common(mutex, Some(self), LockKind::Interruptible)
> + }
> +
> + /// Locks the given [`Mutex`] on this [`AcquireCtx`] using the slow path.
> + ///
> + /// This function should be used when [`Self::lock`] fails (typically due
> + /// to a potential deadlock).
> + pub fn lock_slow<'a, T>(&'a self, mutex: &'a Mutex<'a, T>) -> Result<MutexGuard<'a, T>> {
> + lock_common(mutex, Some(self), LockKind::Slow)
> + }
> +
> + /// Similar to [`Self::lock_slow`], but can be interrupted by signals.
> + pub fn lock_slow_interruptible<'a, T>(
> + &'a self,
> + mutex: &'a Mutex<'a, T>,
> + ) -> Result<MutexGuard<'a, T>> {
> + lock_common(mutex, Some(self), LockKind::SlowInterruptible)
> + }
> +
> + /// Tries to lock the [`Mutex`] on this [`AcquireCtx`] without blocking.
> + ///
> + /// Unlike [`Self::lock`], no deadlock handling is performed.
> + pub fn try_lock<'a, T>(&'a self, mutex: &'a Mutex<'a, T>) -> Result<MutexGuard<'a, T>> {
> + lock_common(mutex, Some(self), LockKind::Try)
> + }
> +}
> +
> +#[pinned_drop]
> +impl PinnedDrop for AcquireCtx<'_> {
> + fn drop(self: Pin<&mut Self>) {
> + // SAFETY: Given the lifetime bounds we know no locks are held,
> + // so calling `ww_acquire_fini` is safe.
> + unsafe { bindings::ww_acquire_fini(self.inner.get()) };
> + }
> +}
> --
> 2.51.2
>
>
I pointed out a few things, but IMHO this is starting to look pretty good :)
— Daniel
On Tue, 2 Dec 2025 15:29:02 -0300
Daniel Almeida <daniel.almeida@collabora.com> wrote:
> Hi Onur,
>
> > On 1 Dec 2025, at 07:28, Onur Özkan <work@onurozkan.dev> wrote:
> >
> > Covers the entire low-level locking API (lock, try_lock,
> > slow path, interruptible variants) and integration with
> > kernel bindings.
> >
> > Signed-off-by: Onur Özkan <work@onurozkan.dev>
> > ---
> > rust/kernel/sync/lock/ww_mutex.rs | 442
> > ++++++++++++++++++ rust/kernel/sync/lock/ww_mutex/acquire_ctx.rs |
> > 172 +++++++ 2 files changed, 614 insertions(+)
> > create mode 100644 rust/kernel/sync/lock/ww_mutex/acquire_ctx.rs
> >
> > diff --git a/rust/kernel/sync/lock/ww_mutex.rs
> > b/rust/kernel/sync/lock/ww_mutex.rs index
> > 727c51cc73af..00e25872a042 100644 ---
> > a/rust/kernel/sync/lock/ww_mutex.rs +++
> > b/rust/kernel/sync/lock/ww_mutex.rs @@ -1,7 +1,449 @@
> > // SPDX-License-Identifier: GPL-2.0
> >
> > //! Rust abstractions for the kernel's wound-wait locking
> > primitives. +//!
> > +//! It is designed to avoid deadlocks when locking multiple
> > [`Mutex`]es +//! that belong to the same [`Class`]. Each lock
> > acquisition uses an +//! [`AcquireCtx`] to track ordering and
> > ensure forward progress. +//!
> > +//! It is recommended to use [`LockSet`] as it provides safe
> > high-level +//! interface that automatically handles deadlocks,
> > retries and context +//! management.
>
> This will break the docs, since LockSet is introduced in the next
> commit. Perhaps add this last paragraph in the next commit?
>
Yeah I realized this as well. I will handle it in the next commit.
> >
> > +use crate::error::to_result;
> > +use crate::prelude::*;
> > +use crate::types::{NotThreadSafe, Opaque};
> > +use crate::{bindings, container_of};
> > +
> > +use core::cell::UnsafeCell;
> > +use core::marker::PhantomData;
> > +
> > +pub use acquire_ctx::AcquireCtx;
> > pub use class::Class;
> > +pub use lock_set::LockSet;
> >
> > +mod acquire_ctx;
> > mod class;
> > +mod lock_set;
> > +
> > +/// A wound-wait (ww) mutex that is powered with deadlock avoidance
> > +/// when acquiring multiple locks of the same [`Class`].
> > +///
> > +/// Each mutex belongs to a [`Class`], which the wound-wait
> > algorithm +/// uses to figure out the order of acquisition and
> > prevent deadlocks. +///
> > +/// # Examples
> > +///
> > +/// ```
> > +/// use kernel::c_str;
> > +/// use kernel::sync::Arc;
> > +/// use kernel::sync::lock::ww_mutex::{AcquireCtx, Class, Mutex};
> > +/// use pin_init::stack_pin_init;
> > +///
> > +/// stack_pin_init!(let class =
> > Class::new_wound_wait(c_str!("some_class"))); +/// let mutex =
> > Arc::pin_init(Mutex::new(42, &class), GFP_KERNEL)?; +///
> > +/// let ctx = KBox::pin_init(AcquireCtx::new(&class), GFP_KERNEL)?;
> > +///
> > +/// let guard = ctx.lock(&mutex)?;
> > +/// assert_eq!(*guard, 42);
> > +///
> > +/// # Ok::<(), Error>(())
> > +/// ```
> > +#[pin_data]
> > +#[repr(C)]
> > +pub struct Mutex<'a, T: ?Sized> {
> > + _p: PhantomData<&'a Class>,
>
> nit: can you keep this last? Let’s not start a #[repr(C)] struct with
> a ZST if we can help it.
>
We can't make it last, the size of `data` is unknown at compile time.
>
> > + #[pin]
> > + inner: Opaque<bindings::ww_mutex>,
> > + data: UnsafeCell<T>,
> > +}
> > +
> > +// SAFETY: `Mutex` can be sent to another thread if the protected
> > +// data `T` can be.
> > +unsafe impl<T: ?Sized + Send> Send for Mutex<'_, T> {}
> > +
> > +// SAFETY: `Mutex` can be shared across threads if the protected
> > +// data `T` can be.
> > +unsafe impl<T: ?Sized + Send + Sync> Sync for Mutex<'_, T> {}
> > +
>
> Foo and impl Foo together, please.
>
> > +impl<'class, T> Mutex<'class, T> {
> > + /// Initializes [`Mutex`] with the given `data` and [`Class`].
> > + pub fn new(data: T, class: &'class Class) -> impl
> > PinInit<Self> {
> > + let class_ptr = class.inner.get();
> > + pin_init!(Mutex {
> > + inner <- Opaque::ffi_init(|slot: *mut
> > bindings::ww_mutex| {
> > + // SAFETY: `class` is valid for the lifetime
> > `'class` captured by `Self`.
> > + unsafe { bindings::ww_mutex_init(slot, class_ptr) }
> > + }),
> > + data: UnsafeCell::new(data),
> > + _p: PhantomData
> > + })
> > + }
> > +}
> > +
> > +impl<'class> Mutex<'class, ()> {
> > + /// Creates a [`Mutex`] from a raw pointer.
> > + ///
> > + /// This function is intended for interoperability with C code.
> > + ///
> > + /// # Safety
> > + ///
> > + /// The caller must ensure that `ptr` is a valid pointer to a
> > `ww_mutex`
> > + /// and that it remains valid for the lifetime `'a`.
> > + pub unsafe fn from_raw<'a>(ptr: *mut bindings::ww_mutex) ->
> > &'a Self {
> > + // SAFETY: By the safety contract, the caller guarantees
> > that `ptr`
> > + // points to a valid `ww_mutex` which is the `inner` field
> > of `Mutex`
> > + // and remains valid for the lifetime `'a`.
> > + //
> > + // Because [`Mutex`] is `#[repr(C)]`, the `inner` field
> > sits at a
> > + // stable offset that `container_of!` can safely rely on.
> > + unsafe { &*container_of!(Opaque::cast_from(ptr), Self,
> > inner) }
> > + }
> > +}
>
> nit: can you move this implementation to be after the fully generic
> one?
>
> > +
> > +impl<'class, T: ?Sized> Mutex<'class, T> {
> > + /// Checks if this [`Mutex`] is currently locked.
> > + ///
> > + /// The returned value is racy as another thread can acquire
> > + /// or release the lock immediately after this call returns.
>
> Then why have this? Apparently there’s only a handful of users in the
> entire kernel?
>
I am fine with removing the pub. It's mainly useful for tests. I am not
sure about other users but if I recall correctly, you were the first
person to request making this pub (please correct me if I am
misremembering).
> > + pub fn is_locked(&self) -> bool {
> > + // SAFETY: It's safe to call `ww_mutex_is_locked` on
> > + // a valid mutex.
> > + unsafe { bindings::ww_mutex_is_locked(self.inner.get()) }
> > + }
> > +
> > + /// Locks this [`Mutex`] without [`AcquireCtx`].
> > + pub fn lock(&self) -> Result<MutexGuard<'_, T>> {
> > + lock_common(self, None, LockKind::Regular)
> > + }
> > +
> > + /// Similar to [`Self::lock`], but can be interrupted by
> > signals.
> > + pub fn lock_interruptible(&self) -> Result<MutexGuard<'_, T>> {
> > + lock_common(self, None, LockKind::Interruptible)
> > + }
> > +
> > + /// Locks this [`Mutex`] without [`AcquireCtx`] using the slow
> > path.
> > + ///
> > + /// This function should be used when [`Self::lock`] fails
> > (typically due
> > + /// to a potential deadlock).
> > + pub fn lock_slow(&self) -> Result<MutexGuard<'_, T>> {
> > + lock_common(self, None, LockKind::Slow)
> > + }
> > +
> > + /// Similar to [`Self::lock_slow`], but can be interrupted by
> > signals.
> > + pub fn lock_slow_interruptible(&self) -> Result<MutexGuard<'_,
> > T>> {
> > + lock_common(self, None, LockKind::SlowInterruptible)
> > + }
> > +
> > + /// Tries to lock this [`Mutex`] with no [`AcquireCtx`] and
> > without blocking.
> > + ///
> > + /// Unlike [`Self::lock`], no deadlock handling is performed.
> > + pub fn try_lock(&self) -> Result<MutexGuard<'_, T>> {
> > + lock_common(self, None, LockKind::Try)
> > + }
> > +}
> > +
> > +/// A guard that provides exclusive access to the data protected
> > +/// by a [`Mutex`].
> > +///
> > +/// # Invariants
> > +///
> > +/// The guard holds an exclusive lock on the associated [`Mutex`].
> > 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 MutexGuard<'a, T: ?Sized> {
> > + mutex: &'a Mutex<'a, T>,
> > + _not_send: NotThreadSafe,
> > +}
> > +
> > +// SAFETY: `MutexGuard` can be shared between threads if the data
> > can. +unsafe impl<T: ?Sized + Sync> Sync for MutexGuard<'_, T> {}
> > +
> > +impl<'a, T: ?Sized> MutexGuard<'a, T> {
> > + /// Creates a new guard for the given [`Mutex`].
> > + fn new(mutex: &'a Mutex<'a, T>) -> Self {
> > + Self {
> > + mutex,
> > + _not_send: NotThreadSafe,
> > + }
> > + }
> > +}
> > +
> > +impl<'a> MutexGuard<'a, ()> {
> > + /// Creates a [`MutexGuard`] from a raw pointer.
> > + ///
> > + /// This function is intended for interoperability with C code.
> > + ///
> > + /// # Safety
> > + ///
> > + /// The caller must ensure that `ptr` is a valid pointer to a
> > `ww_mutex`
> > + /// and that it remains valid for the lifetime `'a`.
>
> The caller must ensure that the ww_mutex is indeed locked, as an
> invariant of Guards is that they’re acquired by locking the
> underlying synchronization primitive.
>
Thanks for catching this!
I got an idea: How about using is_locked on the mutex we create and
return an error if it's not locked? This way we can avoid bloating the
safety contract.
> > + pub unsafe fn from_raw<'b>(ptr: *mut bindings::ww_mutex) ->
> > MutexGuard<'b, ()> {
> > + // SAFETY: By the safety contract, the caller guarantees
> > that `ptr`
> > + // points to a valid `ww_mutex` which is the `mutex` field
> > of `Mutex`
> > + // and remains valid for the lifetime `'a`.
> > + let mutex = unsafe { Mutex::from_raw(ptr) };
> > +
> > + MutexGuard::new(mutex)
> > + }
> > +}
> > +
> > +impl<T: ?Sized> core::ops::Deref for MutexGuard<'_, T> {
> > + type Target = T;
> > +
> > + fn deref(&self) -> &Self::Target {
> > + // SAFETY: We hold the lock, so we have exclusive access.
>
> Not if you call from_raw() on an unlocked ww_mutex.
>
Well, yeah... I can reword it as "self is locked". I wrote this part
way before I added from_raw functions, I must forgot to update comments
here.
> > + unsafe { &*self.mutex.data.get() }
> > + }
> > +}
> > +
> > +impl<T: ?Sized + Unpin> core::ops::DerefMut for MutexGuard<'_, T> {
> > + fn deref_mut(&mut self) -> &mut Self::Target {
> > + // SAFETY: We hold the lock, so we have exclusive access.
>
> Same here.
>
> > + unsafe { &mut *self.mutex.data.get() }
> > + }
> > +}
> > +
> > +impl<T: ?Sized> Drop for MutexGuard<'_, T> {
> > + fn drop(&mut self) {
> > + // SAFETY: We hold the lock and are about to release it.
> > + unsafe { bindings::ww_mutex_unlock(self.mutex.inner.get())
> > };
>
> Same here. Also, unlocking mutex that was not locked in the first
> place would possibly have some unintended consequences.
>
By using the is_locked approach in MutexGuard::from_raw, this would
never happen.
> > + }
> > +}
> > +
> > +/// Locking kinds used by [`lock_common`] to unify the internal
> > +/// locking logic.
> > +///
> > +/// It's best not to expose this type (and [`lock_common`]) to the
> > +/// kernel, as it allows internal API changes without worrying
> > +/// about breaking external compatibility.
> > +#[derive(Copy, Clone, Debug)]
> > +enum LockKind {
> > + /// Blocks until lock is acquired.
> > + Regular,
> > + /// Blocks but can be interrupted by signals.
> > + Interruptible,
> > + /// Used in slow path after deadlock detection.
> > + Slow,
> > + /// Slow path but interruptible.
> > + SlowInterruptible,
> > + /// Does not block, returns immediately if busy.
> > + Try,
> > +}
> > +
> > +/// Internal helper that unifies the different locking kinds.
> > +///
> > +/// Returns [`EINVAL`] if the [`Mutex`] has a different [`Class`].
> > +fn lock_common<'a, T: ?Sized>(
> > + mutex: &'a Mutex<'a, T>,
> > + ctx: Option<&AcquireCtx<'_>>,
> > + kind: LockKind,
> > +) -> Result<MutexGuard<'a, T>> {
> > + let mutex_ptr = mutex.inner.get();
> > +
> > + let ctx_ptr = match ctx {
> > + Some(acquire_ctx) => {
> > + let ctx_ptr = acquire_ctx.inner.get();
> > +
> > + // SAFETY: `ctx_ptr` is a valid pointer for the entire
> > + // lifetime of `ctx`.
> > + let ctx_class = unsafe { (*ctx_ptr).ww_class };
> > +
> > + // SAFETY: `mutex_ptr` is a valid pointer for the
> > entire
> > + // lifetime of `mutex`.
> > + let mutex_class = unsafe { (*mutex_ptr).ww_class };
> > +
> > + // `ctx` and `mutex` must use the same class.
> > + if ctx_class != mutex_class {
>
> IMHO, this is indeed better!
>
> > + return Err(EINVAL);
> > + }
> > +
> > + ctx_ptr
> > + }
> > + None => core::ptr::null_mut(),
> > + };
> > +
> > + match kind {
> > + LockKind::Regular => {
> > + // SAFETY: `Mutex` is always pinned. If `AcquireCtx`
> > is `Some`, it is pinned,
> > + // if `None`, it is set to `core::ptr::null_mut()`.
> > Both cases are safe.
> > + let ret = unsafe { bindings::ww_mutex_lock(mutex_ptr,
> > ctx_ptr) }; +
> > + to_result(ret)?;
> > + }
> > + LockKind::Interruptible => {
> > + // SAFETY: `Mutex` is always pinned. If `AcquireCtx`
> > is `Some`, it is pinned,
> > + // if `None`, it is set to `core::ptr::null_mut()`.
> > Both cases are safe.
> > + let ret = unsafe {
> > bindings::ww_mutex_lock_interruptible(mutex_ptr, ctx_ptr) }; +
> > + to_result(ret)?;
> > + }
> > + LockKind::Slow => {
> > + // SAFETY: `Mutex` is always pinned. If `AcquireCtx`
> > is `Some`, it is pinned,
> > + // if `None`, it is set to `core::ptr::null_mut()`.
> > Both cases are safe.
> > + unsafe { bindings::ww_mutex_lock_slow(mutex_ptr,
> > ctx_ptr) };
> > + }
> > + LockKind::SlowInterruptible => {
> > + // SAFETY: `Mutex` is always pinned. If `AcquireCtx`
> > is `Some`, it is pinned,
> > + // if `None`, it is set to `core::ptr::null_mut()`.
> > Both cases are safe.
> > + let ret = unsafe {
> > bindings::ww_mutex_lock_slow_interruptible(mutex_ptr, ctx_ptr) }; +
> > + to_result(ret)?;
> > + }
> > + LockKind::Try => {
> > + // SAFETY: `Mutex` is always pinned. If `AcquireCtx`
> > is `Some`, it is pinned,
> > + // if `None`, it is set to `core::ptr::null_mut()`.
> > Both cases are safe.
> > + let ret = unsafe {
> > bindings::ww_mutex_trylock(mutex_ptr, ctx_ptr) }; +
> > + if ret == 0 {
> > + return Err(EBUSY);
> > + } else {
> > + to_result(ret)?;
> > + }
> > + }
> > + };
> > +
> > + Ok(MutexGuard::new(mutex))
> > +}
> > +
> > +#[kunit_tests(rust_kernel_ww_mutex)]
> > +mod tests {
> > + use crate::prelude::*;
> > + use crate::sync::Arc;
> > + use crate::{c_str, define_class};
> > + use pin_init::stack_pin_init;
> > +
> > + use super::*;
> > +
> > + // A simple coverage on `define_class` macro.
> > + define_class!(TEST_WOUND_WAIT_CLASS, wound_wait,
> > c_str!("test"));
> > + define_class!(TEST_WAIT_DIE_CLASS, wait_die, c_str!("test"));
> > +
> > + #[test]
> > + fn test_ww_mutex_basic_lock_unlock() -> Result {
> > + stack_pin_init!(let class =
> > Class::new_wound_wait(c_str!("test"))); +
> > + let mutex = Arc::pin_init(Mutex::new(42, &class),
> > GFP_KERNEL)?; +
> > + let ctx = KBox::pin_init(AcquireCtx::new(&class),
> > GFP_KERNEL)?; +
> > + let guard = ctx.lock(&mutex)?;
> > + assert_eq!(*guard, 42);
> > +
> > + // Drop the lock.
> > + drop(guard);
> > +
> > + let mut guard = ctx.lock(&mutex)?;
> > + *guard = 100;
> > + assert_eq!(*guard, 100);
> > +
> > + Ok(())
> > + }
> > +
> > + #[test]
> > + fn test_ww_mutex_trylock() -> Result {
> > + stack_pin_init!(let class =
> > Class::new_wound_wait(c_str!("test"))); +
> > + let mutex = Arc::pin_init(Mutex::new(123, &class),
> > GFP_KERNEL)?; +
> > + let ctx = KBox::pin_init(AcquireCtx::new(&class),
> > GFP_KERNEL)?; +
> > + // `try_lock` on unlocked mutex should succeed.
> > + let guard = ctx.try_lock(&mutex)?;
> > + assert_eq!(*guard, 123);
> > +
> > + // Now it should fail immediately as it's already locked.
> > + assert!(ctx.try_lock(&mutex).is_err());
> > +
> > + Ok(())
> > + }
> > +
> > + #[test]
> > + fn test_ww_mutex_is_locked() -> Result {
> > + stack_pin_init!(let class =
> > Class::new_wait_die(c_str!("test"))); +
> > + let mutex = Arc::pin_init(Mutex::new("hello", &class),
> > GFP_KERNEL)?; +
> > + let ctx = KBox::pin_init(AcquireCtx::new(&class),
> > GFP_KERNEL)?; +
> > + // Should not be locked initially.
> > + assert!(!mutex.is_locked());
> > +
> > + let guard = ctx.lock(&mutex)?;
> > + assert!(mutex.is_locked());
> > +
> > + drop(guard);
> > + assert!(!mutex.is_locked());
> > +
> > + Ok(())
> > + }
> > +
> > + #[test]
> > + fn test_ww_acquire_context_done() -> Result {
> > + stack_pin_init!(let class =
> > Class::new_wound_wait(c_str!("test"))); +
> > + let mutex1 = Arc::pin_init(Mutex::new(1, &class),
> > GFP_KERNEL)?;
> > + let mutex2 = Arc::pin_init(Mutex::new(2, &class),
> > GFP_KERNEL)?; +
> > + let ctx = KBox::pin_init(AcquireCtx::new(&class),
> > GFP_KERNEL)?; +
> > + // Acquire multiple mutexes with the same context.
> > + let guard1 = ctx.lock(&mutex1)?;
> > + let guard2 = ctx.lock(&mutex2)?;
> > +
> > + assert_eq!(*guard1, 1);
> > + assert_eq!(*guard2, 2);
> > +
> > + // SAFETY: It's called exactly once here and nowhere else.
> > + unsafe { ctx.done() };
> > +
> > + // We shouldn't be able to lock once it's `done`.
> > + assert!(ctx.lock(&mutex1).is_err());
> > + assert!(ctx.lock(&mutex2).is_err());
> > +
> > + Ok(())
> > + }
> > +
> > + #[test]
> > + fn test_with_global_classes() -> Result {
> > + let mutex1 = Arc::pin_init(Mutex::new(100,
> > &TEST_WOUND_WAIT_CLASS), GFP_KERNEL)?;
> > + let mutex2 = Arc::pin_init(Mutex::new(200,
> > &TEST_WAIT_DIE_CLASS), GFP_KERNEL)?; +
> > + let ww_ctx =
> > KBox::pin_init(AcquireCtx::new(&TEST_WOUND_WAIT_CLASS),
> > GFP_KERNEL)?;
> > + let wd_ctx =
> > KBox::pin_init(AcquireCtx::new(&TEST_WAIT_DIE_CLASS), GFP_KERNEL)?;
> > +
> > + let ww_guard = ww_ctx.lock(&mutex1)?;
> > + let wd_guard = wd_ctx.lock(&mutex2)?;
> > +
> > + assert_eq!(*ww_guard, 100);
> > + assert_eq!(*wd_guard, 200);
> > +
> > + assert!(mutex1.is_locked());
> > + assert!(mutex2.is_locked());
> > +
> > + drop(ww_guard);
> > + drop(wd_guard);
> > +
> > + assert!(!mutex1.is_locked());
> > + assert!(!mutex2.is_locked());
> > +
> > + Ok(())
> > + }
> > +
> > + #[test]
> > + fn test_mutex_without_ctx() -> Result {
> > + let mutex = Arc::pin_init(Mutex::new(100,
> > &TEST_WOUND_WAIT_CLASS), GFP_KERNEL)?;
> > + let guard = mutex.lock()?;
> > +
> > + assert_eq!(*guard, 100);
> > + assert!(mutex.is_locked());
> > +
> > + drop(guard);
> > +
> > + assert!(!mutex.is_locked());
> > +
> > + Ok(())
> > + }
> > +}
> > diff --git a/rust/kernel/sync/lock/ww_mutex/acquire_ctx.rs
> > b/rust/kernel/sync/lock/ww_mutex/acquire_ctx.rs new file mode 100644
> > index 000000000000..141300e849eb
> > --- /dev/null
> > +++ b/rust/kernel/sync/lock/ww_mutex/acquire_ctx.rs
> > @@ -0,0 +1,172 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +//! Provides [`AcquireCtx`] for managing multiple wound/wait
> > +//! mutexes from the same [`Class`].
> > +
> > +use crate::bindings;
> > +use crate::prelude::*;
> > +use crate::types::Opaque;
> > +
> > +use core::marker::PhantomData;
> > +
> > +use super::{lock_common, Class, LockKind, Mutex, MutexGuard};
> > +
> > +/// Groups multiple [`Mutex`]es for deadlock avoidance when
> > acquired +/// with the same [`Class`].
> > +///
> > +/// # Examples
> > +///
> > +/// ```
> > +/// use kernel::sync::lock::ww_mutex::{Class, AcquireCtx, Mutex};
> > +/// use kernel::c_str;
> > +/// use kernel::sync::Arc;
> > +/// use pin_init::stack_pin_init;
> > +///
> > +/// stack_pin_init!(let class =
> > Class::new_wound_wait(c_str!("demo"))); +///
> > +/// // Create mutexes.
> > +/// let mutex1 = Arc::pin_init(Mutex::new(1, &class), GFP_KERNEL)?;
> > +/// let mutex2 = Arc::pin_init(Mutex::new(2, &class), GFP_KERNEL)?;
> > +///
> > +/// // Create acquire context for deadlock avoidance.
> > +/// let ctx = KBox::pin_init(AcquireCtx::new(&class), GFP_KERNEL)?;
> > +///
> > +/// let guard1 = ctx.lock(&mutex1)?;
> > +/// let guard2 = ctx.lock(&mutex2)?;
> > +///
> > +/// // Mark acquisition phase as complete.
> > +/// // SAFETY: It's called exactly once here and nowhere else.
> > +/// unsafe { ctx.done() };
> > +///
> > +/// # Ok::<(), Error>(())
> > +/// ```
> > +#[pin_data(PinnedDrop)]
> > +#[repr(transparent)]
> > +pub struct AcquireCtx<'a> {
> > + #[pin]
> > + pub(super) inner: Opaque<bindings::ww_acquire_ctx>,
> > + _p: PhantomData<&'a Class>,
> > +}
> > +
> > +impl<'class> AcquireCtx<'class> {
> > + /// Initializes a new [`AcquireCtx`] with the given [`Class`].
> > + pub fn new(class: &'class Class) -> impl PinInit<Self> {
> > + let class_ptr = class.inner.get();
> > + pin_init!(AcquireCtx {
> > + inner <- Opaque::ffi_init(|slot: *mut
> > bindings::ww_acquire_ctx| {
> > + // SAFETY: `class` is valid for the lifetime
> > `'class` captured
> > + // by `AcquireCtx`.
> > + unsafe { bindings::ww_acquire_init(slot,
> > class_ptr) }
> > + }),
> > + _p: PhantomData
> > + })
> > + }
> > +
> > + /// Creates a [`AcquireCtx`] from a raw pointer.
> > + ///
> > + /// This function is intended for interoperability with C code.
> > + ///
> > + /// # Safety
> > + ///
> > + /// The caller must ensure that `ptr` is a valid pointer to
> > the `inner` field
> > + /// of [`AcquireCtx`] and that it remains valid for the
> > lifetime `'a`.
> > + pub unsafe fn from_raw<'a>(ptr: *mut bindings::ww_acquire_ctx)
> > -> &'a Self {
> > + // SAFETY: By the safety contract, `ptr` is valid to
> > construct `AcquireCtx`.
> > + unsafe { &*ptr.cast() }
> > + }
> > +
> > + /// Marks the end of the acquire phase.
> > + ///
> > + /// Calling this function is optional. It is just useful to
> > document
> > + /// the code and clearly designated the acquire phase from
> > actually
> > + /// using the locked data structures.
> > + ///
> > + /// After calling this function, no more mutexes can be
> > acquired with
> > + /// this context.
> > + ///
> > + /// # Safety
> > + ///
> > + /// The caller must ensure that this function is called only
> > once
> > + /// and after calling it, no further mutexes are acquired using
> > + /// this context.
> > + pub unsafe fn done(&self) {
> > + // SAFETY: By the safety contract, the caller guarantees
> > that this
> > + // function is called only once.
> > + unsafe { bindings::ww_acquire_done(self.inner.get()) };
> > + }
> > +
> > + /// Re-initializes the [`AcquireCtx`].
> > + ///
> > + /// Must be called after releasing all locks when [`EDEADLK`]
> > occurs.
> > + ///
> > + /// # Safety
> > + ///
> > + /// The caller must ensure no locks are held in this
> > [`AcquireCtx`].
> > + pub unsafe fn reinit(self: Pin<&mut Self>) {
> > + let ctx = self.inner.get();
> > +
> > + // SAFETY: `ww_class` is always a valid pointer in
> > properly initialized
> > + // `AcquireCtx`.
> > + let class_ptr = unsafe { (*ctx).ww_class };
> > +
> > + // SAFETY:
> > + // - Lifetime of any guard (which hold an immutable
> > borrow of `self`) cannot overlap
> > + // with the execution of this function. This enforces
> > that all locks acquired via
> > + // this context have been released.
> > + //
> > + // - `ctx` is guaranteed to be initialized because
> > `ww_acquire_fini`
> > + // can only be called from the `Drop` implementation.
> > + //
> > + // - `ww_acquire_fini` is safe to call on an initialized
> > context.
> > + unsafe { bindings::ww_acquire_fini(ctx) };
> > +
> > + // SAFETY: `ww_acquire_init` is safe to call with valid
> > pointers
> > + // to initialize an uninitialized context.
> > + unsafe { bindings::ww_acquire_init(ctx, class_ptr) };
> > + }
> > +
> > + /// Locks the given [`Mutex`] on this [`AcquireCtx`].
> > + pub fn lock<'a, T>(&'a self, mutex: &'a Mutex<'a, T>) ->
> > Result<MutexGuard<'a, T>> {
> > + lock_common(mutex, Some(self), LockKind::Regular)
> > + }
> > +
> > + /// Similar to [`Self::lock`], but can be interrupted by
> > signals.
> > + pub fn lock_interruptible<'a, T>(
> > + &'a self,
> > + mutex: &'a Mutex<'a, T>,
> > + ) -> Result<MutexGuard<'a, T>> {
> > + lock_common(mutex, Some(self), LockKind::Interruptible)
> > + }
> > +
> > + /// Locks the given [`Mutex`] on this [`AcquireCtx`] using the
> > slow path.
> > + ///
> > + /// This function should be used when [`Self::lock`] fails
> > (typically due
> > + /// to a potential deadlock).
> > + pub fn lock_slow<'a, T>(&'a self, mutex: &'a Mutex<'a, T>) ->
> > Result<MutexGuard<'a, T>> {
> > + lock_common(mutex, Some(self), LockKind::Slow)
> > + }
> > +
> > + /// Similar to [`Self::lock_slow`], but can be interrupted by
> > signals.
> > + pub fn lock_slow_interruptible<'a, T>(
> > + &'a self,
> > + mutex: &'a Mutex<'a, T>,
> > + ) -> Result<MutexGuard<'a, T>> {
> > + lock_common(mutex, Some(self), LockKind::SlowInterruptible)
> > + }
> > +
> > + /// Tries to lock the [`Mutex`] on this [`AcquireCtx`] without
> > blocking.
> > + ///
> > + /// Unlike [`Self::lock`], no deadlock handling is performed.
> > + pub fn try_lock<'a, T>(&'a self, mutex: &'a Mutex<'a, T>) ->
> > Result<MutexGuard<'a, T>> {
> > + lock_common(mutex, Some(self), LockKind::Try)
> > + }
> > +}
> > +
> > +#[pinned_drop]
> > +impl PinnedDrop for AcquireCtx<'_> {
> > + fn drop(self: Pin<&mut Self>) {
> > + // SAFETY: Given the lifetime bounds we know no locks are
> > held,
> > + // so calling `ww_acquire_fini` is safe.
> > + unsafe { bindings::ww_acquire_fini(self.inner.get()) };
> > + }
> > +}
> > --
> > 2.51.2
> >
> >
>
> I pointed out a few things, but IMHO this is starting to look pretty
> good :)
>
> — Daniel
Thanks for the feedback. Yeah, it's getting better and better in each
iteration :)
Regards,
Onur
Hi Onur,
kernel test robot noticed the following build errors:
[auto build test ERROR on tip/locking/core]
[also build test ERROR on rust/rust-next linus/master v6.18 next-20251201]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Onur-zkan/rust-add-C-wrappers-for-ww_mutex-inline-functions/20251201-184152
base: tip/locking/core
patch link: https://lore.kernel.org/r/20251201102855.4413-6-work%40onurozkan.dev
patch subject: [PATCH v8 5/6] rust: ww_mutex: add Mutex, AcquireCtx and MutexGuard
config: arm64-randconfig-001-20251202 (https://download.01.org/0day-ci/archive/20251202/202512020943.whFrDsXx-lkp@intel.com/config)
compiler: clang version 22.0.0git (https://github.com/llvm/llvm-project b3428bb966f1de8aa48375ffee0eba04ede133b7)
rustc: rustc 1.88.0 (6b00bc388 2025-06-23)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251202/202512020943.whFrDsXx-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202512020943.whFrDsXx-lkp@intel.com/
All errors (new ones prefixed by >>):
>> error[E0609]: no field `ww_class` on type `ww_acquire_ctx`
--> rust/kernel/sync/lock/ww_mutex/acquire_ctx.rs:110:41
|
110 | let class_ptr = unsafe { (*ctx).ww_class };
| ^^^^^^^^ unknown field
|
= note: available field is: `_address`
--
>> error[E0609]: no field `ww_class` on type `ww_acquire_ctx`
--> rust/kernel/sync/lock/ww_mutex.rs:252:49
|
252 | let ctx_class = unsafe { (*ctx_ptr).ww_class };
| ^^^^^^^^ unknown field
|
= note: available field is: `_address`
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
On Tue, 2 Dec 2025 09:49:34 +0800
kernel test robot <lkp@intel.com> wrote:
> Hi Onur,
>
> kernel test robot noticed the following build errors:
>
> [auto build test ERROR on tip/locking/core]
> [also build test ERROR on rust/rust-next linus/master v6.18
> next-20251201] [If your patch is applied to the wrong git tree,
> kindly drop us a note. And when submitting patch, we suggest to use
> '--base' as documented in
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>
> url:
> https://github.com/intel-lab-lkp/linux/commits/Onur-zkan/rust-add-C-wrappers-for-ww_mutex-inline-functions/20251201-184152
> base: tip/locking/core patch link:
> https://lore.kernel.org/r/20251201102855.4413-6-work%40onurozkan.dev
> patch subject: [PATCH v8 5/6] rust: ww_mutex: add Mutex, AcquireCtx
> and MutexGuard config: arm64-randconfig-001-20251202
> (https://download.01.org/0day-ci/archive/20251202/202512020943.whFrDsXx-lkp@intel.com/config)
> compiler: clang version 22.0.0git
> (https://github.com/llvm/llvm-project
> b3428bb966f1de8aa48375ffee0eba04ede133b7) rustc: rustc 1.88.0
> (6b00bc388 2025-06-23) reproduce (this is a W=1 build):
> (https://download.01.org/0day-ci/archive/20251202/202512020943.whFrDsXx-lkp@intel.com/reproduce)
>
> If you fix the issue in a separate patch/commit (i.e. not just a new
> version of the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes:
> https://lore.kernel.org/oe-kbuild-all/202512020943.whFrDsXx-lkp@intel.com/
>
> All errors (new ones prefixed by >>):
>
> >> error[E0609]: no field `ww_class` on type `ww_acquire_ctx`
> --> rust/kernel/sync/lock/ww_mutex/acquire_ctx.rs:110:41
> |
> 110 | let class_ptr = unsafe { (*ctx).ww_class };
> | ^^^^^^^^ unknown field
> |
> = note: available field is: `_address`
> --
> >> error[E0609]: no field `ww_class` on type `ww_acquire_ctx`
> --> rust/kernel/sync/lock/ww_mutex.rs:252:49
> |
> 252 | let ctx_class = unsafe { (*ctx_ptr).ww_class };
> | ^^^^^^^^ unknown
> field |
> = note: available field is: `_address`
>
I got a different error:
CLIPPY L rust/kernel.o
error[E0583]: file not found for module `lock_set`
--> rust/kernel/sync/lock/ww_mutex.rs:27:1
|
27 | mod lock_set;
| ^^^^^^^^^^^^^
|
= help: to create the module `lock_set`, create file
"rust/kernel/sync/lock/ww_mutex/lock_set.rs" or
"rust/kernel/sync/lock/ww_mutex/lock_set/mod.rs" = note: if there is
a `mod lock_set` elsewhere in the crate already, import it with `use
crate::...` instead
I will fix this in the next version. It only appears in the 5th
patch of the series.
But I have no idea about the "no field `ww_class`" error yet. We have
this [1] change so I assume something didn't work properly with the
bindgen generation? I will look more into that later.
[1]: https://github.com/intel-lab-lkp/linux/commit/23ba7e6a3f593455a
-Onur
© 2016 - 2026 Red Hat, Inc.