linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* [PATCH RFC] rust: maple_tree: implement Send and Sync for MapleTree
@ 2026-04-20 20:10 Joel Fernandes
  2026-04-20 22:00 ` Daniel Almeida
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Joel Fernandes @ 2026-04-20 20:10 UTC (permalink / raw)
  To: linux-kernel, Liam R. Howlett, Alice Ryhl, Andrew Ballance,
	Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Trevor Gross, Danilo Krummrich
  Cc: Joel Fernandes, maple-tree, linux-mm, rust-for-linux

The C maple_tree struct contains a *mut c_void, which prevents Rust from
auto-deriving Send/Sync.

Following is an example error message when using MapleTree in nova-core's Vmm.

This propagates up causing NovaCore to fail the Send bound required by
pci::Driver:

  error[E0277]: `*mut c_void` cannot be sent between threads safely
      --> drivers/gpu/nova-core/driver.rs:77:22
       |
  77   | impl pci::Driver for NovaCore {
       |                      ^^^^^^^^ `*mut c_void` cannot be sent between threads safely
       |
       = help: within `MapleTreeAlloc<()>`, the trait `Send` is not implemented for `*mut c_void`
  note: required because it appears within the type `kernel::bindings::maple_tree`
  note: required because it appears within the type `Opaque<kernel::bindings::maple_tree>`
  note: required because it appears within the type `MapleTree<()>`
  note: required because it appears within the type `MapleTreeAlloc<()>`
       = note: required for `Box<MapleTreeAlloc<()>, Kmalloc>` to implement `Send`
  note: required because it appears within the type `core::pin::Pin<Box<MapleTreeAlloc<()>, Kmalloc>>`
  note: required because it appears within the type `Vmm`
  note: required because it appears within the type `BarUser`
  note: required because it appears within the type `Gpu`
  note: required because it appears within the type `NovaCore`
  note: required by a bound in `kernel::pci::Driver`
      --> rust/kernel/pci.rs:294:19

Implement Send and Sync for MapleTree to fix this.

Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
---
 rust/kernel/maple_tree.rs | 27 +++++++++++++++++++++------
 1 file changed, 21 insertions(+), 6 deletions(-)

diff --git a/rust/kernel/maple_tree.rs b/rust/kernel/maple_tree.rs
index 265d6396a78a1..1ecf05871e710 100644
--- a/rust/kernel/maple_tree.rs
+++ b/rust/kernel/maple_tree.rs
@@ -16,7 +16,7 @@
     alloc::Flags,
     error::to_result,
     prelude::*,
-    types::{ForeignOwnable, Opaque},
+    types::{ForeignOwnable, NotThreadSafe, Opaque},
 };
 
 /// A maple tree optimized for storing non-overlapping ranges.
@@ -240,7 +240,10 @@ pub fn lock(&self) -> MapleGuard<'_, T> {
         unsafe { bindings::spin_lock(self.ma_lock()) };
 
         // INVARIANT: We just took the spinlock.
-        MapleGuard(self)
+        MapleGuard {
+            tree: self,
+            _not_send: NotThreadSafe,
+        }
     }
 
     #[inline]
@@ -302,19 +305,31 @@ fn drop(mut self: Pin<&mut Self>) {
     }
 }
 
+// SAFETY: `MapleTree<T>` is `Send` iff `T` is `Send`.  All access to the tree
+// goes through the internal `ma_lock` spinlock or via `&mut MapleTree`.
+unsafe impl<T: ForeignOwnable + Send> Send for MapleTree<T> {}
+// SAFETY: All shared access through `&MapleTree` either acquires `ma_lock`.
+unsafe impl<T: ForeignOwnable + Send> Sync for MapleTree<T> {}
+
 /// A reference to a [`MapleTree`] that owns the inner lock.
 ///
 /// # Invariants
 ///
 /// This guard owns the inner spinlock.
 #[must_use = "if unused, the lock will be immediately unlocked"]
-pub struct MapleGuard<'tree, T: ForeignOwnable>(&'tree MapleTree<T>);
+pub struct MapleGuard<'tree, T: ForeignOwnable> {
+    tree: &'tree MapleTree<T>,
+    // A held spinlock must be released on the same CPU that acquired it.
+    // Prevent `MapleGuard` from auto-deriving `Send` because `&MapleTree<T>`
+    // is `Send` due to `MapleTree<T>` being `Sync`.
+    _not_send: NotThreadSafe,
+}
 
 impl<'tree, T: ForeignOwnable> Drop for MapleGuard<'tree, T> {
     #[inline]
     fn drop(&mut self) {
         // SAFETY: By the type invariants, we hold this spinlock.
-        unsafe { bindings::spin_unlock(self.0.ma_lock()) };
+        unsafe { bindings::spin_unlock(self.tree.ma_lock()) };
     }
 }
 
@@ -323,7 +338,7 @@ impl<'tree, T: ForeignOwnable> MapleGuard<'tree, T> {
     pub fn ma_state(&mut self, first: usize, end: usize) -> MaState<'_, T> {
         // SAFETY: The `MaState` borrows this `MapleGuard`, so it can also borrow the `MapleGuard`s
         // read/write permissions to the maple tree.
-        unsafe { MaState::new_raw(self.0, first, end) }
+        unsafe { MaState::new_raw(self.tree, first, end) }
     }
 
     /// Load the value at the given index.
@@ -375,7 +390,7 @@ pub fn ma_state(&mut self, first: usize, end: usize) -> MaState<'_, T> {
     #[inline]
     pub fn load(&mut self, index: usize) -> Option<T::BorrowedMut<'_>> {
         // SAFETY: `self.tree` contains a valid maple tree.
-        let ret = unsafe { bindings::mtree_load(self.0.tree.get(), index) };
+        let ret = unsafe { bindings::mtree_load(self.tree.tree.get(), index) };
         if ret.is_null() {
             return None;
         }
-- 
2.34.1



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

* Re: [PATCH RFC] rust: maple_tree: implement Send and Sync for MapleTree
  2026-04-20 20:10 [PATCH RFC] rust: maple_tree: implement Send and Sync for MapleTree Joel Fernandes
@ 2026-04-20 22:00 ` Daniel Almeida
  2026-04-20 23:42   ` Joel Fernandes
  2026-04-21 14:19 ` Onur Özkan
  2026-04-21 15:54 ` Gary Guo
  2 siblings, 1 reply; 5+ messages in thread
From: Daniel Almeida @ 2026-04-20 22:00 UTC (permalink / raw)
  To: Joel Fernandes
  Cc: linux-kernel, Liam R. Howlett, Alice Ryhl, Andrew Ballance,
	Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Trevor Gross, Danilo Krummrich,
	maple-tree, linux-mm, rust-for-linux

Hi Joel,

I think there are others in a better position to review this than me, 
however, while reading this patch, I noticed this:
> 
>     #[inline]
> @@ -302,19 +305,31 @@ fn drop(mut self: Pin<&mut Self>) {
>     }
> }
> 
> +// SAFETY: `MapleTree<T>` is `Send` iff `T` is `Send`.  All access to the tree
> +// goes through the internal `ma_lock` spinlock or via `&mut MapleTree`.
> +unsafe impl<T: ForeignOwnable + Send> Send for MapleTree<T> {}
> +// SAFETY: All shared access through `&MapleTree` either acquires `ma_lock`.

^ Missing something here? Either … or? 

— Daniel

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

* Re: [PATCH RFC] rust: maple_tree: implement Send and Sync for MapleTree
  2026-04-20 22:00 ` Daniel Almeida
@ 2026-04-20 23:42   ` Joel Fernandes
  0 siblings, 0 replies; 5+ messages in thread
From: Joel Fernandes @ 2026-04-20 23:42 UTC (permalink / raw)
  To: Daniel Almeida
  Cc: linux-kernel, Liam R. Howlett, Alice Ryhl, Andrew Ballance,
	Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Trevor Gross, Danilo Krummrich,
	maple-tree, linux-mm, rust-for-linux



On 4/20/2026 6:00 PM, Daniel Almeida wrote:
> Hi Joel,
> 
> I think there are others in a better position to review this than me, 
> however, while reading this patch, I noticed this:
>>
>>     #[inline]
>> @@ -302,19 +305,31 @@ fn drop(mut self: Pin<&mut Self>) {
>>     }
>> }
>>
>> +// SAFETY: `MapleTree<T>` is `Send` iff `T` is `Send`.  All access to the tree
>> +// goes through the internal `ma_lock` spinlock or via `&mut MapleTree`.
>> +unsafe impl<T: ForeignOwnable + Send> Send for MapleTree<T> {}
>> +// SAFETY: All shared access through `&MapleTree` either acquires `ma_lock`.
> 
> ^ Missing something here? Either … or? 
> 
Yes, it should be either acquires `ma_lock` or uses &mut. I will adjust it.

thanks,

--
Joel Fernandes



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

* Re: [PATCH RFC] rust: maple_tree: implement Send and Sync for MapleTree
  2026-04-20 20:10 [PATCH RFC] rust: maple_tree: implement Send and Sync for MapleTree Joel Fernandes
  2026-04-20 22:00 ` Daniel Almeida
@ 2026-04-21 14:19 ` Onur Özkan
  2026-04-21 15:54 ` Gary Guo
  2 siblings, 0 replies; 5+ messages in thread
From: Onur Özkan @ 2026-04-21 14:19 UTC (permalink / raw)
  To: Joel Fernandes
  Cc: linux-kernel, Liam R. Howlett, Alice Ryhl, Andrew Ballance,
	Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Trevor Gross, Danilo Krummrich,
	maple-tree, linux-mm, rust-for-linux

Hi Joel,

On Mon, 20 Apr 2026 16:10:39 -0400
Joel Fernandes <joelagnelf@nvidia.com> wrote:

> The C maple_tree struct contains a *mut c_void, which prevents Rust from
> auto-deriving Send/Sync.
> 
> Following is an example error message when using MapleTree in nova-core's Vmm.
> 
> This propagates up causing NovaCore to fail the Send bound required by
> pci::Driver:
> 
>   error[E0277]: `*mut c_void` cannot be sent between threads safely
>       --> drivers/gpu/nova-core/driver.rs:77:22
>        |
>   77   | impl pci::Driver for NovaCore {
>        |                      ^^^^^^^^ `*mut c_void` cannot be sent between threads safely
>        |
>        = help: within `MapleTreeAlloc<()>`, the trait `Send` is not implemented for `*mut c_void`
>   note: required because it appears within the type `kernel::bindings::maple_tree`
>   note: required because it appears within the type `Opaque<kernel::bindings::maple_tree>`
>   note: required because it appears within the type `MapleTree<()>`
>   note: required because it appears within the type `MapleTreeAlloc<()>`
>        = note: required for `Box<MapleTreeAlloc<()>, Kmalloc>` to implement `Send`
>   note: required because it appears within the type `core::pin::Pin<Box<MapleTreeAlloc<()>, Kmalloc>>`
>   note: required because it appears within the type `Vmm`
>   note: required because it appears within the type `BarUser`
>   note: required because it appears within the type `Gpu`
>   note: required because it appears within the type `NovaCore`
>   note: required by a bound in `kernel::pci::Driver`
>       --> rust/kernel/pci.rs:294:19
> 
> Implement Send and Sync for MapleTree to fix this.
> 
> Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
> ---
>  rust/kernel/maple_tree.rs | 27 +++++++++++++++++++++------
>  1 file changed, 21 insertions(+), 6 deletions(-)
> 
> diff --git a/rust/kernel/maple_tree.rs b/rust/kernel/maple_tree.rs
> index 265d6396a78a1..1ecf05871e710 100644
> --- a/rust/kernel/maple_tree.rs
> +++ b/rust/kernel/maple_tree.rs
> @@ -16,7 +16,7 @@
>      alloc::Flags,
>      error::to_result,
>      prelude::*,
> -    types::{ForeignOwnable, Opaque},
> +    types::{ForeignOwnable, NotThreadSafe, Opaque},

We usually prefer vertical formatting e.g.,

	types::{
		ForeignOwnable,
		NotThreadSafe,
		Opaque, //
	},

to avoid merge conflicts as much as possible. The rest of the patch makes
sense to me.

-Onur

>  };
>  
>  /// A maple tree optimized for storing non-overlapping ranges.
> @@ -240,7 +240,10 @@ pub fn lock(&self) -> MapleGuard<'_, T> {
>          unsafe { bindings::spin_lock(self.ma_lock()) };
>  
>          // INVARIANT: We just took the spinlock.
> -        MapleGuard(self)
> +        MapleGuard {
> +            tree: self,
> +            _not_send: NotThreadSafe,
> +        }
>      }
>  
>      #[inline]
> @@ -302,19 +305,31 @@ fn drop(mut self: Pin<&mut Self>) {
>      }
>  }
>  
> +// SAFETY: `MapleTree<T>` is `Send` iff `T` is `Send`.  All access to the tree
> +// goes through the internal `ma_lock` spinlock or via `&mut MapleTree`.
> +unsafe impl<T: ForeignOwnable + Send> Send for MapleTree<T> {}
> +// SAFETY: All shared access through `&MapleTree` either acquires `ma_lock`.
> +unsafe impl<T: ForeignOwnable + Send> Sync for MapleTree<T> {}
> +
>  /// A reference to a [`MapleTree`] that owns the inner lock.
>  ///
>  /// # Invariants
>  ///
>  /// This guard owns the inner spinlock.
>  #[must_use = "if unused, the lock will be immediately unlocked"]
> -pub struct MapleGuard<'tree, T: ForeignOwnable>(&'tree MapleTree<T>);
> +pub struct MapleGuard<'tree, T: ForeignOwnable> {
> +    tree: &'tree MapleTree<T>,
> +    // A held spinlock must be released on the same CPU that acquired it.
> +    // Prevent `MapleGuard` from auto-deriving `Send` because `&MapleTree<T>`
> +    // is `Send` due to `MapleTree<T>` being `Sync`.
> +    _not_send: NotThreadSafe,
> +}
>  
>  impl<'tree, T: ForeignOwnable> Drop for MapleGuard<'tree, T> {
>      #[inline]
>      fn drop(&mut self) {
>          // SAFETY: By the type invariants, we hold this spinlock.
> -        unsafe { bindings::spin_unlock(self.0.ma_lock()) };
> +        unsafe { bindings::spin_unlock(self.tree.ma_lock()) };
>      }
>  }
>  
> @@ -323,7 +338,7 @@ impl<'tree, T: ForeignOwnable> MapleGuard<'tree, T> {
>      pub fn ma_state(&mut self, first: usize, end: usize) -> MaState<'_, T> {
>          // SAFETY: The `MaState` borrows this `MapleGuard`, so it can also borrow the `MapleGuard`s
>          // read/write permissions to the maple tree.
> -        unsafe { MaState::new_raw(self.0, first, end) }
> +        unsafe { MaState::new_raw(self.tree, first, end) }
>      }
>  
>      /// Load the value at the given index.
> @@ -375,7 +390,7 @@ pub fn ma_state(&mut self, first: usize, end: usize) -> MaState<'_, T> {
>      #[inline]
>      pub fn load(&mut self, index: usize) -> Option<T::BorrowedMut<'_>> {
>          // SAFETY: `self.tree` contains a valid maple tree.
> -        let ret = unsafe { bindings::mtree_load(self.0.tree.get(), index) };
> +        let ret = unsafe { bindings::mtree_load(self.tree.tree.get(), index) };
>          if ret.is_null() {
>              return None;
>          }
> -- 
> 2.34.1
> 


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

* Re: [PATCH RFC] rust: maple_tree: implement Send and Sync for MapleTree
  2026-04-20 20:10 [PATCH RFC] rust: maple_tree: implement Send and Sync for MapleTree Joel Fernandes
  2026-04-20 22:00 ` Daniel Almeida
  2026-04-21 14:19 ` Onur Özkan
@ 2026-04-21 15:54 ` Gary Guo
  2 siblings, 0 replies; 5+ messages in thread
From: Gary Guo @ 2026-04-21 15:54 UTC (permalink / raw)
  To: Joel Fernandes, linux-kernel, Liam R. Howlett, Alice Ryhl,
	Andrew Ballance, Miguel Ojeda, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg,
	Trevor Gross, Danilo Krummrich
  Cc: maple-tree, linux-mm, rust-for-linux

On Mon Apr 20, 2026 at 9:10 PM BST, Joel Fernandes wrote:
> The C maple_tree struct contains a *mut c_void, which prevents Rust from
> auto-deriving Send/Sync.
>
> Following is an example error message when using MapleTree in nova-core's Vmm.
>
> This propagates up causing NovaCore to fail the Send bound required by
> pci::Driver:
>
>   error[E0277]: `*mut c_void` cannot be sent between threads safely
>       --> drivers/gpu/nova-core/driver.rs:77:22
>        |
>   77   | impl pci::Driver for NovaCore {
>        |                      ^^^^^^^^ `*mut c_void` cannot be sent between threads safely
>        |
>        = help: within `MapleTreeAlloc<()>`, the trait `Send` is not implemented for `*mut c_void`
>   note: required because it appears within the type `kernel::bindings::maple_tree`
>   note: required because it appears within the type `Opaque<kernel::bindings::maple_tree>`
>   note: required because it appears within the type `MapleTree<()>`
>   note: required because it appears within the type `MapleTreeAlloc<()>`
>        = note: required for `Box<MapleTreeAlloc<()>, Kmalloc>` to implement `Send`
>   note: required because it appears within the type `core::pin::Pin<Box<MapleTreeAlloc<()>, Kmalloc>>`
>   note: required because it appears within the type `Vmm`
>   note: required because it appears within the type `BarUser`
>   note: required because it appears within the type `Gpu`
>   note: required because it appears within the type `NovaCore`
>   note: required by a bound in `kernel::pci::Driver`
>       --> rust/kernel/pci.rs:294:19
>
> Implement Send and Sync for MapleTree to fix this.
>
> Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
> ---
>  rust/kernel/maple_tree.rs | 27 +++++++++++++++++++++------
>  1 file changed, 21 insertions(+), 6 deletions(-)
>
> diff --git a/rust/kernel/maple_tree.rs b/rust/kernel/maple_tree.rs
> index 265d6396a78a1..1ecf05871e710 100644
> --- a/rust/kernel/maple_tree.rs
> +++ b/rust/kernel/maple_tree.rs
> @@ -16,7 +16,7 @@
>      alloc::Flags,
>      error::to_result,
>      prelude::*,
> -    types::{ForeignOwnable, Opaque},
> +    types::{ForeignOwnable, NotThreadSafe, Opaque},
>  };
>  
>  /// A maple tree optimized for storing non-overlapping ranges.
> @@ -240,7 +240,10 @@ pub fn lock(&self) -> MapleGuard<'_, T> {
>          unsafe { bindings::spin_lock(self.ma_lock()) };
>  
>          // INVARIANT: We just took the spinlock.
> -        MapleGuard(self)
> +        MapleGuard {
> +            tree: self,
> +            _not_send: NotThreadSafe,
> +        }
>      }
>  
>      #[inline]
> @@ -302,19 +305,31 @@ fn drop(mut self: Pin<&mut Self>) {
>      }
>  }
>  
> +// SAFETY: `MapleTree<T>` is `Send` iff `T` is `Send`.  All access to the tree
> +// goes through the internal `ma_lock` spinlock or via `&mut MapleTree`.

I don't think this needs to mention about the internal `ma_lock`. The lock
sounds like it'll be relevant for `Sync` impl rather than `Send`. Even if
accesses are completely unsynchronized, it's still okay to implement `Send`,
just not `Sync`.

For owning containers I think usually you don't need to put too much
justifications, I think a statement similar to that of `Vec`'s is sufficient.

    // SAFETY: `Vec` is `Send` if `T` is `Send` because `Vec` owns its elements.

> +unsafe impl<T: ForeignOwnable + Send> Send for MapleTree<T> {}
> +// SAFETY: All shared access through `&MapleTree` either acquires `ma_lock`.

This isn't the full picture. You need to also explain why it's okay for
`MapleTree<T>` to be `Sync` when `T` is merely `Send`, essentially the reasons
that `&MapleTree<T>` doesn't give user ability to have two `&T` that points to
the same object across two threads.

> +unsafe impl<T: ForeignOwnable + Send> Sync for MapleTree<T> {}
> +
>  /// A reference to a [`MapleTree`] that owns the inner lock.
>  ///
>  /// # Invariants
>  ///
>  /// This guard owns the inner spinlock.
>  #[must_use = "if unused, the lock will be immediately unlocked"]
> -pub struct MapleGuard<'tree, T: ForeignOwnable>(&'tree MapleTree<T>);
> +pub struct MapleGuard<'tree, T: ForeignOwnable> {
> +    tree: &'tree MapleTree<T>,
> +    // A held spinlock must be released on the same CPU that acquired it.
> +    // Prevent `MapleGuard` from auto-deriving `Send` because `&MapleTree<T>`
> +    // is `Send` due to `MapleTree<T>` being `Sync`.

nit: I think you can leave out the "auto-deriving" part, you should be allowed
to assume people reading this knows about `Send` and `Sync` are auto traits :)

Just the first line is sufficient. (We probably want to be explicit about
not-thread-safe things even if compiler would get it correct from auto trait
inference anyway.

Best,
Gary

> +    _not_send: NotThreadSafe,
> +}
>  
>  impl<'tree, T: ForeignOwnable> Drop for MapleGuard<'tree, T> {
>      #[inline]
>      fn drop(&mut self) {
>          // SAFETY: By the type invariants, we hold this spinlock.
> -        unsafe { bindings::spin_unlock(self.0.ma_lock()) };
> +        unsafe { bindings::spin_unlock(self.tree.ma_lock()) };
>      }
>  }
>  
> @@ -323,7 +338,7 @@ impl<'tree, T: ForeignOwnable> MapleGuard<'tree, T> {
>      pub fn ma_state(&mut self, first: usize, end: usize) -> MaState<'_, T> {
>          // SAFETY: The `MaState` borrows this `MapleGuard`, so it can also borrow the `MapleGuard`s
>          // read/write permissions to the maple tree.
> -        unsafe { MaState::new_raw(self.0, first, end) }
> +        unsafe { MaState::new_raw(self.tree, first, end) }
>      }
>  
>      /// Load the value at the given index.
> @@ -375,7 +390,7 @@ pub fn ma_state(&mut self, first: usize, end: usize) -> MaState<'_, T> {
>      #[inline]
>      pub fn load(&mut self, index: usize) -> Option<T::BorrowedMut<'_>> {
>          // SAFETY: `self.tree` contains a valid maple tree.
> -        let ret = unsafe { bindings::mtree_load(self.0.tree.get(), index) };
> +        let ret = unsafe { bindings::mtree_load(self.tree.tree.get(), index) };
>          if ret.is_null() {
>              return None;
>          }



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

end of thread, other threads:[~2026-04-21 15:54 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-04-20 20:10 [PATCH RFC] rust: maple_tree: implement Send and Sync for MapleTree Joel Fernandes
2026-04-20 22:00 ` Daniel Almeida
2026-04-20 23:42   ` Joel Fernandes
2026-04-21 14:19 ` Onur Özkan
2026-04-21 15:54 ` Gary Guo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox