On Wed, 2026-06-24 at 13:13 +0200, 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.
In this [1] recent discussion with Tvrtko I point out that we (i.e., DRM) would really greatly benefit from more verbose commit messages. What is completely natural and self-explanatory to us might be far less intuitive for those coming in 5 years. You and I, we understand that dma_fence_is_signaled() can actively poll a fence. I still think it is not intuitive, but it's a complex API. And you personally also understand why an opportunistic poll is not necessary for amdgpu. A new contributor, like a hacker who cares about gaming on AMD hardware, who wants to help improve your driver, does not know that. Someone git-blaming one of those lines in a few years would just see that a function was replaced with a function with a basically identical name, and would not understand why the change was made. The commit simply says "seems unnecessary". Why :) I think fleshing it out more in the classical pattern (description of current situation, description of the problem, then description of the action the patch takes) is indeed more work for the author (to whom it might appear like a waste of time to describe sth obvious), but would also greatly benefit you personally, since it could reduce the amount of times we have to tell folks "I don't know, there was a reason – go ask Christian" ;) Grüße, Philipp [1] https://lore.kernel.org/dri-devel/[email protected]/ > > Signed-off-by: Christian König <[email protected]> > --- > 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); > 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); > > 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); > }
