Commit 1f32f310 authored by Christian König's avatar Christian König
Browse files

dma-buf: inline spinlock for fence protection v5 



Implement per-fence spinlocks, allowing implementations to not give an
external spinlock to protect the fence internal state. 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 lives 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
v5: improve comments

Signed-off-by: default avatarChristian König <christian.koenig@amd.com>
Reviewed-by: default avatarBoris Brezillon <boris.brezillon@collabora.com>
Link: https://lore.kernel.org/r/20260219160822.1529-5-christian.koenig@amd.com
parent 3e506793

drivers/dma-buf/dma-fence.c

+16 −5
Original line number Diff line number Diff line
@@ -343,7 +343,6 @@ void __dma_fence_might_wait(void) 
}

#endif





/**

 * dma_fence_signal_timestamp_locked - signal completion of a fence

 * @fence: the fence to signal
@@ -1070,7 +1069,6 @@ static void 
__dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops,

	         spinlock_t *lock, u64 context, u64 seqno, unsigned long flags)

{

	BUG_ON(!lock);

	BUG_ON(!ops || !ops->get_driver_name || !ops->get_timeline_name);



	kref_init(&fence->refcount);
@@ -1082,10 +1080,15 @@ __dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops, 
	 */

	RCU_INIT_POINTER(fence->ops, ops);

	INIT_LIST_HEAD(&fence->cb_list);

	fence->lock = lock;

	fence->context = context;

	fence->seqno = seqno;

	fence->flags = flags | BIT(DMA_FENCE_FLAG_INITIALIZED_BIT);

	if (lock) {

		fence->extern_lock = lock;

	} else {

		spin_lock_init(&fence->inline_lock);

		fence->flags |= BIT(DMA_FENCE_FLAG_INLINE_LOCK_BIT);

	}

	fence->error = 0;



	trace_dma_fence_init(fence);
@@ -1095,7 +1098,7 @@ __dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops, 
 * dma_fence_init - Initialize a custom fence.

 * @fence: the fence to initialize

 * @ops: the dma_fence_ops for operations on this fence

 * @lock: the irqsafe spinlock to use for locking this fence

 * @lock: optional irqsafe spinlock to use for locking this fence

 * @context: the execution context this fence is run on

 * @seqno: a linear increasing sequence number for this context

 *
@@ -1105,6 +1108,10 @@ __dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops, 
 *

 * context and seqno are used for easy comparison between fences, allowing

 * to check which fence is later by simply using dma_fence_later().

 *

 * It is strongly discouraged to provide an external lock because this couples

 * lock and fence life time. This is only allowed for legacy use cases when

 * multiple fences need to be prevented from signaling out of order.

 */

void

dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops,
@@ -1118,7 +1125,7 @@ EXPORT_SYMBOL(dma_fence_init); 
 * dma_fence_init64 - Initialize a custom fence with 64-bit seqno support.

 * @fence: the fence to initialize

 * @ops: the dma_fence_ops for operations on this fence

 * @lock: the irqsafe spinlock to use for locking this fence

 * @lock: optional irqsafe spinlock to use for locking this fence

 * @context: the execution context this fence is run on

 * @seqno: a linear increasing sequence number for this context

 *
@@ -1128,6 +1135,10 @@ EXPORT_SYMBOL(dma_fence_init); 
 *

 * Context and seqno are used for easy comparison between fences, allowing

 * to check which fence is later by simply using dma_fence_later().

 *

 * It is strongly discouraged to provide an external lock because this couples

 * lock and fence life time. This is only allowed for legacy use cases when

 * multiple fences need to be prevented from signaling out of order.

 */

void

dma_fence_init64(struct dma_fence *fence, const struct dma_fence_ops *ops,

drivers/dma-buf/sync_debug.h

+1 −1
Original line number Diff line number Diff line
@@ -47,7 +47,7 @@ struct sync_timeline { 


static inline struct sync_timeline *dma_fence_parent(struct dma_fence *fence)

{

	return container_of(fence->lock, struct sync_timeline, lock);

	return container_of(fence->extern_lock, struct sync_timeline, lock);

}



/**

drivers/gpu/drm/drm_crtc.c

+1 −1
Original line number Diff line number Diff line
@@ -159,7 +159,7 @@ static const struct dma_fence_ops drm_crtc_fence_ops; 
static struct drm_crtc *fence_to_crtc(struct dma_fence *fence)

{

	BUG_ON(rcu_access_pointer(fence->ops) != &drm_crtc_fence_ops);

	return container_of(fence->lock, struct drm_crtc, fence_lock);

	return container_of(fence->extern_lock, struct drm_crtc, fence_lock);

}



static const char *drm_crtc_fence_get_driver_name(struct dma_fence *fence)

drivers/gpu/drm/drm_writeback.c

+1 −1
Original line number Diff line number Diff line
@@ -81,7 +81,7 @@ 
 *	From userspace, this property will always read as zero.

 */



#define fence_to_wb_connector(x) container_of(x->lock, \

#define fence_to_wb_connector(x) container_of(x->extern_lock, \

					      struct drm_writeback_connector, \

					      fence_lock)

drivers/gpu/drm/nouveau/nouveau_fence.c

+2 −1
Original line number Diff line number Diff line
@@ -41,7 +41,8 @@ static const struct dma_fence_ops nouveau_fence_ops_legacy; 
static inline struct nouveau_fence_chan *

nouveau_fctx(struct nouveau_fence *fence)

{

	return container_of(fence->base.lock, struct nouveau_fence_chan, lock);

	return container_of(fence->base.extern_lock, struct nouveau_fence_chan,

			    lock);

}



static bool