Message-ID: <20240725222822.1784931-3-lyude@redhat.com>
Date: Thu, 25 Jul 2024 18:27:51 -0400
From: Lyude Paul <lyude@...hat.com>
To: rust-for-linux@...r.kernel.org
Cc: 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>,
	Miguel Ojeda <ojeda@...nel.org>,
	Alex Gaynor <alex.gaynor@...il.com>,
	Wedson Almeida Filho <wedsonaf@...il.com>,
	Boqun Feng <boqun.feng@...il.com>,
	Gary Guo <gary@...yguo.net>,
	Björn Roy Baron <bjorn3_gh@...tonmail.com>,
	Benno Lossin <benno.lossin@...ton.me>,
	Andreas Hindborg <a.hindborg@...sung.com>,
	Alice Ryhl <aliceryhl@...gle.com>,
	Martin Rodriguez Reboredo <yakoyoku@...il.com>,
	Valentin Obst <kernel@...entinobst.de>,
	Trevor Gross <tmgross@...ch.edu>,
	Ben Gooding <ben.gooding.dev@...il.com>,
	linux-kernel@...r.kernel.org (open list)
Subject: [PATCH 2/3] rust: sync: Introduce LockContainer trait

We want to be able to use spinlocks in no-interrupt contexts, but our
current `Lock` infrastructure doesn't allow for the ability to pass
arguments when acquiring a lock - meaning that there would be no way for us
to verify interrupts are disabled before granting a lock since we have
nowhere to pass an `IrqGuard`.

It doesn't particularly made sense for us to add the ability to pass such
an argument either: this would technically work, but then we would have to
pass empty units as arguments on all of the many locks that are not grabbed
under interrupts. As a result, we go with a slightly nicer solution:
introducing a trait for types which can contain a lock of a specific type:
LockContainer. This means we can still use locks implemented on top of
other lock types in types such as `LockedBy` - as we convert `LockedBy` to
begin using `LockContainer` internally and implement the trait for all
existing lock types.

Signed-off-by: Lyude Paul <lyude@...hat.com>
---
 rust/kernel/sync.rs           |  1 +
 rust/kernel/sync/lock.rs      | 20 ++++++++++++++++++++
 rust/kernel/sync/locked_by.rs | 11 +++++++++--
 3 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs
index 0ab20975a3b5d..14a79ebbb42d5 100644
--- a/rust/kernel/sync.rs
+++ b/rust/kernel/sync.rs
@@ -16,6 +16,7 @@
 pub use condvar::{new_condvar, CondVar, CondVarTimeoutResult};
 pub use lock::mutex::{new_mutex, Mutex};
 pub use lock::spinlock::{new_spinlock, SpinLock};
+pub use lock::LockContainer;
 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.rs b/rust/kernel/sync/lock.rs
index f6c34ca4d819f..bbd0a7465cae3 100644
--- a/rust/kernel/sync/lock.rs
+++ b/rust/kernel/sync/lock.rs
@@ -195,3 +195,23 @@ pub(crate) unsafe fn new(lock: &'a Lock<T, B>, state: B::GuardState) -> Self {
         }
     }
 }
+
+/// A trait implemented by any type which contains a [`Lock`] with a specific [`Backend`].
+pub trait LockContainer<T: ?Sized, B: Backend> {
+    /// Returns an immutable reference to the lock
+    ///
+    /// # Safety
+    ///
+    /// Since this returns a reference to the contained [`Lock`] without going through the
+    /// [`LockContainer`] implementor, it cannot be guaranteed that it is safe to acquire
+    /// this lock. Thus the caller must promise not to attempt to use the returned immutable
+    /// reference to attempt to grab the underlying lock without ensuring whatever guarantees the
+    /// [`LockContainer`] implementor's interface enforces.
+    unsafe fn get_lock_ref(&self) -> &Lock<T, B>;
+}
+
+impl<T: ?Sized, B: Backend> LockContainer<T, B> for Lock<T, B> {
+    unsafe fn get_lock_ref(&self) -> &Lock<T, B> {
+        &self
+    }
+}
diff --git a/rust/kernel/sync/locked_by.rs b/rust/kernel/sync/locked_by.rs
index babc731bd5f62..d16d89fe74e0b 100644
--- a/rust/kernel/sync/locked_by.rs
+++ b/rust/kernel/sync/locked_by.rs
@@ -95,13 +95,20 @@ impl<T, U> LockedBy<T, U> {
     /// data becomes inaccessible; if another instance of the owner is allocated *on the same
     /// memory location*, the data becomes accessible again: none of this affects memory safety
     /// because in any case at most one thread (or CPU) can access the protected data at a time.
-    pub fn new<B: Backend>(owner: &Lock<U, B>, data: T) -> Self {
+    pub fn new<B, L>(owner: &L, data: T) -> Self
+    where
+        B: Backend,
+        L: super::LockContainer<U, B>,
+    {
         build_assert!(
             size_of::<Lock<U, B>>() > 0,
             "The lock type cannot be a ZST because it may be impossible to distinguish instances"
         );
         Self {
-            owner: owner.data.get(),
+            // SAFETY: We never directly acquire the lock through this reference, we simply use it
+            // to ensure that a `Guard` the user provides us to access this container's contents
+            // belongs to the same lock that owns this data
+            owner: unsafe { owner.get_lock_ref() }.data.get(),
             data: UnsafeCell::new(data),
         }
     }
-- 
2.45.2

Powered by blists - more mailing lists