From 5ae65bdcb867555540169ef57876658262a67d87 Mon Sep 17 00:00:00 2001 From: Lyude Paul Date: Thu, 16 Oct 2025 17:08:14 -0400 Subject: Partially revert "rust: drm: gem: Implement AlwaysRefCounted for all gem objects automatically" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently in order to implement AlwaysRefCounted for gem objects, we use a blanket implementation: unsafe impl AlwaysRefCounted for T { … } While this technically works, it comes with the rather unfortunate downside that attempting to create a similar blanket implementation in any other kernel crate will now fail in a rather confusing way. Using an example from the (not yet upstream) rust DRM KMS bindings, if we were to add: unsafe impl AlwaysRefCounted for T { … } Then the moment that both blanket implementations are present in the same kernel tree, compilation fails with the following: error[E0119]: conflicting implementations of trait `types::AlwaysRefCounted` --> rust/kernel/drm/kms.rs:504:1 | 504 | unsafe impl AlwaysRefCounted for T { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation | ::: rust/kernel/drm/gem/mod.rs:97:1 | 97 | unsafe impl AlwaysRefCounted for T { | ---------------------------------------------------- first implementation here So, revert these changes for now. The proper fix for this is to introduce a macro for copy/pasting the same implementation of AlwaysRefCounted around. This reverts commit 38cb08c3fcd3f3b1d0225dcec8ae50fab5751549. Signed-off-by: Lyude Paul Reviewed-by: Alice Ryhl Link: https://lore.kernel.org/r/20251016210955.2813186-2-lyude@redhat.com --- rust/kernel/drm/gem/mod.rs | 36 ++++++++++++++++-------------------- 1 file changed, 16 insertions(+), 20 deletions(-) (limited to 'rust/kernel/drm/gem/mod.rs') diff --git a/rust/kernel/drm/gem/mod.rs b/rust/kernel/drm/gem/mod.rs index 30c853988b94..20c2769a8c9d 100644 --- a/rust/kernel/drm/gem/mod.rs +++ b/rust/kernel/drm/gem/mod.rs @@ -55,26 +55,6 @@ pub trait IntoGEMObject: Sized + super::private::Sealed + AlwaysRefCounted { unsafe fn from_raw<'a>(self_ptr: *mut bindings::drm_gem_object) -> &'a Self; } -// SAFETY: All gem objects are refcounted. -unsafe impl 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) { - // 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) }; - } -} - extern "C" fn open_callback( raw_obj: *mut bindings::drm_gem_object, raw_file: *mut bindings::drm_file, @@ -273,6 +253,22 @@ impl Object { } } +// SAFETY: Instances of `Object` are always reference-counted. +unsafe impl crate::types::AlwaysRefCounted for Object { + 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) { + // SAFETY: `obj` is a valid pointer to an `Object`. + 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 super::private::Sealed for Object {} impl Deref for Object { -- cgit From d3917368ebc5cd89d7d08eab4673e5c4c73ff42f Mon Sep 17 00:00:00 2001 From: Lyude Paul Date: Tue, 21 Oct 2025 13:21:36 -0400 Subject: rust: drm/gem: Remove Object.dev I noticed by chance that there's actually already a pointer to this in struct drm_gem_object. So, no use in carrying this around! Signed-off-by: Lyude Paul Acked-by: Danilo Krummrich Reviewed-by: Alice Ryhl Link: https://lore.kernel.org/r/20251021172220.252558-1-lyude@redhat.com --- rust/kernel/drm/gem/mod.rs | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) (limited to 'rust/kernel/drm/gem/mod.rs') diff --git a/rust/kernel/drm/gem/mod.rs b/rust/kernel/drm/gem/mod.rs index 20c2769a8c9d..eb5f3feac890 100644 --- a/rust/kernel/drm/gem/mod.rs +++ b/rust/kernel/drm/gem/mod.rs @@ -167,12 +167,10 @@ impl BaseObject for T {} /// Invariants /// /// - `self.obj` is a valid instance of a `struct drm_gem_object`. -/// - `self.dev` is always a valid pointer to a `struct drm_device`. #[repr(C)] #[pin_data] pub struct Object { obj: Opaque, - dev: NonNull>, #[pin] data: T, } @@ -202,9 +200,6 @@ impl Object { try_pin_init!(Self { obj: Opaque::new(bindings::drm_gem_object::default()), data <- T::new(dev, size), - // INVARIANT: The drm subsystem guarantees that the `struct drm_device` will live - // as long as the GEM object lives. - dev: dev.into(), }), GFP_KERNEL, )?; @@ -227,9 +222,13 @@ impl Object { /// Returns the `Device` that owns this GEM object. pub fn dev(&self) -> &drm::Device { - // SAFETY: The DRM subsystem guarantees that the `struct drm_device` will live as long as - // the GEM object lives, hence the pointer must be valid. - unsafe { self.dev.as_ref() } + // SAFETY: + // - `struct drm_gem_object.dev` is initialized and valid for as long as the GEM + // object lives. + // - The device we used for creating the gem object is passed as &drm::Device to + // Object::::new(), so we know that `T::Driver` is the right generic parameter to use + // here. + unsafe { drm::Device::from_raw((*self.as_raw()).dev) } } fn as_raw(&self) -> *mut bindings::drm_gem_object { -- cgit From e54ad0cd3673c93cdafda58505eaa81610fe3aef Mon Sep 17 00:00:00 2001 From: Lyude Paul Date: Fri, 7 Nov 2025 15:25:56 -0500 Subject: rust/drm/gem: Fix missing header in `Object` rustdoc Invariants should be prefixed with a # to turn it into a header. There are no functional changes in this patch. Cc: stable@vger.kernel.org Fixes: c284d3e42338 ("rust: drm: gem: Add GEM object abstraction") Signed-off-by: Lyude Paul Link: https://patch.msgid.link/20251107202603.465932-1-lyude@redhat.com Signed-off-by: Alice Ryhl --- rust/kernel/drm/gem/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'rust/kernel/drm/gem/mod.rs') diff --git a/rust/kernel/drm/gem/mod.rs b/rust/kernel/drm/gem/mod.rs index eb5f3feac890..a7f682e95c01 100644 --- a/rust/kernel/drm/gem/mod.rs +++ b/rust/kernel/drm/gem/mod.rs @@ -164,7 +164,7 @@ impl BaseObject for T {} /// A base GEM object. /// -/// Invariants +/// # Invariants /// /// - `self.obj` is a valid instance of a `struct drm_gem_object`. #[repr(C)] -- cgit