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?

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);

Reply via email to