On Mon, Jul 16, 2018 at 12:05 PM, Michel Dänzer <michel at daenzer.net> wrote: > 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? Fullscreen DRI2 or DRI3 re-imports buffers every frame. It might show up in a profiler. Marek
