[PATCH] drm/prime: remove drm_prime_lookup_buf_by_handle

[Christian König]

On 6/13/25 15:20, Simona Vetter wrote:
> On Fri, Jun 13, 2025 at 02:24:41PM +0200, Christian König wrote:
>> On 6/13/25 14:15, Christian König wrote:
>>> On 6/13/25 14:11, Saarinen, Jani wrote:
>>>> Hi, 
>>>>
>>>>> -----Original Message-----
>>>>> From: dri-devel <dri-devel-bounces at lists.freedesktop.org> On Behalf Of Jani
>>>>> Nikula
>>>>> Sent: Friday, 13 June 2025 14.02
>>>>> To: Tvrtko Ursulin <tursulin at ursulin.net>; Simona Vetter
>>>>> <simona.vetter at ffwll.ch>; Christian König
>>>>> <ckoenig.leichtzumerken at gmail.com>
>>>>> Cc: tzimmermann at suse.de; dri-devel at lists.freedesktop.org
>>>>> Subject: Re: [PATCH] drm/prime: remove drm_prime_lookup_buf_by_handle
>>>>>
>>>>> On Fri, 13 Jun 2025, Tvrtko Ursulin <tursulin at ursulin.net> wrote:
>>>>>> On 13/06/2025 11:09, Jani Nikula wrote:
>>>>>>> On Wed, 04 Jun 2025, Simona Vetter <simona.vetter at ffwll.ch> wrote:
>>>>>>>> On Wed, Jun 04, 2025 at 05:36:22PM +0200, Simona Vetter wrote:
>>>>>>> This regressed one of our CI IGT tests [1].
>>>>>>>
>>>>>>> BR,
>>>>>>> Jani.
>>>>>>>
>>>>>>>
>>>>>>> [1] https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/14463
>>>>>>
>>>>>> It also explodes even more trivially when logging into a KDE Wayland
>>>>>> session:
>>>>>
>>>>> Smells like a revert, and back to the drawing board, perhaps?
>>>
>>> Potentially, but any idea what's going wrong? Sounds like I missed something, but I don't see what.
>>
>> Oh! I now see what's going on.
>>
>> Looks like the code previously had a race condition and by removing the extra check I made the race condition 100% likely.
>>
>> Ups, I think a simple revert won't do it here. Give me a second.
> 
> Please make sure you cc: xe-devel so intel-gfx-ci can pick it up and test.
> It's a bit embarrassing.
> 
> Also since this breaks things quite badly might be good to push the revert
> right away since I don't think we can land the proper fix before the w/e.
> For that
> 
> Acked-by: Simona Vetter <simona.vetter at ffwll.ch>

Done.

The basic problem is that the existing code which checked obj->dma_buf is basically broken.

Because of flink and/or GETFB2 for example it can be that you get two GEM handles for the same GEM object. But in prime we always want to return the same handle for each DMA-buf.

So if somebody would export one of those duplicated handles we would add all of them to the prime rb tree and so change the handle which would be returned. I strongly think that is not something intentional.

I've send out a patch to address this, but I'm not sure about what the preferred approach to fixing this is?

Regards,
Christian.


> 
> Cheers, Sima
>>
>> Regards,
>> Christian.
>>
>>>
>>> Regards,
>>> Christian.
>>>
>>>> I would say so. Looks like this on our CI https://intel-gfx-ci.01.org/tree/drm-tip/igt@prime_self_import@basic-with_one_bo.html 
>>>> And systems stop testing anything after (see eg https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16692/bat-twl-1/igt_runner0.txt ) when aborts happens. 
>>>>
>>>>>
>>>>>
>>>>> BR,
>>>>> Jani.
>>>>
>>>> Br
>>>> Other Jani
>>>>>
>>>>>
>>>>> --
>>>>> Jani Nikula, Intel
>>>
>>
>