[PATCH 8/9] drm/amdgpu: allocate the bo_list array after the list

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

 



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 <ray.huang at amd.com>

> I should probably update the commit message,
> Christian.
> 
> >
> > Thanks,
> > Ray
> >
> >> Signed-off-by: Christian König <christian.koenig at amd.com>
> >> ---
> >>   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
> >> amd-gfx at lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> 


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

  Powered by Linux