On Tue, 15 Apr 2025 14:19:20 +0200 Christian König <christian.koe...@amd.com> wrote:
> 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. I can confirm this is causing a regression in Panthor, where the driver holds references to GEMs that might have been released by userspace, so it's not just drivers calling drm_mode_destroy_dumb_ioctl(). This leads drm_gem_is_imported() to return inconsistent values depending on when the function is called (before/after the handle creation/destruction). This patch seems to fix the problem, but I can't tell if it's the right thing to do.
