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 <[email protected]> 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/[email protected]?part=1
