Re: [PATCH v13 09/17] rust: sync: Add SpinLockIrq

Andreas Hindborg
2025-11-04T13:10:09Z

Lyude Paul <lyude@redhat.com> writes:

> A variant of SpinLock that is expected to be used in noirq contexts, so
> lock() will disable interrupts and unlock() (i.e. `Guard::drop()` will
> undo the interrupt disable.
>
> [Boqun: Port to use spin_lock_irq_disable() and
> spin_unlock_irq_enable()]
>
> Signed-off-by: Lyude Paul <lyude@redhat.com>
> Co-developed-by: Boqun Feng <boqun.feng@gmail.com>
> Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
>
> ---
> V10:
> * Also add support to GlobalLock
> * Documentation fixes from Dirk
> V11:
> * Add unit test requested by Daniel Almeida
>
> rust/kernel/sync.rs | 4 +-
> rust/kernel/sync/lock/global.rs | 3 +
> rust/kernel/sync/lock/spinlock.rs | 229 ++++++++++++++++++++++++++++++
> 3 files changed, 235 insertions(+), 1 deletion(-)
>
> diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs
> index cf5b638a097d9..f293bbe13e855 100644
> --- a/rust/kernel/sync.rs
> +++ b/rust/kernel/sync.rs
> @@ -26,7 +26,9 @@
> 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, SpinLockGuard};
> +pub use lock::spinlock::{
> + new_spinlock, new_spinlock_irq, SpinLock, SpinLockGuard, SpinLockIrq, SpinLockIrqGuard,
> +};
> pub use locked_by::LockedBy;
> pub use refcount::Refcount;
>
> diff --git a/rust/kernel/sync/lock/global.rs b/rust/kernel/sync/lock/global.rs
> index d65f94b5caf26..47e200b750c1d 100644
> --- a/rust/kernel/sync/lock/global.rs
> +++ b/rust/kernel/sync/lock/global.rs
> @@ -299,4 +299,7 @@ macro_rules! global_lock_inner {
> (backend SpinLock) => {
> $crate::sync::lock::spinlock::SpinLockBackend
> };
> + (backend SpinLockIrq) => {
> + $crate::sync::lock::spinlock::SpinLockIrqBackend
> + };
> }
> diff --git a/rust/kernel/sync/lock/spinlock.rs b/rust/kernel/sync/lock/spinlock.rs
> index d7be38ccbdc7d..6e6d571acd90c 100644
> --- a/rust/kernel/sync/lock/spinlock.rs
> +++ b/rust/kernel/sync/lock/spinlock.rs
> @@ -3,6 +3,7 @@
> //! A kernel spinlock.
> //!
> //! This module allows Rust code to use the kernel's `spinlock_t`.
> +use crate::prelude::*;
>
> /// Creates a [`SpinLock`] initialiser with the given name and a newly-created lock class.
> ///
> @@ -139,3 +140,231 @@ unsafe fn assert_is_held(ptr: *mut Self::State) {
> unsafe { bindings::spin_assert_is_held(ptr) }
> }
> }
> +
> +/// Creates a [`SpinLockIrq`] initialiser with the given name and a newly-created lock class.
> +///
> +/// It uses the name if one is given, otherwise it generates one based on the file name and line
> +/// number.
> +#[macro_export]
> +macro_rules! new_spinlock_irq {
> + ($inner:expr $(, $name:literal)? $(,)?) => {
> + $crate::sync::SpinLockIrq::new(
> + $inner, $crate::optional_name!($($name)?), $crate::static_lock_class!())
> + };
> +}
> +pub use new_spinlock_irq;
> +
> +/// A spinlock that may be acquired when local processor interrupts are disabled.
> +///
> +/// This is a version of [`SpinLock`] that can only be used in contexts where interrupts for the
> +/// local CPU are disabled. It can be acquired in two ways:
> +///
> +/// - Using [`lock()`] like any other type of lock, in which case the bindings will modify the
> +/// interrupt state to ensure that local processor interrupts remain disabled for at least as long
> +/// as the [`SpinLockIrqGuard`] exists.
> +/// - Using [`lock_with()`] in contexts where a [`LocalInterruptDisabled`] token is present and
> +/// local processor interrupts are already known to be disabled, in which case the local interrupt
> +/// state will not be touched. This method should be preferred if a [`LocalInterruptDisabled`]
> +/// token is present in the scope.
> +///
> +/// For more info on spinlocks, see [`SpinLock`]. For more information on interrupts,
> +/// [see the interrupt module](kernel::interrupt).
> +///
> +/// # Examples
> +///
> +/// The following example shows how to declare, allocate initialise and access a struct (`Example`)
> +/// that contains an inner struct (`Inner`) that is protected by a spinlock that requires local
> +/// processor interrupts to be disabled.
> +///
> +/// ```
> +/// use kernel::sync::{new_spinlock_irq, SpinLockIrq};
> +///
> +/// struct Inner {
> +/// a: u32,
> +/// b: u32,
> +/// }
> +///
> +/// #[pin_data]
> +/// struct Example {
> +/// #[pin]
> +/// c: SpinLockIrq<Inner>,
> +/// #[pin]
> +/// d: SpinLockIrq<Inner>,
> +/// }
> +///
> +/// impl Example {
> +/// fn new() -> impl PinInit<Self> {
> +/// pin_init!(Self {
> +/// c <- new_spinlock_irq!(Inner { a: 0, b: 10 }),
> +/// d <- new_spinlock_irq!(Inner { a: 20, b: 30 }),
> +/// })
> +/// }
> +/// }
> +///
> +/// // Allocate a boxed `Example`
> +/// let e = KBox::pin_init(Example::new(), GFP_KERNEL)?;
> +///
> +/// // Accessing an `Example` from a context where interrupts may not be disabled already.
> +/// let c_guard = e.c.lock(); // interrupts are disabled now, +1 interrupt disable refcount
> +/// let d_guard = e.d.lock(); // no interrupt state change, +1 interrupt disable refcount
> +///
> +/// assert_eq!(c_guard.a, 0);
> +/// assert_eq!(c_guard.b, 10);
> +/// assert_eq!(d_guard.a, 20);
> +/// assert_eq!(d_guard.b, 30);
> +///
> +/// drop(c_guard); // Dropping c_guard will not re-enable interrupts just yet, since d_guard is
> +/// // still in scope.
> +/// drop(d_guard); // Last interrupt disable reference dropped here, so interrupts are re-enabled
> +/// // now
> +/// # Ok::<(), Error>(())
> +/// ```
> +///
> +/// [`lock()`]: SpinLockIrq::lock
> +/// [`lock_with()`]: SpinLockIrq::lock_with
> +pub type SpinLockIrq<T> = super::Lock<T, SpinLockIrqBackend>;
> +
> +/// A kernel `spinlock_t` lock backend that is acquired in interrupt disabled contexts.

I would suggest rephrasing to "A kernel `spinlock_t` lock backend that can only be
acquired in interrupt disabled contexts.".

Reviewed-by: Andreas Hindborg <a.hindborg@kernel.org>

Best regards,
Andreas Hindborg