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
