On 1/20/26 12:41, Tvrtko Ursulin wrote:
On 20/01/2026 10:54, Christian König wrote:
Implement per-fence spinlocks, allowing implementations to not give an external spinlock to protect the fence internal statei. Instead a spinlock embedded into the fence structure itself is used in this case.
Shared spinlocks have the problem that implementations need to guarantee that the lock live at least as long all fences referencing them.
Using a per-fence spinlock allows completely decoupling spinlock producer and consumer life times, simplifying the handling in most use cases.
v2: improve naming, coverage and function documentation v3: fix one additional locking in the selftests v4: separate out some changes to make the patch smaller, fix one amdgpu crash found by CI systems
Signed-off-by: Christian König christian.koenig@amd.com
drivers/dma-buf/dma-fence.c | 25 +++++++++++++++++------- drivers/dma-buf/sync_debug.h | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 2 +- drivers/gpu/drm/drm_crtc.c | 2 +- drivers/gpu/drm/drm_writeback.c | 2 +- drivers/gpu/drm/nouveau/nouveau_fence.c | 3 ++- drivers/gpu/drm/qxl/qxl_release.c | 3 ++- drivers/gpu/drm/vmwgfx/vmwgfx_fence.c | 3 ++- drivers/gpu/drm/xe/xe_hw_fence.c | 3 ++-
i915 needed changes too, based on the kbuild report.
Going to take a look now.
Have you seen my note about the RCU sparse warning as well?
Nope, I must have missed that mail.
...
+/**
- dma_fence_spinlock - return pointer to the spinlock protecting the fence
- @fence: the fence to get the lock from
- Return either the pointer to the embedded or the external spin lock.
- */
+static inline spinlock_t *dma_fence_spinlock(struct dma_fence *fence) +{ + return test_bit(DMA_FENCE_FLAG_INLINE_LOCK_BIT, &fence->flags) ? + &fence->inline_lock : fence->extern_lock; +}
You did not want to move this helper into "dma-buf: abstract fence locking" ?
I was avoiding that to keep the pre-requisite patch smaller, cause this change here seemed independent to that.
But thinking about it I could make a third patch which introduces dma_fence_spinlock() and changes all the container_of uses.
I think that would have been better to keep everything mechanical in one patch, and then this patch which changes behaviour does not touch any drivers but only dma-fence core.
Also, what about adding something like dma_fence_container_of() in that patch as well?
I would rather like to avoid that. Using the spinlock pointer with container_of seemed to be a bit of a hack to me in the first place and I don't want to encourage people to do that in new code as well.
Regards, Christian.
Regards,
Tvrtko
/** * dma_fence_lock_irqsave - irqsave lock the fence * @fence: the fence to lock @@ -385,7 +403,7 @@ dma_fence_get_rcu_safe(struct dma_fence __rcu **fencep) * Lock the fence, preventing it from changing to the signaled state. */ #define dma_fence_lock_irqsave(fence, flags) \ - spin_lock_irqsave(fence->lock, flags) + spin_lock_irqsave(dma_fence_spinlock(fence), flags) /** * dma_fence_unlock_irqrestore - unlock the fence and irqrestore @@ -395,7 +413,7 @@ dma_fence_get_rcu_safe(struct dma_fence __rcu **fencep) * Unlock the fence, allowing it to change it's state to signaled again. */ #define dma_fence_unlock_irqrestore(fence, flags) \ - spin_unlock_irqrestore(fence->lock, flags) + spin_unlock_irqrestore(dma_fence_spinlock(fence), flags) #ifdef CONFIG_LOCKDEP bool dma_fence_begin_signalling(void);