On 9/9/2026 8:30 AM, SRINIVASAN SHANMUGAM wrote:
On 9/9/2026 1:46 AM, Matthew Brost wrote:
On Tue, Sep 08, 2026 at 08:58:39AM +0000, [email protected] wrote:
All of this looks right. So I believe the solution is:
- s/cancel_work_/disable_work_
- In drm_work_fence_queue if queue_work returns false, call
drm_work_fence_put()
Matt
Agreed — the race is real. Will fix in Patch 1
as you suggested:
- In drm_work_fence_queue: drop ref if queue_work returns false
- In drm_work_fence_cancel_sync: switch to disable_work_sync()
Hi Matt,
Sashiko flagged two more issues on v9 Patch 1 [1][2]:
In v9 we applied suggested fix:
- drm_work_fence_queue: drop ref if queue_work() returns false
- drm_work_fence_cancel_sync: switched to disable_work_sync()
However, Sashiko found two remaining problems:
1. ops->destroy() called in IRQ context:
When the GPU fence signals, the callback runs. If queue_work()
returns false (work already done or disabled), we call
drm_work_fence_put() immediately — but we are inside an interrupt
with IRQs disabled. If that drops the last ref, ops->destroy()
runs in that interrupt context, which is not safe for drivers that
sleep in destroy.
Fix: instead of calling drm_work_fence_put() directly, post a
small cleanup task via schedule_work() on system_wq. This ensures
destroy() always runs in process context where sleeping is allowed.
2. disable_work_sync() still has a timing gap:
The callback does two things in sequence:
a. Removes itself from the fence list
b. Calls queue_work()
If driver teardown starts between (a) and (b), disable_work_sync()
sees no pending work and returns immediately — teardown proceeds.
The callback then calls queue_work() on an already-destroyed
workqueue, causing a use-after-free.
Fix: add an atomic flag. The callback sets the flag ON before (b)
and OFF after. cancel_sync() waits until the flag is OFF before
returning. This closes the gap between (a) and (b).
Are these approaches acceptable, or may I kno pls if do you have a
preferred solution?
Also for Patch 3 — xe_sync_ufence_cancel() is now unused after v9
removed its only caller. Will remove it in v10.
[1] v9 Patch 1:
https://patchwork.freedesktop.org/patch/751979/?series=173582&rev=2
[2] Sashiko v9 review:
https://sashiko.dev/#/patchset/20260909044454.399340-1-srinivasan.shanmugam%40amd.com
Thanks,
Srini
