rust-for-linux.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/3] Groundwork for Lock<T> when T is pinned
@ 2025-08-28 20:52 Daniel Almeida
  2025-08-28 20:52 ` [PATCH v2 1/3] rust: lock: guard: add T: Unpin bound to DerefMut Daniel Almeida
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Daniel Almeida @ 2025-08-28 20:52 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Will Deacon, Boqun Feng, Waiman Long,
	Miguel Ojeda, Alex Gaynor, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Danilo Krummrich
  Cc: linux-kernel, rust-for-linux, Daniel Almeida

It's currently impossible to have a pinned struct within the Lock<T> type.
This is problematic, because drivers might want to do this for various
reasons, specially as they grow in complexity.

A trivial example is:

struct Foo {
  #[pin]
  bar: Mutex<Bar>,
  #[pin]
  p: PhantomPinned,
}

struct Bar {
  #[pin]
  baz: Mutex<Baz>,
  #[pin]
  p: PhantomPinned,
}

Note that Bar is pinned, so having it in a Mutex makes it impossible to
instantiate a Foo that pins the Bar in bar. This is specially undesirable,
since Foo is already pinned, and thus, it could trivially enforce that its
bar field is pinned as well.

This can be trivially solved by using Pin<KBox<Bar>> instead of
structurally pinning, at the cost of an extra (completely unneeded)
allocation and ugly syntax.

This series lays out the groundwork to make the above possible without any
extra allocations.

- Patch 1 constrains the DerefMut implementation for safety reasons
- Patch 2 structurally pins the 'data' field in Lock<T>
- Patch 3 adds an accessor to retrieve a Pin<&mut T>

Note that this is just the beginning of the work needed to make a Pin<&mut
T> actually useful due to pin projections being currently unsupported.

In other words, it is currently impossible (even with the current patch) to
do this:

let mut data: MutexGuard<'_, Data> = mutex.lock();
let mut data: Pin<&mut Data> = data.as_mut();
let foo = &mut data.foo; // <- won't compile

The above is something that Benno is working on.

Thanks Boqun, Benno and the rest of the team for brainstorming the issue
and for and laying out a series of steps to implement a solution.

Changes in v2:

- Rebased on v6.17-rc3
- Collected tags
- Swap the order between patches 1 and 2
- Mention that it's easier to go through DerefMut if T:Unpin in patch 3
- Un-indent the example in patch 3, also make it prettier by adding
  links
- Link to v1: https://lore.kernel.org/rust-for-linux/20250730-lock-t-when-t-is-pinned-v1-0-1b97d5f28aa2@collabora.com/

---
Daniel Almeida (3):
      rust: lock: guard: add T: Unpin bound to DerefMut
      rust: lock: pin the inner data
      rust: lock: add a Pin<&mut T> accessor

 rust/kernel/sync/lock.rs        | 41 +++++++++++++++++++++++++++++++++++++----
 rust/kernel/sync/lock/global.rs |  5 ++++-
 2 files changed, 41 insertions(+), 5 deletions(-)
---
base-commit: 1b237f190eb3d36f52dffe07a40b5eb210280e00
change-id: 20250730-lock-t-when-t-is-pinned-292474504acb

Best regards,
-- 
Daniel Almeida <daniel.almeida@collabora.com>


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2 1/3] rust: lock: guard: add T: Unpin bound to DerefMut
  2025-08-28 20:52 [PATCH v2 0/3] Groundwork for Lock<T> when T is pinned Daniel Almeida
@ 2025-08-28 20:52 ` Daniel Almeida
  2025-08-28 20:52 ` [PATCH v2 2/3] rust: lock: pin the inner data Daniel Almeida
  2025-08-28 20:52 ` [PATCH v2 3/3] rust: lock: add a Pin<&mut T> accessor Daniel Almeida
  2 siblings, 0 replies; 6+ messages in thread
From: Daniel Almeida @ 2025-08-28 20:52 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Will Deacon, Boqun Feng, Waiman Long,
	Miguel Ojeda, Alex Gaynor, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Danilo Krummrich
  Cc: linux-kernel, rust-for-linux, Daniel Almeida

A core property of pinned types is not handing a mutable reference to the
inner data in safe code, as this trivially allows that data to be moved.

Enforce this condition by adding a bound on lock::Guard's DerefMut
implementation, so that it's only implemented for pinning-agnostic types.

Link: https://github.com/Rust-for-Linux/linux/issues/1181
Suggested-by: Benno Lossin <lossin@kernel.org>
Suggested-by: Boqun Feng <boqun.feng@gmail.com>
Reviewed-by: Benno Lossin <lossin@kernel.org>
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Signed-off-by: Daniel Almeida <daniel.almeida@collabora.com>
---
 rust/kernel/sync/lock.rs        | 5 ++++-
 rust/kernel/sync/lock/global.rs | 5 ++++-
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/rust/kernel/sync/lock.rs b/rust/kernel/sync/lock.rs
index 27202beef90c88dda13c58bbea9e8d4ce8d314de..b482f34bf0ce817e70f7a0da93443f77dd19ea79 100644
--- a/rust/kernel/sync/lock.rs
+++ b/rust/kernel/sync/lock.rs
@@ -251,7 +251,10 @@ fn deref(&self) -> &Self::Target {
     }
 }
 
-impl<T: ?Sized, B: Backend> core::ops::DerefMut for Guard<'_, T, B> {
+impl<T: ?Sized, B: Backend> core::ops::DerefMut for Guard<'_, T, B>
+where
+    T: Unpin,
+{
     fn deref_mut(&mut self) -> &mut Self::Target {
         // SAFETY: The caller owns the lock, so it is safe to deref the protected data.
         unsafe { &mut *self.lock.data.get() }
diff --git a/rust/kernel/sync/lock/global.rs b/rust/kernel/sync/lock/global.rs
index d65f94b5caf2668586088417323496629492932f..38b44803279986275616eef499fd40b8d4e97fdf 100644
--- a/rust/kernel/sync/lock/global.rs
+++ b/rust/kernel/sync/lock/global.rs
@@ -106,7 +106,10 @@ fn deref(&self) -> &Self::Target {
     }
 }
 
-impl<B: GlobalLockBackend> core::ops::DerefMut for GlobalGuard<B> {
+impl<B: GlobalLockBackend> core::ops::DerefMut for GlobalGuard<B>
+where
+    B::Item: Unpin,
+{
     fn deref_mut(&mut self) -> &mut Self::Target {
         &mut self.inner
     }

-- 
2.50.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v2 2/3] rust: lock: pin the inner data
  2025-08-28 20:52 [PATCH v2 0/3] Groundwork for Lock<T> when T is pinned Daniel Almeida
  2025-08-28 20:52 ` [PATCH v2 1/3] rust: lock: guard: add T: Unpin bound to DerefMut Daniel Almeida
@ 2025-08-28 20:52 ` Daniel Almeida
  2025-08-28 20:52 ` [PATCH v2 3/3] rust: lock: add a Pin<&mut T> accessor Daniel Almeida
  2 siblings, 0 replies; 6+ messages in thread
From: Daniel Almeida @ 2025-08-28 20:52 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Will Deacon, Boqun Feng, Waiman Long,
	Miguel Ojeda, Alex Gaynor, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Danilo Krummrich
  Cc: linux-kernel, rust-for-linux, Daniel Almeida

In preparation to support Lock<T> where T is pinned, the first thing that
needs to be done is to structurally pin the 'data' member. This switches
the 't' parameter in Lock<T>::new() to take in an impl PinInit<T> instead
of a plain T. This in turn uses the blanket implementation "impl PinInit<T>
for T".

Subsequent patches will touch on Guard<T>.

Link: https://github.com/Rust-for-Linux/linux/issues/1181
Suggested-by: Benno Lossin <lossin@kernel.org>
Suggested-by: Boqun Feng <boqun.feng@gmail.com>
Reviewed-by: Benno Lossin <lossin@kernel.org>
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Signed-off-by: Daniel Almeida <daniel.almeida@collabora.com>
---
 rust/kernel/sync/lock.rs | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/rust/kernel/sync/lock.rs b/rust/kernel/sync/lock.rs
index b482f34bf0ce817e70f7a0da93443f77dd19ea79..9242790d15dbf65d66518d060a8a777aac558cfc 100644
--- a/rust/kernel/sync/lock.rs
+++ b/rust/kernel/sync/lock.rs
@@ -11,7 +11,7 @@
     types::{NotThreadSafe, Opaque, ScopeGuard},
 };
 use core::{cell::UnsafeCell, marker::PhantomPinned, pin::Pin};
-use pin_init::{pin_data, pin_init, PinInit};
+use pin_init::{pin_data, pin_init, PinInit, Wrapper};
 
 pub mod mutex;
 pub mod spinlock;
@@ -115,6 +115,7 @@ pub struct Lock<T: ?Sized, B: Backend> {
     _pin: PhantomPinned,
 
     /// The data protected by the lock.
+    #[pin]
     pub(crate) data: UnsafeCell<T>,
 }
 
@@ -127,9 +128,13 @@ 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: T, name: &'static CStr, key: Pin<&'static LockClassKey>) -> impl PinInit<Self> {
+    pub fn new(
+        t: impl PinInit<T>,
+        name: &'static CStr,
+        key: Pin<&'static LockClassKey>,
+    ) -> impl PinInit<Self> {
         pin_init!(Self {
-            data: UnsafeCell::new(t),
+            data <- UnsafeCell::pin_init(t),
             _pin: PhantomPinned,
             // SAFETY: `slot` is valid while the closure is called and both `name` and `key` have
             // static lifetimes so they live indefinitely.

-- 
2.50.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v2 3/3] rust: lock: add a Pin<&mut T> accessor
  2025-08-28 20:52 [PATCH v2 0/3] Groundwork for Lock<T> when T is pinned Daniel Almeida
  2025-08-28 20:52 ` [PATCH v2 1/3] rust: lock: guard: add T: Unpin bound to DerefMut Daniel Almeida
  2025-08-28 20:52 ` [PATCH v2 2/3] rust: lock: pin the inner data Daniel Almeida
@ 2025-08-28 20:52 ` Daniel Almeida
  2025-09-04 15:13   ` Benno Lossin
  2 siblings, 1 reply; 6+ messages in thread
From: Daniel Almeida @ 2025-08-28 20:52 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Will Deacon, Boqun Feng, Waiman Long,
	Miguel Ojeda, Alex Gaynor, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Danilo Krummrich
  Cc: linux-kernel, rust-for-linux, Daniel Almeida

In order for callers to be able to access the inner T safely if T: !Unpin,
there needs to be a way to get a Pin<&mut T>. Add this accessor and a
corresponding example to tell users how it works.

This is not useful on its own for now, because we do not support pin
projections yet. This means that the following is not going to compile:

    let mut data: MutexGuard<'_, Data> = mutex.lock();
    let mut data: Pin<&mut Data> = data.as_mut();
    let foo = &mut data.foo;

A future patch can enable the behavior above by implementing support for
pin projections. Said patch is in the works already and will possibly
land on 6.18.

Link: https://github.com/Rust-for-Linux/linux/issues/1181
Suggested-by: Benno Lossin <lossin@kernel.org>
Suggested-by: Boqun Feng <boqun.feng@gmail.com>
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Signed-off-by: Daniel Almeida <daniel.almeida@collabora.com>
---
 rust/kernel/sync/lock.rs | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/rust/kernel/sync/lock.rs b/rust/kernel/sync/lock.rs
index 9242790d15dbf65d66518d060a8a777aac558cfc..7191804a244da05db74294fdec598f1a4732682c 100644
--- a/rust/kernel/sync/lock.rs
+++ b/rust/kernel/sync/lock.rs
@@ -245,6 +245,31 @@ pub(crate) fn do_unlocked<U>(&mut self, cb: impl FnOnce() -> U) -> U {
 
         cb()
     }
+
+    /// Returns a pinned mutable reference to the protected data.
+    ///
+    /// The guard implements [`DerefMut`] when `T: Unpin`, so for [`Unpin`]
+    /// types [`DerefMut`] should be used instead of this function.
+    ///
+    /// [`DerefMut`]: core::ops::DerefMut
+    /// [`Unpin`]: core::marker::Unpin
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// # use kernel::sync::{Mutex, MutexGuard};
+    /// # use core::pin::Pin;
+    /// struct Data;
+    ///
+    /// fn example(mutex: &Mutex<Data>) {
+    ///   let mut data: MutexGuard<'_, Data> = mutex.lock();
+    ///   let mut data: Pin<&mut Data> = data.as_mut();
+    ///  }
+    /// ```
+    pub fn as_mut(&mut self) -> Pin<&mut T> {
+        // SAFETY: `self.lock.data` is structurally pinned.
+        unsafe { Pin::new_unchecked(&mut *self.lock.data.get()) }
+    }
 }
 
 impl<T: ?Sized, B: Backend> core::ops::Deref for Guard<'_, T, B> {

-- 
2.50.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 3/3] rust: lock: add a Pin<&mut T> accessor
  2025-08-28 20:52 ` [PATCH v2 3/3] rust: lock: add a Pin<&mut T> accessor Daniel Almeida
@ 2025-09-04 15:13   ` Benno Lossin
  2025-09-04 15:15     ` Daniel Almeida
  0 siblings, 1 reply; 6+ messages in thread
From: Benno Lossin @ 2025-09-04 15:13 UTC (permalink / raw)
  To: Daniel Almeida, Peter Zijlstra, Ingo Molnar, Will Deacon,
	Boqun Feng, Waiman Long, Miguel Ojeda, Alex Gaynor, Gary Guo,
	Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Danilo Krummrich
  Cc: linux-kernel, rust-for-linux

On Thu Aug 28, 2025 at 10:52 PM CEST, Daniel Almeida wrote:
> In order for callers to be able to access the inner T safely if T: !Unpin,
> there needs to be a way to get a Pin<&mut T>. Add this accessor and a
> corresponding example to tell users how it works.
>
> This is not useful on its own for now, because we do not support pin
> projections yet. This means that the following is not going to compile:
>
>     let mut data: MutexGuard<'_, Data> = mutex.lock();
>     let mut data: Pin<&mut Data> = data.as_mut();
>     let foo = &mut data.foo;
>
> A future patch can enable the behavior above by implementing support for
> pin projections. Said patch is in the works already and will possibly
> land on 6.18.
>
> Link: https://github.com/Rust-for-Linux/linux/issues/1181
> Suggested-by: Benno Lossin <lossin@kernel.org>
> Suggested-by: Boqun Feng <boqun.feng@gmail.com>
> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
> Signed-off-by: Daniel Almeida <daniel.almeida@collabora.com>

Reviewed-by: Benno Lossin <lossin@kernel.org>

> ---
>  rust/kernel/sync/lock.rs | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
>
> diff --git a/rust/kernel/sync/lock.rs b/rust/kernel/sync/lock.rs
> index 9242790d15dbf65d66518d060a8a777aac558cfc..7191804a244da05db74294fdec598f1a4732682c 100644
> --- a/rust/kernel/sync/lock.rs
> +++ b/rust/kernel/sync/lock.rs
> @@ -245,6 +245,31 @@ pub(crate) fn do_unlocked<U>(&mut self, cb: impl FnOnce() -> U) -> U {
>  
>          cb()
>      }
> +
> +    /// Returns a pinned mutable reference to the protected data.
> +    ///
> +    /// The guard implements [`DerefMut`] when `T: Unpin`, so for [`Unpin`]
> +    /// types [`DerefMut`] should be used instead of this function.
> +    ///
> +    /// [`DerefMut`]: core::ops::DerefMut
> +    /// [`Unpin`]: core::marker::Unpin
> +    ///
> +    /// # Examples
> +    ///
> +    /// ```
> +    /// # use kernel::sync::{Mutex, MutexGuard};
> +    /// # use core::pin::Pin;
> +    /// struct Data;
> +    ///
> +    /// fn example(mutex: &Mutex<Data>) {
> +    ///   let mut data: MutexGuard<'_, Data> = mutex.lock();
> +    ///   let mut data: Pin<&mut Data> = data.as_mut();
> +    ///  }

The formatting looks off in this one, there should be 4 spaces of
indentation here; there are also 2 spaces in front of the `}`.

Also `Data` implements `Unpin`, so you're not following your own
recommendation from above :)

---
Cheers,
Benno

> +    /// ```
> +    pub fn as_mut(&mut self) -> Pin<&mut T> {
> +        // SAFETY: `self.lock.data` is structurally pinned.
> +        unsafe { Pin::new_unchecked(&mut *self.lock.data.get()) }
> +    }
>  }
>  
>  impl<T: ?Sized, B: Backend> core::ops::Deref for Guard<'_, T, B> {


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 3/3] rust: lock: add a Pin<&mut T> accessor
  2025-09-04 15:13   ` Benno Lossin
@ 2025-09-04 15:15     ` Daniel Almeida
  0 siblings, 0 replies; 6+ messages in thread
From: Daniel Almeida @ 2025-09-04 15:15 UTC (permalink / raw)
  To: Benno Lossin
  Cc: Peter Zijlstra, Ingo Molnar, Will Deacon, Boqun Feng, Waiman Long,
	Miguel Ojeda, Alex Gaynor, Gary Guo, Björn Roy Baron,
	Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
	linux-kernel, rust-for-linux



> On 4 Sep 2025, at 12:13, Benno Lossin <lossin@kernel.org> wrote:
> 
> On Thu Aug 28, 2025 at 10:52 PM CEST, Daniel Almeida wrote:
>> In order for callers to be able to access the inner T safely if T: !Unpin,
>> there needs to be a way to get a Pin<&mut T>. Add this accessor and a
>> corresponding example to tell users how it works.
>> 
>> This is not useful on its own for now, because we do not support pin
>> projections yet. This means that the following is not going to compile:
>> 
>>    let mut data: MutexGuard<'_, Data> = mutex.lock();
>>    let mut data: Pin<&mut Data> = data.as_mut();
>>    let foo = &mut data.foo;
>> 
>> A future patch can enable the behavior above by implementing support for
>> pin projections. Said patch is in the works already and will possibly
>> land on 6.18.
>> 
>> Link: https://github.com/Rust-for-Linux/linux/issues/1181
>> Suggested-by: Benno Lossin <lossin@kernel.org>
>> Suggested-by: Boqun Feng <boqun.feng@gmail.com>
>> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
>> Signed-off-by: Daniel Almeida <daniel.almeida@collabora.com>
> 
> Reviewed-by: Benno Lossin <lossin@kernel.org>
> 
>> ---
>> rust/kernel/sync/lock.rs | 25 +++++++++++++++++++++++++
>> 1 file changed, 25 insertions(+)
>> 
>> diff --git a/rust/kernel/sync/lock.rs b/rust/kernel/sync/lock.rs
>> index 9242790d15dbf65d66518d060a8a777aac558cfc..7191804a244da05db74294fdec598f1a4732682c 100644
>> --- a/rust/kernel/sync/lock.rs
>> +++ b/rust/kernel/sync/lock.rs
>> @@ -245,6 +245,31 @@ pub(crate) fn do_unlocked<U>(&mut self, cb: impl FnOnce() -> U) -> U {
>> 
>>         cb()
>>     }
>> +
>> +    /// Returns a pinned mutable reference to the protected data.
>> +    ///
>> +    /// The guard implements [`DerefMut`] when `T: Unpin`, so for [`Unpin`]
>> +    /// types [`DerefMut`] should be used instead of this function.
>> +    ///
>> +    /// [`DerefMut`]: core::ops::DerefMut
>> +    /// [`Unpin`]: core::marker::Unpin
>> +    ///
>> +    /// # Examples
>> +    ///
>> +    /// ```
>> +    /// # use kernel::sync::{Mutex, MutexGuard};
>> +    /// # use core::pin::Pin;
>> +    /// struct Data;
>> +    ///
>> +    /// fn example(mutex: &Mutex<Data>) {
>> +    ///   let mut data: MutexGuard<'_, Data> = mutex.lock();
>> +    ///   let mut data: Pin<&mut Data> = data.as_mut();
>> +    ///  }
> 
> The formatting looks off in this one, there should be 4 spaces of
> indentation here; there are also 2 spaces in front of the `}`.
> 
> Also `Data` implements `Unpin`, so you're not following your own
> recommendation from above :)

I’ll fix this :)

— Daniel

> 
> ---
> Cheers,
> Benno
> 
>> +    /// ```
>> +    pub fn as_mut(&mut self) -> Pin<&mut T> {
>> +        // SAFETY: `self.lock.data` is structurally pinned.
>> +        unsafe { Pin::new_unchecked(&mut *self.lock.data.get()) }
>> +    }
>> }
>> 
>> impl<T: ?Sized, B: Backend> core::ops::Deref for Guard<'_, T, B> {



^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2025-09-04 15:16 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-28 20:52 [PATCH v2 0/3] Groundwork for Lock<T> when T is pinned Daniel Almeida
2025-08-28 20:52 ` [PATCH v2 1/3] rust: lock: guard: add T: Unpin bound to DerefMut Daniel Almeida
2025-08-28 20:52 ` [PATCH v2 2/3] rust: lock: pin the inner data Daniel Almeida
2025-08-28 20:52 ` [PATCH v2 3/3] rust: lock: add a Pin<&mut T> accessor Daniel Almeida
2025-09-04 15:13   ` Benno Lossin
2025-09-04 15:15     ` Daniel Almeida

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).