On 5/15/25 11:44, Jesse Zhang wrote:
> The original change "drm/amdgpu: promote the implicit sync to the dependent
> read fences"
> (commit 714bbbf20a72) modified fence synchronization to use
> DMA_RESV_USAGE_READ
> instead of DMA_RESV_USAGE_BOOKKEEP. However, the user queue wait ioctl wasn't
> fully
> updated to account for this change, leading to potential synchronization
> issues.
NAK, that is superfluous.
BOOKKEEP fence should never be included here.
Regards,
Christian.
>
> This commit fixes the fence counting logic to properly account for both:
> 1. READ/WRITE fences (for normal synchronization)
> 2. BOOKKEEP fences (for eviction synchronization)
>
> The change ensures:
> - All relevant fences are properly counted before allocation
>
> Fixes: 714bbbf20a72 ("drm/amdgpu: promote the implicit sync to the dependent
> read fences")
>
> Signed-off-by: Jesse Zhang <jesse.zh...@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c | 14 +++++++++++++-
> 1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
> index 029cb24c28b3..8c754474882f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
> @@ -735,10 +735,18 @@ int amdgpu_userq_wait_ioctl(struct drm_device *dev,
> void *data,
> for (i = 0; i < num_read_bo_handles; i++) {
> struct dma_resv_iter resv_cursor;
> struct dma_fence *fence;
> -
> + /*
> + * We must count both READ/WRITE and BOOKKEEP fences
> since:
> + * - BOOKKEEP fences are used for eviction
> synchronization
> + * - The wait operation needs to synchronize with all
> fence types
> + */
> dma_resv_for_each_fence(&resv_cursor,
> gobj_read[i]->resv,
> DMA_RESV_USAGE_READ, fence)
> num_fences++;
> +
> + dma_resv_for_each_fence(&resv_cursor,
> gobj_read[i]->resv,
> + DMA_RESV_USAGE_BOOKKEEP, fence)
> + num_fences++;
> }
>
> for (i = 0; i < num_write_bo_handles; i++) {
> @@ -748,6 +756,10 @@ int amdgpu_userq_wait_ioctl(struct drm_device *dev, void
> *data,
> dma_resv_for_each_fence(&resv_cursor,
> gobj_write[i]->resv,
> DMA_RESV_USAGE_WRITE, fence)
> num_fences++;
> +
> + dma_resv_for_each_fence(&resv_cursor,
> gobj_write[i]->resv,
> + DMA_RESV_USAGE_BOOKKEEP, fence)
> + num_fences++;
> }
>
> /*
