Am 15.04.25 um 12:45 schrieb Thomas Zimmermann: > Hi > > Am 15.04.25 um 11:39 schrieb Christian König: >> Am 15.04.25 um 11:20 schrieb Thomas Zimmermann: >>> Test struct drm_gem_object.import_attach to detect imported objects >>> during cleanup. At that point, the imported DMA buffer might have >>> already been released and the dma_buf field is NULL. The object's >>> free callback then does a cleanup as for native objects. >> I don't think that this is a good idea. >> >> The DMA-buf is separately reference counted through the import attachment. > > I understand that it's not ideal, but testing for import_attach to be set is > what we currently do throughout drivers. Putting this behind an interface is > already a step forward. > >> >>> Happens for calls to drm_mode_destroy_dumb_ioctl() that eventually >>> clear the dma_buf field in drm_gem_object_exported_dma_buf_free(). >> That is for exported DMA-buf and should *NEVER* be used for imported ones. > > Did you look at the discussion at the Closes tag? Where else could dma-buf be > cleared?
Yeah, I've seen that. The solution is just completely wrong. See for the export case obj->dma_buf points to the exported DMA-buf and causes a circle dependency when not set to NULL when the last handle is released. But for the import case obj->dma_buf is actually not that relevant. Instead obj->import_attach->dma_buf should be used. Regards, Christian. > > Best regards > Thomas > >> >> Regards, >> Christian. >> >>> Signed-off-by: Thomas Zimmermann <[email protected]> >>> Fixes: b57aa47d39e9 ("drm/gem: Test for imported GEM buffers with helper") >>> Reported-by: Andy Yan <[email protected]> >>> Closes: >>> https://lore.kernel.org/dri-devel/[email protected]/ >>> Tested-by: Andy Yan <[email protected]> >>> Cc: Thomas Zimmermann <[email protected]> >>> Cc: Anusha Srivatsa <[email protected]> >>> Cc: Christian König <[email protected]> >>> Cc: Maarten Lankhorst <[email protected]> >>> Cc: Maxime Ripard <[email protected]> >>> Cc: David Airlie <[email protected]> >>> Cc: Simona Vetter <[email protected]> >>> Cc: Sumit Semwal <[email protected]> >>> Cc: "Christian König" <[email protected]> >>> Cc: [email protected] >>> Cc: [email protected] >>> Cc: [email protected] >>> --- >>> include/drm/drm_gem.h | 8 +++++++- >>> 1 file changed, 7 insertions(+), 1 deletion(-) >>> >>> diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h >>> index 9b71f7a9f3f8..f09b8afcf86d 100644 >>> --- a/include/drm/drm_gem.h >>> +++ b/include/drm/drm_gem.h >>> @@ -589,7 +589,13 @@ static inline bool >>> drm_gem_object_is_shared_for_memory_stats(struct drm_gem_obje >>> static inline bool drm_gem_is_imported(const struct drm_gem_object *obj) >>> { >>> /* The dma-buf's priv field points to the original GEM object. */ >>> - return obj->dma_buf && (obj->dma_buf->priv != obj); >>> + return (obj->dma_buf && (obj->dma_buf->priv != obj)) || >>> + /* >>> + * TODO: During object release, the dma-buf might already >>> + * be gone. For now keep testing import_attach, but >>> + * this should be removed at some point. >>> + */ >>> + obj->import_attach; >>> } >>> #ifdef CONFIG_LOCKDEP >
