From 0d3547df6934b8f9600630322799a2a76b4567d8 Mon Sep 17 00:00:00 2001 From: Thorsten Blum Date: Wed, 31 Jul 2024 15:58:51 +0200 Subject: [PATCH 01/12] locking/ww_mutex/test: Use swap() macro Fixes the following Coccinelle/coccicheck warning reported by swap.cocci: WARNING opportunity for swap() Compile-tested only. [Boqun: Add the report tags from Jiapeng and Abaci Robot [1].] Reported-by: Abaci Robot Reported-by: Jiapeng Chong Closes: https://bugzilla.openanolis.cn/show_bug.cgi?id=11531 Link: https://lore.kernel.org/r/20241025081455.55089-1-jiapeng.chong@linux.alibaba.com [1] Acked-by: Waiman Long Signed-off-by: Thorsten Blum Signed-off-by: Boqun Feng Link: https://lore.kernel.org/r/20240731135850.81018-2-thorsten.blum@toblux.com --- kernel/locking/test-ww_mutex.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/kernel/locking/test-ww_mutex.c b/kernel/locking/test-ww_mutex.c index 5d58b2c0ef98..bcb1b9fea588 100644 --- a/kernel/locking/test-ww_mutex.c +++ b/kernel/locking/test-ww_mutex.c @@ -404,7 +404,7 @@ static inline u32 prandom_u32_below(u32 ceil) static int *get_random_order(int count) { int *order; - int n, r, tmp; + int n, r; order = kmalloc_array(count, sizeof(*order), GFP_KERNEL); if (!order) @@ -415,11 +415,8 @@ static int *get_random_order(int count) for (n = count - 1; n > 1; n--) { r = prandom_u32_below(n + 1); - if (r != n) { - tmp = order[n]; - order[n] = order[r]; - order[r] = tmp; - } + if (r != n) + swap(order[n], order[r]); } return order; From e638072e61726cae363d48812815197a2a0e097f Mon Sep 17 00:00:00 2001 From: Carlos Llamas Date: Thu, 24 Oct 2024 18:36:26 +0000 Subject: [PATCH 02/12] lockdep: Fix upper limit for LOCKDEP_*_BITS configs Lockdep has a set of configs used to determine the size of the static arrays that it uses. However, the upper limit that was initially setup for these configs is too high (30 bit shift). This equates to several GiB of static memory for individual symbols. Using such high values leads to linker errors: $ make defconfig $ ./scripts/config -e PROVE_LOCKING --set-val LOCKDEP_BITS 30 $ make olddefconfig all [...] ld: kernel image bigger than KERNEL_IMAGE_SIZE ld: section .bss VMA wraps around address space Adjust the upper limits to the maximum values that avoid these issues. The need for anything more, likely points to a problem elsewhere. Note that LOCKDEP_CHAINS_BITS was intentionally left out as its upper limit had a different symptom and has already been fixed [1]. Reported-by: J. R. Okajima Closes: https://lore.kernel.org/all/30795.1620913191@jrobl/ [1] Cc: Peter Zijlstra Cc: Boqun Feng Cc: Ingo Molnar Cc: Waiman Long Cc: Will Deacon Acked-by: Waiman Long Signed-off-by: Carlos Llamas Signed-off-by: Boqun Feng Link: https://lore.kernel.org/r/20241024183631.643450-2-cmllamas@google.com --- lib/Kconfig.debug | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug index 49a3819d4d7c..7635b36ba060 100644 --- a/lib/Kconfig.debug +++ b/lib/Kconfig.debug @@ -1504,7 +1504,7 @@ config LOCKDEP_SMALL config LOCKDEP_BITS int "Bitsize for MAX_LOCKDEP_ENTRIES" depends on LOCKDEP && !LOCKDEP_SMALL - range 10 30 + range 10 24 default 15 help Try increasing this value if you hit "BUG: MAX_LOCKDEP_ENTRIES too low!" message. @@ -1520,7 +1520,7 @@ config LOCKDEP_CHAINS_BITS config LOCKDEP_STACK_TRACE_BITS int "Bitsize for MAX_STACK_TRACE_ENTRIES" depends on LOCKDEP && !LOCKDEP_SMALL - range 10 30 + range 10 26 default 19 help Try increasing this value if you hit "BUG: MAX_STACK_TRACE_ENTRIES too low!" message. @@ -1528,7 +1528,7 @@ config LOCKDEP_STACK_TRACE_BITS config LOCKDEP_STACK_TRACE_HASH_BITS int "Bitsize for STACK_TRACE_HASH_SIZE" depends on LOCKDEP && !LOCKDEP_SMALL - range 10 30 + range 10 26 default 14 help Try increasing this value if you need large STACK_TRACE_HASH_SIZE. @@ -1536,7 +1536,7 @@ config LOCKDEP_STACK_TRACE_HASH_BITS config LOCKDEP_CIRCULAR_QUEUE_BITS int "Bitsize for elements in circular_queue struct" depends on LOCKDEP - range 10 30 + range 10 26 default 12 help Try increasing this value if you hit "lockdep bfs error:-1" warning due to __cq_enqueue() failure. From 88a79e88a97cb9309bb48a472be2bf1316d40adc Mon Sep 17 00:00:00 2001 From: Carlos Llamas Date: Thu, 24 Oct 2024 18:36:27 +0000 Subject: [PATCH 03/12] lockdep: Clarify size for LOCKDEP_*_BITS configs The LOCKDEP_*_BITS configs control the size of internal structures used by lockdep. The size is calculated as a power of two of the configured value (e.g. 16 => 64KB). Update these descriptions to more accurately reflect this, as "Bitsize" can be misleading. Suggested-by: Andrew Morton Cc: Peter Zijlstra Cc: Boqun Feng Cc: Ingo Molnar Cc: Waiman Long Cc: Will Deacon Signed-off-by: Carlos Llamas Acked-by: Waiman Long Signed-off-by: Boqun Feng Link: https://lore.kernel.org/r/20241024183631.643450-3-cmllamas@google.com --- lib/Kconfig.debug | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug index 7635b36ba060..cf2a41dc7682 100644 --- a/lib/Kconfig.debug +++ b/lib/Kconfig.debug @@ -1502,7 +1502,7 @@ config LOCKDEP_SMALL bool config LOCKDEP_BITS - int "Bitsize for MAX_LOCKDEP_ENTRIES" + int "Size for MAX_LOCKDEP_ENTRIES (as Nth power of 2)" depends on LOCKDEP && !LOCKDEP_SMALL range 10 24 default 15 @@ -1510,7 +1510,7 @@ config LOCKDEP_BITS Try increasing this value if you hit "BUG: MAX_LOCKDEP_ENTRIES too low!" message. config LOCKDEP_CHAINS_BITS - int "Bitsize for MAX_LOCKDEP_CHAINS" + int "Size for MAX_LOCKDEP_CHAINS (as Nth power of 2)" depends on LOCKDEP && !LOCKDEP_SMALL range 10 21 default 16 @@ -1518,7 +1518,7 @@ config LOCKDEP_CHAINS_BITS Try increasing this value if you hit "BUG: MAX_LOCKDEP_CHAINS too low!" message. config LOCKDEP_STACK_TRACE_BITS - int "Bitsize for MAX_STACK_TRACE_ENTRIES" + int "Size for MAX_STACK_TRACE_ENTRIES (as Nth power of 2)" depends on LOCKDEP && !LOCKDEP_SMALL range 10 26 default 19 @@ -1526,7 +1526,7 @@ config LOCKDEP_STACK_TRACE_BITS Try increasing this value if you hit "BUG: MAX_STACK_TRACE_ENTRIES too low!" message. config LOCKDEP_STACK_TRACE_HASH_BITS - int "Bitsize for STACK_TRACE_HASH_SIZE" + int "Size for STACK_TRACE_HASH_SIZE (as Nth power of 2)" depends on LOCKDEP && !LOCKDEP_SMALL range 10 26 default 14 @@ -1534,7 +1534,7 @@ config LOCKDEP_STACK_TRACE_HASH_BITS Try increasing this value if you need large STACK_TRACE_HASH_SIZE. config LOCKDEP_CIRCULAR_QUEUE_BITS - int "Bitsize for elements in circular_queue struct" + int "Size for elements in circular_queue struct (as Nth power of 2)" depends on LOCKDEP range 10 26 default 12 From bd7b5ae26618ad2bd6f6264e2cb6c5815d323e75 Mon Sep 17 00:00:00 2001 From: Carlos Llamas Date: Thu, 24 Oct 2024 18:36:28 +0000 Subject: [PATCH 04/12] lockdep: Document MAX_LOCKDEP_CHAIN_HLOCKS calculation Define a macro AVG_LOCKDEP_CHAIN_DEPTH to document the magic number '5' used in the calculation of MAX_LOCKDEP_CHAIN_HLOCKS. The number represents the estimated average depth (number of locks held) of a lock chain. The calculation of MAX_LOCKDEP_CHAIN_HLOCKS was first added in commit 443cd507ce7f ("lockdep: add lock_class information to lock_chain and output it"). Suggested-by: Waiman Long Cc: Huang Ying Cc: J. R. Okajima Cc: Peter Zijlstra Cc: Boqun Feng Cc: Ingo Molnar Cc: Will Deacon Acked-by: Waiman Long Signed-off-by: Carlos Llamas Acked-by: "Huang, Ying" Signed-off-by: Boqun Feng Link: https://lore.kernel.org/r/20241024183631.643450-4-cmllamas@google.com --- kernel/locking/lockdep_internals.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/kernel/locking/lockdep_internals.h b/kernel/locking/lockdep_internals.h index bbe9000260d0..20f9ef58d3d0 100644 --- a/kernel/locking/lockdep_internals.h +++ b/kernel/locking/lockdep_internals.h @@ -119,7 +119,8 @@ static const unsigned long LOCKF_USED_IN_IRQ_READ = #define MAX_LOCKDEP_CHAINS (1UL << MAX_LOCKDEP_CHAINS_BITS) -#define MAX_LOCKDEP_CHAIN_HLOCKS (MAX_LOCKDEP_CHAINS*5) +#define AVG_LOCKDEP_CHAIN_DEPTH 5 +#define MAX_LOCKDEP_CHAIN_HLOCKS (MAX_LOCKDEP_CHAINS * AVG_LOCKDEP_CHAIN_DEPTH) extern struct lock_chain lock_chains[]; From 8148fa2e022bae29f21bb9a2c4cc796334fd372b Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Mon, 9 Dec 2024 19:08:10 +0200 Subject: [PATCH 05/12] lockdep: Mark chain_hlock_class_idx() with __maybe_unused When chain_hlock_class_idx() is unused, it prevents kernel builds with clang, `make W=1` and CONFIG_WERROR=y, CONFIG_LOCKDEP=y and CONFIG_PROVE_LOCKING=n: kernel/locking/lockdep.c:435:28: error: unused function 'chain_hlock_class_idx' [-Werror,-Wunused-function] Fix this by marking it with __maybe_unused. See also commit 6863f5643dd7 ("kbuild: allow Clang to find unused static inline functions for W=1 build"). [Boqun: add more config information of the error] Signed-off-by: Andy Shevchenko Signed-off-by: Boqun Feng Link: https://lore.kernel.org/r/20241209170810.1485183-1-andriy.shevchenko@linux.intel.com --- kernel/locking/lockdep.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c index 2d8ec0351ef9..fe04a2145ca7 100644 --- a/kernel/locking/lockdep.c +++ b/kernel/locking/lockdep.c @@ -430,7 +430,7 @@ static inline u16 hlock_id(struct held_lock *hlock) return (hlock->class_idx | (hlock->read << MAX_LOCKDEP_KEYS_BITS)); } -static inline unsigned int chain_hlock_class_idx(u16 hlock_id) +static inline __maybe_unused unsigned int chain_hlock_class_idx(u16 hlock_id) { return hlock_id & (MAX_LOCKDEP_KEYS - 1); } From 3430600925859be3c8588b8220173758c7860e8c Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Mon, 2 Dec 2024 21:34:45 +0200 Subject: [PATCH 06/12] lockdep: Move lockdep_assert_locked() under #ifdef CONFIG_PROVE_LOCKING When lockdep_assert_locked() is unused, it prevents kernel builds with clang, `make W=1` and CONFIG_WERROR=y, CONFIG_LOCKDEP=y and CONFIG_PROVE_LOCKING=n: kernel/locking/lockdep.c:160:20: error: unused function 'lockdep_assert_locked' [-Werror,-Wunused-function] Fix this by moving it under the respective ifdeffery. See also commit 6863f5643dd7 ("kbuild: allow Clang to find unused static inline functions for W=1 build"). [Boqun: add more config information of the error] Signed-off-by: Andy Shevchenko Signed-off-by: Boqun Feng Link: https://lore.kernel.org/r/20241202193445.769567-1-andriy.shevchenko@linux.intel.com --- kernel/locking/lockdep.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c index fe04a2145ca7..29acd238dad7 100644 --- a/kernel/locking/lockdep.c +++ b/kernel/locking/lockdep.c @@ -157,10 +157,12 @@ static inline void lockdep_unlock(void) __this_cpu_dec(lockdep_recursion); } +#ifdef CONFIG_PROVE_LOCKING static inline bool lockdep_assert_locked(void) { return DEBUG_LOCKS_WARN_ON(__owner != current); } +#endif static struct task_struct *lockdep_selftest_task_struct; From 9793c9bb91f1b05473bb6d4a2323a259ef00ff2e Mon Sep 17 00:00:00 2001 From: Boqun Feng Date: Wed, 27 Nov 2024 10:30:24 -0800 Subject: [PATCH 07/12] locking: MAINTAINERS: Start watching Rust locking primitives It makes sense to add Rust locking primitives under the watch of general locking primitives maintainers. This will encourage more reviews and find potential issues earlier. Hence add related Rust files into the LOCKING PRIMITIVES entry in MAINTAINERS. While we are at it, change the role of myself into the maintainer of LOCKDEP and RUST to reflect my responsibility for the corresponding code. Acked-by: Miguel Ojeda Acked-by: Peter Zijlstra (Intel) Acked-by: Ingo Molnar Signed-off-by: Boqun Feng https://lore.kernel.org/lkml/20241128054022.19586-2-boqun.feng@gmail.com/ --- MAINTAINERS | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/MAINTAINERS b/MAINTAINERS index 1e930c7a58b1..e0495700914d 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -13423,8 +13423,8 @@ LOCKING PRIMITIVES M: Peter Zijlstra M: Ingo Molnar M: Will Deacon +M: Boqun Feng (LOCKDEP & RUST) R: Waiman Long -R: Boqun Feng (LOCKDEP) L: linux-kernel@vger.kernel.org S: Maintained T: git git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git locking/core @@ -13438,6 +13438,11 @@ F: include/linux/seqlock.h F: include/linux/spinlock*.h F: kernel/locking/ F: lib/locking*.[ch] +F: rust/helpers/mutex.c +F: rust/helpers/spinlock.c +F: rust/kernel/sync/lock.rs +F: rust/kernel/sync/lock/ +F: rust/kernel/sync/locked_by.rs X: kernel/locking/locktorture.c LOGICAL DISK MANAGER SUPPORT (LDM, Windows 2000/XP/Vista Dynamic Disks) From 15abc88057eeec052aefde897df277eca2340ac6 Mon Sep 17 00:00:00 2001 From: Lyude Paul Date: Tue, 19 Nov 2024 18:11:03 -0500 Subject: [PATCH 08/12] rust: sync: Add Lock::from_raw() for Lock<(), B> The KMS bindings [1] have a few bindings that require manually acquiring specific locks before calling certain functions. At the moment though, the only way of acquiring these locks in bindings is to simply call the C locking functions directly - since said locks are not initialized on the Rust side of things. However - if we add `#[repr(C)]` to `Lock<(), B>`, then given `()` is a ZST - `Lock<(), B>` becomes equivalent in data layout to its inner `B::State` type. Since locks in C don't have data explicitly associated with them anyway, we can take advantage of this to add a `Lock::from_raw()` function that can translate a raw pointer to `B::State` into its proper `Lock<(), B>` equivalent. This lets us simply acquire a reference to the lock in question and work with it like it was initialized on the Rust side of things, allowing us to use less unsafe code to implement bindings with lock requirements. [Boqun: Use "Link:" instead of a URL and format the commit log] Signed-off-by: Lyude Paul Reviewed-by: Alice Ryhl Link: https://patchwork.freedesktop.org/series/131522/ [1] Signed-off-by: Boqun Feng Link: https://lore.kernel.org/r/20241119231146.2298971-2-lyude@redhat.com --- rust/kernel/sync/lock.rs | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/rust/kernel/sync/lock.rs b/rust/kernel/sync/lock.rs index 41dcddac69e2..57dc2e90e504 100644 --- a/rust/kernel/sync/lock.rs +++ b/rust/kernel/sync/lock.rs @@ -96,6 +96,7 @@ unsafe fn relock(ptr: *mut Self::State, guard_state: &mut Self::GuardState) { /// /// Exposes one of the kernel locking primitives. Which one is exposed depends on the lock /// [`Backend`] specified as the generic parameter `B`. +#[repr(C)] #[pin_data] pub struct Lock { /// The kernel lock object. @@ -134,6 +135,28 @@ pub fn new(t: T, name: &'static CStr, key: &'static LockClassKey) -> impl PinIni } } +impl Lock<(), B> { + /// Constructs a [`Lock`] from a raw pointer. + /// + /// This can be useful for interacting with a lock which was initialised outside of Rust. + /// + /// # Safety + /// + /// The caller promises that `ptr` points to a valid initialised instance of [`State`] during + /// the whole lifetime of `'a`. + /// + /// [`State`]: Backend::State + pub unsafe fn from_raw<'a>(ptr: *mut B::State) -> &'a Self { + // SAFETY: + // - By the safety contract `ptr` must point to a valid initialised instance of `B::State` + // - Since the lock data type is `()` which is a ZST, `state` is the only non-ZST member of + // the struct + // - Combined with `#[repr(C)]`, this guarantees `Self` has an equivalent data layout to + // `B::State`. + unsafe { &*ptr.cast() } + } +} + impl Lock { /// Acquires the lock and gives the caller access to the data protected by it. pub fn lock(&self) -> Guard<'_, T, B> { From daa03fe50ec376aeadd63a264c471c56af194e83 Mon Sep 17 00:00:00 2001 From: Lyude Paul Date: Tue, 19 Nov 2024 18:11:04 -0500 Subject: [PATCH 09/12] rust: sync: Make Guard::new() public Since we added a `Lock::from_raw()` function previously, it makes sense to also introduce an interface for creating a `Guard` from a reference to a `Lock` for instances where we've derived the `Lock` from a raw pointer and know that the lock is already acquired, there are such usages in KMS API. [Boqun: Add backquotes to type names, reformat the commit log, reword a bit on the usage of KMS API] Signed-off-by: Lyude Paul Reviewed-by: Filipe Xavier Reviewed-by: Alice Ryhl Signed-off-by: Boqun Feng Link: https://lore.kernel.org/r/20241119231146.2298971-3-lyude@redhat.com --- rust/kernel/sync/lock.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust/kernel/sync/lock.rs b/rust/kernel/sync/lock.rs index 57dc2e90e504..72dbf3fbb259 100644 --- a/rust/kernel/sync/lock.rs +++ b/rust/kernel/sync/lock.rs @@ -234,7 +234,7 @@ impl<'a, T: ?Sized, B: Backend> Guard<'a, T, B> { /// # Safety /// /// The caller must ensure that it owns the lock. - pub(crate) unsafe fn new(lock: &'a Lock, state: B::GuardState) -> Self { + pub unsafe fn new(lock: &'a Lock, state: B::GuardState) -> Self { Self { lock, state, From 37624dde4768ec25d2f9798aa75bf32e18c0eae2 Mon Sep 17 00:00:00 2001 From: Lyude Paul Date: Wed, 20 Nov 2024 17:26:28 -0500 Subject: [PATCH 10/12] rust: sync: Add MutexGuard type alias A simple helper alias for code that needs to deal with Guard types returned from Mutexes. Signed-off-by: Lyude Paul Reviewed-by: Alice Ryhl Signed-off-by: Boqun Feng Link: https://lore.kernel.org/r/20241120222742.2490495-2-lyude@redhat.com --- rust/kernel/sync.rs | 2 +- rust/kernel/sync/lock/mutex.rs | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs index 1eab7ebf25fd..2721b5c8deda 100644 --- a/rust/kernel/sync.rs +++ b/rust/kernel/sync.rs @@ -16,7 +16,7 @@ pub use arc::{Arc, ArcBorrow, UniqueArc}; pub use condvar::{new_condvar, CondVar, CondVarTimeoutResult}; pub use lock::global::{global_lock, GlobalGuard, GlobalLock, GlobalLockBackend, GlobalLockedBy}; -pub use lock::mutex::{new_mutex, Mutex}; +pub use lock::mutex::{new_mutex, Mutex, MutexGuard}; pub use lock::spinlock::{new_spinlock, SpinLock}; pub use locked_by::LockedBy; diff --git a/rust/kernel/sync/lock/mutex.rs b/rust/kernel/sync/lock/mutex.rs index 0e946ebefce1..10a70c07268d 100644 --- a/rust/kernel/sync/lock/mutex.rs +++ b/rust/kernel/sync/lock/mutex.rs @@ -86,6 +86,14 @@ macro_rules! new_mutex { /// [`struct mutex`]: srctree/include/linux/mutex.h pub type Mutex = super::Lock; +/// A [`Guard`] acquired from locking a [`Mutex`]. +/// +/// This is simply a type alias for a [`Guard`] returned from locking a [`Mutex`]. It will unlock +/// the [`Mutex`] upon being dropped. +/// +/// [`Guard`]: super::Guard +pub type MutexGuard<'a, T> = super::Guard<'a, T, MutexBackend>; + /// A kernel `struct mutex` lock backend. pub struct MutexBackend; From eb5ccb038284dc0e69822d71aafcbf7b57394aad Mon Sep 17 00:00:00 2001 From: Lyude Paul Date: Wed, 20 Nov 2024 17:26:29 -0500 Subject: [PATCH 11/12] rust: sync: Add SpinLockGuard type alias A simple helper alias for code that needs to deal with Guard types returned from SpinLocks. Signed-off-by: Lyude Paul Reviewed-by: Alice Ryhl Signed-off-by: Boqun Feng Link: https://lore.kernel.org/r/20241120222742.2490495-3-lyude@redhat.com --- rust/kernel/sync.rs | 2 +- rust/kernel/sync/lock/spinlock.rs | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs index 2721b5c8deda..dffdaad972ce 100644 --- a/rust/kernel/sync.rs +++ b/rust/kernel/sync.rs @@ -17,7 +17,7 @@ pub use condvar::{new_condvar, CondVar, CondVarTimeoutResult}; pub use lock::global::{global_lock, GlobalGuard, GlobalLock, GlobalLockBackend, GlobalLockedBy}; pub use lock::mutex::{new_mutex, Mutex, MutexGuard}; -pub use lock::spinlock::{new_spinlock, SpinLock}; +pub use lock::spinlock::{new_spinlock, SpinLock, SpinLockGuard}; pub use locked_by::LockedBy; /// Represents a lockdep class. It's a wrapper around C's `lock_class_key`. diff --git a/rust/kernel/sync/lock/spinlock.rs b/rust/kernel/sync/lock/spinlock.rs index 9f4d128bed98..081c0220013b 100644 --- a/rust/kernel/sync/lock/spinlock.rs +++ b/rust/kernel/sync/lock/spinlock.rs @@ -87,6 +87,14 @@ macro_rules! new_spinlock { /// A kernel `spinlock_t` lock backend. pub struct SpinLockBackend; +/// A [`Guard`] acquired from locking a [`SpinLock`]. +/// +/// This is simply a type alias for a [`Guard`] returned from locking a [`SpinLock`]. It will unlock +/// the [`SpinLock`] upon being dropped. +/// +/// [`Guard`]: super::Guard +pub type SpinLockGuard<'a, T> = super::Guard<'a, T, SpinLockBackend>; + // SAFETY: The underlying kernel `spinlock_t` object ensures mutual exclusion. `relock` uses the // default implementation that always calls the same locking method. unsafe impl super::Backend for SpinLockBackend { From fbd7a5a0359bc770e898d918d84977ea61163aad Mon Sep 17 00:00:00 2001 From: Lyude Paul Date: Mon, 25 Nov 2024 15:40:58 -0500 Subject: [PATCH 12/12] rust: sync: Add lock::Backend::assert_is_held() Since we've exposed Lock::from_raw() and Guard::new() publically, we want to be able to make sure that we assert that a lock is actually held when constructing a Guard for it to handle instances of unsafe Guard::new() calls outside of our lock module. Hence add a new method assert_is_held() to Backend, which uses lockdep to check whether or not a lock has been acquired. When lockdep is disabled, this has no overhead. [Boqun: Resolve the conflicts with exposing Guard::new(), reword the commit log a bit and format "unsafe { ; }" into "unsafe { }" for the consistency. ] Signed-off-by: Lyude Paul Signed-off-by: Boqun Feng Link: https://lore.kernel.org/r/20241125204139.656801-1-lyude@redhat.com --- rust/helpers/mutex.c | 5 +++++ rust/helpers/spinlock.c | 5 +++++ rust/kernel/sync/lock.rs | 10 ++++++++++ rust/kernel/sync/lock/mutex.rs | 5 +++++ rust/kernel/sync/lock/spinlock.rs | 5 +++++ 5 files changed, 30 insertions(+) diff --git a/rust/helpers/mutex.c b/rust/helpers/mutex.c index 7e00680958ef..06575553eda5 100644 --- a/rust/helpers/mutex.c +++ b/rust/helpers/mutex.c @@ -12,3 +12,8 @@ void rust_helper___mutex_init(struct mutex *mutex, const char *name, { __mutex_init(mutex, name, key); } + +void rust_helper_mutex_assert_is_held(struct mutex *mutex) +{ + lockdep_assert_held(mutex); +} diff --git a/rust/helpers/spinlock.c b/rust/helpers/spinlock.c index 5971fdf6f755..42c4bf01a23e 100644 --- a/rust/helpers/spinlock.c +++ b/rust/helpers/spinlock.c @@ -30,3 +30,8 @@ int rust_helper_spin_trylock(spinlock_t *lock) { return spin_trylock(lock); } + +void rust_helper_spin_assert_is_held(spinlock_t *lock) +{ + lockdep_assert_held(lock); +} diff --git a/rust/kernel/sync/lock.rs b/rust/kernel/sync/lock.rs index 72dbf3fbb259..eb80048e0110 100644 --- a/rust/kernel/sync/lock.rs +++ b/rust/kernel/sync/lock.rs @@ -90,6 +90,13 @@ unsafe fn relock(ptr: *mut Self::State, guard_state: &mut Self::GuardState) { // SAFETY: The safety requirements ensure that the lock is initialised. *guard_state = unsafe { Self::lock(ptr) }; } + + /// Asserts that the lock is held using lockdep. + /// + /// # Safety + /// + /// Callers must ensure that [`Backend::init`] has been previously called. + unsafe fn assert_is_held(ptr: *mut Self::State); } /// A mutual exclusion primitive. @@ -235,6 +242,9 @@ impl<'a, T: ?Sized, B: Backend> Guard<'a, T, B> { /// /// The caller must ensure that it owns the lock. pub unsafe fn new(lock: &'a Lock, state: B::GuardState) -> Self { + // SAFETY: The caller can only hold the lock if `Backend::init` has already been called. + unsafe { B::assert_is_held(lock.state.get()) }; + Self { lock, state, diff --git a/rust/kernel/sync/lock/mutex.rs b/rust/kernel/sync/lock/mutex.rs index 10a70c07268d..70cadbc2e8e2 100644 --- a/rust/kernel/sync/lock/mutex.rs +++ b/rust/kernel/sync/lock/mutex.rs @@ -134,4 +134,9 @@ unsafe fn try_lock(ptr: *mut Self::State) -> Option { None } } + + unsafe fn assert_is_held(ptr: *mut Self::State) { + // SAFETY: The `ptr` pointer is guaranteed to be valid and initialized before use. + unsafe { bindings::mutex_assert_is_held(ptr) } + } } diff --git a/rust/kernel/sync/lock/spinlock.rs b/rust/kernel/sync/lock/spinlock.rs index 081c0220013b..ab2f8d075311 100644 --- a/rust/kernel/sync/lock/spinlock.rs +++ b/rust/kernel/sync/lock/spinlock.rs @@ -133,4 +133,9 @@ unsafe fn try_lock(ptr: *mut Self::State) -> Option { None } } + + unsafe fn assert_is_held(ptr: *mut Self::State) { + // SAFETY: The `ptr` pointer is guaranteed to be valid and initialized before use. + unsafe { bindings::spin_assert_is_held(ptr) } + } }