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

[PATCH v2] 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 status check 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 hardware poll entirely causes significant
performance regression (e.g. glxgears FPS drop) because the 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_read() directly in
radeon_fence_is_signaled() to poll the hardware fence counter
without updating last_seq or calling wake_up_all(). This restores
timely fence detection while avoiding the original deadlock, since
last_seq update and waiter wakeup remain solely in the interrupt
handler's responsibility.

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 | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/gpu/drm/radeon/radeon_fence.c b/drivers/gpu/drm/radeon/radeon_fence.c
index 02a40e4750c7..047ec5a1d194 100644
--- a/drivers/gpu/drm/radeon/radeon_fence.c
+++ b/drivers/gpu/drm/radeon/radeon_fence.c
@@ -360,6 +360,18 @@ static bool radeon_fence_is_signaled(struct dma_fence *f)
 	if (atomic64_read(&rdev->fence_drv[ring].last_seq) >= seq)
 		return true;
 
+	/*
+	* Poll hardware directly without updating last_seq.
+	* This allows immediate detection of signaled fences (performance)
+	* while leaving last_seq update and wake_up_all() to the interrupt
+	* handler, avoiding:
+	* 1. Deadlock from calling wake_up_all() with fence lock held
+	* 2. Wake event stealing by advancing last_seq without waking waiters
+	* 3. rw_semaphore usage in atomic/irq context (unsafe on PREEMPT_RT)
+	*/
+	if (radeon_fence_read(rdev, ring) >= (u32)seq)
+			return true;
+
 	return false;
 }
 
-- 
2.25.1

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] Direct unsigned comparison of a 32-bit hardware sequence counter against a truncated 64-bit sequence number fails at 32-bit wrap-around boundaries, causing premature fence signaling and memory corruption.
--

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

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

This commit restores hardware polling in radeon_fence_is_signaled() by reading
the hardware fence counter directly. It avoids updating last_seq and calling
wake_up_all(), relying on the interrupt handler for those actions, which
prevents the deadlock that led to the earlier removal of the hardware status
check.

> diff --git a/drivers/gpu/drm/radeon/radeon_fence.c b/drivers/gpu/drm/radeon/radeon_fence.c
> index 02a40e4750c7d..047ec5a1d194b 100644
> --- a/drivers/gpu/drm/radeon/radeon_fence.c
> +++ b/drivers/gpu/drm/radeon/radeon_fence.c
> @@ -360,6 +360,18 @@ static bool radeon_fence_is_signaled(struct dma_fence *f)
>  	if (atomic64_read(&rdev->fence_drv[ring].last_seq) >= seq)
>  		return true;
>  
> +	/*
> +	* Poll hardware directly without updating last_seq.
> +	* This allows immediate detection of signaled fences (performance)
> +	* while leaving last_seq update and wake_up_all() to the interrupt
> +	* handler, avoiding:
> +	* 1. Deadlock from calling wake_up_all() with fence lock held
> +	* 2. Wake event stealing by advancing last_seq without waking waiters
> +	* 3. rw_semaphore usage in atomic/irq context (unsafe on PREEMPT_RT)
> +	*/
> +	if (radeon_fence_read(rdev, ring) >= (u32)seq)
> +			return true;

[Severity: Critical]
Will this comparison evaluate incorrectly during a 32-bit wrap-around?

In radeon_fence_is_signaled(), seq is a 64-bit value. If a userspace
application submits enough work to cross a 32-bit boundary, seq could reach a
value like 0x100000005, which truncates to 5 when cast to u32.

If the hardware counter from radeon_fence_read() has not yet wrapped and
returns a value like 0xFFFFFFFA, the direct unsigned comparison evaluates
0xFFFFFFFA >= 5 as true.

Could this falsely signal that the fence is complete, leading to a
use-after-free regression if the CPU frees or reuses memory buffers while the
GPU is still actively accessing them?

> +
>  	return false;
>  }
>

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