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 <[email protected]> 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 <[email protected]>
>>> ---
>>> 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:
