Merge pull request #539 from wedsonaf/spinlock

rust: Add support for irqsave/irqrestore variant of spin lock/unlock.

Author: ojeda

rust/helpers.c

@@ -114,6 +114,20 @@ void rust_helper_spin_unlock(spinlock_t *lock)
 }
 EXPORT_SYMBOL_GPL(rust_helper_spin_unlock);
 
+unsigned long rust_helper_spin_lock_irqsave(spinlock_t *lock)
+{
+	unsigned long flags;
+	spin_lock_irqsave(lock, flags);
+	return flags;
+}
+EXPORT_SYMBOL_GPL(rust_helper_spin_lock_irqsave);
+
+void rust_helper_spin_unlock_irqrestore(spinlock_t *lock, unsigned long flags)
+{
+	spin_unlock_irqrestore(lock, flags);
+}
+EXPORT_SYMBOL_GPL(rust_helper_spin_unlock_irqrestore);
+
 void rust_helper_init_wait(struct wait_queue_entry *wq_entry)
 {
 	init_wait(wq_entry);

rust/kernel/sync/condvar.rs

@@ -82,7 +82,7 @@ impl CondVar {
         // SAFETY: No arguments, switches to another thread.
         unsafe { bindings::schedule() };
 
-        guard.guard.context = lock.lock_noguard();
+        lock.relock(&mut guard.guard.context);
 
         // SAFETY: Both `wait` and `wait_list` point to valid memory.
         unsafe { bindings::finish_wait(self.wait_list.get(), wait.get()) };

rust/kernel/sync/guard.rs

@@ -122,8 +122,19 @@ pub unsafe trait Lock {
     type GuardContext;
 
     /// Acquires the lock, making the caller its owner.
+    #[must_use]
     fn lock_noguard(&self) -> Self::GuardContext;
 
+    /// Reacquires the lock, making the caller its owner.
+    ///
+    /// The guard context before the last unlock is passed in.
+    ///
+    /// Locks that don't require this state on relock can simply use the default implementation
+    /// that calls [`Lock::lock_noguard`].
+    fn relock(&self, ctx: &mut Self::GuardContext) {
+        *ctx = self.lock_noguard();
+    }
+
     /// Releases the lock, giving up ownership of the lock.
     ///
     /// # Safety

rust/kernel/sync/mutex.rs

@@ -65,9 +65,9 @@ impl<T: ?Sized> Mutex<T> {
     /// Locks the mutex and gives the caller access to the data protected by it. Only one thread at
     /// a time is allowed to access the protected data.
     pub fn lock(&self) -> GuardMut<'_, Self> {
-        self.lock_noguard();
+        let ctx = self.lock_noguard();
         // SAFETY: The mutex was just acquired.
-        unsafe { GuardMut::new(self, ()) }
+        unsafe { GuardMut::new(self, ctx) }
     }
 }
 

rust/kernel/sync/seqlock.rs

@@ -156,6 +156,12 @@ unsafe impl<L: CreatableLock + ?Sized> Lock for SeqLock<L> {
         ctx
     }
 
+    fn relock(&self, ctx: &mut L::GuardContext) {
+        self.write_lock.relock(ctx);
+        // SAFETY: `count` contains valid memory.
+        unsafe { bindings::write_seqcount_begin(self.count.get()) };
+    }
+
     unsafe fn unlock(&self, ctx: &mut L::GuardContext) {
         // SAFETY: The safety requirements of the function ensure that lock is owned by the caller.
         unsafe { bindings::write_seqcount_end(self.count.get()) };

rust/kernel/sync/spinlock.rs

@@ -7,7 +7,7 @@
 //! See <https://www.kernel.org/doc/Documentation/locking/spinlocks.txt>.
 
 use super::{CreatableLock, GuardMut, Lock};
-use crate::{bindings, str::CStr, Opaque};
+use crate::{bindings, c_types, str::CStr, Opaque};
 use core::{cell::UnsafeCell, marker::PhantomPinned, pin::Pin};
 
 /// Safely initialises a [`SpinLock`] with the given name, generating a new lock class.
@@ -26,9 +26,51 @@ macro_rules! spinlock_init {
 /// used. The [`spinlock_init`] macro is provided to automatically assign a new lock class to a
 /// spinlock instance.
 ///
-/// [`SpinLock`] does not manage the interrupt state, so it can be used in only two cases: (a) when
-/// the caller knows that interrupts are disabled, or (b) when callers never use it in interrupt
-/// handlers (in which case it is ok for interrupts to be enabled).
+/// There are two ways to acquire the lock:
+///  - [`SpinLock::lock`], which doesn't manage interrupt state, so it should be used in only two
+///    cases: (a) when the caller knows that interrupts are disabled, or (b) when callers never use
+///    it in atomic context (e.g., interrupt handlers), in which case it is ok for interrupts to be
+///    enabled.
+///  - [`SpinLock::lock_irqdisable`], which disables interrupts if they are enabled before
+///    acquiring the lock. When the lock is released, the interrupt state is automatically returned
+///    to its value before [`SpinLock::lock_irqdisable`] was called.
+///
+/// # Examples
+///
+/// ```
+/// # use kernel::prelude::*;
+/// # use kernel::sync::SpinLock;
+/// # use core::pin::Pin;
+///
+/// struct Example {
+///     a: u32,
+///     b: u32,
+/// }
+///
+/// // Function that acquires spinlock without changing interrupt state.
+/// fn lock_example(value: &SpinLock<Example>) {
+///     let mut guard = value.lock();
+///     guard.a = 10;
+///     guard.b = 20;
+/// }
+///
+/// // Function that acquires spinlock and disables interrupts while holding it.
+/// fn lock_irqdisable_example(value: &SpinLock<Example>) {
+///     let mut guard = value.lock_irqdisable();
+///     guard.a = 30;
+///     guard.b = 40;
+/// }
+///
+/// // Initialises a spinlock and calls the example functions.
+/// pub fn spinlock_example() {
+///     // SAFETY: `spinlock_init` is called below.
+///     let mut value = unsafe { SpinLock::new(Example { a: 1, b: 2 }) };
+///     // SAFETY: We don't move `value`.
+///     kernel::spinlock_init!(unsafe { Pin::new_unchecked(&mut value) }, "value");
+///     lock_example(&value);
+///     lock_irqdisable_example(&value);
+/// }
+/// ```
 ///
 /// [`spinlock_t`]: ../../../include/linux/spinlock.h
 pub struct SpinLock<T: ?Sized> {
@@ -67,9 +109,24 @@ impl<T: ?Sized> SpinLock<T> {
     /// Locks the spinlock and gives the caller access to the data protected by it. Only one thread
     /// at a time is allowed to access the protected data.
     pub fn lock(&self) -> GuardMut<'_, Self> {
-        self.lock_noguard();
+        let ctx = self.lock_noguard();
         // SAFETY: The spinlock was just acquired.
-        unsafe { GuardMut::new(self, ()) }
+        unsafe { GuardMut::new(self, ctx) }
+    }
+
+    /// Locks the spinlock and gives the caller access to the data protected by it. Additionally it
+    /// disables interrupts (if they are enabled).
+    ///
+    /// When the lock in unlocked, the interrupt state (enabled/disabled) is restored.
+    pub fn lock_irqdisable(&self) -> GuardMut<'_, Self> {
+        let ctx = self.internal_lock_irqsave();
+        // SAFETY: The spinlock was just acquired.
+        unsafe { GuardMut::new(self, Some(ctx)) }
+    }
+
+    fn internal_lock_irqsave(&self) -> c_types::c_ulong {
+        // SAFETY: `spin_lock` points to valid memory.
+        unsafe { bindings::spin_lock_irqsave(self.spin_lock.get()) }
     }
 }
 
@@ -91,17 +148,30 @@ impl<T> CreatableLock for SpinLock<T> {
 // SAFETY: The underlying kernel `spinlock_t` object ensures mutual exclusion.
 unsafe impl<T: ?Sized> Lock for SpinLock<T> {
     type Inner = T;
-    type GuardContext = ();
+    type GuardContext = Option<c_types::c_ulong>;
 
-    fn lock_noguard(&self) {
+    fn lock_noguard(&self) -> Option<c_types::c_ulong> {
         // SAFETY: `spin_lock` points to valid memory.
         unsafe { bindings::spin_lock(self.spin_lock.get()) };
+        None
     }
 
-    unsafe fn unlock(&self, _: &mut ()) {
-        // SAFETY: The safety requirements of the function ensure that the spinlock is owned by the
-        // caller.
-        unsafe { bindings::spin_unlock(self.spin_lock.get()) };
+    unsafe fn unlock(&self, ctx: &mut Option<c_types::c_ulong>) {
+        match ctx {
+            // SAFETY: The safety requirements of the function ensure that the spinlock is owned by
+            // the caller.
+            Some(v) => unsafe { bindings::spin_unlock_irqrestore(self.spin_lock.get(), *v) },
+            // SAFETY: The safety requirements of the function ensure that the spinlock is owned by
+            // the caller.
+            None => unsafe { bindings::spin_unlock(self.spin_lock.get()) },
+        }
+    }
+
+    fn relock(&self, ctx: &mut Self::GuardContext) {
+        match ctx {
+            Some(v) => *v = self.internal_lock_irqsave(),
+            None => *ctx = self.lock_noguard(),
+        }
     }
 
     fn locked_data(&self) -> &UnsafeCell<T> {