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

From: Christian König
Date: Wed Aug 10 2022 - 05:16:19 EST


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.