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

[Lizhi Hou <lizhi.hou@amd.com>]

Applied to drm-misc-next

On 9/2/26 11:51, Max Zhen wrote:
>
>
> 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)
>