On 4/8/26 09:45, Prike Liang wrote:
> amdgpu_userq_wait_ioctl() takes extra references on waited-on fence
> drivers and stores them in waitq->fence_drv_xa. When a new userq fence is
> created, those references are transferred into userq_fence->fence_drv_array
> so they can be released when the fence completes.
> 
> However, those inherited references are currently only dropped from
> amdgpu_userq_fence_driver_process(). If a fence never reaches that path,
> such as it is already signaled when created or it is dropped through
> an error/cleanup path, amdgpu_userq_fence_free() frees fence_drv_array
> without putting the referenced fence drivers.

Clear NAK to that as well.

An userq fence must be signaled at some point and when that happens the 
reference fence drivers can be put.

What could be is that we have another call to dma_fence_signal() where we 
forget to do that, but it should *never* be done in amdgpu_userq_fence_free().

Regards,
Christian.

> 
> Signed-off-by: Prike Liang <[email protected]>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c | 17 +++++++++++------
>  1 file changed, 11 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
> index 3be80a82788a..bd196599d3d6 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
> @@ -145,13 +145,21 @@ amdgpu_userq_fence_driver_free(struct 
> amdgpu_usermode_queue *userq)
>       amdgpu_userq_fence_driver_put(userq->fence_drv);
>  }
>  
> +static void
> +amdgpu_userq_fence_put_fence_drv_array(struct amdgpu_userq_fence 
> *userq_fence)
> +{
> +     unsigned long i;
> +     for (i = 0; i < userq_fence->fence_drv_array_count; i++)
> +             amdgpu_userq_fence_driver_put(userq_fence->fence_drv_array[i]);
> +     userq_fence->fence_drv_array_count = 0;
> +}
> +
>  void amdgpu_userq_fence_driver_process(struct amdgpu_userq_fence_driver 
> *fence_drv)
>  {
>       struct amdgpu_userq_fence *userq_fence, *tmp;
>       struct dma_fence *fence;
>       unsigned long flags;
>       u64 rptr;
> -     int i;
>  
>       if (!fence_drv)
>               return;
> @@ -166,10 +174,7 @@ void amdgpu_userq_fence_driver_process(struct 
> amdgpu_userq_fence_driver *fence_d
>                       break;
>  
>               dma_fence_signal(fence);
> -
> -             for (i = 0; i < userq_fence->fence_drv_array_count; i++)
> -                     
> amdgpu_userq_fence_driver_put(userq_fence->fence_drv_array[i]);
> -
> +             amdgpu_userq_fence_put_fence_drv_array(userq_fence);
>               list_del(&userq_fence->link);
>               dma_fence_put(fence);
>       }
> @@ -320,9 +325,9 @@ static void amdgpu_userq_fence_free(struct rcu_head *rcu)
>       struct amdgpu_userq_fence *userq_fence = to_amdgpu_userq_fence(fence);
>       struct amdgpu_userq_fence_driver *fence_drv = userq_fence->fence_drv;
>  
> +     amdgpu_userq_fence_put_fence_drv_array(userq_fence);
>       /* Release the fence driver reference */
>       amdgpu_userq_fence_driver_put(fence_drv);
> -
>       kvfree(userq_fence->fence_drv_array);
>       kmem_cache_free(amdgpu_userq_fence_slab, userq_fence);
>  }

Reply via email to