[Linaro-mm-sig] Re: [PATCH v2] drm/gem: Fix GEM handle release errors

[Christian König]

Hi Jeffy,

Am 10.08.22 um 06:16 schrieb Chen Jeffy:
> Hi Christian,
>
> On 8/9 星期二 18:18, Christian König wrote:
>> Hi Jeffy,
>> [SNIP]
>>> Maybe cache the latest returned handle in the obj(after 
>>> drm_gem_prime_fd_to_handle), and clear it when that handle been 
>>> deleted in drm_gem_handle_delete()?
>>
>> That won't work. The handle is per fpriv, but the same object is used 
>> by multiple fpriv instances. >
>> What we could maybe do is to prevent adding multiple lockup 
>> structures when there is already one, but that's not something I can 
>> easily judge.
>
> So maybe we need to protect that unique lookup structure been deleted 
> before deleting the last handle, and make the handle unique for GEM 
> obj, in case of that unique lookup's handle been deleted earlier that 
> others?
>
> How about adding a GEM obj rbtree too, and make drm_prime_member kref-ed?
>
> So the 
> drm_prime_add_buf_handle/drm_gem_handle_create_tail/drm_gem_handle_delete 
> would be looking up drm_prime_member by GEM obj, then update dmabuf rb 
> and inc/dec drm_prime_member's kref, 
> drm_gem_prime_fd_to_handle/drm_gem_prime_handle_to_fd remain unchanged.

I think we should probably come up with an idea what the UAPI should 
look like before we try to implement this in the kernel, but in general 
I think we should make the solution simpler and not even more complex.

Recording multiple handles for the same DMA-buf/fpriv combination 
doesn't seem to make sense, so the duplicated tracking of 
handle->dma_buf mapping just seems to be overkill.

So my proposal would be this:
1. Only the first used GEM handle is tracker for each DMA-buf/fpriv 
combination.
2. Imports either return this first used handle or allocate a new one if 
there isn't any.
3. If the first used handle is closed we allocate a new one on re-import 
even when there duplicate handles.

The alternative as we have it now is to just return a more or less 
random handle if there are duplicates which doesn't sound like something 
we would want.

Daniel, can we agree on that?

Regards,
Christian.