Currently in order to implement AlwaysRefCounted for gem objects, we use a
blanket implementation:
unsafe impl<T: IntoGEMObject> 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<T: RcModeObject> 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<T: RcModeObject> AlwaysRefCounted for T {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ conflicting
implementation
|
::: rust/kernel/drm/gem/mod.rs:97:1
|
97 | unsafe impl<T: IntoGEMObject> 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 <[email protected]>
---
V2:
* Rewrite the commit message to explain a bit more why we don't want a
blanket implementation for this.
Signed-off-by: Lyude Paul <[email protected]>
---
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 30c853988b942..20c2769a8c9d6 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<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) };
- }
-}
-
extern "C" fn open_callback<T: DriverObject>(
raw_obj: *mut bindings::drm_gem_object,
raw_file: *mut bindings::drm_file,
@@ -273,6 +253,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> {
--
2.51.0
