Re: [PATCH] drm/amdgpu: remove va cursors for all mappings

[Khatri, Sunil]

On 13-05-2026 12:51 pm, Christian König wrote:
On 5/12/26 21:30, Sunil Khatri wrote:
va_cursor struct needs to be cleaned even if the mapping
has been removed already.

Also simplify it by make it a void function as return value
check isn't needed as its called during tear down.

Signed-off-by: Sunil Khatri <sunil.kha...@amd.com>
That patch here is Reviewed-by: Christian König <christian.koe...@amd.com> for 
now since it is clearly fixing an issue.

But of hand the userq_va_cursor design looks like it could be improved.

First of all bo_va->userq_va_mapped shouldn't be an atomic, but rather just a 
flag/boolean.

Then second we should never set this flag back to false since it can be that 
multiple queues refer the same buffer.

Not sure if we should fix those issues in that patch here or just commit this 
patch alone.
I can get this patch in and work on the issues that you highlighted in 
subsequent patches.

Regards
Sunil khatri

Thanks,
Christian.

---
  drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c | 17 +++++++----------
  1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
index f62163917f70..e325c7a350f9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
@@ -303,13 +303,14 @@ 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)
  {
-       atomic_set(&mapping->bo_va->userq_va_mapped, 0);
+       if (mapping)
+               atomic_set(&mapping->bo_va->userq_va_mapped, 0);
        list_del(&va_cursor->list);
        kfree(va_cursor);
  }
  
-static int amdgpu_userq_buffer_vas_list_cleanup(struct amdgpu_device *adev,
-                                               struct amdgpu_usermode_queue 
*queue)
+static void amdgpu_userq_buffer_vas_list_cleanup(struct amdgpu_device *adev,
+                                                struct amdgpu_usermode_queue 
*queue)
  {
        struct amdgpu_userq_va_cursor *va_cursor, *tmp;
        struct amdgpu_bo_va_mapping *mapping;
@@ -319,15 +320,11 @@ static int amdgpu_userq_buffer_vas_list_cleanup(struct 
amdgpu_device *adev,
  
  	list_for_each_entry_safe(va_cursor, tmp, &queue->userq_va_list, list) {
                mapping = amdgpu_vm_bo_lookup_mapping(queue->vm, 
va_cursor->gpu_addr);
-               if (!mapping) {
-                       return -EINVAL;
-               }
-               dev_dbg(adev->dev, "delete the userq:%p va:%llx\n",
-                       queue, va_cursor->gpu_addr);
+               if (mapping)
+                       dev_dbg(adev->dev, "delete the userq:%p va:%llx\n",
+                               queue, va_cursor->gpu_addr);
                amdgpu_userq_buffer_va_list_del(mapping, va_cursor);
        }
-
-       return 0;
  }
  
  static int amdgpu_userq_preempt_helper(struct amdgpu_usermode_queue *queue)