[PATCH 0/9] drm: Revert general use of struct drm_gem_object.dma_buf

[Thomas Zimmermann]

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.vetter at 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.vetter at 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)