Danilo Krummrich
2025-Jul-24 22:27 UTC
[PATCH] Partially revert "rust: drm: gem: Implement AlwaysRefCounted for all gem objects automatically"
On Thu Jul 24, 2025 at 11:06 PM CEST, Lyude Paul wrote:> On Thu, 2025-07-24 at 22:03 +0200, Danilo Krummrich wrote: >> 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 you mean you think the blanket implementation that we had would work, or > that getting rid of it would work?The former.> Since the blanket implementation we have > definitely doesn't compile on my machine once we add more then one > IntoGEMObject impl. (before adding it, it works just fine)Do you have a branch somewhere, where it doesn't compile?
Daniel Almeida
2025-Jul-24 23:13 UTC
[PATCH] Partially revert "rust: drm: gem: Implement AlwaysRefCounted for all gem objects automatically"
> On 24 Jul 2025, at 19:27, Danilo Krummrich <dakr at kernel.org> wrote: > > On Thu Jul 24, 2025 at 11:06 PM CEST, Lyude Paul wrote: >> On Thu, 2025-07-24 at 22:03 +0200, Danilo Krummrich wrote: >>> 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 you mean you think the blanket implementation that we had would work, or >> that getting rid of it would work? > > The former. > >> Since the blanket implementation we have >> definitely doesn't compile on my machine once we add more then one >> IntoGEMObject impl. (before adding it, it works just fine) > > Do you have a branch somewhere, where it doesn't compile?Hi Lyude, I?m somewhat surprised to be honest. Your gem-shmem code works on tyr-next, which is currently on top of 6.16-rc2. What exactly doesn?t compile? [0] https://gitlab.freedesktop.org/panfrost/linux/-/tree/tyr-next?ref_type=heads