rust: add ww_mutex support
Subsystems (3)
Onur Özkan
reviewerv12 cover rust: add ww_mutex support
This series adds Rust abstractions for ww_mutex, including Class, Mutex, AcquireCtx and MutexGuard. LockSet handles deadlock recovery for callers that need to acquire multiple locks.
Changes in v12 (sashiko fixes):
- Give Rust-defined classes separate _acquire and _mutex lockdep names to match the C initializer.
- Correct the Class Sync safety comment and add an invariant comment when constructing MutexGuard.
- Add #[inline] to Class::from_raw(), Mutex::is_locked(), AcquireCtx::done(), MutexGuard::drop() and RawGuard::drop().
Built x86_64 kernels with and without PREEMPT_RT. rustfmtcheck, Clippy and rustdoc passed. All 14 ww_mutex/LockSet KUnit tests passed on both builds with CONFIG_PROVE_LOCKING and CONFIG_DEBUG_WW_MUTEX_SLOWPATH enabled.
Previous version:
https://lore.kernel.org/all/[email protected]/
Onur Özkan (6):
rust: add C wrappers for ww_mutex inline functions
ww_mutex: add ww_class field unconditionally
rust: implement Class for ww_class support
rust: ww_mutex: add Mutex, AcquireCtx and MutexGuard
rust: ww_mutex: implement LockSet
MAINTAINERS: add Onur Özkan as WW MUTEX maintainer
MAINTAINERS | 2 + include/linux/ww_mutex.h | 8 +- rust/helpers/helpers.c | 1 + rust/helpers/ww_mutex.c | 33 ++ rust/kernel/sync/lock.rs | 1 + rust/kernel/sync/lock/ww_mutex.rs | 447 +++++++++++++++++ rust/kernel/sync/lock/ww_mutex/acquire_ctx.rs | 149 ++++++ rust/kernel/sync/lock/ww_mutex/class.rs | 151 ++++++ rust/kernel/sync/lock/ww_mutex/lock_set.rs | 449 ++++++++++++++++++ 9 files changed, 1235 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
Range-diff against v11:
1: 4942155a8873e = 1: 4942155a8873e rust: add C wrappers for ww_mutex inline functions 2: 168394bb52a6c = 2: 168394bb52a6c ww_mutex: add ww_class field unconditionally 3: 003bc88697dc8 ! 3: ba8ea8e2e2b50 rust: implement Class for ww_class support
@@ rust/kernel/sync/lock/ww_mutex/class.rs (new)
+ // SAFETY: This is static, so address is fixed and won't move.
+ unsafe {
+ $crate::sync::lock::ww_mutex::Class::new_unpinned(
-+ $crate::c_str!(::core::stringify!($name)),
++ $crate::c_str!(::core::concat!(::core::stringify!($name), "_acquire")),
++ $crate::c_str!(::core::concat!(::core::stringify!($name), "_mutex")),
+ false,
+ )
+ };
@@ rust/kernel/sync/lock/ww_mutex/class.rs (new)
+ // SAFETY: This is static, so address is fixed and won't move.
+ unsafe {
+ $crate::sync::lock::ww_mutex::Class::new_unpinned(
-+ $crate::c_str!(::core::stringify!($name)),
++ $crate::c_str!(::core::concat!(::core::stringify!($name), "_acquire")),
++ $crate::c_str!(::core::concat!(::core::stringify!($name), "_mutex")),
+ true,
+ )
+ };
@@ rust/kernel/sync/lock/ww_mutex/class.rs (new)
+ ///
+ /// Caller must guarantee that the returned value must be pinned before
+ /// its first use.
-+ pub const unsafe fn new_unpinned(name: &'static CStr, is_wait_die: bool) -> Self {
++ pub const unsafe fn new_unpinned(
++ acquire_name: &'static CStr,
++ mutex_name: &'static CStr,
++ is_wait_die: bool,
++ ) -> Self {
+ Class {
+ inner: Opaque::new(bindings::ww_class {
+ stamp: bindings::atomic_long_t { counter: 0 },
-+ acquire_name: crate::str::as_char_ptr_in_const_context(name),
-+ mutex_name: crate::str::as_char_ptr_in_const_context(name),
++ acquire_name: crate::str::as_char_ptr_in_const_context(acquire_name),
++ mutex_name: crate::str::as_char_ptr_in_const_context(mutex_name),
+ is_wait_die: is_wait_die as u32,
+ // TODO: Replace with bindings::lock_class_key::default() once
+ // stabilized for const.
@@ rust/kernel/sync/lock/ww_mutex/class.rs (new)
+ ///
+ /// The caller must ensure that ptr points to the inner field of
+ /// [Class] and that it remains valid for the lifetime 'a.
++ #[inline]
+ 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() }
+ }
+}
+
-+// SAFETY: [Class] is set up once and never modified. It's fine to share it across threads. ++// SAFETY: [Class] is immutable except for its atomic stamp. 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 {}
4: 293fbbc0bcc0f ! 4: 36bddd73ca296 rust: ww_mutex: add Mutex, AcquireCtx and MutexGuard
@@ rust/kernel/sync/lock/ww_mutex.rs
+ ///
+ /// The returned value is racy as another thread can acquire
+ /// or release the lock immediately after this call returns.
++ #[inline]
+ pub fn is_locked(&self) -> bool {
+ // SAFETY: It's safe to call ww_mutex_is_locked on
+ // a valid mutex.
@@ rust/kernel/sync/lock/ww_mutex.rs
+ fn new(mutex: &'a Mutex<'a, T>) -> Self {
+ assert!(mutex.is_locked());
+
++ // INVARIANT: lock_common and from_raw ensure that the current task holds the mutex
++ // and transfer sole responsibility for unlocking it to this guard.
+ Self {
+ mutex,
+ _not_send: NotThreadSafe,
@@ rust/kernel/sync/lock/ww_mutex.rs
+}
+
+impl<T: ?Sized> Drop for MutexGuard<'_, T> {
++ #[inline]
+ fn drop(&mut self) {
+ // SAFETY: self.mutex is locked and are about to release it.
+ unsafe { bindings::ww_mutex_unlock(self.mutex.inner.get()) };
@@ rust/kernel/sync/lock/ww_mutex/acquire_ctx.rs (new)
+ ///
+ /// After calling this function, no more mutexes can be acquired with
+ /// this context.
++ #[inline]
+ pub fn done(&self) {
+ // SAFETY: self.inner contains a valid, initialized acquire context.
+ unsafe { bindings::ww_acquire_done(self.inner.get()) };
5: d085c15ba9da4 ! 5: 8a3e94391fcee rust: ww_mutex: implement LockSet
@@ rust/kernel/sync/lock/ww_mutex/lock_set.rs (new)
+}
+
+impl Drop for RawGuard {
++ #[inline]
+ 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.
6: 8f0e93bd1781e = 6: 2b7b779f72f77 MAINTAINERS: add Onur Özkan as WW MUTEX maintainer
base-commit: 47324d3a5b3abd781295044d01d92d09f184e872
Signature · 1 line
2.51.2
Onur Özkan
reviewerPATCH v12 1/6 rust: add C wrappers for ww_mutex inline functions
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.
--- MAINTAINERS | 1 + rust/helpers/helpers.c | 1 + rust/helpers/ww_mutex.c | 33 +++++++++++++++++++++++++++++++++ 3 files changed, 35 insertions(+) create mode 100644 rust/helpers/ww_mutex.c
MAINTAINERS +1 -0
@@ -15283,6 +15283,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
rust/helpers/helpers.c +1 -0
@@ -105,4 +105,5 @@#include "vmalloc.c" #include "wait.c" #include "workqueue.c" +#include "ww_mutex.c" #include "xarray.c"
rust/helpers/ww_mutex.c new file +33 -0
@@ -0,0 +1,33 @@ +// SPDX-License-Identifier: GPL-2.0 + +#include <linux/ww_mutex.h> + +__rust_helper +void rust_helper_ww_mutex_init(struct ww_mutex *lock, struct ww_class *ww_class) +{ +ww_mutex_init (lock, ww_class); +} + +__rust_helper +void rust_helper_ww_acquire_init(struct ww_acquire_ctx *ctx, struct ww_class *ww_class) +{ +ww_acquire_init (ctx, ww_class); +} + +__rust_helper +void rust_helper_ww_acquire_done(struct ww_acquire_ctx *ctx) +{ +ww_acquire_done (ctx); +} + +__rust_helper +void rust_helper_ww_acquire_fini(struct ww_acquire_ctx *ctx) +{ +ww_acquire_fini (ctx); +} + +__rust_helper +bool rust_helper_ww_mutex_is_locked(struct ww_mutex *lock) +{ +return ww_mutex_is_locked (lock); +}
Signature · 1 line
2.51.2
Re: [PATCH v12 1/6] rust: add C wrappers for ww_mutex inline functions
Quoted 10 lines · Onur Özkan
Some of the kernel's
ww_mutexfunctions are implemented asstatic 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.Reviewed-by Daniel Almeida <[email protected]>Reviewed-by Alice Ryhl <[email protected]>Reviewed-by Lyude Paul <[email protected]>Signed-off-by Onur Özkan <[email protected]>
Quoted 6 lines
--- MAINTAINERS | 1 + rust/helpers/helpers.c | 1 + rust/helpers/ww_mutex.c | 33 +++++++++++++++++++++++++++++++++ 3 files changed, 35 insertions(+) create mode 100644 rust/helpers/ww_mutex.c
Onur Özkan
reviewerPATCH v12 2/6 ww_mutex: add ww_class field unconditionally
In reply to Onur ÖzkanRemoves DEBUG_WW_MUTEXES guard around the ww_class field in struct ww_mutex and struct ww_acquire_ctx to always initialize it.
Keeping the class pointer available unconditionally allows Rust abstractions to keep from_raw() simple on Mutex and AcquireCtx and validate Mutex and AcquireCtx class pairing in much simpler way.
--- include/linux/ww_mutex.h | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)
include/linux/ww_mutex.h +2 -6
@@ -48,9 +48,7 @@ struct ww_class {context_lock_struct (ww_mutex) {struct WW_MUTEX_BASE base;struct ww_acquire_ctx *ctx; -#ifdef DEBUG_WW_MUTEXES struct ww_class *ww_class; -#endif };context_lock_struct (ww_acquire_ctx) {
@@ -59,9 +57,9 @@ context_lock_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
@@ -111,9 +109,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 }/**
@@ -149,8 +145,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
Signature · 1 line
2.51.2
Gary Guo
maintainerRe: [PATCH v12 2/6] ww_mutex: add ww_class field unconditionally
Quoted 8 lines · Onur Özkan
Removes
DEBUG_WW_MUTEXESguard around theww_classfield in structww_mutexand structww_acquire_ctxto always initialize it.Keeping the class pointer available unconditionally allows Rust abstractions to keep
from_raw()simple on Mutex and AcquireCtx and validate Mutex and AcquireCtx class pairing in much simpler way.
Is a mismatch a memory safety issue or just a bug?
Best,
Gary
Quoted 5 lines
Reviewed-by Daniel Almeida <[email protected]>Reviewed-by Alice Ryhl <[email protected]>Signed-off-by Onur Özkan <[email protected]>
Onur Özkan
reviewerRe: [PATCH v12 2/6] ww_mutex: add ww_class field unconditionally
Quoted 11 lines · On Mon, 12 Oct 2026 12:42:31 +0100 Gary Guo <[email protected]> wrote:
Quoted 8 lines · Onur Özkan
Removes
DEBUG_WW_MUTEXESguard around theww_classfield in structww_mutexand structww_acquire_ctxto always initialize it.Keeping the class pointer available unconditionally allows Rust abstractions to keep
from_raw()simple on Mutex and AcquireCtx and validate Mutex and AcquireCtx class pairing in much simpler way.Is a mismatch a memory safety issue or just a bug?
ww_mutex_lock_acquired() explicitly calls it as an UB:
/* * Naughty, using a different class will lead to undefined behavior! */ DEBUG_LOCKS_WARN_ON(ww_ctx->ww_class != ww->ww_class);
Thanks,
Onur
Quoted 10 lines
Best,
GaryQuoted 5 lines
Reviewed-by Daniel Almeida <[email protected]>Reviewed-by Alice Ryhl <[email protected]>Signed-off-by Onur Özkan <[email protected]>
Onur Özkan
reviewerPATCH v12 3/6 rust: implement Class for ww_class support
In reply to Onur ÖzkanAdds 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_ww_class and define_wd_class macros for safely declaring static instances.
--- rust/kernel/sync/lock.rs | 1 + rust/kernel/sync/lock/ww_mutex.rs | 7 ++ rust/kernel/sync/lock/ww_mutex/class.rs | 151 ++++++++++++++++++++++++ 3 files changed, 159 insertions(+) create mode 100644 rust/kernel/sync/lock/ww_mutex.rs create mode 100644 rust/kernel/sync/lock/ww_mutex/class.rs
rust/kernel/sync/lock.rs +1 -0
@@ -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 };
rust/kernel/sync/lock/ww_mutex.rs new file +7 -0
@@ -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;
rust/kernel/sync/lock/ww_mutex/class.rs new file +151 -0
151 lines not on this page · raw message
Signature · 1 line
2.51.2
Re: [PATCH v12 3/6] rust: implement Class for ww_class support
Quoted 10 lines · Onur Özkan
Adds the Class type, the first step in supporting
ww_mutexin Rust. Class representsww_class, used for deadlock avoidance for supporting both wait-die and wound-wait semantics.Also adds the
define_ww_classanddefine_wd_classmacros for safely declaring static instances.Reviewed-by Alice Ryhl <[email protected]>Reviewed-by Daniel Almeida <[email protected]>Signed-off-by Onur Özkan <[email protected]>
With some nits below.
Quoted 123 lines
--- rust/kernel/sync/lock.rs | 1 + rust/kernel/sync/lock/ww_mutex.rs | 7 ++ rust/kernel/sync/lock/ww_mutex/class.rs | 151 ++++++++++++++++++++++++ 3 files changed, 159 insertions(+) create mode 100644 rust/kernel/sync/lock/ww_mutex.rs create mode 100644 rust/kernel/sync/lock/ww_mutex/class.rsrust/kernel/sync/lock.rs +1
-0@@ -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 };rust/kernel/sync/lock/ww_mutex.rs new file +7
-0@@ -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;rust/kernel/sync/lock/ww_mutex/class.rs new file +84
-0@@ -0,0 +1,151 @@ +// 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_ww_class!`] and [`define_wd_class!`] macros provide safe +//! ways to create classes. + +use crate ::{ + bindings, + prelude::*, + types::Opaque ,// +}; + +/// Defines a static wound-wait [`Class`]. +/// +/// # Examples +/// +/// ``` +/// use kernel::define_ww_class; +/// +/// define_ww_class!(SOME_WW_CLASS); +/// ``` +#[macro_export] +macro_rules! define_ww_class { + ($name:ident) => { +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 ::new_unpinned ( + $crate::c_str!(::core::concat!(::core::stringify!($name),"_acquire" )), + $crate::c_str!(::core::concat!(::core::stringify!($name),"_mutex" )), +false , + ) + }; + }; +} + +/// Defines a static wait-die [`Class`]. +/// +/// # Examples +/// +/// ``` +/// use kernel::define_wd_class; +/// +/// define_wd_class!(SOME_WD_CLASS); +/// ``` +#[macro_export] +macro_rules! define_wd_class { + ($name:ident) => { +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 ::new_unpinned ( + $crate::c_str!(::core::concat!(::core::stringify!($name),"_acquire" )), + $crate::c_str!(::core::concat!(::core::stringify!($name),"_mutex" )), +true , + ) + }; + }; +} + +/// Used to group mutexes together for deadlock avoidance. +/// +/// All mutexes that might be acquired together should use the same class. +/// +/// # Examples +/// +/// ``` +/// use kernel::{ +/// define_wd_class, +/// define_ww_class, // +/// }; +/// +/// define_ww_class!(SOME_WW_CLASS); +/// define_wd_class!(SOME_WD_CLASS); +/// +/// # Ok::<(), Error>(()) +/// ``` +#[pin_data] +#[repr(transparent)] +pub struct Class { + #[pin]
rust/kernel/sync/lock/ww_mutex/class.rs line 94
Quoted 10 lines
rust/kernel/sync/lock/ww_mutex/class.rs new file +10
-0+ pub(super) inner: Opaque<bindings::ww_class>, +} + +impl Class { +/// Creates an unpinned [`Class`]. +/// +/// You should prefer using [`define_ww_class!`] and [`define_wd_class!`] +/// macros. This function is `pub` only so that those macros can use it. +/// The alternative would be to expose the private fields of [`Class`] +/// which is less desirable.
Replace the paragraph with #[doc(hidden)]?
Quoted 24 lines
rust/kernel/sync/lock/ww_mutex/class.rs new file +24
-0+/// +/// # Safety +/// +/// Caller must guarantee that the returned value must be pinned before +/// its first use. +pub const unsafe fn new_unpinned ( + acquire_name: &'staticCStr , + mutex_name: &'staticCStr , + is_wait_die:bool , + ) ->Self { +Class { + inner:Opaque ::new (bindings::ww_class { + stamp: bindings::atomic_long_t { counter:0 }, + acquire_name:crate ::str::as_char_ptr_in_const_context (acquire_name), + mutex_name:crate ::str::as_char_ptr_in_const_context (mutex_name), + is_wait_die: is_wait_dieas 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".
rust/kernel/sync/lock/ww_mutex/class.rs line 128
Quoted 10 lines
rust/kernel/sync/lock/ww_mutex/class.rs new file +10
-0+ 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() },
Does ..pin_init::zeroed() work in this case?
Best,
Gary
Quoted 23 lines
rust/kernel/sync/lock/ww_mutex/class.rs new file +23
-0+ }), + } + } + +/// 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`. +#[inline] +pub const unsafe fn from_raw <'a>(ptr: *mut bindings::ww_class ) -> &'aSelf { +// SAFETY: By the safety contract, `ptr` is valid to construct `Class`. +unsafe { &*ptr.cast () } + } +} + +// SAFETY: [`Class`] is immutable except for its atomic stamp. 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 {}
Onur Özkan
reviewerRe: [PATCH v12 3/6] rust: implement Class for ww_class support
Quoted 143 lines · On Mon, 12 Oct 2026 12:03:04 +0100 Gary Guo <[email protected]> wrote:
Quoted 10 lines · Onur Özkan
Adds the Class type, the first step in supporting
ww_mutexin Rust. Class representsww_class, used for deadlock avoidance for supporting both wait-die and wound-wait semantics.Also adds the
define_ww_classanddefine_wd_classmacros for safely declaring static instances.Reviewed-by Alice Ryhl <[email protected]>Reviewed-by Daniel Almeida <[email protected]>Signed-off-by Onur Özkan <[email protected]>Reviewed-by Gary Guo <[email protected]>With some nits below.
Quoted 127 lines
--- rust/kernel/sync/lock.rs | 1 + rust/kernel/sync/lock/ww_mutex.rs | 7 ++ rust/kernel/sync/lock/ww_mutex/class.rs | 151 ++++++++++++++++++++++++ 3 files changed, 159 insertions(+) create mode 100644 rust/kernel/sync/lock/ww_mutex.rs create mode 100644 rust/kernel/sync/lock/ww_mutex/class.rsrust/kernel/sync/lock.rs +1
-0@@ -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 };rust/kernel/sync/lock/ww_mutex.rs new file +7
-0@@ -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;rust/kernel/sync/lock/ww_mutex/class.rs new file +88
-0@@ -0,0 +1,151 @@ +// 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_ww_class!`] and [`define_wd_class!`] macros provide safe +//! ways to create classes. + +use crate ::{ + bindings, + prelude::*, + types::Opaque ,// +}; + +/// Defines a static wound-wait [`Class`]. +/// +/// # Examples +/// +/// ``` +/// use kernel::define_ww_class; +/// +/// define_ww_class!(SOME_WW_CLASS); +/// ``` +#[macro_export] +macro_rules! define_ww_class { + ($name:ident) => { +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 ::new_unpinned ( + $crate::c_str!(::core::concat!(::core::stringify!($name),"_acquire" )), + $crate::c_str!(::core::concat!(::core::stringify!($name),"_mutex" )), +false , + ) + }; + }; +} + +/// Defines a static wait-die [`Class`]. +/// +/// # Examples +/// +/// ``` +/// use kernel::define_wd_class; +/// +/// define_wd_class!(SOME_WD_CLASS); +/// ``` +#[macro_export] +macro_rules! define_wd_class { + ($name:ident) => { +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 ::new_unpinned ( + $crate::c_str!(::core::concat!(::core::stringify!($name),"_acquire" )), + $crate::c_str!(::core::concat!(::core::stringify!($name),"_mutex" )), +true , + ) + }; + }; +} + +/// Used to group mutexes together for deadlock avoidance. +/// +/// All mutexes that might be acquired together should use the same class. +/// +/// # Examples +/// +/// ``` +/// use kernel::{ +/// define_wd_class, +/// define_ww_class, // +/// }; +/// +/// define_ww_class!(SOME_WW_CLASS); +/// define_wd_class!(SOME_WD_CLASS); +/// +/// # Ok::<(), Error>(()) +/// ``` +#[pin_data] +#[repr(transparent)] +pub struct Class { +#[pin] +pub (super ) inner:Opaque <bindings::ww_class >, +} + +impl Class {
Quoted 8 lines
rust/kernel/sync/lock/ww_mutex/class.rs line 94
Quoted 6 lines
rust/kernel/sync/lock/ww_mutex/class.rs new file +6
-0+/// Creates an unpinned [`Class`]. +/// +/// You should prefer using [`define_ww_class!`] and [`define_wd_class!`] +/// macros. This function is `pub` only so that those macros can use it. +/// The alternative would be to expose the private fields of [`Class`] +/// which is less desirable. Replace the paragraph with
#[doc(hidden)]?
Good idea.
Quoted 36 lines
Quoted 25 lines
rust/kernel/sync/lock/ww_mutex/class.rs new file +24
-0+/// +/// # Safety +/// +/// Caller must guarantee that the returned value must be pinned before +/// its first use. +pub const unsafe fn new_unpinned ( + acquire_name: &'staticCStr , + mutex_name: &'staticCStr , + is_wait_die:bool , + ) ->Self { +Class { + inner:Opaque ::new (bindings::ww_class { + stamp: bindings::atomic_long_t { counter:0 }, + acquire_name:crate ::str::as_char_ptr_in_const_context (acquire_name), + mutex_name:crate ::str::as_char_ptr_in_const_context (mutex_name), + is_wait_die: is_wait_dieas 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". rust/kernel/sync/lock/ww_mutex/class.rs line 128
Quoted 10 lines
rust/kernel/sync/lock/ww_mutex/class.rs new file +10
-0+ 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() },
Quoted 1 line
Does
..pin_init::zeroed()work in this case?
It does.
Thanks,
Onur
Quoted 29 lines
Best,
GaryQuoted 23 lines
rust/kernel/sync/lock/ww_mutex/class.rs new file +23
-0+ }), + } + } + +/// 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`. +#[inline] +pub const unsafe fn from_raw <'a>(ptr: *mut bindings::ww_class ) -> &'aSelf { +// SAFETY: By the safety contract, `ptr` is valid to construct `Class`. +unsafe { &*ptr.cast () } + } +} + +// SAFETY: [`Class`] is immutable except for its atomic stamp. 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 {}
Onur Özkan
reviewerPATCH v12 4/6 rust: ww_mutex: add Mutex, AcquireCtx and MutexGuard
In reply to Onur ÖzkanCovers the low-level locking API (lock, try_lock and interruptible variants) and integration with kernel bindings.
--- rust/kernel/sync/lock/ww_mutex.rs | 434 ++++++++++++++++++ rust/kernel/sync/lock/ww_mutex/acquire_ctx.rs | 149 ++++++ 2 files changed, 583 insertions(+) create mode 100644 rust/kernel/sync/lock/ww_mutex/acquire_ctx.rs
rust/kernel/sync/lock/ww_mutex.rs +434 -0
@@ -1,7 +1,441 @@// 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. +//! +//! See srctree/Documentation/locking/ww-mutex-design.rst for more details. +use crate ::{ + bindings, + container_of, + error::to_result, + prelude::*, + types::{ +NotThreadSafe , +Opaque ,// + },// +}; +use core::{ + cell::UnsafeCell , + marker::PhantomData ,// +}; + +pub use acquire_ctx::AcquireCtx ;pub use class::Class ; +mod acquire_ctx;mod class; + +/// 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::{ +/// define_ww_class, +/// sync::{ +/// lock::ww_mutex::{ +/// AcquireCtx, +/// Class, +/// Mutex, // +/// }, +/// Arc, +/// }, +/// }; +/// use pin_init::stack_pin_init; +/// +/// define_ww_class!(SOME_WW_CLASS); +/// +/// let mutex = Arc::pin_init(Mutex::new(42, &SOME_WW_CLASS), GFP_KERNEL)?; +/// let ctx = KBox::pin_init(AcquireCtx::new(&SOME_WW_CLASS), GFP_KERNEL)?; +/// +/// // SAFETY: The guard is dropped before `ctx`. +/// let guard = unsafe { ctx.lock(&mutex) }?; +/// assert_eq!(*guard, 42); +/// +/// # Ok::<(), Error>(()) +/// ``` +#[pin_data] +#[repr( C )] +pub struct Mutex <'a,T : ?Sized > { + _p:PhantomData <&'aClass >, +#[pin] + inner:Opaque <bindings::ww_mutex >, + data:UnsafeCell <T >, +} + +impl <'class,T >Mutex <'class,T > { +/// Initializes [`Mutex`] with the given `data` and [`Class`]. +pub fn new (data:T , class: &'classClass ) ->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,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. +#[inline] +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 ) + } + +/// 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 ) + } +} + +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`. +/// - `ptr` must remain valid for the lifetime `'a`. +/// - ww_class associated with this mutex must be valid for +/// the lifetime `'class`. +pub unsafe fn from_raw <'a>(ptr: *mut bindings::ww_mutex ) -> &'aSelf { +// SAFETY: By the safety contract, the caller guarantees that `ptr` +// points to a valid `ww_mutex` which is the `inner` field of `Mutex`, +// that it remains valid for the lifetime `'a` and the associated +// ww_class outlives `'class`. +// +// 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) } + } +} + +// 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` serialises access to the protected data, so it is +// `Sync` as long as the data is `Send`. +unsafe impl <T : ?Sized +Send >Sync for Mutex <'_,T > {} + +/// 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: &'aMutex <'a,T >, + _not_send:NotThreadSafe , +} + +impl <'a,T : ?Sized >MutexGuard <'a,T > { +/// Creates a new guard for the given [`Mutex`]. +fn new (mutex: &'aMutex <'a,T >) ->Self { +assert! (mutex.is_locked ()); + +// INVARIANT: `lock_common` and `from_raw` ensure that the current task holds the mutex +// and transfer sole responsibility for unlocking it to this guard. +Self { + mutex, + _not_send:NotThreadSafe , + } + } +} + +impl <'a>MutexGuard <'a, ()> { +/// Creates a [`MutexGuard`] from a raw pointer. +/// +/// If the given pointer refers to a mutex that is not locked, +/// returns [`EINVAL`]. +/// +/// This function is intended for interoperability with C code. +/// +/// # Safety +/// +/// The caller must ensure that: +/// +/// - `ptr` is a valid pointer to a `ww_mutex`. +/// - `ptr` must remain valid for the lifetime `'b`. +/// - The `ww_class` associated with the `ww_mutex` must be valid for the lifetime `'b`. +/// - The current task holds the mutex. +/// - The caller transfers responsibility for unlocking the mutex to the returned guard +/// and must not unlock it separately (including through another guard). +/// - Any associated acquire context remains valid until the mutex is unlocked, +/// even if the returned guard is forgotten. The context must not be finalized or +/// reinitialized before then. +pub unsafe fn from_raw <'b>(ptr: *mut bindings::ww_mutex ) ->Result <MutexGuard <'b, ()>> { +// SAFETY: By this function's safety contract, the caller guarantees that `ptr` points to a +// valid `ww_mutex` which is the `inner` field of a `Mutex`. The caller also guarantees +// that both `ptr` and the associated `ww_class` are valid for the lifetime `'b`. +let mutex =unsafe {Mutex ::from_raw (ptr) }; + +if !mutex.is_locked () { +return Err (EINVAL ); + } + +Ok (MutexGuard ::new (mutex)) + } +} + +impl <T : ?Sized > core::ops::Deref for MutexGuard <'_,T > { +type Target =T ; + +fn deref (&self) -> &Self ::Target { +// SAFETY: self.mutex is locked, 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: self.mutex is locked, so we have exclusive access. +unsafe { &mut *self.mutex.data.get () } + } +} + +impl <T : ?Sized >Drop for MutexGuard <'_,T > { +#[inline] +fn drop (&mut self) { +// SAFETY: self.mutex is locked and are about to release it. +unsafe { bindings::ww_mutex_unlock (self.mutex.inner.get ()) }; + } +} + +// SAFETY: `MutexGuard` can be shared between threads if the data can. +unsafe impl <T : ?Sized +Sync >Sync for MutexGuard <'_,T > {} + +/// 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 , +/// 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: &'aMutex <'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 ::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 super ::*; +use crate ::{ + define_wd_class, + define_ww_class, + sync::Arc ,// + }; + +define_ww_class! (TEST_WOUND_WAIT_CLASS ); +define_wd_class! (TEST_WAIT_DIE_CLASS ); + +#[test] +fn test_ww_mutex_basic_lock_unlock () ->Result { +let mutex =Arc ::pin_init (Mutex ::new (42 , &TEST_WOUND_WAIT_CLASS ),GFP_KERNEL )?; +let ctx =KBox ::pin_init (AcquireCtx ::new (&TEST_WOUND_WAIT_CLASS ),GFP_KERNEL )?; + +// SAFETY: The guard is dropped before `ctx`. +let guard =unsafe { ctx.lock (&mutex) }?; +assert_eq! (*guard,42 ); + +// Drop the lock. +drop (guard); + +// SAFETY: The guard is dropped before `ctx`. +let mut guard =unsafe { ctx.lock (&mutex) }?; + *guard =100 ; +assert_eq! (*guard,100 ); + +Ok (()) + } + +#[test] +fn test_ww_mutex_trylock () ->Result { +let mutex =Arc ::pin_init (Mutex ::new (123 , &TEST_WAIT_DIE_CLASS ),GFP_KERNEL )?; +let ctx =KBox ::pin_init (AcquireCtx ::new (&TEST_WAIT_DIE_CLASS ),GFP_KERNEL )?; + +// `try_lock` on unlocked mutex should succeed. +// SAFETY: The guard is dropped before `ctx`. +let guard =unsafe { ctx.try_lock (&mutex) }?; +assert_eq! (*guard,123 ); + +// Now it should fail immediately as it's already locked. +// SAFETY: Any returned guard is dropped immediately, before `ctx`. +assert! (unsafe { ctx.try_lock (&mutex) }.is_err ()); + +Ok (()) + } + +#[test] +fn test_ww_mutex_is_locked () ->Result { +let mutex =Arc ::pin_init (Mutex ::new ("hello" , &TEST_WOUND_WAIT_CLASS ),GFP_KERNEL )?; +let ctx =KBox ::pin_init (AcquireCtx ::new (&TEST_WOUND_WAIT_CLASS ),GFP_KERNEL )?; + +// Should not be locked initially. +assert! (!mutex.is_locked ()); + +// SAFETY: The guard is dropped before `ctx`. +let guard =unsafe { ctx.lock (&mutex) }?; +assert! (mutex.is_locked ()); + +drop (guard); +assert! (!mutex.is_locked ()); + +Ok (()) + } + +#[test] +fn test_ww_acquire_context_done () ->Result { +let mutex1 =Arc ::pin_init (Mutex ::new (1 , &TEST_WAIT_DIE_CLASS ),GFP_KERNEL )?; +let mutex2 =Arc ::pin_init (Mutex ::new (2 , &TEST_WAIT_DIE_CLASS ),GFP_KERNEL )?; +let ctx =KBox ::pin_init (AcquireCtx ::new (&TEST_WAIT_DIE_CLASS ),GFP_KERNEL )?; + +// Acquire multiple mutexes with the same context. +// SAFETY: The guard is dropped before `ctx`. +let guard1 =unsafe { ctx.lock (&mutex1) }?; +// SAFETY: The guard is dropped before `ctx`. +let guard2 =unsafe { ctx.lock (&mutex2) }?; + +assert_eq! (*guard1,1 ); +assert_eq! (*guard2,2 ); + + ctx.done (); + +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 (()) + } + +#[test] +fn test_guard_from_raw_unlocks_mutex () ->Result { +let mutex =Arc ::pin_init (Mutex ::new ((), &TEST_WOUND_WAIT_CLASS ),GFP_KERNEL )?; +let guard = mutex.lock ()?; + +// Keep the mutex locked without leaving another guard responsible for unlocking it. + core::mem::forget (guard); + +// SAFETY: `mutex` and its class remain valid for this test. The current task holds +// the mutex, and forgetting the previous guard lets this guard take over unlocking it. +let guard =unsafe {MutexGuard ::from_raw (mutex.inner.get ()) }?; +assert! (mutex.is_locked ()); + +drop (guard); +assert! (!mutex.is_locked ()); + +Ok (()) + } +}
rust/kernel/sync/lock/ww_mutex/acquire_ctx.rs new file +149 -0
149 lines not on this page · raw message
Signature · 1 line
2.51.2
Gary Guo
maintainerRe: [PATCH v12 4/6] rust: ww_mutex: add Mutex, AcquireCtx and MutexGuard
Quoted 74 lines · Onur Özkan
Covers the low-level locking API (lock,
try_lockand interruptible variants) and integration with kernel bindings.Reviewed-by Alice Ryhl <[email protected]>Signed-off-by Onur Özkan <[email protected]>--- rust/kernel/sync/lock/ww_mutex.rs | 434 ++++++++++++++++++ rust/kernel/sync/lock/ww_mutex/acquire_ctx.rs | 149 ++++++ 2 files changed, 583 insertions(+) create mode 100644 rust/kernel/sync/lock/ww_mutex/acquire_ctx.rsrust/kernel/sync/lock/ww_mutex.rs +51
-0@@ -1,7 +1,441 @@// 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. +//! +//! See srctree/Documentation/locking/ww-mutex-design.rst for more details. +use crate ::{ + bindings, + container_of, + error::to_result, + prelude::*, + types::{ +NotThreadSafe , +Opaque ,// + },// +}; +use core::{ + cell::UnsafeCell , + marker::PhantomData ,// +}; + +pub use acquire_ctx::AcquireCtx ;pub use class::Class ; +mod acquire_ctx;mod class; + +/// 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::{ +/// define_ww_class, +/// sync::{ +/// lock::ww_mutex::{ +/// AcquireCtx, +/// Class, +/// Mutex, // +/// }, +/// Arc, +/// }, +/// }; +/// use pin_init::stack_pin_init; +/// +/// define_ww_class!(SOME_WW_CLASS); +/// +/// let mutex = Arc::pin_init(Mutex::new(42, &SOME_WW_CLASS), GFP_KERNEL)?; +/// let ctx = KBox::pin_init(AcquireCtx::new(&SOME_WW_CLASS), GFP_KERNEL)?; +///
rust/kernel/sync/lock/ww_mutex.rs line 68
Quoted 10 lines
rust/kernel/sync/lock/ww_mutex.rs +10
-0+/// // SAFETY: The guard is dropped before `ctx`. +/// let guard = unsafe { ctx.lock(&mutex) }?; +/// assert_eq!(*guard, 42); +/// +/// # Ok::<(), Error>(()) +/// ``` +#[pin_data] +#[repr( C )] +pub struct Mutex <'a,T : ?Sized > { + _p:PhantomData <&'aClass >,
Do you expect any users that have non-static class?
So far all in-kernel user of ww mutex has a static class.
Quoted 36 lines
rust/kernel/sync/lock/ww_mutex.rs +36
-0+#[pin] + inner: Opaque<bindings::ww_mutex>, + data: UnsafeCell<T>, +} + +impl <'class,T >Mutex <'class,T > { +/// Initializes [`Mutex`] with the given `data` and [`Class`]. +pub fn new (data:T , class: &'classClass ) ->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,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. +#[inline] +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 ) + }
rust/kernel/sync/lock/ww_mutex.rs line 114
Quoted 10 lines
rust/kernel/sync/lock/ww_mutex.rs +10
-0+ +/// Similar to [`Self::lock`], but can be interrupted by signals. +pub fn lock_interruptible (&self) ->Result <MutexGuard <'_,T >> { +lock_common (self,None ,LockKind ::Interruptible ) + } + +/// 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 >> {
try_lock failure should return Option instead.
Quoted 85 lines
rust/kernel/sync/lock/ww_mutex.rs +85
-0+lock_common (self,None ,LockKind ::Try ) + } +} + +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`. +/// - `ptr` must remain valid for the lifetime `'a`. +/// - ww_class associated with this mutex must be valid for +/// the lifetime `'class`. +pub unsafe fn from_raw <'a>(ptr: *mut bindings::ww_mutex ) -> &'aSelf { +// SAFETY: By the safety contract, the caller guarantees that `ptr` +// points to a valid `ww_mutex` which is the `inner` field of `Mutex`, +// that it remains valid for the lifetime `'a` and the associated +// ww_class outlives `'class`. +// +// 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) } + } +} + +// 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` serialises access to the protected data, so it is +// `Sync` as long as the data is `Send`. +unsafe impl <T : ?Sized +Send >Sync for Mutex <'_,T > {} + +/// 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: &'aMutex <'a,T >, + _not_send:NotThreadSafe , +} + +impl <'a,T : ?Sized >MutexGuard <'a,T > { +/// Creates a new guard for the given [`Mutex`]. +fn new (mutex: &'aMutex <'a,T >) ->Self { +assert! (mutex.is_locked ()); + +// INVARIANT: `lock_common` and `from_raw` ensure that the current task holds the mutex +// and transfer sole responsibility for unlocking it to this guard. +Self { + mutex, + _not_send:NotThreadSafe , + } + } +} + +impl <'a>MutexGuard <'a, ()> { +/// Creates a [`MutexGuard`] from a raw pointer. +/// +/// If the given pointer refers to a mutex that is not locked, +/// returns [`EINVAL`]. +/// +/// This function is intended for interoperability with C code. +/// +/// # Safety +/// +/// The caller must ensure that: +/// +/// - `ptr` is a valid pointer to a `ww_mutex`. +/// - `ptr` must remain valid for the lifetime `'b`. +/// - The `ww_class` associated with the `ww_mutex` must be valid for the lifetime `'b`. +/// - The current task holds the mutex. +/// - The caller transfers responsibility for unlocking the mutex to the returned guard +/// and must not unlock it separately (including through another guard). +/// - Any associated acquire context remains valid until the mutex is unlocked, +/// even if the returned guard is forgotten. The context must not be finalized or
rust/kernel/sync/lock/ww_mutex.rs line 209
Quoted 10 lines
rust/kernel/sync/lock/ww_mutex.rs +10
-0+/// reinitialized before then. +pub unsafe fn from_raw <'b>(ptr: *mut bindings::ww_mutex ) ->Result <MutexGuard <'b, ()>> { +// SAFETY: By this function's safety contract, the caller guarantees that `ptr` points to a +// valid `ww_mutex` which is the `inner` field of a `Mutex`. The caller also guarantees +// that both `ptr` and the associated `ww_class` are valid for the lifetime `'b`. +let mutex =unsafe {Mutex ::from_raw (ptr) }; + +if !mutex.is_locked () { +return Err (EINVAL ); + }
Why is this checked despite being an unsafe function. Do you expect users to call this without knowing the lock is already locked? I think that's unlikely. Perhaps make this a debug_assert instead?
rust/kernel/sync/lock/ww_mutex.rs line 217
Quoted 8 lines
rust/kernel/sync/lock/ww_mutex.rs +8
-0+ +Ok (MutexGuard ::new (mutex)) + } +} + +impl <T : ?Sized > core::ops::Deref for MutexGuard <'_,T > { +type Target =T ; +
#[inline] for this and others.
Quoted 38 lines
rust/kernel/sync/lock/ww_mutex.rs +38
-0+fn deref (&self) -> &Self ::Target { +// SAFETY: self.mutex is locked, 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: self.mutex is locked, so we have exclusive access. +unsafe { &mut *self.mutex.data.get () } + } +} + +impl <T : ?Sized >Drop for MutexGuard <'_,T > { +#[inline] +fn drop (&mut self) { +// SAFETY: self.mutex is locked and are about to release it. +unsafe { bindings::ww_mutex_unlock (self.mutex.inner.get ()) }; + } +} + +// SAFETY: `MutexGuard` can be shared between threads if the data can. +unsafe impl <T : ?Sized +Sync >Sync for MutexGuard <'_,T > {} + +/// 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 , +/// Does not block, returns immediately if busy. +Try ,
rust/kernel/sync/lock/ww_mutex.rs line 265
Quoted 10 lines
rust/kernel/sync/lock/ww_mutex.rs +10
-0+} + +/// Internal helper that unifies the different locking kinds. +/// +/// Returns [`EINVAL`] if the [`Mutex`] has a different [`Class`]. +fn lock_common <'a,T : ?Sized >( + mutex: &'aMutex <'a,T >, + ctx:Option <&AcquireCtx <'_>>, + kind:LockKind , +) ->Result <MutexGuard <'a,T >> {
I think we should have 3 helpers, to avoid having to branch on kind which is compile-tiome known information.
The only part between the 3 kinds that is shared is the ctx_ptr computation. I think it should be extracted to its own helper, and be used for 3 types of locking.
Quoted 8 lines
rust/kernel/sync/lock/ww_mutex.rs +8
-0+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`.
rust/kernel/sync/lock/ww_mutex.rs line 283
Quoted 10 lines
rust/kernel/sync/lock/ww_mutex.rs +10
-0+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 ); + }
This is supposed to be a error condition?
Quoted 113 lines
rust/kernel/sync/lock/ww_mutex.rs +36
-0+ + 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 ::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)) +}rust/kernel/sync/lock/ww_mutex/acquire_ctx.rs new file +68
-0@@ -0,0 +1,149 @@ +// SPDX-License-Identifier: GPL-2.0 + +//! Provides [`AcquireCtx`] for managing multiple wound/wait +//! mutexes from the same [`Class`]. + +use super ::{ + lock_common, +Class , +LockKind , +Mutex , +MutexGuard ,// +}; +use crate ::{ + bindings, + prelude::*, + types::Opaque ,// +}; +use core::marker::PhantomData ; + +/// Groups multiple [`Mutex`]es for deadlock avoidance when acquired +/// with the same [`Class`]. +/// +/// # Examples +/// +/// ``` +/// use kernel::{ +/// define_ww_class, +/// sync::{ +/// lock::ww_mutex::{ +/// AcquireCtx, +/// Class, +/// Mutex, // +/// }, +/// Arc, +/// }, +/// }; +/// use pin_init::stack_pin_init; +/// +/// define_ww_class!(SOME_WW_CLASS); +/// +/// // Create mutexes. +/// let mutex1 = Arc::pin_init(Mutex::new(1, &SOME_WW_CLASS), GFP_KERNEL)?; +/// let mutex2 = Arc::pin_init(Mutex::new(2, &SOME_WW_CLASS), GFP_KERNEL)?; +/// +/// // Create acquire context for deadlock avoidance. +/// let ctx = KBox::pin_init(AcquireCtx::new(&SOME_WW_CLASS), GFP_KERNEL)?; +/// +/// // SAFETY: The guard is dropped before `ctx`. +/// let guard1 = unsafe { ctx.lock(&mutex1) }?; +/// // SAFETY: The guard is dropped before `ctx`. +/// let guard2 = unsafe { ctx.lock(&mutex2) }?; +/// +/// // Mark acquisition phase as complete. +/// ctx.done(); +/// +/// # Ok::<(), Error>(()) +/// ``` +#[pin_data( PinnedDrop )] +#[repr(transparent)] +pub struct AcquireCtx <'a> { +#[pin] +pub (super ) inner:Opaque <bindings::ww_acquire_ctx >, + _p:PhantomData <&'aClass >, +} + +impl <'class>AcquireCtx <'class> { +/// Initializes a new [`AcquireCtx`] with the given [`Class`]. +pub fn new (class: &'classClass ) ->impl PinInit <Self > {
rust/kernel/sync/lock/ww_mutex/acquire_ctx.rs line 78
Quoted 10 lines
rust/kernel/sync/lock/ww_mutex/acquire_ctx.rs new file +10
-0+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 + }) + }
I wonder if a better API would be
class.new_context()
vs.
AcquireCtx::new(&class)
?
Quoted 56 lines
rust/kernel/sync/lock/ww_mutex/acquire_ctx.rs new file +56
-0+ +/// 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 ) -> &'aSelf { +// 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. +#[inline] +pub fn done (&self) { +// SAFETY: `self.inner` contains a valid, initialized acquire context. +unsafe { bindings::ww_acquire_done (self.inner.get ()) }; + } + +/// Locks the given [`Mutex`] on this [`AcquireCtx`]. +/// +/// # Safety +/// +/// On success, the caller must keep this context valid until the mutex is unlocked, +/// even if the returned guard is forgotten. The context must not be finalized or +/// reinitialized while the mutex remains locked. +pub unsafe fn lock <'a,T >(&'a self, mutex: &'aMutex <'a,T >) ->Result <MutexGuard <'a,T >> { +lock_common (mutex,Some (self),LockKind ::Regular ) + } + +/// Similar to [`Self::lock`], but can be interrupted by signals. +/// +/// # Safety +/// +/// The same requirements as [`Self::lock`] apply. +pub unsafe fn lock_interruptible <'a,T >( + &'a self, + mutex: &'aMutex <'a,T >, + ) ->Result <MutexGuard <'a,T >> { +lock_common (mutex,Some (self),LockKind ::Interruptible ) + } + +/// Tries to lock the [`Mutex`] on this [`AcquireCtx`] without blocking. +/// +/// Unlike [`Self::lock`], no deadlock handling is performed. +/// +/// # Safety
rust/kernel/sync/lock/ww_mutex/acquire_ctx.rs line 144
Quoted 10 lines
rust/kernel/sync/lock/ww_mutex/acquire_ctx.rs new file +10
-0+/// +/// The same requirements as [`Self::lock`] apply. +pub unsafe fn try_lock <'a,T >(&'a self, mutex: &'aMutex <'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 >) {
I wonder if we can just do
let acquired = unsafe { (*self.inner.get()).acquired };
assert!(!acquired);to get rid of the unsafe. Somewhat similar to what we do with srcu. Of course, ultimately we want to use !Leak for this and can remove the check.
Thoughts?
Best,
Gary
Quoted 5 lines
rust/kernel/sync/lock/ww_mutex/acquire_ctx.rs new file +5
-0+// SAFETY: The locking methods require callers to release all acquired locks +// before this context is dropped. +unsafe { bindings::ww_acquire_fini (self.inner.get ()) }; + } +}
Onur Özkan
reviewerRe: [PATCH v12 4/6] rust: ww_mutex: add Mutex, AcquireCtx and MutexGuard
Quoted 88 lines · On Mon, 12 Oct 2026 12:40:57 +0100 Gary Guo <[email protected]> wrote:
Quoted 74 lines · Onur Özkan
Covers the low-level locking API (lock,
try_lockand interruptible variants) and integration with kernel bindings.Reviewed-by Alice Ryhl <[email protected]>Signed-off-by Onur Özkan <[email protected]>--- rust/kernel/sync/lock/ww_mutex.rs | 434 ++++++++++++++++++ rust/kernel/sync/lock/ww_mutex/acquire_ctx.rs | 149 ++++++ 2 files changed, 583 insertions(+) create mode 100644 rust/kernel/sync/lock/ww_mutex/acquire_ctx.rsrust/kernel/sync/lock/ww_mutex.rs +51
-0@@ -1,7 +1,441 @@// 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. +//! +//! See srctree/Documentation/locking/ww-mutex-design.rst for more details. +use crate ::{ + bindings, + container_of, + error::to_result, + prelude::*, + types::{ +NotThreadSafe , +Opaque ,// + },// +}; +use core::{ + cell::UnsafeCell , + marker::PhantomData ,// +}; + +pub use acquire_ctx::AcquireCtx ;pub use class::Class ; +mod acquire_ctx;mod class; + +/// 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::{ +/// define_ww_class, +/// sync::{ +/// lock::ww_mutex::{ +/// AcquireCtx, +/// Class, +/// Mutex, // +/// }, +/// Arc, +/// }, +/// }; +/// use pin_init::stack_pin_init; +/// +/// define_ww_class!(SOME_WW_CLASS); +/// +/// let mutex = Arc::pin_init(Mutex::new(42, &SOME_WW_CLASS), GFP_KERNEL)?; +/// let ctx = KBox::pin_init(AcquireCtx::new(&SOME_WW_CLASS), GFP_KERNEL)?; +/// rust/kernel/sync/lock/ww_mutex.rs line 68
Quoted 10 lines
rust/kernel/sync/lock/ww_mutex.rs +10
-0+/// // SAFETY: The guard is dropped before `ctx`. +/// let guard = unsafe { ctx.lock(&mutex) }?; +/// assert_eq!(*guard, 42); +/// +/// # Ok::<(), Error>(()) +/// ``` +#[pin_data] +#[repr( C )] +pub struct Mutex <'a,T : ?Sized > { + _p:PhantomData <&'aClass >,Do you expect any users that have non-static class?
Quoted 1 line
So far all in-kernel user of ww mutex has a static class.
Good finding. No, I don't have a non-static class user in mind. The lifetime was carried over from the earlier versions.
Quoted 41 lines
Quoted 41 lines
rust/kernel/sync/lock/ww_mutex.rs +40
-0+#[pin] + inner: Opaque<bindings::ww_mutex>, + data: UnsafeCell<T>, +} + +impl <'class,T >Mutex <'class,T > { +/// Initializes [`Mutex`] with the given `data` and [`Class`]. +pub fn new (data:T , class: &'classClass ) ->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,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. +#[inline] +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 )
Quoted 8 lines
rust/kernel/sync/lock/ww_mutex.rs line 114
Quoted 6 lines
rust/kernel/sync/lock/ww_mutex.rs +6
-0+ } + +/// 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 >> {
try_lockfailure should returnOptioninstead.
Okay.
Quoted 92 lines
Quoted 92 lines
rust/kernel/sync/lock/ww_mutex.rs +91
-0+lock_common (self,None ,LockKind ::Try ) + } +} + +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`. +/// - `ptr` must remain valid for the lifetime `'a`. +/// - ww_class associated with this mutex must be valid for +/// the lifetime `'class`. +pub unsafe fn from_raw <'a>(ptr: *mut bindings::ww_mutex ) -> &'aSelf { +// SAFETY: By the safety contract, the caller guarantees that `ptr` +// points to a valid `ww_mutex` which is the `inner` field of `Mutex`, +// that it remains valid for the lifetime `'a` and the associated +// ww_class outlives `'class`. +// +// 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) } + } +} + +// 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` serialises access to the protected data, so it is +// `Sync` as long as the data is `Send`. +unsafe impl <T : ?Sized +Send >Sync for Mutex <'_,T > {} + +/// 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: &'aMutex <'a,T >, + _not_send:NotThreadSafe , +} + +impl <'a,T : ?Sized >MutexGuard <'a,T > { +/// Creates a new guard for the given [`Mutex`]. +fn new (mutex: &'aMutex <'a,T >) ->Self { +assert! (mutex.is_locked ()); + +// INVARIANT: `lock_common` and `from_raw` ensure that the current task holds the mutex +// and transfer sole responsibility for unlocking it to this guard. +Self { + mutex, + _not_send:NotThreadSafe , + } + } +} + +impl <'a>MutexGuard <'a, ()> { +/// Creates a [`MutexGuard`] from a raw pointer. +/// +/// If the given pointer refers to a mutex that is not locked, +/// returns [`EINVAL`]. +/// +/// This function is intended for interoperability with C code. +/// +/// # Safety +/// +/// The caller must ensure that: +/// +/// - `ptr` is a valid pointer to a `ww_mutex`. +/// - `ptr` must remain valid for the lifetime `'b`. +/// - The `ww_class` associated with the `ww_mutex` must be valid for the lifetime `'b`. +/// - The current task holds the mutex. +/// - The caller transfers responsibility for unlocking the mutex to the returned guard +/// and must not unlock it separately (including through another guard). +/// - Any associated acquire context remains valid until the mutex is unlocked, +/// even if the returned guard is forgotten. The context must not be finalized or +/// reinitialized before then. +pub unsafe fn from_raw <'b>(ptr: *mut bindings::ww_mutex ) ->Result <MutexGuard <'b, ()>> { +// SAFETY: By this function's safety contract, the caller guarantees that `ptr` points to a +// valid `ww_mutex` which is the `inner` field of a `Mutex`. The caller also guarantees +// that both `ptr` and the associated `ww_class` are valid for the lifetime `'b`. +let mutex =unsafe {Mutex ::from_raw (ptr) };
Quoted 8 lines
rust/kernel/sync/lock/ww_mutex.rs line 209
Quoted 4 lines
rust/kernel/sync/lock/ww_mutex.rs +4
-0+ +if !mutex.is_locked () { +return Err (EINVAL ); + }Why is this checked despite being an unsafe function. Do you expect users to call this without knowing the lock is already locked? I think that's unlikely. Perhaps make this a
debug_assertinstead?
Sure, I don't have strong feeling on keeping it the same or making it debug_assert.
Quoted 64 lines
rust/kernel/sync/lock/ww_mutex.rs line 217
Quoted 9 lines
rust/kernel/sync/lock/ww_mutex.rs +8
-0+ +Ok (MutexGuard ::new (mutex)) + } +} + +impl <T : ?Sized > core::ops::Deref for MutexGuard <'_,T > { +type Target =T ; +
#[inline]for this and others.Quoted 38 lines
rust/kernel/sync/lock/ww_mutex.rs +38
-0+fn deref (&self) -> &Self ::Target { +// SAFETY: self.mutex is locked, 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: self.mutex is locked, so we have exclusive access. +unsafe { &mut *self.mutex.data.get () } + } +} + +impl <T : ?Sized >Drop for MutexGuard <'_,T > { +#[inline] +fn drop (&mut self) { +// SAFETY: self.mutex is locked and are about to release it. +unsafe { bindings::ww_mutex_unlock (self.mutex.inner.get ()) }; + } +} + +// SAFETY: `MutexGuard` can be shared between threads if the data can. +unsafe impl <T : ?Sized +Sync >Sync for MutexGuard <'_,T > {} + +/// 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 , +/// Does not block, returns immediately if busy. +Try ,rust/kernel/sync/lock/ww_mutex.rs line 265
Quoted 10 lines
rust/kernel/sync/lock/ww_mutex.rs +10
-0+} + +/// Internal helper that unifies the different locking kinds. +/// +/// Returns [`EINVAL`] if the [`Mutex`] has a different [`Class`]. +fn lock_common <'a,T : ?Sized >( + mutex: &'aMutex <'a,T >, + ctx:Option <&AcquireCtx <'_>>, + kind:LockKind , +) ->Result <MutexGuard <'a,T >> {I think we should have 3 helpers, to avoid having to branch on
kindwhich is compile-tiome known information.
Quoted 3 lines
The only part between the 3 kinds that is shared is the
ctx_ptrcomputation. I think it should be extracted to its own helper, and be used for 3 types of locking.
This one also carries the design from earlier versions. Previously we had slow locks as well so it wasn't only 3. I will fix this one too.
Quoted 13 lines
Quoted 13 lines
rust/kernel/sync/lock/ww_mutex.rs +12
-0+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`.
Quoted 8 lines
rust/kernel/sync/lock/ww_mutex.rs line 283
Quoted 6 lines
rust/kernel/sync/lock/ww_mutex.rs +6
-0+let mutex_class =unsafe { (*mutex_ptr).ww_class }; + +// `ctx` and `mutex` must use the same class. +if ctx_class != mutex_class { +return Err (EINVAL ); + }This is supposed to be a error condition?
Yes, or panic. Both works.
Quoted 127 lines
Quoted 114 lines
rust/kernel/sync/lock/ww_mutex.rs +36
-0+ + 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 ::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)) +}rust/kernel/sync/lock/ww_mutex/acquire_ctx.rs new file +68
-0@@ -0,0 +1,149 @@ +// SPDX-License-Identifier: GPL-2.0 + +//! Provides [`AcquireCtx`] for managing multiple wound/wait +//! mutexes from the same [`Class`]. + +use super ::{ + lock_common, +Class , +LockKind , +Mutex , +MutexGuard ,// +}; +use crate ::{ + bindings, + prelude::*, + types::Opaque ,// +}; +use core::marker::PhantomData ; + +/// Groups multiple [`Mutex`]es for deadlock avoidance when acquired +/// with the same [`Class`]. +/// +/// # Examples +/// +/// ``` +/// use kernel::{ +/// define_ww_class, +/// sync::{ +/// lock::ww_mutex::{ +/// AcquireCtx, +/// Class, +/// Mutex, // +/// }, +/// Arc, +/// }, +/// }; +/// use pin_init::stack_pin_init; +/// +/// define_ww_class!(SOME_WW_CLASS); +/// +/// // Create mutexes. +/// let mutex1 = Arc::pin_init(Mutex::new(1, &SOME_WW_CLASS), GFP_KERNEL)?; +/// let mutex2 = Arc::pin_init(Mutex::new(2, &SOME_WW_CLASS), GFP_KERNEL)?; +/// +/// // Create acquire context for deadlock avoidance. +/// let ctx = KBox::pin_init(AcquireCtx::new(&SOME_WW_CLASS), GFP_KERNEL)?; +/// +/// // SAFETY: The guard is dropped before `ctx`. +/// let guard1 = unsafe { ctx.lock(&mutex1) }?; +/// // SAFETY: The guard is dropped before `ctx`. +/// let guard2 = unsafe { ctx.lock(&mutex2) }?; +/// +/// // Mark acquisition phase as complete. +/// ctx.done(); +/// +/// # Ok::<(), Error>(()) +/// ``` +#[pin_data( PinnedDrop )] +#[repr(transparent)] +pub struct AcquireCtx <'a> { +#[pin] +pub (super ) inner:Opaque <bindings::ww_acquire_ctx >, + _p:PhantomData <&'aClass >, +} + +impl <'class>AcquireCtx <'class> { +/// Initializes a new [`AcquireCtx`] with the given [`Class`]. +pub fn new (class: &'classClass ) ->impl PinInit <Self > {rust/kernel/sync/lock/ww_mutex/acquire_ctx.rs line 78
Quoted 10 lines
rust/kernel/sync/lock/ww_mutex/acquire_ctx.rs new file +10
-0+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 + }) + }I wonder if a better API would be
Quoted 7 lines
class.new_context()vs.
AcquireCtx::new(&class)?
I would prefer to keep AcquireCtx::new(&class). It follows the same constructor style as Mutex::new(data, &class). I don't see much benefit in moving it to Class::new_context().
Quoted 70 lines
Quoted 57 lines
rust/kernel/sync/lock/ww_mutex/acquire_ctx.rs new file +56
-0+ +/// 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 ) -> &'aSelf { +// 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. +#[inline] +pub fn done (&self) { +// SAFETY: `self.inner` contains a valid, initialized acquire context. +unsafe { bindings::ww_acquire_done (self.inner.get ()) }; + } + +/// Locks the given [`Mutex`] on this [`AcquireCtx`]. +/// +/// # Safety +/// +/// On success, the caller must keep this context valid until the mutex is unlocked, +/// even if the returned guard is forgotten. The context must not be finalized or +/// reinitialized while the mutex remains locked. +pub unsafe fn lock <'a,T >(&'a self, mutex: &'aMutex <'a,T >) ->Result <MutexGuard <'a,T >> { +lock_common (mutex,Some (self),LockKind ::Regular ) + } + +/// Similar to [`Self::lock`], but can be interrupted by signals. +/// +/// # Safety +/// +/// The same requirements as [`Self::lock`] apply. +pub unsafe fn lock_interruptible <'a,T >( + &'a self, + mutex: &'aMutex <'a,T >, + ) ->Result <MutexGuard <'a,T >> { +lock_common (mutex,Some (self),LockKind ::Interruptible ) + } + +/// Tries to lock the [`Mutex`] on this [`AcquireCtx`] without blocking. +/// +/// Unlike [`Self::lock`], no deadlock handling is performed. +/// +/// # Safety rust/kernel/sync/lock/ww_mutex/acquire_ctx.rs line 144
Quoted 10 lines
rust/kernel/sync/lock/ww_mutex/acquire_ctx.rs new file +10
-0+/// +/// The same requirements as [`Self::lock`] apply. +pub unsafe fn try_lock <'a,T >(&'a self, mutex: &'aMutex <'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 >) {I wonder if we can just do
Quoted 5 lines
let acquired = unsafe { (*self.inner.get()).acquired }; assert!(!acquired);to get rid of the
unsafe. Somewhat similar to what we do with srcu. Of course, ultimately we want to use!Leakfor this and can remove the check.
I didn't understand this. You also wrote an unsafe code?
Quoted 13 lines
Thoughts?
Best,
GaryQuoted 5 lines
rust/kernel/sync/lock/ww_mutex/acquire_ctx.rs new file +5
-0+// SAFETY: The locking methods require callers to release all acquired locks +// before this context is dropped. +unsafe { bindings::ww_acquire_fini (self.inner.get ()) }; + } +}
Gary Guo
maintainerRe: [PATCH v12 4/6] rust: ww_mutex: add Mutex, AcquireCtx and MutexGuard
Quoted 31 lines · Onur Özkan
Quoted 31 lines
Quoted 18 lines
rust/kernel/sync/lock/ww_mutex/acquire_ctx.rs new file +18
-0+ +/// Similar to [`Self::lock`], but can be interrupted by signals. +/// +/// # Safety +/// +/// The same requirements as [`Self::lock`] apply. +pub unsafe fn lock_interruptible <'a,T >( + &'a self, + mutex: &'aMutex <'a,T >, + ) ->Result <MutexGuard <'a,T >> { +lock_common (mutex,Some (self),LockKind ::Interruptible ) + } + +/// Tries to lock the [`Mutex`] on this [`AcquireCtx`] without blocking. +/// +/// Unlike [`Self::lock`], no deadlock handling is performed. +/// +/// # Safety rust/kernel/sync/lock/ww_mutex/acquire_ctx.rs line 144
Quoted 10 lines
rust/kernel/sync/lock/ww_mutex/acquire_ctx.rs new file +10
-0+/// +/// The same requirements as [`Self::lock`] apply. +pub unsafe fn try_lock <'a,T >(&'a self, mutex: &'aMutex <'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 >) {I wonder if we can just do
Quoted 7 lines
Quoted 5 lines
let acquired = unsafe { (*self.inner.get()).acquired }; assert!(!acquired);to get rid of the
unsafe. Somewhat similar to what we do with srcu. Of course, ultimately we want to use!Leakfor this and can remove the check.I didn't understand this. You also wrote an unsafe code?
Sorry, I meant to get rid of "unsafe" for lock/try_lock APIs, because by adding the assertion we don't have to worry about leaked guards.
Best,
Gary
Onur Özkan
reviewerPATCH v12 5/6 rust: ww_mutex: implement LockSet
In reply to Onur ÖzkanLockSet 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 and waits for the contended mutex. It then retries the given (by the user) locking algorithm with that mutex held and the same acquire context until it succeeds.
--- rust/kernel/sync/lock/ww_mutex.rs | 6 + rust/kernel/sync/lock/ww_mutex/lock_set.rs | 449 +++++++++++++++++++++ 2 files changed, 455 insertions(+) create mode 100644 rust/kernel/sync/lock/ww_mutex/lock_set.rs
rust/kernel/sync/lock/ww_mutex.rs +6 -0
@@ -6,6 +6,10 @@//! 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. +//! //! See srctree/Documentation/locking/ww-mutex-design.rst for more details. use crate ::{
@@ -25,9 +29,11 @@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`].
rust/kernel/sync/lock/ww_mutex/lock_set.rs new file +449 -0
449 lines not on this page · raw message
Signature · 1 line
2.51.2
Gary Guo
maintainerRe: [PATCH v12 5/6] rust: ww_mutex: implement LockSet
Quoted 8 lines · Onur Özkan
LockSet is a high-level and safe API built on top of
ww_mutexwhich provides a safe and easy to use API while keeping theww_mutexsemantics.When EDEADLK is hit it drops all held locks and waits for the contended mutex. It then retries the given (by the user) locking algorithm with that mutex held and the same acquire context until it succeeds.
Do we really need this, or this can be achieved by simple retry loops?
fn retry_if_deadlock<R>(f: impl FnOnce() -> Result<R>) -> Result<R> {
loop {
match f() {
Err(EDEADLK) => (),
e => return e,
}
}
}
stack_pin_init!(let acq_ctx = AcquireCtx::new(&class));
retry_if_deadlock(|| {
let mut gaurd1 = acq_ctx.lock(&mutex1)?;
let mut guard2 = acq_ctx.lock(&mutex2)?;
acq_ctx.done();
*guard1 += 1;
*guard2 += 1;
});I think the only difference is that the LockSet acquires the contending mutex first. However, I imagine we can solve this by having a wrapped AcquireCtx that provides that.
Best,
Gary
Quoted 7 lines
Signed-off-by Onur Özkan <[email protected]>--- rust/kernel/sync/lock/ww_mutex.rs | 6 + rust/kernel/sync/lock/ww_mutex/lock_set.rs | 449 +++++++++++++++++++++ 2 files changed, 455 insertions(+) create mode 100644 rust/kernel/sync/lock/ww_mutex/lock_set.rs
Onur Özkan
reviewerRe: [PATCH v12 5/6] rust: ww_mutex: implement LockSet
Quoted 32 lines · On Mon, 12 Oct 2026 13:02:15 +0100 Gary Guo <[email protected]> wrote:
Quoted 8 lines · Onur Özkan
LockSet is a high-level and safe API built on top of
ww_mutexwhich provides a safe and easy to use API while keeping theww_mutexsemantics.When EDEADLK is hit it drops all held locks and waits for the contended mutex. It then retries the given (by the user) locking algorithm with that mutex held and the same acquire context until it succeeds.
Do we really need this, or this can be achieved by simple retry loops?
fn retry_if_deadlock<R>(f: impl FnOnce() -> Result<R>) -> Result<R> { loop { match f() { Err(EDEADLK) => (), e => return e, } } } stack_pin_init!(let acq_ctx = AcquireCtx::new(&class)); retry_if_deadlock(|| { let mut gaurd1 = acq_ctx.lock(&mutex1)?; let mut guard2 = acq_ctx.lock(&mutex2)?; acq_ctx.done(); *guard1 += 1; *guard2 += 1; });
Quoted 3 lines
I think the only difference is that the
LockSetacquires the contending mutex first. However, I imagine we can solve this by having a wrappedAcquireCtxthat provides that.
That's what LockSet already is. How would you design that wrapped AcquireCtx and what would we gain by replacing LockSet with another wrapper?
You need to remember the contended mutex, release all acquired locks before waiting for it and retry with that mutex held and the same acquire context. Also for a dynamic set, you also need to keep the acquired locks held across loop iterations and helper calls and handle repeated objects. With ordinary guards, that bookkeeping falls on the caller unless the wrapper handles it. LockSet already does all this.
Thanks,
Onur
Quoted 12 lines
Best,
GaryQuoted 7 lines
Signed-off-by Onur Özkan <[email protected]>--- rust/kernel/sync/lock/ww_mutex.rs | 6 + rust/kernel/sync/lock/ww_mutex/lock_set.rs | 449 +++++++++++++++++++++ 2 files changed, 455 insertions(+) create mode 100644 rust/kernel/sync/lock/ww_mutex/lock_set.rs
Gary Guo
maintainerRe: [PATCH v12 5/6] rust: ww_mutex: implement LockSet
Quoted 35 lines · Onur Özkan
Quoted 32 lines · On Mon, 12 Oct 2026 13:02:15 +0100 Gary Guo <[email protected]> wrote:
Quoted 8 lines · Onur Özkan
LockSet is a high-level and safe API built on top of
ww_mutexwhich provides a safe and easy to use API while keeping theww_mutexsemantics.When EDEADLK is hit it drops all held locks and waits for the contended mutex. It then retries the given (by the user) locking algorithm with that mutex held and the same acquire context until it succeeds.
Do we really need this, or this can be achieved by simple retry loops?
fn retry_if_deadlock<R>(f: impl FnOnce() -> Result<R>) -> Result<R> { loop { match f() { Err(EDEADLK) => (), e => return e, } } } stack_pin_init!(let acq_ctx = AcquireCtx::new(&class)); retry_if_deadlock(|| { let mut gaurd1 = acq_ctx.lock(&mutex1)?; let mut guard2 = acq_ctx.lock(&mutex2)?; acq_ctx.done(); *guard1 += 1; *guard2 += 1; });
Quoted 6 lines
Quoted 3 lines
I think the only difference is that the
LockSetacquires the contending mutex first. However, I imagine we can solve this by having a wrappedAcquireCtxthat provides that.That's what LockSet already is. How would you design that wrapped AcquireCtx and what would we gain by replacing LockSet with another wrapper?
The LockSet API have multiple issues that I consider problematic:
* It always allocate, even if you don't need a dynamic set.
* Accessing data searches through the vec. To access all data, you will have O(n^2) complexity spent just on vec traversal.
* There's no way of getting multiple mutable borrow of the distinct locks.
My example above has none of the issue.
If you do not need the "acquire the contended lock first" behaviour, then there's no need for wrapper, even. Also, if this behaviour is truly universal, perhaps it should become part of AcquireCtx's default behaviour instead.
Quoted 3 lines
You need to remember the contended mutex, release all acquired locks before waiting for it and retry with that mutex held and the same acquire context.
My example above doesn't have these issues. Releasing acquired locks if free with ?. Retrying is done by the helper function and not handled by user. Acquire context if really easy to be misued, can be done by the helper function too:
fn ww_transaction<R>(
class: &Class, f: impl FnOnce(&AcquireCtx) -> Result<R>
) -> Result<R> {
stack_pin_init!(let acq_ctx = AcquireCtx::new(&class));
loop {
match f(&acq_Ctx) {
Err(EDEADLK) => (),
e => return e,
}
}
}
ww_transaction(&class, |acq_ctx| {
let mut guard1 = acq_ctx.lock(&mutex1)?;
let mut guard2 = acq_ctx.lock(&mutex2)?;
acq_ctx.done();
*guard1 += 1;
*guard2 += 1;
})Quoted 4 lines
Also for a dynamic set, you also need to keep the acquired locks held across loop iterations and helper calls and handle repeated objects. With ordinary guards, that bookkeeping falls on the caller unless the wrapper handles it. LockSet already does all this.
You can make dynamic lock set just a data structure and not do everything at once, something like:
ww_transaction(&class, |acq_ctx| {
let dynset = DynLockSet::new()?;
// Check if already inside the set, if not, lock it.
dynset.lock(acq_ctx, &mutex1)?;
dynset.lock(acq_ctx, &mutex2)?;
acq_ctx.done();
dynset.with_locked(&mutex1, |guard1| *guard1 += 1)?;
dynset.with_locked(&mutex2, |guard2| *guard2 += 1)?;
})Best,
Gary
Onur Özkan
reviewerRe: [PATCH v12 5/6] rust: ww_mutex: implement LockSet
Quoted 43 lines · On Thu, 15 Oct 2026 12:53:10 +0100 Gary Guo <[email protected]> wrote:
Quoted 35 lines · Onur Özkan
Quoted 32 lines · On Mon, 12 Oct 2026 13:02:15 +0100 Gary Guo <[email protected]> wrote:
Quoted 8 lines · Onur Özkan
LockSet is a high-level and safe API built on top of
ww_mutexwhich provides a safe and easy to use API while keeping theww_mutexsemantics.When EDEADLK is hit it drops all held locks and waits for the contended mutex. It then retries the given (by the user) locking algorithm with that mutex held and the same acquire context until it succeeds.
Do we really need this, or this can be achieved by simple retry loops?
fn retry_if_deadlock<R>(f: impl FnOnce() -> Result<R>) -> Result<R> { loop { match f() { Err(EDEADLK) => (), e => return e, } } } stack_pin_init!(let acq_ctx = AcquireCtx::new(&class)); retry_if_deadlock(|| { let mut gaurd1 = acq_ctx.lock(&mutex1)?; let mut guard2 = acq_ctx.lock(&mutex2)?; acq_ctx.done(); *guard1 += 1; *guard2 += 1; });Quoted 6 lines
Quoted 3 lines
I think the only difference is that the
LockSetacquires the contending mutex first. However, I imagine we can solve this by having a wrappedAcquireCtxthat provides that.That's what LockSet already is. How would you design that wrapped AcquireCtx and what would we gain by replacing LockSet with another wrapper?
Quoted 8 lines
The LockSet API have multiple issues that I consider problematic:
* It always allocate, even if you don't need a dynamic set.
* Accessing data searches through the vec. To access all data, you will have O(n^2) complexity spent just on vec traversal.
* There's no way of getting multiple mutable borrow of the distinct locks.
I can work on these.
Quoted 6 lines
My example above has none of the issue.
If you do not need the "acquire the contended lock first" behaviour, then there's no need for wrapper, even. Also, if this behaviour is truly universal, perhaps it should become part of
AcquireCtx's default behaviour instead.
We need that functionality for the DRM use-case, it's not something that should be applied to the ww mutex primitives but I would like to hear @Daniel's opinion on this.
Quoted 23 lines
Quoted 4 lines
You need to remember the contended mutex, release all acquired locks before waiting for it and retry with that mutex held and the same acquire context.
My example above doesn't have these issues. Releasing acquired locks if free with
?. Retrying is done by the helper function and not handled by user. Acquire context if really easy to be misued, can be done by the helper function too:fn ww_transaction<R>( class: &Class, f: impl FnOnce(&AcquireCtx) -> Result<R> ) -> Result<R> { stack_pin_init!(let acq_ctx = AcquireCtx::new(&class)); loop { match f(&acq_Ctx) { Err(EDEADLK) => (), e => return e, } } } ww_transaction(&class, |acq_ctx| {
Quoted 9 lines
let mut guard1 = acq_ctx.lock(&mutex1)?;
let mut guard2 = acq_ctx.lock(&mutex2)?;
acq_ctx.done();
*guard1 += 1;
*guard2 += 1;
})This still needs the contended-lock backoff when retrying the full transaction. Simply restarting the callback can repeatedly hit EDEADLK, so the proposed code is not going to work.
Cc @Daniel
Thanks,
Onur
Quoted 24 lines
Quoted 5 lines
Also for a dynamic set, you also need to keep the acquired locks held across loop iterations and helper calls and handle repeated objects. With ordinary guards, that bookkeeping falls on the caller unless the wrapper handles it. LockSet already does all this.
You can make dynamic lock set just a data structure and not do everything at once, something like:
ww_transaction(&class, |acq_ctx| { let dynset = DynLockSet::new()?; // Check if already inside the set, if not, lock it. dynset.lock(acq_ctx, &mutex1)?; dynset.lock(acq_ctx, &mutex2)?; acq_ctx.done(); dynset.with_locked(&mutex1, |guard1| *guard1 += 1)?; dynset.with_locked(&mutex2, |guard2| *guard2 += 1)?; })Best,
Gary
Gary Guo
maintainerRe: [PATCH v12 5/6] rust: ww_mutex: implement LockSet
Quoted 11 lines · Onur Özkan
Quoted 3 lines · On Thu, 15 Oct 2026 12:53:10 +0100 Gary Guo <[email protected]> wrote:
If you do not need the "acquire the contended lock first" behaviour, then there's no need for wrapper, even. Also, if this behaviour is truly universal, perhaps it should become part of
AcquireCtx's default behaviour instead.We need that functionality for the DRM use-case, it's not something that should be applied to the ww mutex primitives but I would like to hear @Daniel's opinion on this.
Quoted 5 lines
[snip]
This still needs the contended-lock backoff when retrying the full transaction. Simply restarting the callback can repeatedly hit EDEADLK, so the proposed code is not going to work.
WW mutex is fair so the transaction will progress eventually when it becomes the oldest transaction. I think acquring the contended lock first is basically just a protection against pathological cases where say, old txn and new txn acquires lock in opposite order, than new txn would have to wound repeatedly as old txn make its way through..
But regardless I think this behaviour can just become a thin wrapper on AcquireCtx like my earlier email. Something like
#[pin_data]
struct AcquireCtxBackoff<'a> {
#[pin]
ctx: AcquireCtx,
// Set by `lock()` on `EDEADLK`; the mutex remains valid for `'a`.
contended: Option<NonNull<bindings::ww_mutex>>,
contend_acquired: bool,
}
impl<'a> AcquireCtxBackOff<'a> {
fn lock(&mut self, m: &'a Mutex<T>) -> Result<MutexGuard<'_, T>> {
if let Some(contended) = self.contended {
if !self.contended_acquired {
lock contended
}
if m.as_ptr() == contended.as_ptr() {
self.contended = None;
self.contended_acquired = false;
return a guard
}
}
match ... {
Ok(...) => ...,
Err(EDEADLK) => {
if self.contended_acquired {
unlock contended
}
self.contended = Some(m.as_ptr());
Err(EDEADLK)?
}
Err(err) => Err(err)?
}
}
}Then code that want backoff behaviour can just use this ctx.
Best,
Gary
Onur Özkan
reviewerPATCH v12 6/6 MAINTAINERS: add Onur Özkan as WW MUTEX maintainer
In reply to Onur ÖzkanAdd myself as a WW MUTEX maintainer to reflect ongoing work and future maintenance of WW MUTEX.
--- MAINTAINERS | 1 + 1 file changed, 1 insertion(+)
MAINTAINERS +1 -0
@@ -15265,6 +15265,7 @@ M: Peter Zijlstra <[email protected]> M: Ingo Molnar <[email protected]> M: Will Deacon <[email protected]> M: Boqun Feng <[email protected]> (LOCKDEP & RUST) +M: Onur Özkan <[email protected]> (WW MUTEX) R: Waiman Long <[email protected]> L: [email protected] S: Maintained
Signature · 1 line
2.51.2