On Tue, Jul 31, 2018 at 03:04:42PM +0800, Christian König wrote: > Am 31.07.2018 um 09:12 schrieb Huang Rui: > > On Mon, Jul 30, 2018 at 04:51:58PM +0200, Christian König wrote: > >> This avoids multiple allocations for the head and the array. > >> > > I am afraid I don't get the point that how to avoid multiple times of > > allocations. Could you please explain more? > > Allocating the head and the array separately has more overhead because > you need to do two allocations. >
I see. You allocated the whole memory include bo_list(head) and all number of bo_list_entry one time, then use amdgpu_bo_list_array_entry to get the array. Acked-by: Huang Rui <[email protected]> > I should probably update the commit message, > Christian. > > > > > Thanks, > > Ray > > > >> Signed-off-by: Christian König <[email protected]> > >> --- > >> drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c | 114 > >> +++++++++++----------------- > >> drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.h | 17 +++-- > >> 2 files changed, 57 insertions(+), 74 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c > >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c > >> index 096bcf4a6334..d472a2c8399f 100644 > >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c > >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c > >> @@ -35,13 +35,15 @@ > >> #define AMDGPU_BO_LIST_MAX_PRIORITY 32u > >> #define AMDGPU_BO_LIST_NUM_BUCKETS (AMDGPU_BO_LIST_MAX_PRIORITY + > >> 1) > >> > >> -static int amdgpu_bo_list_set(struct amdgpu_device *adev, > >> - struct drm_file *filp, > >> - struct amdgpu_bo_list *list, > >> - struct drm_amdgpu_bo_list_entry *info, > >> - unsigned num_entries); > >> +static void amdgpu_bo_list_free_rcu(struct rcu_head *rcu) > >> +{ > >> + struct amdgpu_bo_list *list = container_of(rcu, struct amdgpu_bo_list, > >> + rhead); > >> + > >> + kvfree(list); > >> +} > >> > >> -static void amdgpu_bo_list_release_rcu(struct kref *ref) > >> +static void amdgpu_bo_list_free(struct kref *ref) > >> { > >> struct amdgpu_bo_list *list = container_of(ref, struct amdgpu_bo_list, > >> refcount); > >> @@ -50,67 +52,36 @@ static void amdgpu_bo_list_release_rcu(struct kref > >> *ref) > >> amdgpu_bo_list_for_each_entry(e, list) > >> amdgpu_bo_unref(&e->robj); > >> > >> - kvfree(list->array); > >> - kfree_rcu(list, rhead); > >> + call_rcu(&list->rhead, amdgpu_bo_list_free_rcu); > >> } > >> > >> -int amdgpu_bo_list_create(struct amdgpu_device *adev, > >> - struct drm_file *filp, > >> - struct drm_amdgpu_bo_list_entry *info, > >> - unsigned num_entries, > >> - struct amdgpu_bo_list **list_out) > >> +int amdgpu_bo_list_create(struct amdgpu_device *adev, struct drm_file > >> *filp, > >> + struct drm_amdgpu_bo_list_entry *info, > >> + unsigned num_entries, struct amdgpu_bo_list **result) > >> { > >> + unsigned last_entry = 0, first_userptr = num_entries; > >> + struct amdgpu_bo_list_entry *array; > >> struct amdgpu_bo_list *list; > >> + uint64_t total_size = 0; > >> + size_t size; > >> + unsigned i; > >> int r; > >> > >> + if (num_entries > SIZE_MAX / sizeof(struct amdgpu_bo_list_entry)) > >> + return -EINVAL; > >> > >> - list = kzalloc(sizeof(struct amdgpu_bo_list), GFP_KERNEL); > >> + size = sizeof(struct amdgpu_bo_list); > >> + size += num_entries * sizeof(struct amdgpu_bo_list_entry); > >> + list = kvmalloc(size, GFP_KERNEL); > >> if (!list) > >> return -ENOMEM; > >> > >> - /* initialize bo list*/ > >> kref_init(&list->refcount); > >> - r = amdgpu_bo_list_set(adev, filp, list, info, num_entries); > >> - if (r) { > >> - kfree(list); > >> - return r; > >> - } > >> - > >> - *list_out = list; > >> - return 0; > >> -} > >> - > >> -static void amdgpu_bo_list_destroy(struct amdgpu_fpriv *fpriv, int id) > >> -{ > >> - struct amdgpu_bo_list *list; > >> - > >> - mutex_lock(&fpriv->bo_list_lock); > >> - list = idr_remove(&fpriv->bo_list_handles, id); > >> - mutex_unlock(&fpriv->bo_list_lock); > >> - if (list) > >> - kref_put(&list->refcount, amdgpu_bo_list_release_rcu); > >> -} > >> - > >> -static int amdgpu_bo_list_set(struct amdgpu_device *adev, > >> - struct drm_file *filp, > >> - struct amdgpu_bo_list *list, > >> - struct drm_amdgpu_bo_list_entry *info, > >> - unsigned num_entries) > >> -{ > >> - struct amdgpu_bo_list_entry *array; > >> - struct amdgpu_bo *gds_obj = adev->gds.gds_gfx_bo; > >> - struct amdgpu_bo *gws_obj = adev->gds.gws_gfx_bo; > >> - struct amdgpu_bo *oa_obj = adev->gds.oa_gfx_bo; > >> - > >> - unsigned last_entry = 0, first_userptr = num_entries; > >> - struct amdgpu_bo_list_entry *e; > >> - uint64_t total_size = 0; > >> - unsigned i; > >> - int r; > >> + list->gds_obj = adev->gds.gds_gfx_bo; > >> + list->gws_obj = adev->gds.gws_gfx_bo; > >> + list->oa_obj = adev->gds.oa_gfx_bo; > >> > >> - array = kvmalloc_array(num_entries, sizeof(struct > >> amdgpu_bo_list_entry), GFP_KERNEL); > >> - if (!array) > >> - return -ENOMEM; > >> + array = amdgpu_bo_list_array_entry(list, 0); > >> memset(array, 0, num_entries * sizeof(struct amdgpu_bo_list_entry)); > >> > >> for (i = 0; i < num_entries; ++i) { > >> @@ -147,36 +118,41 @@ static int amdgpu_bo_list_set(struct amdgpu_device > >> *adev, > >> entry->tv.shared = !entry->robj->prime_shared_count; > >> > >> if (entry->robj->preferred_domains == AMDGPU_GEM_DOMAIN_GDS) > >> - gds_obj = entry->robj; > >> + list->gds_obj = entry->robj; > >> if (entry->robj->preferred_domains == AMDGPU_GEM_DOMAIN_GWS) > >> - gws_obj = entry->robj; > >> + list->gws_obj = entry->robj; > >> if (entry->robj->preferred_domains == AMDGPU_GEM_DOMAIN_OA) > >> - oa_obj = entry->robj; > >> + list->oa_obj = entry->robj; > >> > >> total_size += amdgpu_bo_size(entry->robj); > >> trace_amdgpu_bo_list_set(list, entry->robj); > >> } > >> > >> - amdgpu_bo_list_for_each_entry(e, list) > >> - amdgpu_bo_unref(&list->array[i].robj); > >> - > >> - kvfree(list->array); > >> - > >> - list->gds_obj = gds_obj; > >> - list->gws_obj = gws_obj; > >> - list->oa_obj = oa_obj; > >> list->first_userptr = first_userptr; > >> - list->array = array; > >> list->num_entries = num_entries; > >> > >> trace_amdgpu_cs_bo_status(list->num_entries, total_size); > >> + > >> + *result = list; > >> return 0; > >> > >> error_free: > >> while (i--) > >> amdgpu_bo_unref(&array[i].robj); > >> - kvfree(array); > >> + kvfree(list); > >> return r; > >> + > >> +} > >> + > >> +static void amdgpu_bo_list_destroy(struct amdgpu_fpriv *fpriv, int id) > >> +{ > >> + struct amdgpu_bo_list *list; > >> + > >> + mutex_lock(&fpriv->bo_list_lock); > >> + list = idr_remove(&fpriv->bo_list_handles, id); > >> + mutex_unlock(&fpriv->bo_list_lock); > >> + if (list) > >> + kref_put(&list->refcount, amdgpu_bo_list_free); > >> } > >> > >> int amdgpu_bo_list_get(struct amdgpu_fpriv *fpriv, int id, > >> @@ -229,7 +205,7 @@ void amdgpu_bo_list_get_list(struct amdgpu_bo_list > >> *list, > >> > >> void amdgpu_bo_list_put(struct amdgpu_bo_list *list) > >> { > >> - kref_put(&list->refcount, amdgpu_bo_list_release_rcu); > >> + kref_put(&list->refcount, amdgpu_bo_list_free); > >> } > >> > >> int amdgpu_bo_create_list_entry_array(struct drm_amdgpu_bo_list_in *in, > >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.h > >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.h > >> index 3d77abfcd4a6..61b089768e1c 100644 > >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.h > >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.h > >> @@ -48,7 +48,6 @@ struct amdgpu_bo_list { > >> struct amdgpu_bo *oa_obj; > >> unsigned first_userptr; > >> unsigned num_entries; > >> - struct amdgpu_bo_list_entry *array; > >> }; > >> > >> int amdgpu_bo_list_get(struct amdgpu_fpriv *fpriv, int id, > >> @@ -65,14 +64,22 @@ int amdgpu_bo_list_create(struct amdgpu_device *adev, > >> unsigned num_entries, > >> struct amdgpu_bo_list **list); > >> > >> +static inline struct amdgpu_bo_list_entry * > >> +amdgpu_bo_list_array_entry(struct amdgpu_bo_list *list, unsigned index) > >> +{ > >> + struct amdgpu_bo_list_entry *array = (void *)&list[1]; > >> + > >> + return &array[index]; > >> +} > >> + > >> #define amdgpu_bo_list_for_each_entry(e, list) \ > >> - for (e = &(list)->array[0]; \ > >> - e != &(list)->array[(list)->num_entries]; \ > >> + for (e = amdgpu_bo_list_array_entry(list, 0); \ > >> + e != amdgpu_bo_list_array_entry(list, (list)->num_entries); \ > >> ++e) > >> > >> #define amdgpu_bo_list_for_each_userptr_entry(e, list) \ > >> - for (e = &(list)->array[(list)->first_userptr]; \ > >> - e != &(list)->array[(list)->num_entries]; \ > >> + for (e = amdgpu_bo_list_array_entry(list, (list)->first_userptr); \ > >> + e != amdgpu_bo_list_array_entry(list, (list)->num_entries); \ > >> ++e) > >> > >> #endif > >> -- > >> 2.14.1 > >> > >> _______________________________________________ > >> amd-gfx mailing list > >> [email protected] > >> https://lists.freedesktop.org/mailman/listinfo/amd-gfx > _______________________________________________ amd-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/amd-gfx
