[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Zvnp0v-x1JZxnjmN@boqun-archlinux>
Date: Sun, 29 Sep 2024 16:59:14 -0700
From: Boqun Feng <boqun.feng@...il.com>
To: Lyude Paul <lyude@...hat.com>
Cc: rust-for-linux@...r.kernel.org, Danilo Krummrich <dakr@...hat.com>,
airlied@...hat.com, Ingo Molnar <mingo@...hat.com>,
Will Deacon <will@...nel.org>, Waiman Long <longman@...hat.com>,
Peter Zijlstra <peterz@...radead.org>,
Thomas Gleixner <tglx@...utronix.de>, linux-kernel@...r.kernel.org,
Benno Lossin <benno.lossin@...ton.me>,
Daniel Almeida <daniel.almeida@...labora.com>,
Gary Guo <gary@...yguo.net>, Miguel Ojeda <ojeda@...nel.org>,
Alex Gaynor <alex.gaynor@...il.com>,
Wedson Almeida Filho <wedsonaf@...il.com>,
Björn Roy Baron <bjorn3_gh@...tonmail.com>,
Andreas Hindborg <a.hindborg@...sung.com>,
Alice Ryhl <aliceryhl@...gle.com>, Trevor Gross <tmgross@...ch.edu>,
Martin Rodriguez Reboredo <yakoyoku@...il.com>,
Valentin Obst <kernel@...entinobst.de>
Subject: Re: [PATCH v6 3/3] rust: sync: Add SpinLockIrq
On Mon, Sep 16, 2024 at 05:28:06PM -0400, Lyude Paul wrote:
> A variant of SpinLock that is expected to be used in noirq contexts, and
> thus requires that the user provide an kernel::irq::IrqDisabled to prove
> they are in such a context upon lock acquisition. This is the rust
> equivalent of spin_lock_irqsave()/spin_lock_irqrestore().
>
> Signed-off-by: Lyude Paul <lyude@...hat.com>
> Reviewed-by: Benno Lossin <benno.lossin@...ton.me>
> Reviewed-by: Daniel Almeida <daniel.almeida@...labora.com>
> Reviewed-by: Gary Guo <gary@...yguo.net>
Reviewed-by: Boqun Feng <boqun.feng@...il.com>
Regards,
Boqun
>
> ---
>
> V2:
> * s/IrqSpinLock/SpinLockIrq/
> * Implement `lock::Backend` now that we have `Context`
> * Add missing periods
> * Make sure rustdoc examples compile correctly
> * Add documentation suggestions
>
> ---
> rust/kernel/sync.rs | 2 +-
> rust/kernel/sync/lock/spinlock.rs | 104 ++++++++++++++++++++++++++++++
> 2 files changed, 105 insertions(+), 1 deletion(-)
>
> diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs
> index 0ab20975a3b5d..b028ee325f2a6 100644
> --- a/rust/kernel/sync.rs
> +++ b/rust/kernel/sync.rs
> @@ -15,7 +15,7 @@
> pub use arc::{Arc, ArcBorrow, UniqueArc};
> pub use condvar::{new_condvar, CondVar, CondVarTimeoutResult};
> pub use lock::mutex::{new_mutex, Mutex};
> -pub use lock::spinlock::{new_spinlock, SpinLock};
> +pub use lock::spinlock::{new_spinlock, new_spinlock_irq, SpinLock, SpinLockIrq};
> 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 97d85a5576615..47c71d779062a 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 kernel::irq::*;
>
> /// Creates a [`SpinLock`] initialiser with the given name and a newly-created lock class.
> ///
> @@ -116,3 +117,106 @@ unsafe fn unlock(ptr: *mut Self::State, _guard_state: &Self::GuardState) {
> unsafe { bindings::spin_unlock(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 interrupts are disabled.
> +///
> +/// A version of [`SpinLock`] that can only be used in contexts where interrupts for the local CPU
> +/// are disabled. It requires that the user acquiring the lock provide proof that interrupts are
> +/// disabled through [`IrqDisabled`].
> +///
> +/// For more info, see [`SpinLock`].
> +///
> +/// # 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.
> +///
> +/// ```
> +/// use kernel::{
> +/// sync::{new_spinlock_irq, SpinLockIrq},
> +/// irq::{with_irqs_disabled, IrqDisabled}
> +/// };
> +///
> +/// struct Inner {
> +/// a: u32,
> +/// b: u32,
> +/// }
> +///
> +/// #[pin_data]
> +/// struct Example {
> +/// c: u32,
> +/// #[pin]
> +/// d: SpinLockIrq<Inner>,
> +/// }
> +///
> +/// impl Example {
> +/// fn new() -> impl PinInit<Self> {
> +/// pin_init!(Self {
> +/// c: 10,
> +/// d <- new_spinlock_irq!(Inner { a: 20, b: 30 }),
> +/// })
> +/// }
> +/// }
> +///
> +/// // Accessing an `Example` from a function that can only be called in no-irq contexts
> +/// fn noirq_work(e: &Example, irq: IrqDisabled<'_>) {
> +/// assert_eq!(e.c, 10);
> +/// assert_eq!(e.d.lock_with(irq).a, 20);
> +/// }
> +///
> +/// // Allocate a boxed `Example`
> +/// let e = Box::pin_init(Example::new(), GFP_KERNEL)?;
> +///
> +/// // Accessing an `Example` from a context where IRQs may not be disabled already.
> +/// let b = with_irqs_disabled(|irq| {
> +/// noirq_work(&e, irq);
> +/// e.d.lock_with(irq).b
> +/// });
> +/// assert_eq!(b, 30);
> +/// # Ok::<(), Error>(())
> +/// ```
> +pub type SpinLockIrq<T> = super::Lock<T, SpinLockIrqBackend>;
> +
> +/// A kernel `spinlock_t` lock backend that is acquired in no-irq contexts.
> +pub struct SpinLockIrqBackend;
> +
> +unsafe impl super::Backend for SpinLockIrqBackend {
> + type State = bindings::spinlock_t;
> + type GuardState = ();
> + type Context<'a> = IrqDisabled<'a>;
> +
> + unsafe fn init(
> + ptr: *mut Self::State,
> + name: *const core::ffi::c_char,
> + key: *mut bindings::lock_class_key,
> + ) {
> + // SAFETY: The safety requirements ensure that `ptr` is valid for writes, and `name` and
> + // `key` are valid for read indefinitely.
> + unsafe { bindings::__spin_lock_init(ptr, name, key) }
> + }
> +
> + unsafe fn lock(ptr: *mut Self::State) -> Self::GuardState {
> + // SAFETY: The safety requirements of this function ensure that `ptr` points to valid
> + // memory, and that it has been initialised before.
> + unsafe { bindings::spin_lock(ptr) }
> + }
> +
> + unsafe fn unlock(ptr: *mut Self::State, _guard_state: &Self::GuardState) {
> + // SAFETY: The safety requirements of this function ensure that `ptr` is valid and that the
> + // caller is the owner of the spinlock.
> + unsafe { bindings::spin_unlock(ptr) }
> + }
> +}
> --
> 2.46.0
>
Powered by blists - more mailing lists