On 5/18/26 16:57, Khatri, Sunil wrote:
> On 18-05-2026 08:25 pm, Christian König wrote:
>> On 5/13/26 10:14, Sunil Khatri wrote:
>>> Multiple queues needs these bo_va objects belonging to
>>> the same uq_mgr. So once they are mapped lets not unmap
>>> them as at any point of time any of the queues might be
>>> using it.
>>>
>>> Also userq_va_mapped should be a boolean than atomic.
>>>
>>> Signed-off-by: Sunil Khatri <[email protected]>
>> Reviewed-by: Christian König <[email protected]> for this one here,
>> but I think we also need some follow up cleanup.
>>
>> What is the userq_va_cursor actually used for?
> It holds the gpu address and the membership to the list. we just add the gpu
> address from the mapping of the buffers of interest from queue like wptr rptr
> etc.
> struct amdgpu_userq_va_cursor {
> u64 gpu_addr;
> struct list_head list;
> };
Mhm, that's a bit overkill. We could use an array instead since the number of
VA addresses is pretty fixed.
But yeah something for the nice to have list.
Thanks,
Christian.
>
> Regards
> Sunil Khatri
>>
>> Regards,
>> Christian.
>>
>>> ---
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 3 ++-
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c | 6 ++----
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 2 +-
>>> 3 files changed, 5 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
>>> index 912c9afaf9e1..4d68732d6223 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
>>> @@ -96,7 +96,8 @@ struct amdgpu_bo_va {
>>> * if non-zero, cannot unmap from GPU because user queues may still
>>> access it
>>> */
>>> unsigned int queue_refcount;
>>> - atomic_t userq_va_mapped;
>>> + /* Indicates if this buffer is mapped for any user queue. Once set,
>>> never reset. */
>>> + bool userq_va_mapped;
>>> };
>>> struct amdgpu_bo {
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
>>> index 24b172a0d9ac..9225b3795e74 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
>>> @@ -227,7 +227,7 @@ static int amdgpu_userq_buffer_va_list_add(struct
>>> amdgpu_usermode_queue *queue,
>>> INIT_LIST_HEAD(&va_cursor->list);
>>> va_cursor->gpu_addr = addr;
>>> - atomic_set(&va_map->bo_va->userq_va_mapped, 1);
>>> + va_map->bo_va->userq_va_mapped = true;
>>> list_add(&va_cursor->list, &queue->userq_va_list);
>>> return 0;
>>> @@ -274,7 +274,7 @@ static bool amdgpu_userq_buffer_va_mapped(struct
>>> amdgpu_vm *vm, u64 addr)
>>> dma_resv_assert_held(vm->root.bo->tbo.base.resv);
>>> mapping = amdgpu_vm_bo_lookup_mapping(vm, addr);
>>> - if (!IS_ERR_OR_NULL(mapping) &&
>>> atomic_read(&mapping->bo_va->userq_va_mapped))
>>> + if (!IS_ERR_OR_NULL(mapping) && mapping->bo_va->userq_va_mapped)
>>> r = true;
>>> else
>>> r = false;
>>> @@ -303,8 +303,6 @@ static bool amdgpu_userq_buffer_vas_mapped(struct
>>> amdgpu_usermode_queue *queue)
>>> static void amdgpu_userq_buffer_va_list_del(struct amdgpu_bo_va_mapping
>>> *mapping,
>>> struct amdgpu_userq_va_cursor *va_cursor)
>>> {
>>> - if (mapping)
>>> - atomic_set(&mapping->bo_va->userq_va_mapped, 0);
>>> list_del(&va_cursor->list);
>>> kfree(va_cursor);
>>> }
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> index 82a1c19350ee..47c531ffc065 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> @@ -2002,7 +2002,7 @@ int amdgpu_vm_bo_unmap(struct amdgpu_device *adev,
>>> * during user requests GEM unmap IOCTL except for forcing the unmap
>>> * from user space.
>>> */
>>> - if (unlikely(atomic_read(&bo_va->userq_va_mapped) > 0))
>>> + if (unlikely(bo_va->userq_va_mapped > 0))
>>> amdgpu_userq_gem_va_unmap_validate(adev, mapping, saddr);
>>> list_del(&mapping->list);
