[Linaro-mm-sig] Re: [PATCH 03/10] drm/amdgpu: use dma_fence_test_signaled_flag()

On 24/06/2026 12:13, Christian König wrote:

Instead of dma_fence_is_signaled_locked() use dma_fence_test_signaled_flag().

The extra polling check seems unecessary for those use cases.

Signed-off-by: Christian König christian.koenig@amd.com

drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c | 8 ++++---- drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c index ea69b1bac7c6..1192b9800ff2 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c @@ -652,7 +652,7 @@ void amdgpu_fence_driver_set_error(struct amdgpu_ring *ring, int error) fence = rcu_dereference_protected(drv->fences[i], lockdep_is_held(&drv->lock));

• if (fence && !dma_fence_is_signaled_locked(fence))
  

• if (fence && !dma_fence_test_signaled_flag(fence))
  dma_fence_set_error(fence, error);
  

  } spin_unlock_irqrestore(&drv->lock, flags);

@@ -677,7 +677,7 @@ void amdgpu_fence_driver_force_completion(struct amdgpu_ring *ring, fence = rcu_dereference_protected(drv->fences[i], lockdep_is_held(&drv->lock));

• if (fence && !dma_fence_is_signaled_locked(fence)) {
  

• if (fence && !dma_fence_test_signaled_flag(fence)) {
  if (fence == timedout_fence)
  	dma_fence_set_error(fence, -ETIME);
  else
  

@@ -738,7 +738,7 @@ void amdgpu_ring_set_fence_errors_and_reemit(struct amdgpu_ring *ring, rcu_read_lock(); unprocessed = rcu_dereference(*ptr);

• if (unprocessed && !dma_fence_is_signaled_locked(unprocessed)) {
  

• if (unprocessed && !dma_fence_test_signaled_flag(unprocessed)) {
  fence = container_of(unprocessed, struct amdgpu_fence, base);
  is_guilty_fence = fence == guilty_fence;
  is_guilty_context = fence->context == guilty_fence->context;
  

@@ -802,7 +802,7 @@ void amdgpu_ring_backup_unprocessed_commands(struct amdgpu_ring *ring, rcu_read_lock(); unprocessed = rcu_dereference(*ptr);

• if (unprocessed && !dma_fence_is_signaled(unprocessed)) {
  

• if (unprocessed && !dma_fence_test_signaled_flag(unprocessed)) {
  fence = container_of(unprocessed, struct amdgpu_fence, base);
  

amdgpu_ring_backup_unprocessed_command(ring, fence);

No fence->ops->signaled so all good.

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c index d6bee5c30073..ae9d6a2eefab 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c @@ -460,7 +460,7 @@ bool amdgpu_ring_soft_recovery(struct amdgpu_ring *ring, unsigned int vmid, return false; dma_fence_lock_irqsave(fence, flags);

• if (!dma_fence_is_signaled_locked(fence))

• if (!dma_fence_test_signaled_flag(fence)) dma_fence_set_error(fence, -ENODATA); dma_fence_unlock_irqrestore(fence, flags);

A bit of an odd function? With this change it becomes:

dma_fence_lock_irqsave(fence, flags); if (!dma_fence_test_signaled_flag(fence)) dma_fence_set_error(fence, -ENODATA); dma_fence_unlock_irqrestore(fence, flags);

while (!dma_fence_is_signaled(fence) && ktime_to_ns(ktime_sub(deadline, ktime_get())) > 0) ring->funcs->soft_recovery(ring, vmid);

ret = dma_fence_is_signaled(fence);

Does this flow make sense?

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c index a41fb72dba94..2cc6552a6399 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c @@ -426,7 +426,7 @@ amdgpu_userq_fence_driver_set_error(struct amdgpu_userq_fence *fence, f = rcu_dereference_protected(&fence->base, lockdep_is_held(&fence_drv->fence_list_lock));

• if (f && !dma_fence_is_signaled_locked(f))

• if (f && !dma_fence_test_signaled_flag(f)) dma_fence_set_error(f, error); spin_unlock_irqrestore(&fence_drv->fence_list_lock, flags); }

amdgpu_userq_fence_signaled does exists so is there any scope for this to open a false positive window?

Regards,

Tvrtko