[PATCH libdrm] amdgpu: add amdgpu_bo_handle_type_kms_noimport

[Marek Olšák]

On Fri, Jul 13, 2018 at 4:28 AM, Michel Dänzer <michel at daenzer.net> wrote:
> On 2018-07-12 07:03 PM, Marek Olšák wrote:
>> On Thu, Jul 12, 2018, 3:31 AM Michel Dänzer <michel at daenzer.net> wrote:
>>> On 2018-07-12 02:47 AM, Marek Olšák wrote:
>>>> From: Marek Olšák <marek.olsak at amd.com>
>>>>
>>>> diff --git a/amdgpu/amdgpu_bo.c b/amdgpu/amdgpu_bo.c
>>>> index 9e37b149..d29be244 100644
>>>> --- a/amdgpu/amdgpu_bo.c
>>>> +++ b/amdgpu/amdgpu_bo.c
>>>> @@ -234,20 +234,22 @@ int amdgpu_bo_export(amdgpu_bo_handle bo,
>>>>       case amdgpu_bo_handle_type_gem_flink_name:
>>>>               r = amdgpu_bo_export_flink(bo);
>>>>               if (r)
>>>>                       return r;
>>>>
>>>>               *shared_handle = bo->flink_name;
>>>>               return 0;
>>>>
>>>>       case amdgpu_bo_handle_type_kms:
>>>>               amdgpu_add_handle_to_table(bo);
>>>> +             /* fall through */
>>>> +     case amdgpu_bo_handle_type_kms_noimport:
>>>>               *shared_handle = bo->handle;
>>>>               return 0;
>>>
>>> What is the rationale for this? I.e. why do you want to not store some
>>> handles in the hash table?
>>
>>
>> Because I have the option.
>
> Seems like you're expecting this patch to be accepted without providing
> any real justification for it (here or in the corresponding Mesa patch).
> NAK from me if so.

The real justification is implied by the patch. See: amdgpu_add_handle_to_table
Like I said: There is no risk of regression and it simplifies one
simple case trivially. We shouldn't have to even talk about it.

>
>
>>> And how can code using amdgpu_bo_handle_type_kms_noimport be sure that
>>> the BO will never be re-imported via dma-buf?
>>
>> That's for the user to decide and prove when it's safe.
>
> We shouldn't even have to think about this, let's use the mental
> capacity for more useful things. :)

Mental capacity spent to write the patch: 15 seconds
Mental capacity spent for bike-shedding: Minutes? Tens of minutes?

>
> I'd rather add the handle to the hash table in amdgpu_bo_alloc,
> amdgpu_create_bo_from_user_mem and amdgpu_bo_import instead of in
> amdgpu_bo_export, making amdgpu_bo_export(bo, amdgpu_bo_handle_type_kms,
> ...) essentially free. In the unlikely (since allocating a BO from the
> kernel is expensive) case that the hash table shows up on profiles, we
> can optimize it.

The hash table isn't very good for high BO counts. The time complexity
of a lookup is O(n).

Marek