* [PATCH v8 0/6] rust: add ww_mutex support
@ 2025-12-01 10:28 Onur Özkan
2025-12-01 10:28 ` [PATCH v8 1/6] rust: add C wrappers for ww_mutex inline functions Onur Özkan
` (5 more replies)
0 siblings, 6 replies; 24+ messages in thread
From: Onur Özkan @ 2025-12-01 10:28 UTC (permalink / raw)
To: rust-for-linux
Cc: lossin, lyude, ojeda, alex.gaynor, boqun.feng, gary, a.hindborg,
aliceryhl, tmgross, dakr, peterz, mingo, will, longman,
felipe_life, daniel, daniel.almeida, thomas.hellstrom,
linux-kernel, Onur Özkan
I would like to request becoming a maintainer of this work.
If this is acceptable, please let me know so that I can add
myself to the MAINTAINERS file in the next revision.
See the changes of this version below.
Changes in v8:
- Various minor updates (e.g., doc updates, removing unnecessary
lifetime annotations, etc.).
- Changed C implementation to always provide ww_class field in
ww_mutex and ww_acquire_ctx types so we can access it from Rust
easily.
- Locking functions are now safe to call.
- Added class validation logic to ww_mutex::lock_common.
Changes in v7:
- Split Class and AcquireCtx into separate modules.
- Removed "Ww" prefixes from type names.
- Renamed exec.rs -> lock_set.rs and ExecContext -> LockSet.
- Removed transmute logic from LockSet (formerly ExecContext).
- Improved various doc-comments.
- Marked certain AcquireCtx functions as unsafe.
- Added from_raw constructors for Mutex, MutexGuard, AcquireCtx
and Class.
- LockSet::cleanup_on_deadlock no longer triggers reallocations when
reinitializing AcquireCtx.
- Incorporated various minor improvements suggested on the v6 series.
Changes in v6:
- Added `unpinned_new` constructor for `WwClass` and updated
global macros.
- Changed all tests (and docs) to use Arc/KBox instead of
`stack_pin_init` for `WwMutex` and `WwAcquireCtx`.
- Added `LockKind` and `lock_common` helper to unify locking logic.
- Added context-based and context-free locking functions for `WwMutex`.
- Added `ww_mutex/exec` module, a high-level API with auto `EDEADLK`
handling mechanism.
Changes in v5:
- Addressed documentation review notes.
- Removed `unwrap()`s in examples and KUnit tests.
Onur Özkan (6):
rust: add C wrappers for ww_mutex inline functions
ww_mutex: add `ww_class` field unconditionally
rust: error: add EDEADLK
rust: implement Class for ww_class support
rust: ww_mutex: add Mutex, AcquireCtx and MutexGuard
rust: ww_mutex: implement LockSet
MAINTAINERS | 1 +
include/linux/ww_mutex.h | 8 +-
rust/helpers/helpers.c | 1 +
rust/helpers/ww_mutex.c | 39 ++
rust/kernel/error.rs | 1 +
rust/kernel/sync/lock.rs | 1 +
rust/kernel/sync/lock/ww_mutex.rs | 449 ++++++++++++++++++
rust/kernel/sync/lock/ww_mutex/acquire_ctx.rs | 172 +++++++
rust/kernel/sync/lock/ww_mutex/class.rs | 156 ++++++
rust/kernel/sync/lock/ww_mutex/lock_set.rs | 383 +++++++++++++++
10 files changed, 1205 insertions(+), 6 deletions(-)
create mode 100644 rust/helpers/ww_mutex.c
create mode 100644 rust/kernel/sync/lock/ww_mutex.rs
create mode 100644 rust/kernel/sync/lock/ww_mutex/acquire_ctx.rs
create mode 100644 rust/kernel/sync/lock/ww_mutex/class.rs
create mode 100644 rust/kernel/sync/lock/ww_mutex/lock_set.rs
--
2.51.2
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v8 1/6] rust: add C wrappers for ww_mutex inline functions
2025-12-01 10:28 [PATCH v8 0/6] rust: add ww_mutex support Onur Özkan
@ 2025-12-01 10:28 ` Onur Özkan
2025-12-02 17:38 ` Daniel Almeida
2025-12-01 10:28 ` [PATCH v8 2/6] ww_mutex: add `ww_class` field unconditionally Onur Özkan
` (4 subsequent siblings)
5 siblings, 1 reply; 24+ messages in thread
From: Onur Özkan @ 2025-12-01 10:28 UTC (permalink / raw)
To: rust-for-linux
Cc: lossin, lyude, ojeda, alex.gaynor, boqun.feng, gary, a.hindborg,
aliceryhl, tmgross, dakr, peterz, mingo, will, longman,
felipe_life, daniel, daniel.almeida, thomas.hellstrom,
linux-kernel, Onur Özkan
Some of the kernel's `ww_mutex` functions are implemented as
`static inline`, so they are inaccessible from Rust as bindgen
can't generate code on them. This patch provides C function wrappers
around these inline implementations, so bindgen can see them and generate
the corresponding Rust code.
Signed-off-by: Onur Özkan <work@onurozkan.dev>
---
MAINTAINERS | 1 +
rust/helpers/helpers.c | 1 +
rust/helpers/ww_mutex.c | 39 +++++++++++++++++++++++++++++++++++++++
3 files changed, 41 insertions(+)
create mode 100644 rust/helpers/ww_mutex.c
diff --git a/MAINTAINERS b/MAINTAINERS
index e8f06145fb54..bc6ea9cc1588 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14546,6 +14546,7 @@ F: kernel/locking/
F: lib/locking*.[ch]
F: rust/helpers/mutex.c
F: rust/helpers/spinlock.c
+F: rust/helpers/ww_mutex.c
F: rust/kernel/sync/lock.rs
F: rust/kernel/sync/lock/
F: rust/kernel/sync/locked_by.rs
diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c
index 551da6c9b506..58cf14d74516 100644
--- a/rust/helpers/helpers.c
+++ b/rust/helpers/helpers.c
@@ -59,4 +59,5 @@
#include "vmalloc.c"
#include "wait.c"
#include "workqueue.c"
+#include "ww_mutex.c"
#include "xarray.c"
diff --git a/rust/helpers/ww_mutex.c b/rust/helpers/ww_mutex.c
new file mode 100644
index 000000000000..61a487653394
--- /dev/null
+++ b/rust/helpers/ww_mutex.c
@@ -0,0 +1,39 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/ww_mutex.h>
+
+void rust_helper_ww_mutex_init(struct ww_mutex *lock, struct ww_class *ww_class)
+{
+ ww_mutex_init(lock, ww_class);
+}
+
+void rust_helper_ww_acquire_init(struct ww_acquire_ctx *ctx, struct ww_class *ww_class)
+{
+ ww_acquire_init(ctx, ww_class);
+}
+
+void rust_helper_ww_acquire_done(struct ww_acquire_ctx *ctx)
+{
+ ww_acquire_done(ctx);
+}
+
+void rust_helper_ww_acquire_fini(struct ww_acquire_ctx *ctx)
+{
+ ww_acquire_fini(ctx);
+}
+
+void rust_helper_ww_mutex_lock_slow(struct ww_mutex *lock, struct ww_acquire_ctx *ctx)
+{
+ ww_mutex_lock_slow(lock, ctx);
+}
+
+int rust_helper_ww_mutex_lock_slow_interruptible(struct ww_mutex *lock, struct ww_acquire_ctx *ctx)
+{
+ return ww_mutex_lock_slow_interruptible(lock, ctx);
+}
+
+bool rust_helper_ww_mutex_is_locked(struct ww_mutex *lock)
+{
+ return ww_mutex_is_locked(lock);
+}
+
--
2.51.2
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v8 2/6] ww_mutex: add `ww_class` field unconditionally
2025-12-01 10:28 [PATCH v8 0/6] rust: add ww_mutex support Onur Özkan
2025-12-01 10:28 ` [PATCH v8 1/6] rust: add C wrappers for ww_mutex inline functions Onur Özkan
@ 2025-12-01 10:28 ` Onur Özkan
2025-12-02 17:42 ` Daniel Almeida
2025-12-01 10:28 ` [PATCH v8 3/6] rust: error: add EDEADLK Onur Özkan
` (3 subsequent siblings)
5 siblings, 1 reply; 24+ messages in thread
From: Onur Özkan @ 2025-12-01 10:28 UTC (permalink / raw)
To: rust-for-linux
Cc: lossin, lyude, ojeda, alex.gaynor, boqun.feng, gary, a.hindborg,
aliceryhl, tmgross, dakr, peterz, mingo, will, longman,
felipe_life, daniel, daniel.almeida, thomas.hellstrom,
linux-kernel, Onur Özkan
Removes `DEBUG_WW_MUTEXES` guard on ww_class field
from ww_acquire_ctx and ww_mutex.
This helps us to write much safer Rust abstraction
in very simple way.
Link: https://lore.kernel.org/all/ECC0425A-8B18-4626-8EA8-2F843C45E0A1@collabora.com/
Signed-off-by: Onur Özkan <work@onurozkan.dev>
---
include/linux/ww_mutex.h | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/include/linux/ww_mutex.h b/include/linux/ww_mutex.h
index 45ff6f7a872b..c9835035b6d6 100644
--- a/include/linux/ww_mutex.h
+++ b/include/linux/ww_mutex.h
@@ -47,9 +47,7 @@ struct ww_class {
struct ww_mutex {
struct WW_MUTEX_BASE base;
struct ww_acquire_ctx *ctx;
-#ifdef DEBUG_WW_MUTEXES
struct ww_class *ww_class;
-#endif
};
struct ww_acquire_ctx {
@@ -58,9 +56,9 @@ struct ww_acquire_ctx {
unsigned int acquired;
unsigned short wounded;
unsigned short is_wait_die;
+ struct ww_class *ww_class;
#ifdef DEBUG_WW_MUTEXES
unsigned int done_acquire;
- struct ww_class *ww_class;
void *contending_lock;
#endif
#ifdef CONFIG_DEBUG_LOCK_ALLOC
@@ -110,9 +108,7 @@ static inline void ww_mutex_init(struct ww_mutex *lock,
{
ww_mutex_base_init(&lock->base, ww_class->mutex_name, &ww_class->mutex_key);
lock->ctx = NULL;
-#ifdef DEBUG_WW_MUTEXES
lock->ww_class = ww_class;
-#endif
}
/**
@@ -147,8 +143,8 @@ static inline void ww_acquire_init(struct ww_acquire_ctx *ctx,
ctx->acquired = 0;
ctx->wounded = false;
ctx->is_wait_die = ww_class->is_wait_die;
-#ifdef DEBUG_WW_MUTEXES
ctx->ww_class = ww_class;
+#ifdef DEBUG_WW_MUTEXES
ctx->done_acquire = 0;
ctx->contending_lock = NULL;
#endif
--
2.51.2
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v8 3/6] rust: error: add EDEADLK
2025-12-01 10:28 [PATCH v8 0/6] rust: add ww_mutex support Onur Özkan
2025-12-01 10:28 ` [PATCH v8 1/6] rust: add C wrappers for ww_mutex inline functions Onur Özkan
2025-12-01 10:28 ` [PATCH v8 2/6] ww_mutex: add `ww_class` field unconditionally Onur Özkan
@ 2025-12-01 10:28 ` Onur Özkan
2025-12-02 17:43 ` Daniel Almeida
2025-12-01 10:28 ` [PATCH v8 4/6] rust: implement Class for ww_class support Onur Özkan
` (2 subsequent siblings)
5 siblings, 1 reply; 24+ messages in thread
From: Onur Özkan @ 2025-12-01 10:28 UTC (permalink / raw)
To: rust-for-linux
Cc: lossin, lyude, ojeda, alex.gaynor, boqun.feng, gary, a.hindborg,
aliceryhl, tmgross, dakr, peterz, mingo, will, longman,
felipe_life, daniel, daniel.almeida, thomas.hellstrom,
linux-kernel, Onur Özkan
This is needed for the ww_mutex implementation so
we can handle EDEADLK on lock attempts.
Signed-off-by: Onur Özkan <work@onurozkan.dev>
---
rust/kernel/error.rs | 1 +
1 file changed, 1 insertion(+)
diff --git a/rust/kernel/error.rs b/rust/kernel/error.rs
index 1c0e0e241daa..9bf1072cfe19 100644
--- a/rust/kernel/error.rs
+++ b/rust/kernel/error.rs
@@ -66,6 +66,7 @@ macro_rules! declare_err {
declare_err!(EPIPE, "Broken pipe.");
declare_err!(EDOM, "Math argument out of domain of func.");
declare_err!(ERANGE, "Math result not representable.");
+ declare_err!(EDEADLK, "Resource deadlock avoided.");
declare_err!(EOVERFLOW, "Value too large for defined data type.");
declare_err!(ETIMEDOUT, "Connection timed out.");
declare_err!(ERESTARTSYS, "Restart the system call.");
--
2.51.2
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v8 4/6] rust: implement Class for ww_class support
2025-12-01 10:28 [PATCH v8 0/6] rust: add ww_mutex support Onur Özkan
` (2 preceding siblings ...)
2025-12-01 10:28 ` [PATCH v8 3/6] rust: error: add EDEADLK Onur Özkan
@ 2025-12-01 10:28 ` Onur Özkan
2025-12-02 17:59 ` Daniel Almeida
2025-12-03 13:10 ` Alice Ryhl
2025-12-01 10:28 ` [PATCH v8 5/6] rust: ww_mutex: add Mutex, AcquireCtx and MutexGuard Onur Özkan
2025-12-01 10:28 ` [PATCH v8 6/6] rust: ww_mutex: implement LockSet Onur Özkan
5 siblings, 2 replies; 24+ messages in thread
From: Onur Özkan @ 2025-12-01 10:28 UTC (permalink / raw)
To: rust-for-linux
Cc: lossin, lyude, ojeda, alex.gaynor, boqun.feng, gary, a.hindborg,
aliceryhl, tmgross, dakr, peterz, mingo, will, longman,
felipe_life, daniel, daniel.almeida, thomas.hellstrom,
linux-kernel, Onur Özkan
Adds the Class type, the first step in supporting
ww_mutex in Rust. Class represents ww_class, used
for deadlock avoidance for supporting both wait-die
and wound-wait semantics.
Also adds the define_class macro for safely declaring
static instances.
Signed-off-by: Onur Özkan <work@onurozkan.dev>
---
rust/kernel/sync/lock.rs | 1 +
rust/kernel/sync/lock/ww_mutex.rs | 7 ++
rust/kernel/sync/lock/ww_mutex/class.rs | 156 ++++++++++++++++++++++++
3 files changed, 164 insertions(+)
create mode 100644 rust/kernel/sync/lock/ww_mutex.rs
create mode 100644 rust/kernel/sync/lock/ww_mutex/class.rs
diff --git a/rust/kernel/sync/lock.rs b/rust/kernel/sync/lock.rs
index 27202beef90c..5b320c2b28c1 100644
--- a/rust/kernel/sync/lock.rs
+++ b/rust/kernel/sync/lock.rs
@@ -15,6 +15,7 @@
pub mod mutex;
pub mod spinlock;
+pub mod ww_mutex;
pub(super) mod global;
pub use global::{GlobalGuard, GlobalLock, GlobalLockBackend, GlobalLockedBy};
diff --git a/rust/kernel/sync/lock/ww_mutex.rs b/rust/kernel/sync/lock/ww_mutex.rs
new file mode 100644
index 000000000000..727c51cc73af
--- /dev/null
+++ b/rust/kernel/sync/lock/ww_mutex.rs
@@ -0,0 +1,7 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Rust abstractions for the kernel's wound-wait locking primitives.
+
+pub use class::Class;
+
+mod class;
diff --git a/rust/kernel/sync/lock/ww_mutex/class.rs b/rust/kernel/sync/lock/ww_mutex/class.rs
new file mode 100644
index 000000000000..b0753093f1e0
--- /dev/null
+++ b/rust/kernel/sync/lock/ww_mutex/class.rs
@@ -0,0 +1,156 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Provides [`Class`] to group wound/wait mutexes to be acquired together
+//! and specifies which deadlock avoidance algorithm to use (e.g., wound-wait
+//! or wait-die).
+//!
+//! The [`define_class`] macro and [`Class::new_wait_die`]/[`Class::new_wound_wait`]
+//! constructors provide safe ways to create classes.
+
+use crate::bindings;
+use crate::prelude::*;
+use crate::types::Opaque;
+
+/// Creates static [`Class`] instances.
+///
+/// # Examples
+///
+/// ```
+/// use kernel::{c_str, define_class};
+///
+/// define_class!(WOUND_WAIT_GLOBAL_CLASS, wound_wait, c_str!("wound_wait_global_class"));
+/// define_class!(WAIT_DIE_GLOBAL_CLASS, wait_die, c_str!("wait_die_global_class"));
+/// ```
+#[macro_export]
+macro_rules! define_class {
+ ($name:ident, wound_wait, $class_name:expr) => {
+ static $name: $crate::sync::lock::ww_mutex::Class =
+ // SAFETY: This is `static`, so address is fixed and won't move.
+ unsafe { $crate::sync::lock::ww_mutex::Class::unpinned_new($class_name, false) };
+ };
+ ($name:ident, wait_die, $class_name:expr) => {
+ static $name: $crate::sync::lock::ww_mutex::Class =
+ // SAFETY: This is `static`, so address is fixed and won't move.
+ unsafe { $crate::sync::lock::ww_mutex::Class::unpinned_new($class_name, true) };
+ };
+}
+
+/// Used to group mutexes together for deadlock avoidance.
+///
+/// All mutexes that might be acquired together should use the same class.
+///
+/// # Examples
+///
+/// ```
+/// use kernel::sync::lock::ww_mutex::Class;
+/// use kernel::c_str;
+/// use pin_init::stack_pin_init;
+///
+/// stack_pin_init!(let _wait_die_class = Class::new_wait_die(c_str!("some_class")));
+/// stack_pin_init!(let _wound_wait_class = Class::new_wound_wait(c_str!("some_other_class")));
+///
+/// # Ok::<(), Error>(())
+/// ```
+#[pin_data]
+#[repr(transparent)]
+pub struct Class {
+ #[pin]
+ pub(super) inner: Opaque<bindings::ww_class>,
+}
+
+// SAFETY: [`Class`] is set up once and never modified. It's fine to share it across threads.
+unsafe impl Sync for Class {}
+// SAFETY: Doesn't hold anything thread-specific. It's safe to send to other threads.
+unsafe impl Send for Class {}
+
+impl Class {
+ /// Creates an unpinned [`Class`].
+ ///
+ /// # Safety
+ ///
+ /// Caller must guarantee that the returned value is not moved after creation.
+ pub const unsafe fn unpinned_new(name: &'static CStr, is_wait_die: bool) -> Self {
+ Class {
+ inner: Opaque::new(bindings::ww_class {
+ stamp: bindings::atomic_long_t { counter: 0 },
+ acquire_name: name.as_char_ptr(),
+ mutex_name: name.as_char_ptr(),
+ is_wait_die: is_wait_die as u32,
+ // TODO: Replace with `bindings::lock_class_key::default()` once
+ // stabilized for `const`.
+ //
+ // SAFETY: This is always zero-initialized when defined with
+ // `DEFINE_WD_CLASS` globally on C side.
+ //
+ // For reference, see __WW_CLASS_INITIALIZER() in
+ // "include/linux/ww_mutex.h".
+ acquire_key: unsafe { core::mem::zeroed() },
+ // TODO: Replace with `bindings::lock_class_key::default()` once
+ // stabilized for `const`.
+ //
+ // SAFETY: This is always zero-initialized when defined with
+ // `DEFINE_WD_CLASS` globally on C side.
+ //
+ // For reference, see __WW_CLASS_INITIALIZER() in
+ // "include/linux/ww_mutex.h".
+ mutex_key: unsafe { core::mem::zeroed() },
+ }),
+ }
+ }
+
+ /// Creates a [`Class`].
+ ///
+ /// You should not use this function directly. Use the [`define_class!`]
+ /// macro or call [`Class::new_wait_die`] or [`Class::new_wound_wait`] instead.
+ fn new(name: &'static CStr, is_wait_die: bool) -> impl PinInit<Self> {
+ pin_init! {
+ Self {
+ inner <- Opaque::ffi_init(|slot: *mut bindings::ww_class| {
+ // SAFETY: The fields are being initialized. The `name` pointer is valid for a
+ // static lifetime. The keys are zeroed, which is what the C side does.
+ unsafe {
+ slot.write(bindings::ww_class {
+ stamp: bindings::atomic_long_t { counter: 0 },
+ acquire_name: name.as_char_ptr(),
+ mutex_name: name.as_char_ptr(),
+ is_wait_die: is_wait_die.into(),
+ // TODO: Replace with `bindings::lock_class_key::default()` once
+ // stabilized for `const`.
+ //
+ // SAFETY: This is always zero-initialized when defined with
+ // `DEFINE_WD_CLASS` globally on C side.
+ //
+ // For reference, see __WW_CLASS_INITIALIZER() in
+ // "include/linux/ww_mutex.h".
+ acquire_key: core::mem::zeroed(),
+ mutex_key: core::mem::zeroed(),
+ });
+ }
+ }),
+ }
+ }
+ }
+
+ /// Creates wait-die [`Class`].
+ pub fn new_wait_die(name: &'static CStr) -> impl PinInit<Self> {
+ Self::new(name, true)
+ }
+
+ /// Creates wound-wait [`Class`].
+ pub fn new_wound_wait(name: &'static CStr) -> impl PinInit<Self> {
+ Self::new(name, false)
+ }
+
+ /// Creates a [`Class`] from a raw pointer.
+ ///
+ /// This function is intended for interoperability with C code.
+ ///
+ /// # Safety
+ ///
+ /// The caller must ensure that `ptr` points to the `inner` field of
+ /// [`Class`] and that it remains valid for the lifetime `'a`.
+ pub const unsafe fn from_raw<'a>(ptr: *mut bindings::ww_class) -> &'a Self {
+ // SAFETY: By the safety contract, `ptr` is valid to construct `Class`.
+ unsafe { &*ptr.cast() }
+ }
+}
--
2.51.2
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v8 5/6] rust: ww_mutex: add Mutex, AcquireCtx and MutexGuard
2025-12-01 10:28 [PATCH v8 0/6] rust: add ww_mutex support Onur Özkan
` (3 preceding siblings ...)
2025-12-01 10:28 ` [PATCH v8 4/6] rust: implement Class for ww_class support Onur Özkan
@ 2025-12-01 10:28 ` Onur Özkan
2025-12-02 1:49 ` kernel test robot
` (2 more replies)
2025-12-01 10:28 ` [PATCH v8 6/6] rust: ww_mutex: implement LockSet Onur Özkan
5 siblings, 3 replies; 24+ messages in thread
From: Onur Özkan @ 2025-12-01 10:28 UTC (permalink / raw)
To: rust-for-linux
Cc: lossin, lyude, ojeda, alex.gaynor, boqun.feng, gary, a.hindborg,
aliceryhl, tmgross, dakr, peterz, mingo, will, longman,
felipe_life, daniel, daniel.almeida, thomas.hellstrom,
linux-kernel, Onur Özkan
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
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v8 6/6] rust: ww_mutex: implement LockSet
2025-12-01 10:28 [PATCH v8 0/6] rust: add ww_mutex support Onur Özkan
` (4 preceding siblings ...)
2025-12-01 10:28 ` [PATCH v8 5/6] rust: ww_mutex: add Mutex, AcquireCtx and MutexGuard Onur Özkan
@ 2025-12-01 10:28 ` Onur Özkan
5 siblings, 0 replies; 24+ messages in thread
From: Onur Özkan @ 2025-12-01 10:28 UTC (permalink / raw)
To: rust-for-linux
Cc: lossin, lyude, ojeda, alex.gaynor, boqun.feng, gary, a.hindborg,
aliceryhl, tmgross, dakr, peterz, mingo, will, longman,
felipe_life, daniel, daniel.almeida, thomas.hellstrom,
linux-kernel, Onur Özkan
LockSet is a high-level and safe API built on top of
ww_mutex which provides a safe and easy to use API
while keeping the ww_mutex semantics.
When EDEADLK is hit it drops all held locks, resets
the acquire context and retries the given (by the user)
locking algorithm until it succeeds.
Signed-off-by: Onur Özkan <work@onurozkan.dev>
---
rust/kernel/sync/lock/ww_mutex/lock_set.rs | 383 +++++++++++++++++++++
1 file changed, 383 insertions(+)
create mode 100644 rust/kernel/sync/lock/ww_mutex/lock_set.rs
diff --git a/rust/kernel/sync/lock/ww_mutex/lock_set.rs b/rust/kernel/sync/lock/ww_mutex/lock_set.rs
new file mode 100644
index 000000000000..d1faef120911
--- /dev/null
+++ b/rust/kernel/sync/lock/ww_mutex/lock_set.rs
@@ -0,0 +1,383 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Provides [`LockSet`] which automatically detects [`EDEADLK`],
+//! releases all locks, resets the state and retries the user
+//! supplied locking algorithm until success.
+
+use super::{AcquireCtx, Class, Mutex};
+use crate::bindings;
+use crate::prelude::*;
+use crate::types::NotThreadSafe;
+use core::ptr::NonNull;
+
+/// A tracked set of [`Mutex`] locks acquired under the same [`Class`].
+///
+/// It ensures proper cleanup and retry mechanism on deadlocks and provides
+/// safe access to locked data via [`LockSet::with_locked`].
+///
+/// Typical usage is through [`LockSet::lock_all`], which retries a
+/// user supplied locking algorithm until it succeeds without deadlock.
+pub struct LockSet<'a> {
+ acquire_ctx: Pin<KBox<AcquireCtx<'a>>>,
+ taken: KVec<RawGuard>,
+}
+
+/// Used by [`LockSet`] to track acquired locks.
+///
+/// This type is strictly crate-private and must never be exposed
+/// outside this crate.
+struct RawGuard {
+ mutex_ptr: NonNull<bindings::ww_mutex>,
+ _not_send: NotThreadSafe,
+}
+
+impl Drop for RawGuard {
+ fn drop(&mut self) {
+ // SAFETY: `mutex_ptr` originates from a locked `Mutex` and remains
+ // valid for the lifetime of this guard, so unlocking here is sound.
+ unsafe { bindings::ww_mutex_unlock(self.mutex_ptr.as_ptr()) };
+ }
+}
+
+impl<'a> Drop for LockSet<'a> {
+ fn drop(&mut self) {
+ self.release_all_locks();
+ }
+}
+
+impl<'a> LockSet<'a> {
+ /// Creates a new [`LockSet`] with the given [`Class`].
+ ///
+ /// All locks taken through this [`LockSet`] must belong to the
+ /// same [`Class`].
+ pub fn new(class: &'a Class) -> Result<Self> {
+ Ok(Self {
+ acquire_ctx: KBox::pin_init(AcquireCtx::new(class), GFP_KERNEL)?,
+ taken: KVec::new(),
+ })
+ }
+
+ /// Creates a new [`LockSet`] using an existing [`AcquireCtx`].
+ ///
+ /// # Safety
+ ///
+ /// The caller must ensure that `acquire_ctx` is properly initialized
+ /// and holds no [`Mutex`]es.
+ pub unsafe fn new_with_acquire_ctx(acquire_ctx: Pin<KBox<AcquireCtx<'a>>>) -> Self {
+ Self {
+ acquire_ctx,
+ taken: KVec::new(),
+ }
+ }
+
+ /// Attempts to lock the given [`Mutex`] and stores a guard for it.
+ pub fn lock<T>(&mut self, mutex: &'a Mutex<'a, T>) -> Result {
+ let guard = self.acquire_ctx.lock(mutex)?;
+
+ self.taken.push(
+ RawGuard {
+ // SAFETY: We just locked it above so it's a valid pointer.
+ mutex_ptr: unsafe { NonNull::new_unchecked(guard.mutex.inner.get()) },
+ _not_send: NotThreadSafe,
+ },
+ GFP_KERNEL,
+ )?;
+
+ // Avoid unlocking here; `release_all_locks` (also run by `Drop`)
+ // performs the unlock for `LockSet`.
+ core::mem::forget(guard);
+
+ Ok(())
+ }
+
+ /// Runs `locking_algorithm` until success with retrying on deadlock.
+ ///
+ /// `locking_algorithm` should attempt to acquire all needed locks.
+ /// If [`EDEADLK`] is detected, this function will roll back, reset
+ /// the context and retry automatically.
+ ///
+ /// Once all locks are acquired successfully, `on_all_locks_taken` is
+ /// invoked for exclusive access to the locked values. Afterwards, all
+ /// locks are released.
+ ///
+ /// # Example
+ ///
+ /// ```
+ /// use kernel::alloc::KBox;
+ /// use kernel::c_str;
+ /// use kernel::prelude::*;
+ /// use kernel::sync::Arc;
+ /// use kernel::sync::lock::ww_mutex::{Class, LockSet, Mutex};
+ /// use pin_init::stack_pin_init;
+ ///
+ /// stack_pin_init!(let class = Class::new_wound_wait(c_str!("test")));
+ ///
+ /// let mutex1 = Arc::pin_init(Mutex::new(0, &class), GFP_KERNEL)?;
+ /// let mutex2 = Arc::pin_init(Mutex::new(0, &class), GFP_KERNEL)?;
+ /// let mut lock_set = KBox::pin_init(LockSet::new(&class)?, GFP_KERNEL)?;
+ ///
+ /// lock_set.lock_all(
+ /// // `locking_algorithm` closure
+ /// |lock_set| {
+ /// lock_set.lock(&mutex1)?;
+ /// lock_set.lock(&mutex2)?;
+ ///
+ /// Ok(())
+ /// },
+ /// // `on_all_locks_taken` closure
+ /// |lock_set| {
+ /// // Safely mutate both values while holding the locks.
+ /// lock_set.with_locked(&mutex1, |v| *v += 1)?;
+ /// lock_set.with_locked(&mutex2, |v| *v += 1)?;
+ ///
+ /// Ok(())
+ /// },
+ /// )?;
+ ///
+ /// # Ok::<(), Error>(())
+ /// ```
+ pub fn lock_all<T, Y, Z>(
+ &mut self,
+ mut locking_algorithm: T,
+ mut on_all_locks_taken: Y,
+ ) -> Result<Z>
+ where
+ T: FnMut(&mut LockSet<'a>) -> Result,
+ Y: FnMut(&mut LockSet<'a>) -> Result<Z>,
+ {
+ loop {
+ match locking_algorithm(self) {
+ Ok(()) => {
+ // All locks in `locking_algorithm` succeeded.
+ // The user can now safely use them in `on_all_locks_taken`.
+ let res = on_all_locks_taken(self);
+ self.release_all_locks();
+
+ return res;
+ }
+ Err(e) if e == EDEADLK => {
+ // Deadlock detected, retry from scratch.
+ self.cleanup_on_deadlock();
+ continue;
+ }
+ Err(e) => {
+ self.release_all_locks();
+ return Err(e);
+ }
+ }
+ }
+ }
+
+ /// Executes `access` with a mutable reference to the data behind [`Mutex`].
+ ///
+ /// Fails with [`EINVAL`] if the [`Mutex`] was not locked in this [`LockSet`].
+ pub fn with_locked<T: Unpin, Y>(
+ &mut self,
+ mutex: &'a Mutex<'a, T>,
+ access: impl for<'b> FnOnce(&'b mut T) -> Y,
+ ) -> Result<Y> {
+ let mutex_ptr = mutex.inner.get();
+
+ if self
+ .taken
+ .iter()
+ .any(|guard| guard.mutex_ptr.as_ptr() == mutex_ptr)
+ {
+ // SAFETY: We hold the lock corresponding to `mutex`, so we have
+ // exclusive access to its protected data.
+ let value = unsafe { &mut *mutex.data.get() };
+ Ok(access(value))
+ } else {
+ // `mutex` isn't locked in this `LockSet`.
+ Err(EINVAL)
+ }
+ }
+
+ /// Releases all currently held locks in this [`LockSet`].
+ fn release_all_locks(&mut self) {
+ // `Drop` implementation of the `RawGuard` takes care of the unlocking.
+ self.taken.clear();
+ }
+
+ /// Resets this [`LockSet`] after a deadlock detection.
+ ///
+ /// Drops all held locks and reinitializes the [`AcquireCtx`].
+ ///
+ /// It is intended to be used for internal implementation only.
+ fn cleanup_on_deadlock(&mut self) {
+ self.release_all_locks();
+
+ // SAFETY: We released all the locks just above.
+ unsafe { self.acquire_ctx.as_mut().reinit() };
+ }
+}
+
+#[kunit_tests(rust_kernel_lock_set)]
+mod tests {
+ use crate::c_str;
+ use crate::prelude::*;
+ use crate::sync::Arc;
+ use pin_init::stack_pin_init;
+
+ use super::*;
+
+ #[test]
+ fn test_lock_set_basic_lock_unlock() -> Result {
+ stack_pin_init!(let class = Class::new_wound_wait(c_str!("test")));
+
+ let mutex = Arc::pin_init(Mutex::new(10, &class), GFP_KERNEL)?;
+ let mut lock_set = KBox::pin_init(LockSet::new(&class)?, GFP_KERNEL)?;
+
+ lock_set.lock(&mutex)?;
+
+ lock_set.with_locked(&mutex, |v| {
+ assert_eq!(*v, 10);
+ })?;
+
+ lock_set.release_all_locks();
+ assert!(!mutex.is_locked());
+
+ Ok(())
+ }
+
+ #[test]
+ fn test_lock_set_with_locked_mutates_data() -> Result {
+ stack_pin_init!(let class = Class::new_wound_wait(c_str!("test")));
+
+ let mutex = Arc::pin_init(Mutex::new(5, &class), GFP_KERNEL)?;
+ let mut lock_set = KBox::pin_init(LockSet::new(&class)?, GFP_KERNEL)?;
+
+ lock_set.lock(&mutex)?;
+
+ lock_set.with_locked(&mutex, |v| {
+ assert_eq!(*v, 5);
+ // Increment the value.
+ *v += 7;
+ })?;
+
+ lock_set.with_locked(&mutex, |v| {
+ // Check that mutation took effect.
+ assert_eq!(*v, 12);
+ })?;
+
+ Ok(())
+ }
+
+ #[test]
+ fn test_lock_all_success() -> 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 mut lock_set = KBox::pin_init(LockSet::new(&class)?, GFP_KERNEL)?;
+
+ let res = lock_set.lock_all(
+ // `locking_algorithm` closure
+ |lock_set| {
+ let _ = lock_set.lock(&mutex1)?;
+ let _ = lock_set.lock(&mutex2)?;
+ Ok(())
+ },
+ // `on_all_locks_taken` closure
+ |lock_set| {
+ lock_set.with_locked(&mutex1, |v| *v += 10)?;
+ lock_set.with_locked(&mutex2, |v| *v += 20)?;
+ Ok((
+ lock_set.with_locked(&mutex1, |v| *v)?,
+ lock_set.with_locked(&mutex2, |v| *v)?,
+ ))
+ },
+ )?;
+
+ assert_eq!(res, (11, 22));
+ assert!(!mutex1.is_locked());
+ assert!(!mutex2.is_locked());
+
+ Ok(())
+ }
+
+ #[test]
+ fn test_with_different_input_type() -> 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("hello", &class), GFP_KERNEL)?;
+ let mut lock_set = KBox::pin_init(LockSet::new(&class)?, GFP_KERNEL)?;
+
+ lock_set.lock_all(
+ // `locking_algorithm` closure
+ |lock_set| {
+ lock_set.lock(&mutex1)?;
+ lock_set.lock(&mutex2)?;
+
+ Ok(())
+ },
+ // `on_all_locks_taken` closure
+ |lock_set| {
+ lock_set.with_locked(&mutex1, |v| assert_eq!(*v, 1))?;
+ lock_set.with_locked(&mutex2, |v| assert_eq!(*v, "hello"))?;
+ Ok(())
+ },
+ )?;
+
+ Ok(())
+ }
+
+ #[test]
+ fn test_lock_all_retries_on_deadlock() -> Result {
+ stack_pin_init!(let class = Class::new_wound_wait(c_str!("test")));
+
+ let mutex = Arc::pin_init(Mutex::new(99, &class), GFP_KERNEL)?;
+ let mut lock_set = KBox::pin_init(LockSet::new(&class)?, GFP_KERNEL)?;
+ let mut first_try = true;
+
+ let res = lock_set.lock_all(
+ // `locking_algorithm` closure
+ |lock_set| {
+ if first_try {
+ first_try = false;
+ // Simulate deadlock on first attempt.
+ return Err(EDEADLK);
+ }
+ lock_set.lock(&mutex)
+ },
+ // `on_all_locks_taken` closure
+ |lock_set| {
+ lock_set.with_locked(&mutex, |v| {
+ *v += 1;
+ *v
+ })
+ },
+ )?;
+
+ assert_eq!(res, 100);
+ Ok(())
+ }
+
+ #[test]
+ fn test_with_locked_on_unlocked_mutex() -> Result {
+ stack_pin_init!(let class = Class::new_wound_wait(c_str!("test")));
+
+ let mutex = Arc::pin_init(Mutex::new(5, &class), GFP_KERNEL)?;
+ let mut lock_set = KBox::pin_init(LockSet::new(&class)?, GFP_KERNEL)?;
+
+ let ecode = lock_set.with_locked(&mutex, |_v| {}).unwrap_err();
+ assert_eq!(EINVAL, ecode);
+
+ Ok(())
+ }
+
+ #[test]
+ fn test_with_different_classes() -> Result {
+ stack_pin_init!(let class1 = Class::new_wound_wait(c_str!("class1")));
+ stack_pin_init!(let class2 = Class::new_wound_wait(c_str!("class2")));
+
+ let mutex = Arc::pin_init(Mutex::new(5, &class1), GFP_KERNEL)?;
+ let mut lock_set = KBox::pin_init(LockSet::new(&class2)?, GFP_KERNEL)?;
+
+ let ecode = lock_set.lock(&mutex).unwrap_err();
+ assert_eq!(EINVAL, ecode);
+
+ Ok(())
+ }
+}
--
2.51.2
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH v8 5/6] rust: ww_mutex: add Mutex, AcquireCtx and MutexGuard
2025-12-01 10:28 ` [PATCH v8 5/6] rust: ww_mutex: add Mutex, AcquireCtx and MutexGuard Onur Özkan
@ 2025-12-02 1:49 ` kernel test robot
2025-12-02 10:20 ` Onur Özkan
2025-12-02 18:29 ` Daniel Almeida
2025-12-03 13:26 ` Alice Ryhl
2 siblings, 1 reply; 24+ messages in thread
From: kernel test robot @ 2025-12-02 1:49 UTC (permalink / raw)
To: Onur Özkan, rust-for-linux
Cc: llvm, oe-kbuild-all, lossin, lyude, ojeda, alex.gaynor,
boqun.feng, gary, a.hindborg, aliceryhl, tmgross, dakr, peterz,
mingo, will, longman, felipe_life, daniel, daniel.almeida,
thomas.hellstrom, linux-kernel, Onur Özkan
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
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v8 5/6] rust: ww_mutex: add Mutex, AcquireCtx and MutexGuard
2025-12-02 1:49 ` kernel test robot
@ 2025-12-02 10:20 ` Onur Özkan
0 siblings, 0 replies; 24+ messages in thread
From: Onur Özkan @ 2025-12-02 10:20 UTC (permalink / raw)
To: kernel test robot
Cc: rust-for-linux, llvm, oe-kbuild-all, lossin, lyude, ojeda,
alex.gaynor, boqun.feng, gary, a.hindborg, aliceryhl, tmgross,
dakr, peterz, mingo, will, longman, felipe_life, daniel,
daniel.almeida, thomas.hellstrom, linux-kernel
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
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v8 1/6] rust: add C wrappers for ww_mutex inline functions
2025-12-01 10:28 ` [PATCH v8 1/6] rust: add C wrappers for ww_mutex inline functions Onur Özkan
@ 2025-12-02 17:38 ` Daniel Almeida
0 siblings, 0 replies; 24+ messages in thread
From: Daniel Almeida @ 2025-12-02 17:38 UTC (permalink / raw)
To: Onur Özkan
Cc: rust-for-linux, lossin, lyude, ojeda, alex.gaynor, boqun.feng,
gary, a.hindborg, aliceryhl, tmgross, dakr, peterz, mingo, will,
longman, felipe_life, daniel, thomas.hellstrom, linux-kernel
Hi Onur,
> On 1 Dec 2025, at 07:28, Onur Özkan <work@onurozkan.dev> wrote:
>
> Some of the kernel's `ww_mutex` functions are implemented as
> `static inline`, so they are inaccessible from Rust as bindgen
> can't generate code on them. This patch provides C function wrappers
> around these inline implementations, so bindgen can see them and generate
> the corresponding Rust code.
>
> Signed-off-by: Onur Özkan <work@onurozkan.dev>
Did you forget to pick up tags? (b4 trailers -u)
> ---
> MAINTAINERS | 1 +
> rust/helpers/helpers.c | 1 +
> rust/helpers/ww_mutex.c | 39 +++++++++++++++++++++++++++++++++++++++
> 3 files changed, 41 insertions(+)
> create mode 100644 rust/helpers/ww_mutex.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index e8f06145fb54..bc6ea9cc1588 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -14546,6 +14546,7 @@ F: kernel/locking/
> F: lib/locking*.[ch]
> F: rust/helpers/mutex.c
> F: rust/helpers/spinlock.c
> +F: rust/helpers/ww_mutex.c
> F: rust/kernel/sync/lock.rs
> F: rust/kernel/sync/lock/
> F: rust/kernel/sync/locked_by.rs
> diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c
> index 551da6c9b506..58cf14d74516 100644
> --- a/rust/helpers/helpers.c
> +++ b/rust/helpers/helpers.c
> @@ -59,4 +59,5 @@
> #include "vmalloc.c"
> #include "wait.c"
> #include "workqueue.c"
> +#include "ww_mutex.c"
> #include "xarray.c"
> diff --git a/rust/helpers/ww_mutex.c b/rust/helpers/ww_mutex.c
> new file mode 100644
> index 000000000000..61a487653394
> --- /dev/null
> +++ b/rust/helpers/ww_mutex.c
> @@ -0,0 +1,39 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <linux/ww_mutex.h>
> +
> +void rust_helper_ww_mutex_init(struct ww_mutex *lock, struct ww_class *ww_class)
> +{
> + ww_mutex_init(lock, ww_class);
> +}
> +
> +void rust_helper_ww_acquire_init(struct ww_acquire_ctx *ctx, struct ww_class *ww_class)
> +{
> + ww_acquire_init(ctx, ww_class);
> +}
> +
> +void rust_helper_ww_acquire_done(struct ww_acquire_ctx *ctx)
> +{
> + ww_acquire_done(ctx);
> +}
> +
> +void rust_helper_ww_acquire_fini(struct ww_acquire_ctx *ctx)
> +{
> + ww_acquire_fini(ctx);
> +}
> +
> +void rust_helper_ww_mutex_lock_slow(struct ww_mutex *lock, struct ww_acquire_ctx *ctx)
> +{
> + ww_mutex_lock_slow(lock, ctx);
> +}
> +
> +int rust_helper_ww_mutex_lock_slow_interruptible(struct ww_mutex *lock, struct ww_acquire_ctx *ctx)
> +{
> + return ww_mutex_lock_slow_interruptible(lock, ctx);
> +}
> +
> +bool rust_helper_ww_mutex_is_locked(struct ww_mutex *lock)
> +{
> + return ww_mutex_is_locked(lock);
> +}
> +
> --
> 2.51.2
>
>
— Daniel
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v8 2/6] ww_mutex: add `ww_class` field unconditionally
2025-12-01 10:28 ` [PATCH v8 2/6] ww_mutex: add `ww_class` field unconditionally Onur Özkan
@ 2025-12-02 17:42 ` Daniel Almeida
0 siblings, 0 replies; 24+ messages in thread
From: Daniel Almeida @ 2025-12-02 17:42 UTC (permalink / raw)
To: Onur Özkan
Cc: rust-for-linux, lossin, lyude, ojeda, alex.gaynor, boqun.feng,
gary, a.hindborg, aliceryhl, tmgross, dakr, peterz, mingo, will,
longman, felipe_life, daniel, thomas.hellstrom, linux-kernel
Hi Onur,
> On 1 Dec 2025, at 07:28, Onur Özkan <work@onurozkan.dev> wrote:
>
> Removes `DEBUG_WW_MUTEXES` guard on ww_class field
> from ww_acquire_ctx and ww_mutex.
>
> This helps us to write much safer Rust abstraction
> in very simple way.
Please extend this a bit. i.e.:
> This helps us to write much safer Rust abstraction in very simple way.
How so? This is what you should expand on in the commit message.
Also the wording/grammar is slightly off.
>
> Link: https://lore.kernel.org/all/ECC0425A-8B18-4626-8EA8-2F843C45E0A1@collabora.com/
> Signed-off-by: Onur Özkan <work@onurozkan.dev>
> ---
> include/linux/ww_mutex.h | 8 ++------
> 1 file changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/include/linux/ww_mutex.h b/include/linux/ww_mutex.h
> index 45ff6f7a872b..c9835035b6d6 100644
> --- a/include/linux/ww_mutex.h
> +++ b/include/linux/ww_mutex.h
> @@ -47,9 +47,7 @@ struct ww_class {
> struct ww_mutex {
> struct WW_MUTEX_BASE base;
> struct ww_acquire_ctx *ctx;
> -#ifdef DEBUG_WW_MUTEXES
> struct ww_class *ww_class;
> -#endif
> };
>
> struct ww_acquire_ctx {
> @@ -58,9 +56,9 @@ struct ww_acquire_ctx {
> unsigned int acquired;
> unsigned short wounded;
> unsigned short is_wait_die;
> + struct ww_class *ww_class;
> #ifdef DEBUG_WW_MUTEXES
> unsigned int done_acquire;
> - struct ww_class *ww_class;
> void *contending_lock;
> #endif
> #ifdef CONFIG_DEBUG_LOCK_ALLOC
> @@ -110,9 +108,7 @@ static inline void ww_mutex_init(struct ww_mutex *lock,
> {
> ww_mutex_base_init(&lock->base, ww_class->mutex_name, &ww_class->mutex_key);
> lock->ctx = NULL;
> -#ifdef DEBUG_WW_MUTEXES
> lock->ww_class = ww_class;
> -#endif
> }
>
> /**
> @@ -147,8 +143,8 @@ static inline void ww_acquire_init(struct ww_acquire_ctx *ctx,
> ctx->acquired = 0;
> ctx->wounded = false;
> ctx->is_wait_die = ww_class->is_wait_die;
> -#ifdef DEBUG_WW_MUTEXES
> ctx->ww_class = ww_class;
> +#ifdef DEBUG_WW_MUTEXES
> ctx->done_acquire = 0;
> ctx->contending_lock = NULL;
> #endif
> --
> 2.51.2
>
With the change above,
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v8 3/6] rust: error: add EDEADLK
2025-12-01 10:28 ` [PATCH v8 3/6] rust: error: add EDEADLK Onur Özkan
@ 2025-12-02 17:43 ` Daniel Almeida
0 siblings, 0 replies; 24+ messages in thread
From: Daniel Almeida @ 2025-12-02 17:43 UTC (permalink / raw)
To: Onur Özkan
Cc: rust-for-linux, lossin, lyude, ojeda, alex.gaynor, boqun.feng,
gary, a.hindborg, aliceryhl, tmgross, dakr, peterz, mingo, will,
longman, felipe_life, daniel, thomas.hellstrom, linux-kernel
> On 1 Dec 2025, at 07:28, Onur Özkan <work@onurozkan.dev> wrote:
>
> This is needed for the ww_mutex implementation so
> we can handle EDEADLK on lock attempts.
>
> Signed-off-by: Onur Özkan <work@onurozkan.dev>
FYI: missing tags here as well
> ---
> rust/kernel/error.rs | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/rust/kernel/error.rs b/rust/kernel/error.rs
> index 1c0e0e241daa..9bf1072cfe19 100644
> --- a/rust/kernel/error.rs
> +++ b/rust/kernel/error.rs
> @@ -66,6 +66,7 @@ macro_rules! declare_err {
> declare_err!(EPIPE, "Broken pipe.");
> declare_err!(EDOM, "Math argument out of domain of func.");
> declare_err!(ERANGE, "Math result not representable.");
> + declare_err!(EDEADLK, "Resource deadlock avoided.");
> declare_err!(EOVERFLOW, "Value too large for defined data type.");
> declare_err!(ETIMEDOUT, "Connection timed out.");
> declare_err!(ERESTARTSYS, "Restart the system call.");
> --
> 2.51.2
>
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v8 4/6] rust: implement Class for ww_class support
2025-12-01 10:28 ` [PATCH v8 4/6] rust: implement Class for ww_class support Onur Özkan
@ 2025-12-02 17:59 ` Daniel Almeida
2025-12-03 13:10 ` Alice Ryhl
1 sibling, 0 replies; 24+ messages in thread
From: Daniel Almeida @ 2025-12-02 17:59 UTC (permalink / raw)
To: Onur Özkan
Cc: rust-for-linux, lossin, lyude, ojeda, alex.gaynor, boqun.feng,
gary, a.hindborg, aliceryhl, tmgross, dakr, peterz, mingo, will,
longman, felipe_life, daniel, thomas.hellstrom, linux-kernel
Hi Onur,
> On 1 Dec 2025, at 07:28, Onur Özkan <work@onurozkan.dev> wrote:
>
> Adds the Class type, the first step in supporting
> ww_mutex in Rust. Class represents ww_class, used
> for deadlock avoidance for supporting both wait-die
> and wound-wait semantics.
>
> Also adds the define_class macro for safely declaring
> static instances.
>
> Signed-off-by: Onur Özkan <work@onurozkan.dev>
> ---
> rust/kernel/sync/lock.rs | 1 +
> rust/kernel/sync/lock/ww_mutex.rs | 7 ++
> rust/kernel/sync/lock/ww_mutex/class.rs | 156 ++++++++++++++++++++++++
> 3 files changed, 164 insertions(+)
> create mode 100644 rust/kernel/sync/lock/ww_mutex.rs
> create mode 100644 rust/kernel/sync/lock/ww_mutex/class.rs
>
> diff --git a/rust/kernel/sync/lock.rs b/rust/kernel/sync/lock.rs
> index 27202beef90c..5b320c2b28c1 100644
> --- a/rust/kernel/sync/lock.rs
> +++ b/rust/kernel/sync/lock.rs
> @@ -15,6 +15,7 @@
>
> pub mod mutex;
> pub mod spinlock;
> +pub mod ww_mutex;
>
> pub(super) mod global;
> pub use global::{GlobalGuard, GlobalLock, GlobalLockBackend, GlobalLockedBy};
> diff --git a/rust/kernel/sync/lock/ww_mutex.rs b/rust/kernel/sync/lock/ww_mutex.rs
> new file mode 100644
> index 000000000000..727c51cc73af
> --- /dev/null
> +++ b/rust/kernel/sync/lock/ww_mutex.rs
> @@ -0,0 +1,7 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Rust abstractions for the kernel's wound-wait locking primitives.
Can you link to the docs here?
> +
> +pub use class::Class;
> +
> +mod class;
> diff --git a/rust/kernel/sync/lock/ww_mutex/class.rs b/rust/kernel/sync/lock/ww_mutex/class.rs
> new file mode 100644
> index 000000000000..b0753093f1e0
> --- /dev/null
> +++ b/rust/kernel/sync/lock/ww_mutex/class.rs
> @@ -0,0 +1,156 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Provides [`Class`] to group wound/wait mutexes to be acquired together
> +//! and specifies which deadlock avoidance algorithm to use (e.g., wound-wait
> +//! or wait-die).
> +//!
> +//! The [`define_class`] macro and [`Class::new_wait_die`]/[`Class::new_wound_wait`]
> +//! constructors provide safe ways to create classes.
> +
> +use crate::bindings;
> +use crate::prelude::*;
> +use crate::types::Opaque;
> +
> +/// Creates static [`Class`] instances.
> +///
> +/// # Examples
> +///
> +/// ```
> +/// use kernel::{c_str, define_class};
> +///
> +/// define_class!(WOUND_WAIT_GLOBAL_CLASS, wound_wait, c_str!("wound_wait_global_class"));
> +/// define_class!(WAIT_DIE_GLOBAL_CLASS, wait_die, c_str!("wait_die_global_class"));
> +/// ```
> +#[macro_export]
> +macro_rules! define_class {
> + ($name:ident, wound_wait, $class_name:expr) => {
> + static $name: $crate::sync::lock::ww_mutex::Class =
> + // SAFETY: This is `static`, so address is fixed and won't move.
> + unsafe { $crate::sync::lock::ww_mutex::Class::unpinned_new($class_name, false) };
> + };
> + ($name:ident, wait_die, $class_name:expr) => {
> + static $name: $crate::sync::lock::ww_mutex::Class =
> + // SAFETY: This is `static`, so address is fixed and won't move.
> + unsafe { $crate::sync::lock::ww_mutex::Class::unpinned_new($class_name, true) };
> + };
> +}
> +
> +/// Used to group mutexes together for deadlock avoidance.
> +///
> +/// All mutexes that might be acquired together should use the same class.
> +///
> +/// # Examples
> +///
> +/// ```
> +/// use kernel::sync::lock::ww_mutex::Class;
> +/// use kernel::c_str;
> +/// use pin_init::stack_pin_init;
> +///
> +/// stack_pin_init!(let _wait_die_class = Class::new_wait_die(c_str!("some_class")));
> +/// stack_pin_init!(let _wound_wait_class = Class::new_wound_wait(c_str!("some_other_class")));
> +///
> +/// # Ok::<(), Error>(())
> +/// ```
> +#[pin_data]
> +#[repr(transparent)]
> +pub struct Class {
> + #[pin]
> + pub(super) inner: Opaque<bindings::ww_class>,
Do you plan on ever extending this? A tuple struct might make more sense otherwise.
> +}
> +
> +// SAFETY: [`Class`] is set up once and never modified. It's fine to share it across threads.
> +unsafe impl Sync for Class {}
> +// SAFETY: Doesn't hold anything thread-specific. It's safe to send to other threads.
> +unsafe impl Send for Class {}
Please keep Foo and impl Foo together. The very next thing one wants to see after a
type definition is its implementation.
i.e.:
struct Foo;
impl Foo {…}
impl Bar for Foo {…}
> +
> +impl Class {
> + /// Creates an unpinned [`Class`].
> + ///
> + /// # Safety
> + ///
> + /// Caller must guarantee that the returned value is not moved after creation.
> + pub const unsafe fn unpinned_new(name: &'static CStr, is_wait_die: bool) -> Self {
IMHO new_unpinned() would be a better name. Also, where is this used?
By the way, this will immediately move Self out of this function, which is
technically after its creation (i.e.: it’s after the Class {…}
construct). Is this ok?
> + Class {
> + inner: Opaque::new(bindings::ww_class {
> + stamp: bindings::atomic_long_t { counter: 0 },
> + acquire_name: name.as_char_ptr(),
> + mutex_name: name.as_char_ptr(),
> + is_wait_die: is_wait_die as u32,
> + // TODO: Replace with `bindings::lock_class_key::default()` once
> + // stabilized for `const`.
> + //
> + // SAFETY: This is always zero-initialized when defined with
> + // `DEFINE_WD_CLASS` globally on C side.
> + //
> + // For reference, see __WW_CLASS_INITIALIZER() in
> + // "include/linux/ww_mutex.h".
> + acquire_key: unsafe { core::mem::zeroed() },
> + // TODO: Replace with `bindings::lock_class_key::default()` once
> + // stabilized for `const`.
> + //
> + // SAFETY: This is always zero-initialized when defined with
> + // `DEFINE_WD_CLASS` globally on C side.
> + //
> + // For reference, see __WW_CLASS_INITIALIZER() in
> + // "include/linux/ww_mutex.h".
> + mutex_key: unsafe { core::mem::zeroed() },
> + }),
> + }
> + }
> +
> + /// Creates a [`Class`].
> + ///
> + /// You should not use this function directly. Use the [`define_class!`]
> + /// macro or call [`Class::new_wait_die`] or [`Class::new_wound_wait`] instead.
> + fn new(name: &'static CStr, is_wait_die: bool) -> impl PinInit<Self> {
> + pin_init! {
> + Self {
> + inner <- Opaque::ffi_init(|slot: *mut bindings::ww_class| {
> + // SAFETY: The fields are being initialized. The `name` pointer is valid for a
> + // static lifetime. The keys are zeroed, which is what the C side does.
> + unsafe {
> + slot.write(bindings::ww_class {
> + stamp: bindings::atomic_long_t { counter: 0 },
> + acquire_name: name.as_char_ptr(),
> + mutex_name: name.as_char_ptr(),
> + is_wait_die: is_wait_die.into(),
> + // TODO: Replace with `bindings::lock_class_key::default()` once
> + // stabilized for `const`.
> + //
> + // SAFETY: This is always zero-initialized when defined with
> + // `DEFINE_WD_CLASS` globally on C side.
> + //
> + // For reference, see __WW_CLASS_INITIALIZER() in
> + // "include/linux/ww_mutex.h".
> + acquire_key: core::mem::zeroed(),
> + mutex_key: core::mem::zeroed(),
> + });
> + }
> + }),
> + }
> + }
> + }
> +
> + /// Creates wait-die [`Class`].
> + pub fn new_wait_die(name: &'static CStr) -> impl PinInit<Self> {
> + Self::new(name, true)
> + }
> +
> + /// Creates wound-wait [`Class`].
> + pub fn new_wound_wait(name: &'static CStr) -> impl PinInit<Self> {
> + Self::new(name, false)
> + }
> +
> + /// Creates a [`Class`] from a raw pointer.
> + ///
> + /// This function is intended for interoperability with C code.
> + ///
> + /// # Safety
> + ///
> + /// The caller must ensure that `ptr` points to the `inner` field of
> + /// [`Class`] and that it remains valid for the lifetime `'a`.
> + pub const unsafe fn from_raw<'a>(ptr: *mut bindings::ww_class) -> &'a Self {
> + // SAFETY: By the safety contract, `ptr` is valid to construct `Class`.
> + unsafe { &*ptr.cast() }
> + }
> +}
> --
> 2.51.2
>
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v8 5/6] rust: ww_mutex: add Mutex, AcquireCtx and MutexGuard
2025-12-01 10:28 ` [PATCH v8 5/6] rust: ww_mutex: add Mutex, AcquireCtx and MutexGuard Onur Özkan
2025-12-02 1:49 ` kernel test robot
@ 2025-12-02 18:29 ` Daniel Almeida
2025-12-03 15:49 ` Onur Özkan
2025-12-03 13:26 ` Alice Ryhl
2 siblings, 1 reply; 24+ messages in thread
From: Daniel Almeida @ 2025-12-02 18:29 UTC (permalink / raw)
To: Onur Özkan
Cc: rust-for-linux, lossin, lyude, ojeda, alex.gaynor, boqun.feng,
gary, a.hindborg, aliceryhl, tmgross, dakr, peterz, mingo, will,
longman, felipe_life, daniel, thomas.hellstrom, linux-kernel
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
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v8 4/6] rust: implement Class for ww_class support
2025-12-01 10:28 ` [PATCH v8 4/6] rust: implement Class for ww_class support Onur Özkan
2025-12-02 17:59 ` Daniel Almeida
@ 2025-12-03 13:10 ` Alice Ryhl
2025-12-03 16:06 ` Onur Özkan
1 sibling, 1 reply; 24+ messages in thread
From: Alice Ryhl @ 2025-12-03 13:10 UTC (permalink / raw)
To: Onur Özkan
Cc: rust-for-linux, lossin, lyude, ojeda, alex.gaynor, boqun.feng,
gary, a.hindborg, tmgross, dakr, peterz, mingo, will, longman,
felipe_life, daniel, daniel.almeida, thomas.hellstrom,
linux-kernel
On Mon, Dec 01, 2025 at 01:28:53PM +0300, Onur Özkan wrote:
> Adds the Class type, the first step in supporting
> ww_mutex in Rust. Class represents ww_class, used
> for deadlock avoidance for supporting both wait-die
> and wound-wait semantics.
>
> Also adds the define_class macro for safely declaring
> static instances.
> +impl Class {
> + /// Creates an unpinned [`Class`].
> + ///
> + /// # Safety
> + ///
> + /// Caller must guarantee that the returned value is not moved after creation.
The value is moved when you return it. Perhaps you meant that it must be
pinned before first use?
> + // TODO: Replace with `bindings::lock_class_key::default()` once
> + // stabilized for `const`.
> + //
> + // SAFETY: This is always zero-initialized when defined with
> + // `DEFINE_WD_CLASS` globally on C side.
> + //
> + // For reference, see __WW_CLASS_INITIALIZER() in
> + // "include/linux/ww_mutex.h".
> + acquire_key: core::mem::zeroed(),
> + mutex_key: core::mem::zeroed(),
For global lock class keys, this is fine, but this constructor seems to
be for the non-global case. In that case, lockdep_register_key() must be
called.
Is a ww_class ever created as a non-global? I don't really think so. If
not, then let's not support it.
Alice
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v8 5/6] rust: ww_mutex: add Mutex, AcquireCtx and MutexGuard
2025-12-01 10:28 ` [PATCH v8 5/6] rust: ww_mutex: add Mutex, AcquireCtx and MutexGuard Onur Özkan
2025-12-02 1:49 ` kernel test robot
2025-12-02 18:29 ` Daniel Almeida
@ 2025-12-03 13:26 ` Alice Ryhl
2025-12-03 16:02 ` Onur Özkan
2025-12-03 17:23 ` Daniel Almeida
2 siblings, 2 replies; 24+ messages in thread
From: Alice Ryhl @ 2025-12-03 13:26 UTC (permalink / raw)
To: Onur Özkan
Cc: rust-for-linux, lossin, lyude, ojeda, alex.gaynor, boqun.feng,
gary, a.hindborg, tmgross, dakr, peterz, mingo, will, longman,
felipe_life, daniel, daniel.almeida, thomas.hellstrom,
linux-kernel
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
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v8 5/6] rust: ww_mutex: add Mutex, AcquireCtx and MutexGuard
2025-12-02 18:29 ` Daniel Almeida
@ 2025-12-03 15:49 ` Onur Özkan
0 siblings, 0 replies; 24+ messages in thread
From: Onur Özkan @ 2025-12-03 15:49 UTC (permalink / raw)
To: Daniel Almeida
Cc: rust-for-linux, lossin, lyude, ojeda, alex.gaynor, boqun.feng,
gary, a.hindborg, aliceryhl, tmgross, dakr, peterz, mingo, will,
longman, felipe_life, daniel, thomas.hellstrom, linux-kernel
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
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v8 5/6] rust: ww_mutex: add Mutex, AcquireCtx and MutexGuard
2025-12-03 13:26 ` Alice Ryhl
@ 2025-12-03 16:02 ` Onur Özkan
2025-12-04 9:08 ` Alice Ryhl
2025-12-03 17:23 ` Daniel Almeida
1 sibling, 1 reply; 24+ messages in thread
From: Onur Özkan @ 2025-12-03 16:02 UTC (permalink / raw)
To: Alice Ryhl
Cc: rust-for-linux, lossin, lyude, ojeda, alex.gaynor, boqun.feng,
gary, a.hindborg, tmgross, dakr, peterz, mingo, will, longman,
felipe_life, daniel, daniel.almeida, thomas.hellstrom,
linux-kernel
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
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v8 4/6] rust: implement Class for ww_class support
2025-12-03 13:10 ` Alice Ryhl
@ 2025-12-03 16:06 ` Onur Özkan
0 siblings, 0 replies; 24+ messages in thread
From: Onur Özkan @ 2025-12-03 16:06 UTC (permalink / raw)
To: Alice Ryhl
Cc: rust-for-linux, lossin, lyude, ojeda, alex.gaynor, boqun.feng,
gary, a.hindborg, tmgross, dakr, peterz, mingo, will, longman,
felipe_life, daniel, daniel.almeida, thomas.hellstrom,
linux-kernel
On Wed, 3 Dec 2025 13:10:44 +0000
Alice Ryhl <aliceryhl@google.com> wrote:
> On Mon, Dec 01, 2025 at 01:28:53PM +0300, Onur Özkan wrote:
> > Adds the Class type, the first step in supporting
> > ww_mutex in Rust. Class represents ww_class, used
> > for deadlock avoidance for supporting both wait-die
> > and wound-wait semantics.
> >
> > Also adds the define_class macro for safely declaring
> > static instances.
>
> > +impl Class {
> > + /// Creates an unpinned [`Class`].
> > + ///
> > + /// # Safety
> > + ///
> > + /// Caller must guarantee that the returned value is not moved
> > after creation.
>
> The value is moved when you return it. Perhaps you meant that it must
> be pinned before first use?
>
Yes, that was my point.
> > + // TODO: Replace with
> > `bindings::lock_class_key::default()` once
> > + // stabilized for `const`.
> > + //
> > + // SAFETY: This is always
> > zero-initialized when defined with
> > + // `DEFINE_WD_CLASS` globally on C
> > side.
> > + //
> > + // For reference, see
> > __WW_CLASS_INITIALIZER() in
> > + // "include/linux/ww_mutex.h".
> > + acquire_key: core::mem::zeroed(),
> > + mutex_key: core::mem::zeroed(),
>
> For global lock class keys, this is fine, but this constructor seems
> to be for the non-global case. In that case, lockdep_register_key()
> must be called.
>
> Is a ww_class ever created as a non-global? I don't really think so.
> If not, then let's not support it.
>
> Alice
No, it's always used as global on the C side. Good point, will make the
change in the next version.
-Onur
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v8 5/6] rust: ww_mutex: add Mutex, AcquireCtx and MutexGuard
2025-12-03 13:26 ` Alice Ryhl
2025-12-03 16:02 ` Onur Özkan
@ 2025-12-03 17:23 ` Daniel Almeida
2025-12-04 9:07 ` Alice Ryhl
1 sibling, 1 reply; 24+ messages in thread
From: Daniel Almeida @ 2025-12-03 17:23 UTC (permalink / raw)
To: Alice Ryhl
Cc: Onur Özkan, rust-for-linux, lossin, lyude, ojeda,
alex.gaynor, boqun.feng, gary, a.hindborg, tmgross, dakr, peterz,
mingo, will, longman, felipe_life, daniel, thomas.hellstrom,
linux-kernel
> 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
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v8 5/6] rust: ww_mutex: add Mutex, AcquireCtx and MutexGuard
2025-12-03 17:23 ` Daniel Almeida
@ 2025-12-04 9:07 ` Alice Ryhl
2025-12-04 13:26 ` Daniel Almeida
0 siblings, 1 reply; 24+ messages in thread
From: Alice Ryhl @ 2025-12-04 9:07 UTC (permalink / raw)
To: Daniel Almeida
Cc: Onur Özkan, rust-for-linux, lossin, lyude, ojeda,
alex.gaynor, boqun.feng, gary, a.hindborg, tmgross, dakr, peterz,
mingo, will, longman, felipe_life, daniel, thomas.hellstrom,
linux-kernel
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
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v8 5/6] rust: ww_mutex: add Mutex, AcquireCtx and MutexGuard
2025-12-03 16:02 ` Onur Özkan
@ 2025-12-04 9:08 ` Alice Ryhl
0 siblings, 0 replies; 24+ messages in thread
From: Alice Ryhl @ 2025-12-04 9:08 UTC (permalink / raw)
To: Onur Özkan
Cc: rust-for-linux, lossin, lyude, ojeda, alex.gaynor, boqun.feng,
gary, a.hindborg, tmgross, dakr, peterz, mingo, will, longman,
felipe_life, daniel, daniel.almeida, thomas.hellstrom,
linux-kernel
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
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v8 5/6] rust: ww_mutex: add Mutex, AcquireCtx and MutexGuard
2025-12-04 9:07 ` Alice Ryhl
@ 2025-12-04 13:26 ` Daniel Almeida
2025-12-04 13:33 ` Alice Ryhl
0 siblings, 1 reply; 24+ messages in thread
From: Daniel Almeida @ 2025-12-04 13:26 UTC (permalink / raw)
To: Alice Ryhl
Cc: Onur Özkan, rust-for-linux, lossin, lyude, ojeda,
alex.gaynor, boqun.feng, gary, a.hindborg, tmgross, dakr, peterz,
mingo, will, longman, felipe_life, daniel, thomas.hellstrom,
linux-kernel
> 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
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v8 5/6] rust: ww_mutex: add Mutex, AcquireCtx and MutexGuard
2025-12-04 13:26 ` Daniel Almeida
@ 2025-12-04 13:33 ` Alice Ryhl
0 siblings, 0 replies; 24+ messages in thread
From: Alice Ryhl @ 2025-12-04 13:33 UTC (permalink / raw)
To: Daniel Almeida
Cc: Onur Özkan, rust-for-linux, lossin, lyude, ojeda,
alex.gaynor, boqun.feng, gary, a.hindborg, tmgross, dakr, peterz,
mingo, will, longman, felipe_life, daniel, thomas.hellstrom,
linux-kernel
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
^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2025-12-04 13:33 UTC | newest]
Thread overview: 24+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-01 10:28 [PATCH v8 0/6] rust: add ww_mutex support Onur Özkan
2025-12-01 10:28 ` [PATCH v8 1/6] rust: add C wrappers for ww_mutex inline functions Onur Özkan
2025-12-02 17:38 ` Daniel Almeida
2025-12-01 10:28 ` [PATCH v8 2/6] ww_mutex: add `ww_class` field unconditionally Onur Özkan
2025-12-02 17:42 ` Daniel Almeida
2025-12-01 10:28 ` [PATCH v8 3/6] rust: error: add EDEADLK Onur Özkan
2025-12-02 17:43 ` Daniel Almeida
2025-12-01 10:28 ` [PATCH v8 4/6] rust: implement Class for ww_class support Onur Özkan
2025-12-02 17:59 ` Daniel Almeida
2025-12-03 13:10 ` Alice Ryhl
2025-12-03 16:06 ` Onur Özkan
2025-12-01 10:28 ` [PATCH v8 5/6] rust: ww_mutex: add Mutex, AcquireCtx and MutexGuard Onur Özkan
2025-12-02 1:49 ` kernel test robot
2025-12-02 10:20 ` Onur Özkan
2025-12-02 18:29 ` Daniel Almeida
2025-12-03 15:49 ` Onur Özkan
2025-12-03 13:26 ` Alice Ryhl
2025-12-03 16:02 ` Onur Özkan
2025-12-04 9:08 ` Alice Ryhl
2025-12-03 17:23 ` Daniel Almeida
2025-12-04 9:07 ` Alice Ryhl
2025-12-04 13:26 ` Daniel Almeida
2025-12-04 13:33 ` Alice Ryhl
2025-12-01 10:28 ` [PATCH v8 6/6] rust: ww_mutex: implement LockSet Onur Özkan
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).