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 at amd.com>
>
> Signed-off-by: Christian König <christian.koenig at 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 at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
