[PATCH v5 09/10] drm/amdgpu: cleanup leftover queues

[Christian König]

Am 06.07.23 um 14:36 schrieb Shashank Sharma:
> This patch adds code to cleanup any leftover userqueues which
> a user might have missed to destroy due to a crash or any other
> programming error.
>
> Cc: Alex Deucher <alexander.deucher at amd.com>
> Cc: Christian Koenig <christian.koenig at amd.com>
> Suggested-by: Bas Nieuwenhuizen <bas at basnieuwenhuizen.nl>
> Signed-off-by: Bas Nieuwenhuizen <bas at basnieuwenhuizen.nl>
> Signed-off-by: Shashank Sharma <shashank.sharma at amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.c | 31 ++++++++++++++++---
>   1 file changed, 26 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.c
> index 61064266c4f8..6e32e2854a58 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.c
> @@ -57,12 +57,23 @@ amdgpu_userqueue_get_doorbell_index(struct amdgpu_userq_mgr *uq_mgr,
>   	return index;
>   }
>   
> +static void
> +amdgpu_userqueue_cleanup(struct amdgpu_userq_mgr *uq_mgr,
> +			 struct amdgpu_usermode_queue *queue,
> +			 int queue_id)
> +{
> +	const struct amdgpu_userq_funcs *uq_funcs = uq_mgr->userq_funcs[queue->queue_type];
> +
> +	uq_funcs->mqd_destroy(uq_mgr, queue);
> +	idr_remove(&uq_mgr->userq_idr, queue_id);
> +	kfree(queue);
> +}
> +
>   static int
>   amdgpu_userqueue_destroy(struct drm_file *filp, int queue_id)
>   {
>   	struct amdgpu_fpriv *fpriv = filp->driver_priv;
>   	struct amdgpu_userq_mgr *uq_mgr = &fpriv->userq_mgr;
> -	const struct amdgpu_userq_funcs *uq_funcs;
>   	struct amdgpu_usermode_queue *queue;
>   
>   	mutex_lock(&uq_mgr->userq_mutex);
> @@ -73,11 +84,8 @@ amdgpu_userqueue_destroy(struct drm_file *filp, int queue_id)
>   		mutex_unlock(&uq_mgr->userq_mutex);
>   		return -EINVAL;
>   	}
> -	uq_funcs = uq_mgr->userq_funcs[queue->queue_type];
> -	uq_funcs->mqd_destroy(uq_mgr, queue);
> -	idr_remove(&uq_mgr->userq_idr, queue_id);
> -	kfree(queue);
>   
> +	amdgpu_userqueue_cleanup(uq_mgr, queue, queue_id);
>   	mutex_unlock(&uq_mgr->userq_mutex);
>   	return 0;
>   }
> @@ -193,8 +201,21 @@ int amdgpu_userq_mgr_init(struct amdgpu_userq_mgr *userq_mgr, struct amdgpu_devi
>   	return 0;
>   }
>   
> +static int amdgpu_userqueue_cleanup_residue(int queue_id, void *ptr, void *data)
> +{
> +	struct amdgpu_userq_mgr *uq_mgr = data;
> +	struct amdgpu_usermode_queue *queue = ptr;
> +
> +	amdgpu_userqueue_cleanup(uq_mgr, queue, queue_id);
> +	return 0;
> +}
> +
>   void amdgpu_userq_mgr_fini(struct amdgpu_userq_mgr *userq_mgr)
>   {
> +	idr_for_each(&userq_mgr->userq_idr,
> +		     amdgpu_userqueue_cleanup_residue,
> +		     userq_mgr);
> +

Better use idr_for_each_entry() here which avoids the mid layer function.

Apart from that it would be nice to have to merge this patch into the 
original one adding the user queues, but really only nice to have.

Christian.

>   	idr_destroy(&userq_mgr->userq_idr);
>   	mutex_destroy(&userq_mgr->userq_mutex);
>   }