Re: [PATCH] drm/gpuvm: take refcount on DRM device

["Thomas Hellström" <thomas.hellstrom@linux.intel.com>]

On Mon, 2026-04-27 at 07:34 +0000, Alice Ryhl wrote:
> On Mon, Apr 20, 2026 at 06:19:02PM +0200, Thomas Hellström wrote:
> > On Mon, 2026-04-20 at 17:08 +0200, Danilo Krummrich wrote:
> > > On Mon Apr 20, 2026 at 11:28 AM CEST, Thomas Hellström wrote:
> > > > I agree with your reasoning here, but current fact is that most
> > > > (if
> > > > not
> > > > all) holders of a drm device reference (files, pagemaps, dma-
> > > > bufs)
> > > > currently also hold a module reference to protect against this,
> > > > and
> > > > drm_gpuvm would be an outlier.
> > > 
> > > I'm not convinced; if the DRM device has the requirement to not
> > > outlive the
> > > module it is associated with, then the DRM device code has to
> > > take
> > > care of this
> > > requirement, and not every caller of drm_dev_get().
> > > 
> > > Besides that, if GPUVM holds the module reference count on behalf
> > > of
> > > the DRM
> > > device, it has the same effect that you rightfully point out
> > > below --
> > > it breaks
> > > rmmod.
> > > 
> > > > To fix this properly (lifting that requirement) one could
> > > > introduce
> > > > a
> > > > drm device count in the module and have the module exit
> > > > function
> > > > wait
> > > > for it to become zero, *and* that the code that did the last
> > > > decrement
> > > > finished executing.
> > > > 
> > > > https://patchwork.freedesktop.org/patch/712146/?series=163298&rev=1
> > > 
> > > This looks like a reasonable fix to me. And it makes me conclude
> > > that
> > > we
> > > basically agree on everything. :)
> > 
> > Yes, unless we'd want to do a similar wait for gpuvms before
> > returning
> > from the close() callback: If we assume all GPUVMs are tied to an
> > open
> > drm file, that would conceptually be nicer IMO but I agree if gpuvm
> > drivers implement something like the above per-driver device count,
> > that would be unnecessary.
> 
> Just to confirm, it sounds like no changes to my patch are required
> here?

I guess not, Although in the end it currently just moves one potential
bug to another, but as long as people involved are aware, that's OK
from my POW.

Thanks,
Thomas


> 
> Alice