On Sat, Dec 20, 2025 at 09:48:17AM +0000, Alice Ryhl wrote:
> On Fri, Dec 19, 2025 at 04:35:00PM +0100, Danilo Krummrich wrote:
> > On Fri Nov 28, 2025 at 3:14 PM CET, Alice Ryhl wrote:
> > > + /// Returns a [`GpuVmBoObtain`] for the provided GEM object.
> > > + #[inline]
> > > + pub fn obtain(
> > > + &self,
> > > + obj: &T::Object,
> > > + data: impl PinInit<T::VmBoData>,
> > > + ) -> Result<GpuVmBoObtain<T>, AllocError> {
> > > + Ok(GpuVmBoAlloc::new(self, obj, data)?.obtain())
> > > + }
> >
> > Does this method make sense? We usually preallocate a VM_BO, then enter the
> > fence signalling critical path and then obtain the VM_BO.
>
> Hmm, but there is something tricky here. When do we add it to the extobj
> list, then? If we add it before starting the critical path, then we must
> also call drm_gpuvm_bo_obtain_prealloc() before starting the critical
> path because obtain must happen before drm_gpuvm_bo_extobj_add(). And
> adding it to extobj after signalling the fence seems error prone.
>
> And besides, adding it to the extobj list before the critical path
> means that we can have drm_gpuvm_exec_lock() lock the new BO without
> having to do anything special - it's simply in the extobj list by the
> time we call drm_gpuvm_exec_lock().
>
> > > +impl<T: DriverGpuVm> DerefMut for GpuVmCore<T> {
> > > + #[inline]
> > > + fn deref_mut(&mut self) -> &mut T {
> > > + // SAFETY: By the type invariants we may access `core`.
> > > + unsafe { &mut *self.0.core.get() }
> > > + }
> > > +}
> >
> > Hm..it seems more natural to me to deref to &GpuVm<T> and provide data() and
> > data_mut().
>
> That's fair.
>
> > > +impl<T: DriverGpuVm> Drop for GpuVmBoAlloc<T> {
> > > + #[inline]
> > > + fn drop(&mut self) {
> > > + // SAFETY: It's safe to perform a deferred put in any context.
> > > + unsafe { bindings::drm_gpuvm_bo_put_deferred(self.as_raw()) };
> >
> > This does not need to be deferred, no?
>
> I think what I *actually* want to call here is
>
> kref_put(&self->kref, drm_gpuvm_bo_destroy_not_in_lists_kref);
>
> like what drm_gpuvm_bo_obtain_prealloc() does as of the first patch in
> this series.
>
> > > + }
> > > +}
> > > +
> > > +/// A [`GpuVmBo`] object in the GEM list.
> > > +///
> > > +/// # Invariants
> > > +///
> > > +/// Points at a `drm_gpuvm_bo` that contains a valid `T::VmBoData` and
> > > is present in the gem list.
> > > +pub struct GpuVmBoObtain<T: DriverGpuVm>(NonNull<GpuVmBo<T>>);
> >
> > How is this different from GpuVmBo? The only object that is not in the GEM
> > list
> > should be GpuVmBoAlloc, i.e. the preallocated one.
>
> The difference is whether there is pointer indirection or not.
>
> This type is morally an ARef<GpuVm<T>>, except I don't expose any way
> to increment the refcount.
>
> Alice
