Nouveau - Jul 2025 - [PATCH] Partially revert "rust: drm: gem: Implement AlwaysRefCounted for all gem objects automatically"

I made a very silly mistake with this commit that managed to slip by
because I forgot to mzke sure rvkms was rebased before testing my work last
- we can't do blanket implementations like this due to rust's orphan
rule.

The code -does- build just fine right now, but it doesn't with the ongoing
bindings for gem shmem. So, just revert this and we'll introduce a macro
for implementing AlwaysRefCounted individually for each type of gem
implementation.

Note that we leave the IntoGEMObject since it is true that all gem objects
are refcounted, so any implementations that are added should be
implementing AlwaysRefCounted anyhow.

This reverts commit 38cb08c3fcd3f3b1d0225dcec8ae50fab5751549.

Signed-off-by: Lyude Paul <lyude at redhat.com>
---
 rust/kernel/drm/gem/mod.rs | 36 ++++++++++++++++--------------------
 1 file changed, 16 insertions(+), 20 deletions(-)

diff --git a/rust/kernel/drm/gem/mod.rs b/rust/kernel/drm/gem/mod.rs
index 4cd69fa84318c..db65807dbce88 100644
--- a/rust/kernel/drm/gem/mod.rs
+++ b/rust/kernel/drm/gem/mod.rs
@@ -54,26 +54,6 @@ pub trait IntoGEMObject: Sized + super::private::Sealed +
AlwaysRefCounted {
     unsafe fn as_ref<'a>(self_ptr: *mut bindings::drm_gem_object)
-> &'a Self;
 }
 
-// SAFETY: All gem objects are refcounted.
-unsafe impl<T: IntoGEMObject> AlwaysRefCounted for T {
-    fn inc_ref(&self) {
-        // SAFETY: The existence of a shared reference guarantees that the
refcount is non-zero.
-        unsafe { bindings::drm_gem_object_get(self.as_raw()) };
-    }
-
-    unsafe fn dec_ref(obj: NonNull<Self>) {
-        // SAFETY: We either hold the only refcount on `obj`, or one of many -
meaning that no one
-        // else could possibly hold a mutable reference to `obj` and thus this
immutable reference
-        // is safe.
-        let obj = unsafe { obj.as_ref() }.as_raw();
-
-        // SAFETY:
-        // - The safety requirements guarantee that the refcount is non-zero.
-        // - We hold no references to `obj` now, making it safe for us to
potentially deallocate it.
-        unsafe { bindings::drm_gem_object_put(obj) };
-    }
-}
-
 /// Trait which must be implemented by drivers using base GEM objects.
 pub trait DriverObject: BaseDriverObject<Object<Self>> {
     /// Parent `Driver` for this object.
@@ -287,6 +267,22 @@ extern "C" fn free_callback(obj: *mut
bindings::drm_gem_object) {
     }
 }
 
+// SAFETY: Instances of `Object<T>` are always reference-counted.
+unsafe impl<T: DriverObject> crate::types::AlwaysRefCounted for
Object<T> {
+    fn inc_ref(&self) {
+        // SAFETY: The existence of a shared reference guarantees that the
refcount is non-zero.
+        unsafe { bindings::drm_gem_object_get(self.as_raw()) };
+    }
+
+    unsafe fn dec_ref(obj: NonNull<Self>) {
+        // SAFETY: `obj` is a valid pointer to an `Object<T>`.
+        let obj = unsafe { obj.as_ref() };
+
+        // SAFETY: The safety requirements guarantee that the refcount is
non-zero.
+        unsafe { bindings::drm_gem_object_put(obj.as_raw()) }
+    }
+}
+
 impl<T: DriverObject> super::private::Sealed for Object<T> {}
 
 impl<T: DriverObject> Deref for Object<T> {

base-commit: 89be9a83ccf1f88522317ce02f854f30d6115c41
-- 
2.50.0

On Thu Jul 24, 2025 at 9:15 PM CEST, Lyude Paul wrote:
> -// SAFETY: All gem objects are refcounted.
> -unsafe impl<T: IntoGEMObject> AlwaysRefCounted for T {
> -    fn inc_ref(&self) {
> -        // SAFETY: The existence of a shared reference guarantees that the
refcount is non-zero.
> -        unsafe { bindings::drm_gem_object_get(self.as_raw()) };
> -    }
> -
> -    unsafe fn dec_ref(obj: NonNull<Self>) {
> -        // SAFETY: We either hold the only refcount on `obj`, or one of
many - meaning that no one
> -        // else could possibly hold a mutable reference to `obj` and thus
this immutable reference
> -        // is safe.
> -        let obj = unsafe { obj.as_ref() }.as_raw();
> -
> -        // SAFETY:
> -        // - The safety requirements guarantee that the refcount is
non-zero.
> -        // - We hold no references to `obj` now, making it safe for us to
potentially deallocate it.
> -        unsafe { bindings::drm_gem_object_put(obj) };
> -    }
> -}
IIUC, you'll add rust/kernel/drm/gem/shmem.rs with a new type shmem::Object
that
implements IntoGEMObject, right?

If this is correct, I think that should work.

Do I miss anything?