[PATCH libdrm] amdgpu: add amdgpu_bo_handle_type_kms_noimport

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2018-07-13 08:47 PM, Marek Olšák wrote:
> 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:
>>>>
>>>> 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.

IMO you haven't provided enough justification for adding API which is
prone to breakage if used incorrectly.

Other opinions?


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

A lookup is only needed in amdgpu_bo_import. amdgpu_bo_alloc and
amdgpu_create_bo_from_user_mem can just add the handle to the hash
bucket directly.

Do you know of, or can you imagine, any workload where amdgpu_bo_import
is called often enough for this to be a concern?


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer


[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux