On 2018-07-17 08:50 AM, Christian König wrote: > Am 16.07.2018 um 18:05 schrieb Michel Dänzer: >> 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 understand the reason why Marek wants to do this, but I agree that > this is a little bit dangerous if used incorrectly. > > On the other hand I don't see any other way to sanely handle it either. Sanely handle what exactly? :) I still haven't seen any description of an actual problem, other than "the handle is stored in the hash table". >>>> 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? > > MM interop with GFX usually imports BOs on each frame, so that can hurt > here. That's always the same set of BOs in the steady state, right? So it's easy to make the repeated lookups fast, by moving them to the start of their hash buckets. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer
