[PATCH 0/2] rust: sync: lock: allow fallible initializers in Lock::new

[PATCH 0/2] rust: sync: lock: allow fallible initializers in Lock::new

[Ivan Tverdokhleb]

GpuBuddyInner keeps the C gpu_buddy structure next to a separate
Mutex<()> and has a TODO to move the structure inside the mutex once
Mutex::new() accepts a fallible initializer.

Patch 1 makes Lock::new() generic over the error type of the initializer
for the protected data, so a fallible initializer such as
Opaque::try_ffi_init() can be wrapped in a Mutex or SpinLock.

Patch 2 uses it to put the gpu_buddy allocator directly inside the Mutex
in GPU buddy, so that the allocator can only be reached through the lock
guard, and resolves the TODO.

Tested by building the rust/kernel crate (x86_64 defconfig with
CONFIG_RUST=y and CONFIG_GPU_BUDDY=y, built with LLVM=1). Not tested at
runtime.

Ivan Tverdokhleb (2):
  rust: sync: lock: allow fallible initializers in Lock::new
  rust: gpu: buddy: store the allocator inside the Mutex

 rust/kernel/gpu/buddy.rs | 38 +++++++++++++++-----------------------
 rust/kernel/sync/lock.rs |  8 ++++----
 2 files changed, 19 insertions(+), 27 deletions(-)

-- 
2.56.1

[PATCH 1/2] rust: sync: lock: allow fallible initializers in Lock::new

[Ivan Tverdokhleb]

Currently Lock::new() only accepts an infallible initializer for the
protected data, because impl PinInit<T> defaults the error type to
Infallible. This prevents wrapping a fallible initializer, such as
Opaque::try_ffi_init(), in a Mutex or SpinLock.

Make Lock::new() generic over the error type E, so that it accepts any
impl PinInit<T, E> and returns impl PinInit<Self, E>. E must implement
From<Infallible>, since initializing the lock state itself cannot fail.

This is needed, for example, by the GPU buddy allocator, which has a
TODO to move its gpu_buddy structure inside the mutex.

Signed-off-by: Ivan Tverdokhleb <mbkchelofficialdev@gmail.com>
---
 rust/kernel/sync/lock.rs | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/rust/kernel/sync/lock.rs b/rust/kernel/sync/lock.rs
index 10b6b5e9b024..26c071088b02 100644
--- a/rust/kernel/sync/lock.rs
+++ b/rust/kernel/sync/lock.rs
@@ -128,11 +128,11 @@ unsafe impl<T: ?Sized + Send, B: Backend> Sync for Lock<T, B> {}
 
 impl<T, B: Backend> Lock<T, B> {
     /// Constructs a new lock initialiser.
-    pub fn new(
-        t: impl PinInit<T>,
+    pub fn new<E: core::convert::From<core::convert::Infallible>>(
+        t: impl PinInit<T, E>,
         name: &'static CStr,
         key: Pin<&'static LockClassKey>,
-    ) -> impl PinInit<Self> {
+    ) -> impl PinInit<Self, E> {
         pin_init!(Self {
             data <- UnsafeCell::pin_init(t),
             _pin: PhantomPinned,
@@ -141,7 +141,7 @@ pub fn new(
             state <- Opaque::ffi_init(|slot| unsafe {
                 B::init(slot, name.as_char_ptr(), key.as_ptr())
             }),
-        })
+        }? E)
     }
 }
 
-- 
2.56.1

[PATCH 2/2] rust: gpu: buddy: store the allocator inside the Mutex

[Ivan Tverdokhleb]

GpuBuddyInner keeps the C gpu_buddy structure next to a separate
Mutex<()>, so nothing in the type system ties the data to the lock.
A TODO asked to replace Mutex<()> with Mutex<Opaque<..>> once
Mutex::new() accepts a fallible initializer.

That is now possible, so put the Opaque<gpu_buddy> directly inside the
Mutex. The allocator can then only be reached through the lock guard,
which lets the compiler enforce that the lock is held. This also
removes the now unneeded `inner` field from GpuBuddyGuard.

Update the safety comments that referred to the removed `inner` field.

Signed-off-by: Ivan Tverdokhleb <mbkchelofficialdev@gmail.com>
---
 rust/kernel/gpu/buddy.rs | 38 +++++++++++++++-----------------------
 1 file changed, 15 insertions(+), 23 deletions(-)

diff --git a/rust/kernel/gpu/buddy.rs b/rust/kernel/gpu/buddy.rs
index d502ada6ebbd..3397eb584ead 100644
--- a/rust/kernel/gpu/buddy.rs
+++ b/rust/kernel/gpu/buddy.rs
@@ -313,16 +313,11 @@ pub struct GpuBuddyParams {
 ///
 /// # Invariants
 ///
-/// The inner [`Opaque`] contains an initialized buddy allocator.
+/// The `lock` field contains an initialized buddy allocator wrapped in `Opaque`.
 #[pin_data(PinnedDrop)]
 struct GpuBuddyInner {
     #[pin]
-    inner: Opaque<bindings::gpu_buddy>,
-
-    // TODO: Replace `Mutex<()>` with `Mutex<Opaque<..>>` once `Mutex::new()`
-    // accepts `impl PinInit<T>`.
-    #[pin]
-    lock: Mutex<()>,
+    lock: Mutex<Opaque<bindings::gpu_buddy>>,
     /// Cached creation parameters (do not change after init).
     params: GpuBuddyParams,
 }
@@ -338,22 +333,20 @@ fn new(params: GpuBuddyParams) -> impl PinInit<Self, Error> {
         // `gpu_buddy_*` APIs. `try_pin_init!` only completes if all fields succeed,
         // so the invariant holds when construction finishes.
         try_pin_init!(Self {
-            inner <- Opaque::try_ffi_init(|ptr| {
-                // SAFETY: `ptr` points to valid uninitialized memory from the pin-init
-                // infrastructure. `gpu_buddy_init` will initialize the structure.
-                to_result(unsafe {
-                    bindings::gpu_buddy_init(ptr, size, chunk_size.as_usize() as u64)
-                })
-            }),
-            lock <- new_mutex!(()),
-            params,
+            lock <- new_mutex!(Opaque::try_ffi_init(|ptr| {
+            // SAFETY: `ptr` points to valid uninitialized memory from the pin-init
+            // infrastructure. `gpu_buddy_init` will initialize the structure.
+            to_result(unsafe {
+                bindings::gpu_buddy_init(ptr, size, chunk_size.as_usize() as u64)
+            })
+        })),
+         params,
         })
     }
 
     /// Lock the mutex and return a guard for accessing the allocator.
     fn lock(&self) -> GpuBuddyGuard<'_> {
         GpuBuddyGuard {
-            inner: self,
             _guard: self.lock.lock(),
         }
     }
@@ -364,7 +357,7 @@ impl PinnedDrop for GpuBuddyInner {
     fn drop(self: Pin<&mut Self>) {
         let guard = self.lock();
 
-        // SAFETY: Per the type invariant, `inner` contains an initialized
+        // SAFETY: Per the type invariant, `lock` contains an initialized
         // allocator. `guard` provides exclusive access.
         unsafe { bindings::gpu_buddy_fini(guard.as_raw()) };
     }
@@ -381,14 +374,13 @@ unsafe impl Sync for GpuBuddyInner {}
 ///
 /// The `_guard` holds the lock for the duration of this guard's lifetime.
 struct GpuBuddyGuard<'a> {
-    inner: &'a GpuBuddyInner,
-    _guard: MutexGuard<'a, ()>,
+    _guard: MutexGuard<'a, Opaque<bindings::gpu_buddy>>,
 }
 
 impl GpuBuddyGuard<'_> {
     /// Get a raw pointer to the underlying C `gpu_buddy` structure.
     fn as_raw(&self) -> *mut bindings::gpu_buddy {
-        self.inner.inner.get()
+        self._guard.get()
     }
 }
 
@@ -428,7 +420,7 @@ pub fn size(&self) -> u64 {
     pub fn avail(&self) -> u64 {
         let guard = self.0.lock();
 
-        // SAFETY: Per the type invariant, `inner` contains an initialized allocator.
+        // SAFETY: Per the type invariant, `lock` contains an initialized allocator.
         // `guard` provides exclusive access.
         unsafe { (*guard.as_raw()).avail }
     }
@@ -463,7 +455,7 @@ pub fn alloc_blocks(
                 // Lock while allocating to serialize with concurrent frees.
                 let guard = buddy.lock();
 
-                // SAFETY: Per the type invariant, `inner` contains an initialized
+                // SAFETY: Per the type invariant, `lock` contains an initialized
                 // allocator. `guard` provides exclusive access.
                 to_result(unsafe {
                     bindings::gpu_buddy_alloc_blocks(
-- 
2.56.1