Re: [PATCH v6 2/8] drm/amdgpu/userq: Fix the mutex_init cleanup for fence_drv_lock

["Christian König" <christian.koenig@amd.com>]

On 5/19/26 15:09, Khatri, Sunil wrote:
> 
> On 19-05-2026 06:08 pm, Christian König wrote:
>> On 5/19/26 13:17, Sunil Khatri wrote:
>>> mutex fence_drv_lock is destroyed in amdgpu_userq_fence_driver_free
>>> also in one of the jump condition mutex_destroy is also called leading
>>> to double mutex_destroy.
>>>
>>> So rearranging the code so amdgpu_userq_fence_driver_free takes care
>>> of the clean up along with mutex_destroy.
>> Please also move amdgpu_userq_fence_driver_free() into amdgpu_userq.c or eventually completely drop it.
>>
>> The cleanup done in there is actually on the queue and not the fence driver
> 
> There is no clear demarcation here, we are doing amdgpu_userq_walk_and_drop_fence_drv and amdgpu_userq_fence_driver_put in the clean up function.  If it's ok we could pick that up later for code
> 
> organization as its mixed use case right now and all the function called from clean up also needs to be pulled in.

Yeah all of that looks pretty mixed up. Maybe we should move the handling more into amdgpu_userq_fence.c, I don't really know what would be cleaner.

Anyway Reviewed-by: Christian König <christian.koenig@amd.com> for this patch at the moment since it is clearly fixing a bug.

Thanks,
Christian.

> 
> Regards
> Sunil Khatri
> 
>>
>> Regards,
>> Christian.
>>
>>> Signed-off-by: Sunil Khatri <sunil.khatri@amd.com>
>>> ---
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c | 5 ++---
>>>   1 file changed, 2 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
>>> index eedea84c5e0f..3bfb9ae2cb3a 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
>>> @@ -748,12 +748,12 @@ amdgpu_userq_create(struct drm_file *filp, union drm_amdgpu_userq *args)
>>>       INIT_DELAYED_WORK(&queue->hang_detect_work,
>>>                 amdgpu_userq_hang_detect_work);
>>>   -    mutex_init(&queue->fence_drv_lock);
>>> -    xa_init_flags(&queue->fence_drv_xa, XA_FLAGS_ALLOC);
>>>       r = amdgpu_userq_fence_driver_alloc(adev, &queue->fence_drv);
>>>       if (r)
>>>           goto free_queue;
>>>   +    xa_init_flags(&queue->fence_drv_xa, XA_FLAGS_ALLOC);
>>> +    mutex_init(&queue->fence_drv_lock);
>>>       /* Make sure the queue can actually run with those virtual addresses. */
>>>       r = amdgpu_bo_reserve(fpriv->vm.root.bo, false);
>>>       if (r)
>>> @@ -844,7 +844,6 @@ amdgpu_userq_create(struct drm_file *filp, union drm_amdgpu_userq *args)
>>>       amdgpu_bo_reserve(fpriv->vm.root.bo, true);
>>>       amdgpu_userq_buffer_vas_list_cleanup(adev, queue);
>>>       amdgpu_bo_unreserve(fpriv->vm.root.bo);
>>> -    mutex_destroy(&queue->fence_drv_lock);
>>>   free_fence_drv:
>>>       amdgpu_userq_fence_driver_free(queue);
>>>   free_queue: