Hi
Am 14.07.25 um 14:39 schrieb Simona Vetter:
On Fri, Jul 11, 2025 at 11:35:15AM +0200, Thomas Zimmermann wrote:
Revert the use of drm_gem_object.dma_buf back to .import_attach->dmabuf
in the affected places. Also revert any fixes on top. Separates references
to imported and exported DMA bufs within a GEM object; as before.
Using the dma_buf as the one authoritative field for the DMA buf turns
out to be fragile. The GEM object's dma_buf pointer can be NULL if
userspace releases the GEM handle too early. Sima mentioned that the fix
in commit 5307dce878d4 ("drm/gem: Acquire references on GEM handles for
framebuffers") is conceptionally broken. Linus still notices boot-up
hangs that might be related.
Reverting the whole thing is the only sensible action here.
Tested on virtio; and amdgpu, simpledrm plus udl for dma-buf sharing.
Thomas Zimmermann (9):
Revert "drm/framebuffer: Acquire internal references on GEM handles"
Revert "drm/gem: Acquire references on GEM handles for framebuffers"
Ok, I think all the below we should still apply for -fixes, because
fundamentally gem_bo->dma_buf is not invariant over the lifetime of the
buffer, while gem_bo->import_attach.dmabuf is. And so we blow up.
For display drivers the handle_count reference mostly papers over the
issues, but even display drivers are allowed to keep internal references
to the underlying gem bo for longer. So there could be a bunch of really
tricky bugs lurking.
For render drivers it's even clearer, they don't have framebuffers as
objects, so there the fb handle_count references does not help.
I'm not opposed to trying to unify these fields for imported dma_buf, but
currently they're just not. Hence all the reverts.
Thanks for the write up.
The patches also need Fixes: and as needed, cc: stable added for merging.
With that and the above text as additional justification added:
Reviewed-by: Simona Vetter <simona.vet...@ffwll.ch>
Also we'd need to chase down any addiotional conversions that have only
landed in -next meanwhile of course.
₣or the handle_count patches I'm less sure. I don't think they're
justified for fixing the gem_bo->dma_buf NULL pointer issues, but they do
probably help with the GETFB/2 confusion Christian has pointed out.
Personally my preference is:
1. Apply the two reverts.
2. Create an igt testcase for the GETFB confusion
3. Figure out what the right fix for that is, which might or might not be
the handle_count reference of drm_fb.
But with my maintainer hat on I don't mind about the exact path, as long
as we get there somehow. If you decide to do land the reverts, they also
have my:
Reviewed-by: Simona Vetter <simona.vet...@ffwll.ch>
Let's first revert all the dma_buf switching in drm-misc and other
trees. They should
be easy. If we revert the framebuffer-related A changes first, we might
end up with
these intermediate errors again.
There's no hurry with the framebuffer changes. We can address them after
upstream
picked up the dma-buf reverts.
Best regards
Thomas
Cheers, Sima
Revert "drm/virtio: Use dma_buf from GEM object instance"
Revert "drm/vmwgfx: Use dma_buf from GEM object instance"
Revert "drm/etnaviv: Use dma_buf from GEM object instance"
Revert "drm/prime: Use dma_buf from GEM object instance"
Revert "drm/gem-framebuffer: Use dma_buf from GEM object instance"
Revert "drm/gem-shmem: Use dma_buf from GEM object instance"
Revert "drm/gem-dma: Use dma_buf from GEM object instance"
drivers/gpu/drm/drm_framebuffer.c | 31 +---------
drivers/gpu/drm/drm_gem.c | 64 +++-----------------
drivers/gpu/drm/drm_gem_dma_helper.c | 2 +-
drivers/gpu/drm/drm_gem_framebuffer_helper.c | 8 ++-
drivers/gpu/drm/drm_gem_shmem_helper.c | 4 +-
drivers/gpu/drm/drm_internal.h | 2 -
drivers/gpu/drm/drm_prime.c | 8 ++-
drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c | 4 +-
drivers/gpu/drm/virtio/virtgpu_prime.c | 5 +-
drivers/gpu/drm/vmwgfx/vmwgfx_gem.c | 6 +-
include/drm/drm_framebuffer.h | 7 ---
11 files changed, 35 insertions(+), 106 deletions(-)
--
2.50.0
--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)
