[PATCH] drm/radeon: restore hardware polling in fence_is_signaled to fix performance regression

[PATCH] drm/radeon: restore hardware polling in fence_is_signaled to fix performance regression

[2564278112]

From: Wang Jiang <jiangwang@kylinos.cn>

Commit 9eb00b5f5697b ("drm/radeon: delete radeon_fence_process in
is_signaled, no deadlock") removed the hardware polling from
radeon_fence_is_signaled() to fix a self-deadlock caused by
wake_up_all(&rdev->fence_queue) being called with the fence queue
lock held.

However, removing the polling entirely causes significant performance
regression (e.g. glxgears FPS drop) because the fence signaled check
becomes purely passive — it only reads the cached last_seq without
probing the GPU, so completed GPU work is not detected in time,
causing unnecessary CPU stalls in sync-heavy workloads.

Fix this by calling radeon_fence_activity() directly instead of
radeon_fence_process(). radeon_fence_activity() reads the hardware
fence counter and updates last_seq via atomic ops without calling
wake_up_all(), thus avoiding the deadlock while restoring timely
fence detection.

Fixes: 9eb00b5f5697b ("drm/radeon: delete radeon_fence_process in is_signaled, no deadlock")
Signed-off-by: Wang Jiang <jiangwang@kylinos.cn>
---
 drivers/gpu/drm/radeon/radeon_fence.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/radeon/radeon_fence.c b/drivers/gpu/drm/radeon/radeon_fence.c
index 02a40e4750c7..299865569252 100644
--- a/drivers/gpu/drm/radeon/radeon_fence.c
+++ b/drivers/gpu/drm/radeon/radeon_fence.c
@@ -360,6 +360,13 @@ static bool radeon_fence_is_signaled(struct dma_fence *f)
 	if (atomic64_read(&rdev->fence_drv[ring].last_seq) >= seq)
 		return true;
 
+	if (down_read_trylock(&rdev->exclusive_lock)) {
+		radeon_fence_activity(rdev, ring);
+		up_read(&rdev->exclusive_lock);
+
+		if (atomic64_read(&rdev->fence_drv[ring].last_seq) >= seq)
+			return true;
+	}
 	return false;
 }
 
-- 
2.25.1

Re: [PATCH] drm/radeon: restore hardware polling in fence_is_signaled to fix performance regression

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Calling `radeon_fence_activity()` in `.signaled` callback consumes hardware state without waking up wait queues, leading to permanent hangs.
- [Medium] Acquiring an `rw_semaphore` in the `dma_fence_ops.signaled` callback violates wait-context rules and can crash PREEMPT_RT kernels.
--

--- Patch [1]: [PATCH] drm/radeon: restore hardware polling in fence_is_signaled to fix performance regression ---
commit 23682e8220434ea07b863fe650003e08c35ce798
Author: Wang Jiang <jiangwang@kylinos.cn>

drm/radeon: restore hardware polling in fence_is_signaled to fix performance regression

This commit restores hardware polling by calling radeon_fence_activity()
wrapped in down_read_trylock() inside radeon_fence_is_signaled(). This is
intended to fix performance regressions without re-introducing deadlocks.

> diff --git a/drivers/gpu/drm/radeon/radeon_fence.c b/drivers/gpu/drm/radeon/radeon_fence.c
> index 02a40e4750c7d..299865569252d 100644
> --- a/drivers/gpu/drm/radeon/radeon_fence.c
> +++ b/drivers/gpu/drm/radeon/radeon_fence.c
> @@ -360,6 +360,13 @@ static bool radeon_fence_is_signaled(struct dma_fence *f)
>  	if (atomic64_read(&rdev->fence_drv[ring].last_seq) >= seq)
>  		return true;
>  
> +	if (down_read_trylock(&rdev->exclusive_lock)) {
> +		radeon_fence_activity(rdev, ring);

[Severity: High]
Does this consume the hardware state and drop the wake signal?

When a thread calls dma_fence_is_signaled() via polling, it invokes
radeon_fence_is_signaled(), which then calls radeon_fence_activity(). 
The hardware sequence is read, last_seq is advanced, and it returns true.

However, radeon_fence_is_signaled() intentionally drops this return value and
skips wake_up_all(). Later, when the actual hardware interrupt fires,
radeon_fence_process() calls radeon_fence_activity() again.

Because last_seq was already advanced, it sees no new activity
(seq <= last_seq), returns false, and skips wake_up_all().

Could this steal the wake event and cause tasks waiting on fences to hang
indefinitely?

> +		up_read(&rdev->exclusive_lock);

[Severity: Medium]
Is it safe to manipulate an rw_semaphore in the dma_fence_ops.signaled
callback?

The dma_fence_ops.signaled callback can be called from atomic context or
directly from a foreign device's hardirq handler due to cross-device
dependencies.

While down_read_trylock() avoids sleeping on acquire, up_read() still
manipulates the underlying lock state. On PREEMPT_RT, this is an rt_mutex,
and manipulating it from hardirq context violates RT constraints, potentially
leading to a crash.

> +
> +		if (atomic64_read(&rdev->fence_drv[ring].last_seq) >= seq)
> +			return true;
> +	}
>  	return false;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/tencent_BB2A7CAE632BA0AC08D9296E78A15786200A@qq.com?part=1