Re: [PATCH v3] drm/gem: Internally test import_attach for imported objects

[Boris Brezillon <boris.brezillon@collabora.com>]

On Wed, 30 Apr 2025 11:07:17 +0200
Simona Vetter <simona.vetter@ffwll.ch> wrote:

> On Wed, Apr 16, 2025 at 11:38:03AM +0200, Simona Vetter wrote:
> > On Wed, Apr 16, 2025 at 08:57:45AM +0200, Thomas Zimmermann wrote:  
> > > Test struct drm_gem_object.import_attach to detect imported objects.
> > > 
> > > During object clenanup, the dma_buf field might be NULL. Testing it in
> > > an object's free callback then incorrectly does a cleanup as for native
> > > objects. Happens for calls to drm_mode_destroy_dumb_ioctl() that
> > > clears the dma_buf field in drm_gem_object_exported_dma_buf_free().
> > > 
> > > v3:
> > > - only test for import_attach (Boris)
> > > v2:
> > > - use import_attach.dmabuf instead of dma_buf (Christian)
> > > 
> > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> > > Fixes: b57aa47d39e9 ("drm/gem: Test for imported GEM buffers with helper")
> > > Reported-by: Andy Yan <andyshrk@163.com>
> > > Closes: https://lore.kernel.org/dri-devel/38d09d34.4354.196379aa560.Coremail.andyshrk@163.com/
> > > Tested-by: Andy Yan <andyshrk@163.com>
> > > Cc: Thomas Zimmermann <tzimmermann@suse.de>
> > > Cc: Anusha Srivatsa <asrivats@redhat.com>
> > > Cc: Christian König <christian.koenig@amd.com>
> > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > > Cc: Maxime Ripard <mripard@kernel.org>
> > > Cc: David Airlie <airlied@gmail.com>
> > > Cc: Simona Vetter <simona@ffwll.ch>
> > > Cc: Sumit Semwal <sumit.semwal@linaro.org>
> > > Cc: "Christian König" <christian.koenig@amd.com>
> > > Cc: dri-devel@lists.freedesktop.org
> > > Cc: linux-media@vger.kernel.org
> > > Cc: linaro-mm-sig@lists.linaro.org  
> > 
> > Reviewed-by: Simona Vetter <simona.vetter@ffwll.ch>  
> 
> Also quick doc request: We do have a bit of overview documentation for
> prime here about specifically this lifetime fun, and why there's a chain
> of references and hence a distinction between imported foreign dma-buf and
> re-imported native dma-buf:
> 
> https://dri.freedesktop.org/docs/drm/gpu/drm-mm.html#reference-counting-for-gem-drivers
> 
> I think it would be good to augment this with more links to functions
> (like this one recently added and fixed in this patch here) and struct
> members to that overview. And maybe also link from key function and struct
> functions back to that overview doc. Otherwise I think the next person
> will get confused by this rather tricky code again and break a corner
> cases.

BTW, could we also backmerge 6.15-rc5 into drm-misc-next so the fix is
also present in drm-misc-next?