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 <[email protected]>
That patch here is Reviewed-by: Christian König <[email protected]> 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. 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)
