Re: [PATCH 5/9] drm/amdgpu: always recreate bo_list

[Huang Rui <ray.huang-5C7GfCeVMHo@public.gmane.org>]

On Mon, Jul 30, 2018 at 04:51:55PM +0200, Christian König wrote:
> When changing a list always completely recreate it. This avoids locking
> in the hot path because the list always stays like it is until it is
> unreferenced.

The fpriv->bo_list_handles is allocated by OP_CREATE, so here we just
re-create bo list and replace the handles in OP_UPDATE. Then we don't need
locking to protect amdgpu_bo_list because it always be re-created.

Acked-by: Huang Rui <ray.huang@amd.com>

> 
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c | 23 ++++++++++++-----------
>  drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.h |  1 -
>  drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c      |  3 ---
>  3 files changed, 12 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
> index 6728448167ba..556040e45931 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
> @@ -50,7 +50,6 @@ static void amdgpu_bo_list_release_rcu(struct kref *ref)
>  	for (i = 0; i < list->num_entries; ++i)
>  		amdgpu_bo_unref(&list->array[i].robj);
>  
> -	mutex_destroy(&list->lock);
>  	kvfree(list->array);
>  	kfree_rcu(list, rhead);
>  }
> @@ -70,7 +69,6 @@ int amdgpu_bo_list_create(struct amdgpu_device *adev,
>  		return -ENOMEM;
>  
>  	/* initialize bo list*/
> -	mutex_init(&list->lock);
>  	kref_init(&list->refcount);
>  	r = amdgpu_bo_list_set(adev, filp, list, info, num_entries);
>  	if (r) {
> @@ -188,7 +186,6 @@ int amdgpu_bo_list_get(struct amdgpu_fpriv *fpriv, int id,
>  
>  	if (*result && kref_get_unless_zero(&(*result)->refcount)) {
>  		rcu_read_unlock();
> -		mutex_lock(&(*result)->lock);
>  		return 0;
>  	}
>  
> @@ -231,7 +228,6 @@ void amdgpu_bo_list_get_list(struct amdgpu_bo_list *list,
>  
>  void amdgpu_bo_list_put(struct amdgpu_bo_list *list)
>  {
> -	mutex_unlock(&list->lock);
>  	kref_put(&list->refcount, amdgpu_bo_list_release_rcu);
>  }
>  
> @@ -242,7 +238,6 @@ void amdgpu_bo_list_free(struct amdgpu_bo_list *list)
>  	for (i = 0; i < list->num_entries; ++i)
>  		amdgpu_bo_unref(&list->array[i].robj);
>  
> -	mutex_destroy(&list->lock);
>  	kvfree(list->array);
>  	kfree(list);
>  }
> @@ -297,7 +292,7 @@ int amdgpu_bo_list_ioctl(struct drm_device *dev, void *data,
>  	union drm_amdgpu_bo_list *args = data;
>  	uint32_t handle = args->in.list_handle;
>  	struct drm_amdgpu_bo_list_entry *info = NULL;
> -	struct amdgpu_bo_list *list;
> +	struct amdgpu_bo_list *list, *old;
>  	int r;
>  
>  	r = amdgpu_bo_create_list_entry_array(&args->in, &info);
> @@ -328,16 +323,22 @@ int amdgpu_bo_list_ioctl(struct drm_device *dev, void *data,
>  		break;
>  
>  	case AMDGPU_BO_LIST_OP_UPDATE:
> -		r = amdgpu_bo_list_get(fpriv, handle, &list);
> +		r = amdgpu_bo_list_create(adev, filp, info, args->in.bo_number,
> +					  &list);
>  		if (r)
>  			goto error_free;
>  
> -		r = amdgpu_bo_list_set(adev, filp, list, info,
> -					      args->in.bo_number);
> -		amdgpu_bo_list_put(list);
> -		if (r)
> +		mutex_lock(&fpriv->bo_list_lock);
> +		old = idr_replace(&fpriv->bo_list_handles, list, handle);
> +		mutex_unlock(&fpriv->bo_list_lock);
> +
> +		if (IS_ERR(old)) {
> +			amdgpu_bo_list_put(list);
> +			r = PTR_ERR(old);
>  			goto error_free;
> +		}
>  
> +		amdgpu_bo_list_put(old);
>  		break;
>  
>  	default:
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.h
> index 833f846bfdad..89195fdcb1ef 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.h
> @@ -41,7 +41,6 @@ struct amdgpu_bo_list_entry {
>  };
>  
>  struct amdgpu_bo_list {
> -	struct mutex lock;
>  	struct rcu_head rhead;
>  	struct kref refcount;
>  	struct amdgpu_bo *gds_obj;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> index 0295666968da..f7154f3ed807 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> @@ -580,9 +580,6 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p,
>  				       &p->bo_list);
>  		if (r)
>  			return r;
> -
> -	} else if (p->bo_list) {
> -		mutex_lock(&p->bo_list->lock);
>  	}
>  
>  	if (p->bo_list) {
> -- 
> 2.14.1
> 
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx