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 <tzimmerm...@suse.de>
>>> Fixes: b57aa47d39e9 ("drm/gem: Test for imported GEM buffers with helper")
>>> Reported-by: Andy Yan <andys...@163.com>
>>> Closes:
>>> https://lore.kernel.org/dri-devel/38d09d34.4354.196379aa560.coremail.andys...@163.com/
>>> Tested-by: Andy Yan <andys...@163.com>
>>> Cc: Thomas Zimmermann <tzimmerm...@suse.de>
>>> Cc: Anusha Srivatsa <asriv...@redhat.com>
>>> Cc: Christian König <christian.koe...@amd.com>
>>> Cc: Maarten Lankhorst <maarten.lankho...@linux.intel.com>
>>> Cc: Maxime Ripard <mrip...@kernel.org>
>>> Cc: David Airlie <airl...@gmail.com>
>>> Cc: Simona Vetter <sim...@ffwll.ch>
>>> Cc: Sumit Semwal <sumit.sem...@linaro.org>
>>> Cc: "Christian König" <christian.koe...@amd.com>
>>> Cc: dri-devel@lists.freedesktop.org
>>> Cc: linux-me...@vger.kernel.org
>>> Cc: linaro-mm-...@lists.linaro.org
>>> ---
>>> 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
>
