Re: [PATCH V1 2/2] accel/amdxdna: Fix potential deadlock in BO open and close callbacks

[Max Zhen <max.zhen@amd.com>]

On 9/2/2026 Wed 11:09, Lizhi Hou wrote:
> In amdxdna_gem_obj_open(), abo->lock is held when calling
> amdxdna_gem_add_bo_usage(), which then acquires client->mm_lock.
> 
> However, the heap update path may acquire these locks in the reverse
> order, creating a potential deadlock.
> 
> Fix this by saving the client pointer locally before acquiring abo->lock,
> and releasing abo->lock before calling amdxdna_gem_add_bo_usage().
> 
> Apply the same change to amdxdna_gem_obj_close().
> 
> Fixes: 1f513a3ec3a9 ("accel/amdxdna: Add per-process BO memory usage query support")
> Signed-off-by: Lizhi Hou <lizhi.hou@amd.com>
Reviewed-by: Max Zhen <max.zhen@amd.com>
> ---
>   drivers/accel/amdxdna/amdxdna_gem.c | 34 +++++++++++++++++++----------
>   1 file changed, 22 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/accel/amdxdna/amdxdna_gem.c b/drivers/accel/amdxdna/amdxdna_gem.c
> index 1353393194e2..0d165b66c1fc 100644
> --- a/drivers/accel/amdxdna/amdxdna_gem.c
> +++ b/drivers/accel/amdxdna/amdxdna_gem.c
> @@ -641,10 +641,8 @@ amdxdna_gem_skip_bo_usage(struct amdxdna_gem_obj *abo)
>   }
>   
>   static void
> -amdxdna_gem_add_bo_usage(struct amdxdna_gem_obj *abo)
> +amdxdna_gem_add_bo_usage(struct amdxdna_client *client, struct amdxdna_gem_obj *abo)
>   {
> -	struct amdxdna_client *client = abo->client;
> -
>   	if (amdxdna_gem_skip_bo_usage(abo))
>   		return;
>   
> @@ -656,10 +654,8 @@ amdxdna_gem_add_bo_usage(struct amdxdna_gem_obj *abo)
>   }
>   
>   static void
> -amdxdna_gem_del_bo_usage(struct amdxdna_gem_obj *abo)
> +amdxdna_gem_del_bo_usage(struct amdxdna_client *client, struct amdxdna_gem_obj *abo)
>   {
> -	struct amdxdna_client *client = abo->client;
> -
>   	if (amdxdna_gem_skip_bo_usage(abo))
>   		return;
>   
> @@ -694,14 +690,20 @@ static int amdxdna_gem_obj_open(struct drm_gem_object *gobj, struct drm_file *fi
>   {
>   	struct amdxdna_dev *xdna = to_xdna_dev(gobj->dev);
>   	struct amdxdna_gem_obj *abo = to_xdna_obj(gobj);
> +	struct amdxdna_client *client;
>   	int ret;
>   
> -	guard(mutex)(&abo->lock);
> -	if (abo->open_ref > 0 && filp->driver_priv != abo->client)
> +	mutex_lock(&abo->lock);
> +	if (abo->open_ref > 0 && filp->driver_priv != abo->client) {
> +		mutex_unlock(&abo->lock);
>   		return -EPERM;
> +	}
> +
>   	abo->open_ref++;
> -	if (abo->open_ref > 1)
> +	if (abo->open_ref > 1) {
> +		mutex_unlock(&abo->lock);
>   		return 0;
> +	}
>   
>   	/* Attached to the client when first opened by it. */
>   	abo->client = filp->driver_priv;
> @@ -712,26 +714,34 @@ static int amdxdna_gem_obj_open(struct drm_gem_object *gobj, struct drm_file *fi
>   		if (ret) {
>   			abo->open_ref--;
>   			abo->client = NULL;
> +			mutex_unlock(&abo->lock);
>   			return ret;
>   		}
>   	}
> +	client = abo->client;
> +	mutex_unlock(&abo->lock);
>   
> -	amdxdna_gem_add_bo_usage(abo);
> +	amdxdna_gem_add_bo_usage(client, abo);
>   	return 0;
>   }
>   
>   static void amdxdna_gem_obj_close(struct drm_gem_object *gobj, struct drm_file *filp)
>   {
>   	struct amdxdna_gem_obj *abo = to_xdna_obj(gobj);
> +	struct amdxdna_client *client = NULL;
>   
> -	guard(mutex)(&abo->lock);
> +	mutex_lock(&abo->lock);
>   	abo->open_ref--;
>   
>   	if (abo->open_ref == 0) {
> -		amdxdna_gem_del_bo_usage(abo);
>   		/* Detach from the client when last closed by it. */
> +		client = abo->client;
>   		abo->client = NULL;
>   	}
> +	mutex_unlock(&abo->lock);
> +
> +	if (client)
> +		amdxdna_gem_del_bo_usage(client, abo);
>   }
>   
>   static int amdxdna_gem_obj_vmap(struct drm_gem_object *obj, struct iosys_map *map)