[PATCH v4 0/9] Some (drm_sched_|dma_)fence lifetime issues

[PATCH v4 0/9] Some (drm_sched_|dma_)fence lifetime issues

[Tvrtko Ursulin]

Hi all,

tl;dr;
Xe and probably some other drivers can tear down the internal state referenced
by an exported sync_file fence which then causes a null pointer derefences on
accessing said fence.

IGT that exploits the problem:
https://patchwork.freedesktop.org/patch/642709/?series=146211&rev=2

It seems there is a consensus this is a known problem with the dma-fence design,
where internal state shouldn't really be accessed after the fence has been
signaled. However currently the code is mostly unaware of that hence the use-
after-free potential.

To fix it, between the option of adding more reference counting and trying to
"revoke" the fence, suggestion is to focus on the later.

Reference to the recent discussion:
https://lore.kernel.org/dri-devel/20250418164246.72426-1-tvrtko.ursulin@igalia.com/

This series therefore attempts to implement a solution along those lines.

Most of the description what and how can be found in:
 "dma-fence: Add safe access helpers and document the rules"

Other than that, the series starts with some cleanups, with the general goal of
hiding more of the dma-fence implementation details behind explicit API. This
means adding helpers for access to driver and timeline name, and also moving as
much as it is easily possible of driver allocated state into the fence object
itself. Because dma-fence is already reference counted, any state we can embed
automatically becomes safe.

Having said that, the series only addreses the runtime use-after-free scenarios,
such as the above explained situation with the xe driver. For now the module
unload problem is deliberately left for later. (Although again, some of the
early patches do make it safer, and will make future improvements easier due
fewer accesses to fence->ops.)

Final patch in the series is the one which makes xe compliant with the rules
and API proposed earlier in the series. It does so by ensuring there is at least
one RCU grace period between fences being signaled and driver allocated memory
accessible from xe fences getting freed. Which couples with the earlier (from
the series) refactors which added dma_fence_access_begin/end() protection to
the relevant call sites.

If this approach is acceptable the next steps will be to see if any other
drivers will need similar changes. And also to discuss whether we want to go a
step futher and later move to SRCU, so code would be protected against module
unload as well.

v2:
 * Dropped module unload handling.
 * Proposing real API instead of hacks.

v3:
 * Dropped the dma_fence_is_array|chain ops to flags conversion.
 * Dropped the i915 cleanup patch which is now independent.
 * Squashed dma-fence helpers with internal usage patches.
 * Restored missing hunk in "dma-fence: Use a flag for 64-bit seqnos".
 * Removed the AMDGPU_JOB_GET_TIMELINE_NAME macro.
 * Applied some r-b tags.

v4:
 * Tidied 64-bit seqno flags patch and fixed for amdgpu user queues which landed
   since.
 * Adjusted some dma-fence tracepoints to avoid asserts.
 * Protected tracepoints in dma_fence_wait_timeout() with the safe access
   annotations.
 * Dropped driver/timeline helper usage from amdgpu_trace.h.
 * Dropped signaled fence protection from i915 timeline name vfunc.

Tvrtko Ursulin (9):
  dma-fence: Change signature of __dma_fence_is_later
  dma-fence: Use a flag for 64-bit seqnos
  dma-fence: Add helpers for accessing driver and timeline name
  sync_file: Use dma-fence driver and timeline name helpers
  drm/i915: Use dma-fence driver and timeline name helpers
  dma-fence: Add safe access helpers and document the rules
  sync_file: Protect access to driver and timeline name
  drm/i915: Protect access to driver and timeline name
  drm/xe: Make dma-fences compliant with the safe access rules

 drivers/dma-buf/dma-fence-chain.c             |   7 +-
 drivers/dma-buf/dma-fence.c                   | 159 +++++++++++++++---
 drivers/dma-buf/sw_sync.c                     |   2 +-
 drivers/dma-buf/sync_file.c                   |  14 +-
 .../drm/amd/amdgpu/amdgpu_eviction_fence.c    |   7 +-
 .../gpu/drm/amd/amdgpu/amdgpu_userq_fence.c   |   5 +-
 .../gpu/drm/amd/amdgpu/amdgpu_vm_tlb_fence.c  |   5 +-
 drivers/gpu/drm/i915/gt/intel_gt_requests.c   |   6 +-
 drivers/gpu/drm/i915/i915_request.c           |  17 +-
 drivers/gpu/drm/i915/i915_sw_fence.c          |   6 +-
 drivers/gpu/drm/xe/xe_guc_exec_queue_types.h  |   2 +
 drivers/gpu/drm/xe/xe_guc_submit.c            |   7 +-
 drivers/gpu/drm/xe/xe_hw_fence.c              |   5 +-
 drivers/gpu/drm/xe/xe_sched_job.c             |  14 +-
 include/linux/dma-fence.h                     |  47 ++++--
 include/trace/events/dma_fence.h              |  38 ++++-
 16 files changed, 254 insertions(+), 87 deletions(-)

-- 
2.48.0

[PATCH v4 1/9] dma-fence: Change signature of __dma_fence_is_later

[Tvrtko Ursulin]

With the goal of reducing the need for drivers to touch (and dereference)
fence->ops, we change the prototype of __dma_fence_is_later() to take
fence instead of fence->ops.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Reviewed-by: Christian König <christian.koenig@amd.com>
---
 drivers/dma-buf/dma-fence-chain.c |  2 +-
 drivers/dma-buf/sw_sync.c         |  2 +-
 drivers/gpu/drm/xe/xe_hw_fence.c  |  2 +-
 drivers/gpu/drm/xe/xe_sched_job.c | 14 ++++++++------
 include/linux/dma-fence.h         |  9 ++++-----
 5 files changed, 15 insertions(+), 14 deletions(-)

diff --git a/drivers/dma-buf/dma-fence-chain.c b/drivers/dma-buf/dma-fence-chain.c
index 9663ba1bb6ac..90424f23fd73 100644
--- a/drivers/dma-buf/dma-fence-chain.c
+++ b/drivers/dma-buf/dma-fence-chain.c
@@ -252,7 +252,7 @@ void dma_fence_chain_init(struct dma_fence_chain *chain,
 	chain->prev_seqno = 0;
 
 	/* Try to reuse the context of the previous chain node. */
-	if (prev_chain && __dma_fence_is_later(seqno, prev->seqno, prev->ops)) {
+	if (prev_chain && __dma_fence_is_later(prev, seqno, prev->seqno)) {
 		context = prev->context;
 		chain->prev_seqno = prev->seqno;
 	} else {
diff --git a/drivers/dma-buf/sw_sync.c b/drivers/dma-buf/sw_sync.c
index 4f27ee93a00c..3c20f1d31cf5 100644
--- a/drivers/dma-buf/sw_sync.c
+++ b/drivers/dma-buf/sw_sync.c
@@ -170,7 +170,7 @@ static bool timeline_fence_signaled(struct dma_fence *fence)
 {
 	struct sync_timeline *parent = dma_fence_parent(fence);
 
-	return !__dma_fence_is_later(fence->seqno, parent->value, fence->ops);
+	return !__dma_fence_is_later(fence, fence->seqno, parent->value);
 }
 
 static void timeline_fence_set_deadline(struct dma_fence *fence, ktime_t deadline)
diff --git a/drivers/gpu/drm/xe/xe_hw_fence.c b/drivers/gpu/drm/xe/xe_hw_fence.c
index 0b4f12be3692..03eb8c6d1616 100644
--- a/drivers/gpu/drm/xe/xe_hw_fence.c
+++ b/drivers/gpu/drm/xe/xe_hw_fence.c
@@ -165,7 +165,7 @@ static bool xe_hw_fence_signaled(struct dma_fence *dma_fence)
 	u32 seqno = xe_map_rd(xe, &fence->seqno_map, 0, u32);
 
 	return dma_fence->error ||
-		!__dma_fence_is_later(dma_fence->seqno, seqno, dma_fence->ops);
+		!__dma_fence_is_later(dma_fence, dma_fence->seqno, seqno);
 }
 
 static bool xe_hw_fence_enable_signaling(struct dma_fence *dma_fence)
diff --git a/drivers/gpu/drm/xe/xe_sched_job.c b/drivers/gpu/drm/xe/xe_sched_job.c
index 1905ca590965..f0a6ce610948 100644
--- a/drivers/gpu/drm/xe/xe_sched_job.c
+++ b/drivers/gpu/drm/xe/xe_sched_job.c
@@ -216,15 +216,17 @@ void xe_sched_job_set_error(struct xe_sched_job *job, int error)
 
 bool xe_sched_job_started(struct xe_sched_job *job)
 {
+	struct dma_fence *fence = dma_fence_chain_contained(job->fence);
 	struct xe_lrc *lrc = job->q->lrc[0];
 
-	return !__dma_fence_is_later(xe_sched_job_lrc_seqno(job),
-				     xe_lrc_start_seqno(lrc),
-				     dma_fence_chain_contained(job->fence)->ops);
+	return !__dma_fence_is_later(fence,
+				     xe_sched_job_lrc_seqno(job),
+				     xe_lrc_start_seqno(lrc));
 }
 
 bool xe_sched_job_completed(struct xe_sched_job *job)
 {
+	struct dma_fence *fence = dma_fence_chain_contained(job->fence);
 	struct xe_lrc *lrc = job->q->lrc[0];
 
 	/*
@@ -232,9 +234,9 @@ bool xe_sched_job_completed(struct xe_sched_job *job)
 	 * parallel handshake is done.
 	 */
 
-	return !__dma_fence_is_later(xe_sched_job_lrc_seqno(job),
-				     xe_lrc_seqno(lrc),
-				     dma_fence_chain_contained(job->fence)->ops);
+	return !__dma_fence_is_later(fence,
+				     xe_sched_job_lrc_seqno(job),
+				     xe_lrc_seqno(lrc));
 }
 
 void xe_sched_job_arm(struct xe_sched_job *job)
diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
index b12776883d14..48b5202c531d 100644
--- a/include/linux/dma-fence.h
+++ b/include/linux/dma-fence.h
@@ -441,21 +441,20 @@ dma_fence_is_signaled(struct dma_fence *fence)
 
 /**
  * __dma_fence_is_later - return if f1 is chronologically later than f2
+ * @fence: fence in whose context to do the comparison
  * @f1: the first fence's seqno
  * @f2: the second fence's seqno from the same context
- * @ops: dma_fence_ops associated with the seqno
  *
  * Returns true if f1 is chronologically later than f2. Both fences must be
  * from the same context, since a seqno is not common across contexts.
  */
-static inline bool __dma_fence_is_later(u64 f1, u64 f2,
-					const struct dma_fence_ops *ops)
+static inline bool __dma_fence_is_later(struct dma_fence *fence, u64 f1, u64 f2)
 {
 	/* This is for backward compatibility with drivers which can only handle
 	 * 32bit sequence numbers. Use a 64bit compare when the driver says to
 	 * do so.
 	 */
-	if (ops->use_64bit_seqno)
+	if (fence->ops->use_64bit_seqno)
 		return f1 > f2;
 
 	return (int)(lower_32_bits(f1) - lower_32_bits(f2)) > 0;
@@ -475,7 +474,7 @@ static inline bool dma_fence_is_later(struct dma_fence *f1,
 	if (WARN_ON(f1->context != f2->context))
 		return false;
 
-	return __dma_fence_is_later(f1->seqno, f2->seqno, f1->ops);
+	return __dma_fence_is_later(f1, f1->seqno, f2->seqno);
 }
 
 /**
-- 
2.48.0

[PATCH v4 2/9] dma-fence: Use a flag for 64-bit seqnos

[Tvrtko Ursulin]

With the goal of reducing the need for drivers to touch (and dereference)
fence->ops, we move the 64-bit seqnos flag from struct dma_fence_ops to
the fence->flags.

Drivers which were setting this flag are changed to use new
dma_fence_init64() instead of dma_fence_init().

v2:
 * Streamlined init and added kerneldoc.
 * Rebase for amdgpu userq which landed since.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Reviewed-by: Christian König <christian.koenig@amd.com> # v1
---
 drivers/dma-buf/dma-fence-chain.c             |  5 +-
 drivers/dma-buf/dma-fence.c                   | 69 ++++++++++++++-----
 .../drm/amd/amdgpu/amdgpu_eviction_fence.c    |  7 +-
 .../gpu/drm/amd/amdgpu/amdgpu_userq_fence.c   |  5 +-
 .../gpu/drm/amd/amdgpu/amdgpu_vm_tlb_fence.c  |  5 +-
 include/linux/dma-fence.h                     | 14 ++--
 6 files changed, 64 insertions(+), 41 deletions(-)

diff --git a/drivers/dma-buf/dma-fence-chain.c b/drivers/dma-buf/dma-fence-chain.c
index 90424f23fd73..a8a90acf4f34 100644
--- a/drivers/dma-buf/dma-fence-chain.c
+++ b/drivers/dma-buf/dma-fence-chain.c
@@ -218,7 +218,6 @@ static void dma_fence_chain_set_deadline(struct dma_fence *fence,
 }
 
 const struct dma_fence_ops dma_fence_chain_ops = {
-	.use_64bit_seqno = true,
 	.get_driver_name = dma_fence_chain_get_driver_name,
 	.get_timeline_name = dma_fence_chain_get_timeline_name,
 	.enable_signaling = dma_fence_chain_enable_signaling,
@@ -262,8 +261,8 @@ void dma_fence_chain_init(struct dma_fence_chain *chain,
 			seqno = max(prev->seqno, seqno);
 	}
 
-	dma_fence_init(&chain->base, &dma_fence_chain_ops,
-		       &chain->lock, context, seqno);
+	dma_fence_init64(&chain->base, &dma_fence_chain_ops, &chain->lock,
+			 context, seqno);
 
 	/*
 	 * Chaining dma_fence_chain container together is only allowed through
diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
index f0cdd3e99d36..705b59787731 100644
--- a/drivers/dma-buf/dma-fence.c
+++ b/drivers/dma-buf/dma-fence.c
@@ -989,24 +989,9 @@ void dma_fence_describe(struct dma_fence *fence, struct seq_file *seq)
 }
 EXPORT_SYMBOL(dma_fence_describe);
 
-/**
- * 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
- * @context: the execution context this fence is run on
- * @seqno: a linear increasing sequence number for this context
- *
- * Initializes an allocated fence, the caller doesn't have to keep its
- * refcount after committing with this fence, but it will need to hold a
- * refcount again if &dma_fence_ops.enable_signaling gets called.
- *
- * context and seqno are used for easy comparison between fences, allowing
- * to check which fence is later by simply using dma_fence_later().
- */
-void
-dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops,
-	       spinlock_t *lock, u64 context, u64 seqno)
+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);
@@ -1017,9 +1002,55 @@ dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops,
 	fence->lock = lock;
 	fence->context = context;
 	fence->seqno = seqno;
-	fence->flags = 0UL;
+	fence->flags = flags;
 	fence->error = 0;
 
 	trace_dma_fence_init(fence);
 }
+
+/**
+ * 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
+ * @context: the execution context this fence is run on
+ * @seqno: a linear increasing sequence number for this context
+ *
+ * Initializes an allocated fence, the caller doesn't have to keep its
+ * refcount after committing with this fence, but it will need to hold a
+ * refcount again if &dma_fence_ops.enable_signaling gets called.
+ *
+ * context and seqno are used for easy comparison between fences, allowing
+ * to check which fence is later by simply using dma_fence_later().
+ */
+void
+dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops,
+	       spinlock_t *lock, u64 context, u64 seqno)
+{
+	__dma_fence_init(fence, ops, lock, context, seqno, 0UL);
+}
 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
+ * @context: the execution context this fence is run on
+ * @seqno: a linear increasing sequence number for this context
+ *
+ * Initializes an allocated fence, the caller doesn't have to keep its
+ * refcount after committing with this fence, but it will need to hold a
+ * refcount again if &dma_fence_ops.enable_signaling gets called.
+ *
+ * Context and seqno are used for easy comparison between fences, allowing
+ * to check which fence is later by simply using dma_fence_later().
+ */
+void
+dma_fence_init64(struct dma_fence *fence, const struct dma_fence_ops *ops,
+		 spinlock_t *lock, u64 context, u64 seqno)
+{
+	__dma_fence_init(fence, ops, lock, context, seqno,
+			 BIT(DMA_FENCE_FLAG_SEQNO64_BIT));
+}
+EXPORT_SYMBOL(dma_fence_init64);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.c
index 1a7469543db5..79713421bffe 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.c
@@ -134,7 +134,6 @@ static bool amdgpu_eviction_fence_enable_signaling(struct dma_fence *f)
 }
 
 static const struct dma_fence_ops amdgpu_eviction_fence_ops = {
-	.use_64bit_seqno = true,
 	.get_driver_name = amdgpu_eviction_fence_get_driver_name,
 	.get_timeline_name = amdgpu_eviction_fence_get_timeline_name,
 	.enable_signaling = amdgpu_eviction_fence_enable_signaling,
@@ -160,9 +159,9 @@ amdgpu_eviction_fence_create(struct amdgpu_eviction_fence_mgr *evf_mgr)
 	ev_fence->evf_mgr = evf_mgr;
 	get_task_comm(ev_fence->timeline_name, current);
 	spin_lock_init(&ev_fence->lock);
-	dma_fence_init(&ev_fence->base, &amdgpu_eviction_fence_ops,
-		       &ev_fence->lock, evf_mgr->ev_fence_ctx,
-		       atomic_inc_return(&evf_mgr->ev_fence_seq));
+	dma_fence_init64(&ev_fence->base, &amdgpu_eviction_fence_ops,
+			 &ev_fence->lock, evf_mgr->ev_fence_ctx,
+			 atomic_inc_return(&evf_mgr->ev_fence_seq));
 	return ev_fence;
 }
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
index 029cb24c28b3..5e92d00a591f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
@@ -239,8 +239,8 @@ static int amdgpu_userq_fence_create(struct amdgpu_usermode_queue *userq,
 	fence = &userq_fence->base;
 	userq_fence->fence_drv = fence_drv;
 
-	dma_fence_init(fence, &amdgpu_userq_fence_ops, &userq_fence->lock,
-		       fence_drv->context, seq);
+	dma_fence_init64(fence, &amdgpu_userq_fence_ops, &userq_fence->lock,
+			 fence_drv->context, seq);
 
 	amdgpu_userq_fence_driver_get(fence_drv);
 	dma_fence_get(fence);
@@ -334,7 +334,6 @@ static void amdgpu_userq_fence_release(struct dma_fence *f)
 }
 
 static const struct dma_fence_ops amdgpu_userq_fence_ops = {
-	.use_64bit_seqno = true,
 	.get_driver_name = amdgpu_userq_fence_get_driver_name,
 	.get_timeline_name = amdgpu_userq_fence_get_timeline_name,
 	.signaled = amdgpu_userq_fence_signaled,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_tlb_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_tlb_fence.c
index 51cddfa3f1e8..5d26797356a3 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_tlb_fence.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_tlb_fence.c
@@ -71,7 +71,6 @@ static void amdgpu_tlb_fence_work(struct work_struct *work)
 }
 
 static const struct dma_fence_ops amdgpu_tlb_fence_ops = {
-	.use_64bit_seqno = true,
 	.get_driver_name = amdgpu_tlb_fence_get_driver_name,
 	.get_timeline_name = amdgpu_tlb_fence_get_timeline_name
 };
@@ -101,8 +100,8 @@ void amdgpu_vm_tlb_fence_create(struct amdgpu_device *adev, struct amdgpu_vm *vm
 	INIT_WORK(&f->work, amdgpu_tlb_fence_work);
 	spin_lock_init(&f->lock);
 
-	dma_fence_init(&f->base, &amdgpu_tlb_fence_ops, &f->lock,
-		       vm->tlb_fence_context, atomic64_read(&vm->tlb_seq));
+	dma_fence_init64(&f->base, &amdgpu_tlb_fence_ops, &f->lock,
+			 vm->tlb_fence_context, atomic64_read(&vm->tlb_seq));
 
 	/* TODO: We probably need a separate wq here */
 	dma_fence_get(&f->base);
diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
index 48b5202c531d..a34a0dcdc446 100644
--- a/include/linux/dma-fence.h
+++ b/include/linux/dma-fence.h
@@ -97,6 +97,7 @@ struct dma_fence {
 };
 
 enum dma_fence_flag_bits {
+	DMA_FENCE_FLAG_SEQNO64_BIT,
 	DMA_FENCE_FLAG_SIGNALED_BIT,
 	DMA_FENCE_FLAG_TIMESTAMP_BIT,
 	DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT,
@@ -124,14 +125,6 @@ struct dma_fence_cb {
  *
  */
 struct dma_fence_ops {
-	/**
-	 * @use_64bit_seqno:
-	 *
-	 * True if this dma_fence implementation uses 64bit seqno, false
-	 * otherwise.
-	 */
-	bool use_64bit_seqno;
-
 	/**
 	 * @get_driver_name:
 	 *
@@ -262,6 +255,9 @@ struct dma_fence_ops {
 void dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops,
 		    spinlock_t *lock, u64 context, u64 seqno);
 
+void dma_fence_init64(struct dma_fence *fence, const struct dma_fence_ops *ops,
+		      spinlock_t *lock, u64 context, u64 seqno);
+
 void dma_fence_release(struct kref *kref);
 void dma_fence_free(struct dma_fence *fence);
 void dma_fence_describe(struct dma_fence *fence, struct seq_file *seq);
@@ -454,7 +450,7 @@ static inline bool __dma_fence_is_later(struct dma_fence *fence, u64 f1, u64 f2)
 	 * 32bit sequence numbers. Use a 64bit compare when the driver says to
 	 * do so.
 	 */
-	if (fence->ops->use_64bit_seqno)
+	if (test_bit(DMA_FENCE_FLAG_SEQNO64_BIT, &fence->flags))
 		return f1 > f2;
 
 	return (int)(lower_32_bits(f1) - lower_32_bits(f2)) > 0;
-- 
2.48.0

[PATCH v4 3/9] dma-fence: Add helpers for accessing driver and timeline name

[Tvrtko Ursulin]

Add some helpers in order to enable preventing dma-fence users accessing
the implementation details directly and make the implementation itself use
them.

This will also enable later adding some asserts to a consolidated
location.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Reviewed-by: Christian König <christian.koenig@amd.com>
---
 drivers/dma-buf/dma-fence.c      |  9 +++++----
 include/linux/dma-fence.h        | 10 ++++++++++
 include/trace/events/dma_fence.h |  4 ++--
 3 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
index 705b59787731..74f9e4b665e3 100644
--- a/drivers/dma-buf/dma-fence.c
+++ b/drivers/dma-buf/dma-fence.c
@@ -538,8 +538,8 @@ void dma_fence_release(struct kref *kref)
 	if (WARN(!list_empty(&fence->cb_list) &&
 		 !test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags),
 		 "Fence %s:%s:%llx:%llx released with pending signals!\n",
-		 fence->ops->get_driver_name(fence),
-		 fence->ops->get_timeline_name(fence),
+		 dma_fence_driver_name(fence),
+		 dma_fence_timeline_name(fence),
 		 fence->context, fence->seqno)) {
 		unsigned long flags;
 
@@ -983,8 +983,9 @@ EXPORT_SYMBOL(dma_fence_set_deadline);
 void dma_fence_describe(struct dma_fence *fence, struct seq_file *seq)
 {
 	seq_printf(seq, "%s %s seq %llu %ssignalled\n",
-		   fence->ops->get_driver_name(fence),
-		   fence->ops->get_timeline_name(fence), fence->seqno,
+		   dma_fence_driver_name(fence),
+		   dma_fence_timeline_name(fence),
+		   fence->seqno,
 		   dma_fence_is_signaled(fence) ? "" : "un");
 }
 EXPORT_SYMBOL(dma_fence_describe);
diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
index a34a0dcdc446..c5ac37e10d85 100644
--- a/include/linux/dma-fence.h
+++ b/include/linux/dma-fence.h
@@ -377,6 +377,16 @@ bool dma_fence_remove_callback(struct dma_fence *fence,
 			       struct dma_fence_cb *cb);
 void dma_fence_enable_sw_signaling(struct dma_fence *fence);
 
+static inline const char *dma_fence_driver_name(struct dma_fence *fence)
+{
+	return fence->ops->get_driver_name(fence);
+}
+
+static inline const char *dma_fence_timeline_name(struct dma_fence *fence)
+{
+	return fence->ops->get_timeline_name(fence);
+}
+
 /**
  * dma_fence_is_signaled_locked - Return an indication if the fence
  *                                is signaled yet.
diff --git a/include/trace/events/dma_fence.h b/include/trace/events/dma_fence.h
index a4de3df8500b..84c83074ee81 100644
--- a/include/trace/events/dma_fence.h
+++ b/include/trace/events/dma_fence.h
@@ -16,8 +16,8 @@ DECLARE_EVENT_CLASS(dma_fence,
 	TP_ARGS(fence),
 
 	TP_STRUCT__entry(
-		__string(driver, fence->ops->get_driver_name(fence))
-		__string(timeline, fence->ops->get_timeline_name(fence))
+		__string(driver, dma_fence_driver_name(fence))
+		__string(timeline, dma_fence_timeline_name(fence))
 		__field(unsigned int, context)
 		__field(unsigned int, seqno)
 	),
-- 
2.48.0

[PATCH v4 4/9] sync_file: Use dma-fence driver and timeline name helpers

[Tvrtko Ursulin]

Access the dma-fence internals via the previously added helpers.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Reviewed-by: Christian König <christian.koenig@amd.com>
---
 drivers/dma-buf/sync_file.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/dma-buf/sync_file.c b/drivers/dma-buf/sync_file.c
index d9b1c1b2a72b..212df4b849fe 100644
--- a/drivers/dma-buf/sync_file.c
+++ b/drivers/dma-buf/sync_file.c
@@ -137,8 +137,8 @@ char *sync_file_get_name(struct sync_file *sync_file, char *buf, int len)
 		struct dma_fence *fence = sync_file->fence;
 
 		snprintf(buf, len, "%s-%s%llu-%lld",
-			 fence->ops->get_driver_name(fence),
-			 fence->ops->get_timeline_name(fence),
+			 dma_fence_driver_name(fence),
+			 dma_fence_timeline_name(fence),
 			 fence->context,
 			 fence->seqno);
 	}
@@ -262,9 +262,9 @@ static long sync_file_ioctl_merge(struct sync_file *sync_file,
 static int sync_fill_fence_info(struct dma_fence *fence,
 				 struct sync_fence_info *info)
 {
-	strscpy(info->obj_name, fence->ops->get_timeline_name(fence),
+	strscpy(info->obj_name, dma_fence_timeline_name(fence),
 		sizeof(info->obj_name));
-	strscpy(info->driver_name, fence->ops->get_driver_name(fence),
+	strscpy(info->driver_name, dma_fence_driver_name(fence),
 		sizeof(info->driver_name));
 
 	info->status = dma_fence_get_status(fence);
-- 
2.48.0

[PATCH v4 5/9] drm/i915: Use dma-fence driver and timeline name helpers

[Tvrtko Ursulin]

Access the dma-fence internals via the previously added helpers.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Reviewed-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/i915/gt/intel_gt_requests.c | 4 ++--
 drivers/gpu/drm/i915/i915_request.c         | 2 +-
 drivers/gpu/drm/i915/i915_sw_fence.c        | 4 ++--
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_gt_requests.c b/drivers/gpu/drm/i915/gt/intel_gt_requests.c
index d1a382dfaa1d..ae3557ed6c1e 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_requests.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt_requests.c
@@ -252,8 +252,8 @@ void intel_gt_watchdog_work(struct work_struct *work)
 			struct dma_fence *f = &rq->fence;
 
 			pr_notice("Fence expiration time out i915-%s:%s:%llx!\n",
-				  f->ops->get_driver_name(f),
-				  f->ops->get_timeline_name(f),
+				  dma_fence_driver_name(f),
+				  dma_fence_timeline_name(f),
 				  f->seqno);
 			i915_request_cancel(rq, -EINTR);
 		}
diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
index c3d27eadc0a7..4874c4f1e4ab 100644
--- a/drivers/gpu/drm/i915/i915_request.c
+++ b/drivers/gpu/drm/i915/i915_request.c
@@ -2184,7 +2184,7 @@ void i915_request_show(struct drm_printer *m,
 		       const char *prefix,
 		       int indent)
 {
-	const char *name = rq->fence.ops->get_timeline_name((struct dma_fence *)&rq->fence);
+	const char *name = dma_fence_timeline_name((struct dma_fence *)&rq->fence);
 	char buf[80] = "";
 	int x = 0;
 
diff --git a/drivers/gpu/drm/i915/i915_sw_fence.c b/drivers/gpu/drm/i915/i915_sw_fence.c
index 1d4cc91c0e40..e51ca7e50a4e 100644
--- a/drivers/gpu/drm/i915/i915_sw_fence.c
+++ b/drivers/gpu/drm/i915/i915_sw_fence.c
@@ -435,8 +435,8 @@ static void timer_i915_sw_fence_wake(struct timer_list *t)
 		return;
 
 	pr_notice("Asynchronous wait on fence %s:%s:%llx timed out (hint:%ps)\n",
-		  cb->dma->ops->get_driver_name(cb->dma),
-		  cb->dma->ops->get_timeline_name(cb->dma),
+		  dma_fence_driver_name(cb->dma),
+		  dma_fence_timeline_name(cb->dma),
 		  cb->dma->seqno,
 		  i915_sw_fence_debug_hint(fence));
 
-- 
2.48.0

[PATCH v4 6/9] dma-fence: Add safe access helpers and document the rules

[Tvrtko Ursulin]

Dma-fence objects currently suffer from a potential use after free problem
where fences exported to userspace and other drivers can outlive the
exporting driver, or the associated data structures.

The discussion on how to address this concluded that adding reference
counting to all the involved objects is not desirable, since it would need
to be very wide reaching and could cause unloadable drivers if another
entity would be holding onto a signaled fence reference potentially
indefinitely.

This patch enables the safe access by introducing and documenting a
contract between fence exporters and users. It documents a set of
contraints and adds helpers which a) drivers with potential to suffer from
the use after free must use and b) users of the dma-fence API must use as
well.

Premise of the design has multiple sides:

1. Drivers (fence exporters) MUST ensure a RCU grace period between
signalling a fence and freeing the driver private data associated with it.

The grace period does not have to follow the signalling immediately but
HAS to happen before data is freed.

2. Users of the dma-fence API marked with such requirement MUST contain
the complete access to the data within a single code block guarded by the
new dma_fence_access_begin() and dma_fence_access_end() helpers.

The combination of the two ensures that whoever sees the
DMA_FENCE_FLAG_SIGNALED_BIT not set is guaranteed to have access to a
valid fence->lock and valid data potentially accessed by the fence->ops
virtual functions, until the call to dma_fence_access_end().

3. Module unload (fence->ops) disappearing is for now explicitly not
handled. That would required a more complex protection, possibly needing
SRCU instead of RCU to handle callers such as dma_fence_wait_timeout(),
where race between dma_fence_enable_sw_signaling, signalling, and
dereference of fence->ops->wait() would need a sleeping SRCU context.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
---
 drivers/dma-buf/dma-fence.c      | 81 +++++++++++++++++++++++++++++++-
 include/linux/dma-fence.h        | 32 +++++++++----
 include/trace/events/dma_fence.h | 38 +++++++++++++--
 3 files changed, 137 insertions(+), 14 deletions(-)

diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
index 74f9e4b665e3..c467e9c013d1 100644
--- a/drivers/dma-buf/dma-fence.c
+++ b/drivers/dma-buf/dma-fence.c
@@ -511,12 +511,20 @@ dma_fence_wait_timeout(struct dma_fence *fence, bool intr, signed long timeout)
 
 	dma_fence_enable_sw_signaling(fence);
 
-	trace_dma_fence_wait_start(fence);
+	if (trace_dma_fence_wait_start_enabled()) {
+		dma_fence_access_begin();
+		trace_dma_fence_wait_start(fence);
+		dma_fence_access_end();
+	}
 	if (fence->ops->wait)
 		ret = fence->ops->wait(fence, intr, timeout);
 	else
 		ret = dma_fence_default_wait(fence, intr, timeout);
-	trace_dma_fence_wait_end(fence);
+	if (trace_dma_fence_wait_end_enabled()) {
+		dma_fence_access_begin();
+		trace_dma_fence_wait_end(fence);
+		dma_fence_access_end();
+	}
 	return ret;
 }
 EXPORT_SYMBOL(dma_fence_wait_timeout);
@@ -533,6 +541,7 @@ void dma_fence_release(struct kref *kref)
 	struct dma_fence *fence =
 		container_of(kref, struct dma_fence, refcount);
 
+	dma_fence_access_begin();
 	trace_dma_fence_destroy(fence);
 
 	if (WARN(!list_empty(&fence->cb_list) &&
@@ -560,6 +569,8 @@ void dma_fence_release(struct kref *kref)
 		fence->ops->release(fence);
 	else
 		dma_fence_free(fence);
+
+	dma_fence_access_end();
 }
 EXPORT_SYMBOL(dma_fence_release);
 
@@ -982,11 +993,13 @@ EXPORT_SYMBOL(dma_fence_set_deadline);
  */
 void dma_fence_describe(struct dma_fence *fence, struct seq_file *seq)
 {
+	dma_fence_access_begin();
 	seq_printf(seq, "%s %s seq %llu %ssignalled\n",
 		   dma_fence_driver_name(fence),
 		   dma_fence_timeline_name(fence),
 		   fence->seqno,
 		   dma_fence_is_signaled(fence) ? "" : "un");
+	dma_fence_access_end();
 }
 EXPORT_SYMBOL(dma_fence_describe);
 
@@ -1055,3 +1068,67 @@ dma_fence_init64(struct dma_fence *fence, const struct dma_fence_ops *ops,
 			 BIT(DMA_FENCE_FLAG_SEQNO64_BIT));
 }
 EXPORT_SYMBOL(dma_fence_init64);
+
+/**
+ * dma_fence_driver_name - Access the driver name
+ * @fence: the fence to query
+ *
+ * Returns a driver name backing the dma-fence implementation.
+ *
+ * IMPORTANT CONSIDERATION:
+ * Dma-fence contract stipulates that access to driver provided data (data not
+ * directly embedded into the object itself), such as the &dma_fence.lock and
+ * memory potentially accessed by the &dma_fence.ops functions, is forbidden
+ * after the fence has been signalled. Drivers are allowed to free that data,
+ * and some do.
+ *
+ * To allow safe access drivers are mandated to guarantee a RCU grace period
+ * between signalling the fence and freeing said data.
+ *
+ * As such access to the driver name is only valid inside a RCU locked section.
+ * The pointer MUST be both queried and USED ONLY WITHIN a SINGLE block guarded
+ * by the &dma_fence_access_being and &dma_fence_access_end pair.
+ */
+const char *dma_fence_driver_name(struct dma_fence *fence)
+{
+	RCU_LOCKDEP_WARN(!rcu_read_lock_held(),
+			 "dma_fence_access_begin/end() are required for safe access to returned string");
+
+	if (!test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
+		return fence->ops->get_driver_name(fence);
+	else
+		return "detached-driver";
+}
+EXPORT_SYMBOL(dma_fence_driver_name);
+
+/**
+ * dma_fence_timeline_name - Access the timeline name
+ * @fence: the fence to query
+ *
+ * Returns a timeline name provided by the dma-fence implementation.
+ *
+ * IMPORTANT CONSIDERATION:
+ * Dma-fence contract stipulates that access to driver provided data (data not
+ * directly embedded into the object itself), such as the &dma_fence.lock and
+ * memory potentially accessed by the &dma_fence.ops functions, is forbidden
+ * after the fence has been signalled. Drivers are allowed to free that data,
+ * and some do.
+ *
+ * To allow safe access drivers are mandated to guarantee a RCU grace period
+ * between signalling the fence and freeing said data.
+ *
+ * As such access to the driver name is only valid inside a RCU locked section.
+ * The pointer MUST be both queried and USED ONLY WITHIN a SINGLE block guarded
+ * by the &dma_fence_access_being and &dma_fence_access_end pair.
+ */
+const char *dma_fence_timeline_name(struct dma_fence *fence)
+{
+	RCU_LOCKDEP_WARN(!rcu_read_lock_held(),
+			 "dma_fence_access_begin/end() are required for safe access to returned string");
+
+	if (!test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
+		return fence->ops->get_driver_name(fence);
+	else
+		return "signaled-timeline";
+}
+EXPORT_SYMBOL(dma_fence_timeline_name);
diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
index c5ac37e10d85..b39e430142ea 100644
--- a/include/linux/dma-fence.h
+++ b/include/linux/dma-fence.h
@@ -377,15 +377,31 @@ bool dma_fence_remove_callback(struct dma_fence *fence,
 			       struct dma_fence_cb *cb);
 void dma_fence_enable_sw_signaling(struct dma_fence *fence);
 
-static inline const char *dma_fence_driver_name(struct dma_fence *fence)
-{
-	return fence->ops->get_driver_name(fence);
-}
+/**
+ * DOC: Safe external access to driver provided object members
+ *
+ * All data not stored directly in the dma-fence object, such as the
+ * &dma_fence.lock and memory potentially accessed by functions in the
+ * &dma_fence.ops table, MUST NOT be accessed after the fence has been signalled
+ * because after that point drivers are allowed to free it.
+ *
+ * All code accessing that data via the dma-fence API (or directly, which is
+ * discouraged), MUST make sure to contain the complete access within a
+ * &dma_fence_access_begin and &dma_fence_access_end pair.
+ *
+ * Some dma-fence API handles this automatically, while other, as for example
+ * &dma_fence_driver_name and &dma_fence_timeline_name, leave that
+ * responsibility to the caller.
+ *
+ * To enable this scheme to work drivers MUST ensure a RCU grace period elapses
+ * between signalling the fence and freeing the said data.
+ *
+ */
+#define dma_fence_access_begin	rcu_read_lock
+#define dma_fence_access_end	rcu_read_unlock
 
-static inline const char *dma_fence_timeline_name(struct dma_fence *fence)
-{
-	return fence->ops->get_timeline_name(fence);
-}
+const char *dma_fence_driver_name(struct dma_fence *fence);
+const char *dma_fence_timeline_name(struct dma_fence *fence);
 
 /**
  * dma_fence_is_signaled_locked - Return an indication if the fence
diff --git a/include/trace/events/dma_fence.h b/include/trace/events/dma_fence.h
index 84c83074ee81..4814a65b68dc 100644
--- a/include/trace/events/dma_fence.h
+++ b/include/trace/events/dma_fence.h
@@ -34,14 +34,44 @@ DECLARE_EVENT_CLASS(dma_fence,
 		  __entry->seqno)
 );
 
-DEFINE_EVENT(dma_fence, dma_fence_emit,
+/*
+ * Safe only for call sites which are guaranteed to not race with fence
+ * signaling,holding the fence->lock and having checked for not signaled, or the
+ * signaling path itself.
+ */
+DECLARE_EVENT_CLASS(dma_fence_unsignaled,
+
+	TP_PROTO(struct dma_fence *fence),
+
+	TP_ARGS(fence),
+
+	TP_STRUCT__entry(
+		__string(driver, fence->ops->get_driver_name(fence))
+		__string(timeline, fence->ops->get_timeline_name(fence))
+		__field(unsigned int, context)
+		__field(unsigned int, seqno)
+	),
+
+	TP_fast_assign(
+		__assign_str(driver);
+		__assign_str(timeline);
+		__entry->context = fence->context;
+		__entry->seqno = fence->seqno;
+	),
+
+	TP_printk("driver=%s timeline=%s context=%u seqno=%u",
+		  __get_str(driver), __get_str(timeline), __entry->context,
+		  __entry->seqno)
+);
+
+DEFINE_EVENT(dma_fence_unsignaled, dma_fence_emit,
 
 	TP_PROTO(struct dma_fence *fence),
 
 	TP_ARGS(fence)
 );
 
-DEFINE_EVENT(dma_fence, dma_fence_init,
+DEFINE_EVENT(dma_fence_unsignaled, dma_fence_init,
 
 	TP_PROTO(struct dma_fence *fence),
 
@@ -55,14 +85,14 @@ DEFINE_EVENT(dma_fence, dma_fence_destroy,
 	TP_ARGS(fence)
 );
 
-DEFINE_EVENT(dma_fence, dma_fence_enable_signal,
+DEFINE_EVENT(dma_fence_unsignaled, dma_fence_enable_signal,
 
 	TP_PROTO(struct dma_fence *fence),
 
 	TP_ARGS(fence)
 );
 
-DEFINE_EVENT(dma_fence, dma_fence_signaled,
+DEFINE_EVENT(dma_fence_unsignaled, dma_fence_signaled,
 
 	TP_PROTO(struct dma_fence *fence),
 
-- 
2.48.0

[PATCH v4 7/9] sync_file: Protect access to driver and timeline name

[Tvrtko Ursulin]

Protect the access to driver and timeline name which otherwise could be
freed as dma-fence exported is signalling fences.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
---
 drivers/dma-buf/sync_file.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/dma-buf/sync_file.c b/drivers/dma-buf/sync_file.c
index 212df4b849fe..ad87116baa24 100644
--- a/drivers/dma-buf/sync_file.c
+++ b/drivers/dma-buf/sync_file.c
@@ -136,11 +136,13 @@ char *sync_file_get_name(struct sync_file *sync_file, char *buf, int len)
 	} else {
 		struct dma_fence *fence = sync_file->fence;
 
+		dma_fence_access_begin();
 		snprintf(buf, len, "%s-%s%llu-%lld",
 			 dma_fence_driver_name(fence),
 			 dma_fence_timeline_name(fence),
 			 fence->context,
 			 fence->seqno);
+		dma_fence_access_end();
 	}
 
 	return buf;
@@ -262,6 +264,8 @@ static long sync_file_ioctl_merge(struct sync_file *sync_file,
 static int sync_fill_fence_info(struct dma_fence *fence,
 				 struct sync_fence_info *info)
 {
+	dma_fence_access_begin();
+
 	strscpy(info->obj_name, dma_fence_timeline_name(fence),
 		sizeof(info->obj_name));
 	strscpy(info->driver_name, dma_fence_driver_name(fence),
@@ -273,6 +277,8 @@ static int sync_fill_fence_info(struct dma_fence *fence,
 			ktime_to_ns(dma_fence_timestamp(fence)) :
 			ktime_set(0, 0);
 
+	dma_fence_access_end();
+
 	return info->status;
 }
 
-- 
2.48.0

[PATCH v4 8/9] drm/i915: Protect access to driver and timeline name

[Tvrtko Ursulin]

Protect the access to driver and timeline name which otherwise could be
freed as dma-fence exported is signalling fences.

Now that the safe access is handled in the dma-fence API, the external
callers such as sync_file, and our internal code paths, we can drop the
similar protection from i915_fence_get_timeline_name().

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
---
 drivers/gpu/drm/i915/gt/intel_gt_requests.c |  2 ++
 drivers/gpu/drm/i915/i915_request.c         | 17 +++--------------
 drivers/gpu/drm/i915/i915_sw_fence.c        |  2 ++
 3 files changed, 7 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_gt_requests.c b/drivers/gpu/drm/i915/gt/intel_gt_requests.c
index ae3557ed6c1e..11fca24c8b5b 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_requests.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt_requests.c
@@ -251,10 +251,12 @@ void intel_gt_watchdog_work(struct work_struct *work)
 		if (!i915_request_completed(rq)) {
 			struct dma_fence *f = &rq->fence;
 
+			dma_fence_access_begin();
 			pr_notice("Fence expiration time out i915-%s:%s:%llx!\n",
 				  dma_fence_driver_name(f),
 				  dma_fence_timeline_name(f),
 				  f->seqno);
+			dma_fence_access_end();
 			i915_request_cancel(rq, -EINTR);
 		}
 		i915_request_put(rq);
diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
index 4874c4f1e4ab..a8de736ff556 100644
--- a/drivers/gpu/drm/i915/i915_request.c
+++ b/drivers/gpu/drm/i915/i915_request.c
@@ -66,18 +66,6 @@ static const char *i915_fence_get_timeline_name(struct dma_fence *fence)
 {
 	const struct i915_gem_context *ctx;
 
-	/*
-	 * The timeline struct (as part of the ppgtt underneath a context)
-	 * may be freed when the request is no longer in use by the GPU.
-	 * We could extend the life of a context to beyond that of all
-	 * fences, possibly keeping the hw resource around indefinitely,
-	 * or we just give them a false name. Since
-	 * dma_fence_ops.get_timeline_name is a debug feature, the occasional
-	 * lie seems justifiable.
-	 */
-	if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
-		return "signaled";
-
 	ctx = i915_request_gem_context(to_request(fence));
 	if (!ctx)
 		return "[" DRIVER_NAME "]";
@@ -2184,7 +2172,6 @@ void i915_request_show(struct drm_printer *m,
 		       const char *prefix,
 		       int indent)
 {
-	const char *name = dma_fence_timeline_name((struct dma_fence *)&rq->fence);
 	char buf[80] = "";
 	int x = 0;
 
@@ -2220,6 +2207,7 @@ void i915_request_show(struct drm_printer *m,
 
 	x = print_sched_attr(&rq->sched.attr, buf, x, sizeof(buf));
 
+	dma_fence_access_begin();
 	drm_printf(m, "%s%.*s%c %llx:%lld%s%s %s @ %dms: %s\n",
 		   prefix, indent, "                ",
 		   queue_status(rq),
@@ -2228,7 +2216,8 @@ void i915_request_show(struct drm_printer *m,
 		   fence_status(rq),
 		   buf,
 		   jiffies_to_msecs(jiffies - rq->emitted_jiffies),
-		   name);
+		   dma_fence_timeline_name((struct dma_fence *)&rq->fence));
+	dma_fence_access_end();
 }
 
 static bool engine_match_ring(struct intel_engine_cs *engine, struct i915_request *rq)
diff --git a/drivers/gpu/drm/i915/i915_sw_fence.c b/drivers/gpu/drm/i915/i915_sw_fence.c
index e51ca7e50a4e..e7bdc1165b90 100644
--- a/drivers/gpu/drm/i915/i915_sw_fence.c
+++ b/drivers/gpu/drm/i915/i915_sw_fence.c
@@ -434,11 +434,13 @@ static void timer_i915_sw_fence_wake(struct timer_list *t)
 	if (!fence)
 		return;
 
+	dma_fence_access_begin();
 	pr_notice("Asynchronous wait on fence %s:%s:%llx timed out (hint:%ps)\n",
 		  dma_fence_driver_name(cb->dma),
 		  dma_fence_timeline_name(cb->dma),
 		  cb->dma->seqno,
 		  i915_sw_fence_debug_hint(fence));
+	dma_fence_access_end();
 
 	i915_sw_fence_set_error_once(fence, -ETIMEDOUT);
 	i915_sw_fence_complete(fence);
-- 
2.48.0

[PATCH v4 9/9] drm/xe: Make dma-fences compliant with the safe access rules

[Tvrtko Ursulin]

Xe can free some of the data pointed to by the dma-fences it exports. Most
notably the timeline name can get freed if userspace closes the associated
submit queue. At the same time the fence could have been exported to a
third party (for example a sync_fence fd) which will then cause an use-
after-free on subsequent access.

To make this safe we need to make the driver compliant with the newly
documented dma-fence rules. Driver has to ensure a RCU grace period
between signalling a fence and freeing any data pointed to by said fence.

For the timeline name we simply make the queue be freed via kfree_rcu and
for the shared lock associated with multiple queues we add a RCU grace
period before freeing the per GT structure holding the lock.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Reviewed-by: Matthew Brost <matthew.brost@intel.com>
---
 drivers/gpu/drm/xe/xe_guc_exec_queue_types.h | 2 ++
 drivers/gpu/drm/xe/xe_guc_submit.c           | 7 ++++++-
 drivers/gpu/drm/xe/xe_hw_fence.c             | 3 +++
 3 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/xe/xe_guc_exec_queue_types.h b/drivers/gpu/drm/xe/xe_guc_exec_queue_types.h
index 4c39f01e4f52..a3f421e2adc0 100644
--- a/drivers/gpu/drm/xe/xe_guc_exec_queue_types.h
+++ b/drivers/gpu/drm/xe/xe_guc_exec_queue_types.h
@@ -20,6 +20,8 @@ struct xe_exec_queue;
 struct xe_guc_exec_queue {
 	/** @q: Backpointer to parent xe_exec_queue */
 	struct xe_exec_queue *q;
+	/** @rcu: For safe freeing of exported dma fences */
+	struct rcu_head rcu;
 	/** @sched: GPU scheduler for this xe_exec_queue */
 	struct xe_gpu_scheduler sched;
 	/** @entity: Scheduler entity for this xe_exec_queue */
diff --git a/drivers/gpu/drm/xe/xe_guc_submit.c b/drivers/gpu/drm/xe/xe_guc_submit.c
index fb125f940de8..879a4474bf51 100644
--- a/drivers/gpu/drm/xe/xe_guc_submit.c
+++ b/drivers/gpu/drm/xe/xe_guc_submit.c
@@ -1291,7 +1291,11 @@ static void __guc_exec_queue_fini_async(struct work_struct *w)
 	xe_sched_entity_fini(&ge->entity);
 	xe_sched_fini(&ge->sched);
 
-	kfree(ge);
+	/*
+	 * RCU free due sched being exported via DRM scheduler fences
+	 * (timeline name).
+	 */
+	kfree_rcu(ge, rcu);
 	xe_exec_queue_fini(q);
 	xe_pm_runtime_put(guc_to_xe(guc));
 }
@@ -1474,6 +1478,7 @@ static int guc_exec_queue_init(struct xe_exec_queue *q)
 
 	q->guc = ge;
 	ge->q = q;
+	init_rcu_head(&ge->rcu);
 	init_waitqueue_head(&ge->suspend_wait);
 
 	for (i = 0; i < MAX_STATIC_MSG_TYPE; ++i)
diff --git a/drivers/gpu/drm/xe/xe_hw_fence.c b/drivers/gpu/drm/xe/xe_hw_fence.c
index 03eb8c6d1616..b2a0c46dfcd4 100644
--- a/drivers/gpu/drm/xe/xe_hw_fence.c
+++ b/drivers/gpu/drm/xe/xe_hw_fence.c
@@ -100,6 +100,9 @@ void xe_hw_fence_irq_finish(struct xe_hw_fence_irq *irq)
 		spin_unlock_irqrestore(&irq->lock, flags);
 		dma_fence_end_signalling(tmp);
 	}
+
+	/* Safe release of the irq->lock used in dma_fence_init. */
+	synchronize_rcu();
 }
 
 void xe_hw_fence_irq_run(struct xe_hw_fence_irq *irq)
-- 
2.48.0

Re: [PATCH v4 2/9] dma-fence: Use a flag for 64-bit seqnos

[Christian König]

Hey drm-misc maintainers,

can you guys please backmerge drm-next into drm-misc-next?

I want to push this patch here but it depends on changes which are partially in drm-next and partially in drm-misc-next.

Thanks in advance,
Christian.

On 5/15/25 11:49, Tvrtko Ursulin wrote:
> With the goal of reducing the need for drivers to touch (and dereference)
> fence->ops, we move the 64-bit seqnos flag from struct dma_fence_ops to
> the fence->flags.
> 
> Drivers which were setting this flag are changed to use new
> dma_fence_init64() instead of dma_fence_init().
> 
> v2:
>  * Streamlined init and added kerneldoc.
>  * Rebase for amdgpu userq which landed since.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
> Reviewed-by: Christian König <christian.koenig@amd.com> # v1
> ---
>  drivers/dma-buf/dma-fence-chain.c             |  5 +-
>  drivers/dma-buf/dma-fence.c                   | 69 ++++++++++++++-----
>  .../drm/amd/amdgpu/amdgpu_eviction_fence.c    |  7 +-
>  .../gpu/drm/amd/amdgpu/amdgpu_userq_fence.c   |  5 +-
>  .../gpu/drm/amd/amdgpu/amdgpu_vm_tlb_fence.c  |  5 +-
>  include/linux/dma-fence.h                     | 14 ++--
>  6 files changed, 64 insertions(+), 41 deletions(-)
> 
> diff --git a/drivers/dma-buf/dma-fence-chain.c b/drivers/dma-buf/dma-fence-chain.c
> index 90424f23fd73..a8a90acf4f34 100644
> --- a/drivers/dma-buf/dma-fence-chain.c
> +++ b/drivers/dma-buf/dma-fence-chain.c
> @@ -218,7 +218,6 @@ static void dma_fence_chain_set_deadline(struct dma_fence *fence,
>  }
>  
>  const struct dma_fence_ops dma_fence_chain_ops = {
> -	.use_64bit_seqno = true,
>  	.get_driver_name = dma_fence_chain_get_driver_name,
>  	.get_timeline_name = dma_fence_chain_get_timeline_name,
>  	.enable_signaling = dma_fence_chain_enable_signaling,
> @@ -262,8 +261,8 @@ void dma_fence_chain_init(struct dma_fence_chain *chain,
>  			seqno = max(prev->seqno, seqno);
>  	}
>  
> -	dma_fence_init(&chain->base, &dma_fence_chain_ops,
> -		       &chain->lock, context, seqno);
> +	dma_fence_init64(&chain->base, &dma_fence_chain_ops, &chain->lock,
> +			 context, seqno);
>  
>  	/*
>  	 * Chaining dma_fence_chain container together is only allowed through
> diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
> index f0cdd3e99d36..705b59787731 100644
> --- a/drivers/dma-buf/dma-fence.c
> +++ b/drivers/dma-buf/dma-fence.c
> @@ -989,24 +989,9 @@ void dma_fence_describe(struct dma_fence *fence, struct seq_file *seq)
>  }
>  EXPORT_SYMBOL(dma_fence_describe);
>  
> -/**
> - * 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
> - * @context: the execution context this fence is run on
> - * @seqno: a linear increasing sequence number for this context
> - *
> - * Initializes an allocated fence, the caller doesn't have to keep its
> - * refcount after committing with this fence, but it will need to hold a
> - * refcount again if &dma_fence_ops.enable_signaling gets called.
> - *
> - * context and seqno are used for easy comparison between fences, allowing
> - * to check which fence is later by simply using dma_fence_later().
> - */
> -void
> -dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops,
> -	       spinlock_t *lock, u64 context, u64 seqno)
> +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);
> @@ -1017,9 +1002,55 @@ dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops,
>  	fence->lock = lock;
>  	fence->context = context;
>  	fence->seqno = seqno;
> -	fence->flags = 0UL;
> +	fence->flags = flags;
>  	fence->error = 0;
>  
>  	trace_dma_fence_init(fence);
>  }
> +
> +/**
> + * 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
> + * @context: the execution context this fence is run on
> + * @seqno: a linear increasing sequence number for this context
> + *
> + * Initializes an allocated fence, the caller doesn't have to keep its
> + * refcount after committing with this fence, but it will need to hold a
> + * refcount again if &dma_fence_ops.enable_signaling gets called.
> + *
> + * context and seqno are used for easy comparison between fences, allowing
> + * to check which fence is later by simply using dma_fence_later().
> + */
> +void
> +dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops,
> +	       spinlock_t *lock, u64 context, u64 seqno)
> +{
> +	__dma_fence_init(fence, ops, lock, context, seqno, 0UL);
> +}
>  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
> + * @context: the execution context this fence is run on
> + * @seqno: a linear increasing sequence number for this context
> + *
> + * Initializes an allocated fence, the caller doesn't have to keep its
> + * refcount after committing with this fence, but it will need to hold a
> + * refcount again if &dma_fence_ops.enable_signaling gets called.
> + *
> + * Context and seqno are used for easy comparison between fences, allowing
> + * to check which fence is later by simply using dma_fence_later().
> + */
> +void
> +dma_fence_init64(struct dma_fence *fence, const struct dma_fence_ops *ops,
> +		 spinlock_t *lock, u64 context, u64 seqno)
> +{
> +	__dma_fence_init(fence, ops, lock, context, seqno,
> +			 BIT(DMA_FENCE_FLAG_SEQNO64_BIT));
> +}
> +EXPORT_SYMBOL(dma_fence_init64);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.c
> index 1a7469543db5..79713421bffe 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.c
> @@ -134,7 +134,6 @@ static bool amdgpu_eviction_fence_enable_signaling(struct dma_fence *f)
>  }
>  
>  static const struct dma_fence_ops amdgpu_eviction_fence_ops = {
> -	.use_64bit_seqno = true,
>  	.get_driver_name = amdgpu_eviction_fence_get_driver_name,
>  	.get_timeline_name = amdgpu_eviction_fence_get_timeline_name,
>  	.enable_signaling = amdgpu_eviction_fence_enable_signaling,
> @@ -160,9 +159,9 @@ amdgpu_eviction_fence_create(struct amdgpu_eviction_fence_mgr *evf_mgr)
>  	ev_fence->evf_mgr = evf_mgr;
>  	get_task_comm(ev_fence->timeline_name, current);
>  	spin_lock_init(&ev_fence->lock);
> -	dma_fence_init(&ev_fence->base, &amdgpu_eviction_fence_ops,
> -		       &ev_fence->lock, evf_mgr->ev_fence_ctx,
> -		       atomic_inc_return(&evf_mgr->ev_fence_seq));
> +	dma_fence_init64(&ev_fence->base, &amdgpu_eviction_fence_ops,
> +			 &ev_fence->lock, evf_mgr->ev_fence_ctx,
> +			 atomic_inc_return(&evf_mgr->ev_fence_seq));
>  	return ev_fence;
>  }
>  
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
> index 029cb24c28b3..5e92d00a591f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
> @@ -239,8 +239,8 @@ static int amdgpu_userq_fence_create(struct amdgpu_usermode_queue *userq,
>  	fence = &userq_fence->base;
>  	userq_fence->fence_drv = fence_drv;
>  
> -	dma_fence_init(fence, &amdgpu_userq_fence_ops, &userq_fence->lock,
> -		       fence_drv->context, seq);
> +	dma_fence_init64(fence, &amdgpu_userq_fence_ops, &userq_fence->lock,
> +			 fence_drv->context, seq);
>  
>  	amdgpu_userq_fence_driver_get(fence_drv);
>  	dma_fence_get(fence);
> @@ -334,7 +334,6 @@ static void amdgpu_userq_fence_release(struct dma_fence *f)
>  }
>  
>  static const struct dma_fence_ops amdgpu_userq_fence_ops = {
> -	.use_64bit_seqno = true,
>  	.get_driver_name = amdgpu_userq_fence_get_driver_name,
>  	.get_timeline_name = amdgpu_userq_fence_get_timeline_name,
>  	.signaled = amdgpu_userq_fence_signaled,
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_tlb_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_tlb_fence.c
> index 51cddfa3f1e8..5d26797356a3 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_tlb_fence.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_tlb_fence.c
> @@ -71,7 +71,6 @@ static void amdgpu_tlb_fence_work(struct work_struct *work)
>  }
>  
>  static const struct dma_fence_ops amdgpu_tlb_fence_ops = {
> -	.use_64bit_seqno = true,
>  	.get_driver_name = amdgpu_tlb_fence_get_driver_name,
>  	.get_timeline_name = amdgpu_tlb_fence_get_timeline_name
>  };
> @@ -101,8 +100,8 @@ void amdgpu_vm_tlb_fence_create(struct amdgpu_device *adev, struct amdgpu_vm *vm
>  	INIT_WORK(&f->work, amdgpu_tlb_fence_work);
>  	spin_lock_init(&f->lock);
>  
> -	dma_fence_init(&f->base, &amdgpu_tlb_fence_ops, &f->lock,
> -		       vm->tlb_fence_context, atomic64_read(&vm->tlb_seq));
> +	dma_fence_init64(&f->base, &amdgpu_tlb_fence_ops, &f->lock,
> +			 vm->tlb_fence_context, atomic64_read(&vm->tlb_seq));
>  
>  	/* TODO: We probably need a separate wq here */
>  	dma_fence_get(&f->base);
> diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
> index 48b5202c531d..a34a0dcdc446 100644
> --- a/include/linux/dma-fence.h
> +++ b/include/linux/dma-fence.h
> @@ -97,6 +97,7 @@ struct dma_fence {
>  };
>  
>  enum dma_fence_flag_bits {
> +	DMA_FENCE_FLAG_SEQNO64_BIT,
>  	DMA_FENCE_FLAG_SIGNALED_BIT,
>  	DMA_FENCE_FLAG_TIMESTAMP_BIT,
>  	DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT,
> @@ -124,14 +125,6 @@ struct dma_fence_cb {
>   *
>   */
>  struct dma_fence_ops {
> -	/**
> -	 * @use_64bit_seqno:
> -	 *
> -	 * True if this dma_fence implementation uses 64bit seqno, false
> -	 * otherwise.
> -	 */
> -	bool use_64bit_seqno;
> -
>  	/**
>  	 * @get_driver_name:
>  	 *
> @@ -262,6 +255,9 @@ struct dma_fence_ops {
>  void dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops,
>  		    spinlock_t *lock, u64 context, u64 seqno);
>  
> +void dma_fence_init64(struct dma_fence *fence, const struct dma_fence_ops *ops,
> +		      spinlock_t *lock, u64 context, u64 seqno);
> +
>  void dma_fence_release(struct kref *kref);
>  void dma_fence_free(struct dma_fence *fence);
>  void dma_fence_describe(struct dma_fence *fence, struct seq_file *seq);
> @@ -454,7 +450,7 @@ static inline bool __dma_fence_is_later(struct dma_fence *fence, u64 f1, u64 f2)
>  	 * 32bit sequence numbers. Use a 64bit compare when the driver says to
>  	 * do so.
>  	 */
> -	if (fence->ops->use_64bit_seqno)
> +	if (test_bit(DMA_FENCE_FLAG_SEQNO64_BIT, &fence->flags))
>  		return f1 > f2;
>  
>  	return (int)(lower_32_bits(f1) - lower_32_bits(f2)) > 0;

Re: [PATCH v4 5/9] drm/i915: Use dma-fence driver and timeline name helpers

[Andi Shyti]

Hi Tvrtko,

On Thu, May 15, 2025 at 10:50:00AM +0100, Tvrtko Ursulin wrote:
> Access the dma-fence internals via the previously added helpers.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
> Reviewed-by: Christian König <christian.koenig@amd.com>

Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>

Thanks,
Andi

Re: [PATCH v4 8/9] drm/i915: Protect access to driver and timeline name

[Andi Shyti]

Hi Tvrtko,

On Thu, May 15, 2025 at 10:50:03AM +0100, Tvrtko Ursulin wrote:
> Protect the access to driver and timeline name which otherwise could be
> freed as dma-fence exported is signalling fences.
> 
> Now that the safe access is handled in the dma-fence API, the external
> callers such as sync_file, and our internal code paths, we can drop the
> similar protection from i915_fence_get_timeline_name().
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>

Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>

Thanks,
Andi

Re: [PATCH v4 2/9] dma-fence: Use a flag for 64-bit seqnos

[Tvrtko Ursulin]

Hi,

On 15/05/2025 14:15, Christian König wrote:
> Hey drm-misc maintainers,
> 
> can you guys please backmerge drm-next into drm-misc-next?
> 
> I want to push this patch here but it depends on changes which are partially in drm-next and partially in drm-misc-next.

Looks like the backmerge is still pending?

In the meantime, Christian, any chance you will have some bandwith to 
think about the tail end of the series? Specifically patch 6 and how 
that is used onward.

Regards,

Tvrtko

> On 5/15/25 11:49, Tvrtko Ursulin wrote:
>> With the goal of reducing the need for drivers to touch (and dereference)
>> fence->ops, we move the 64-bit seqnos flag from struct dma_fence_ops to
>> the fence->flags.
>>
>> Drivers which were setting this flag are changed to use new
>> dma_fence_init64() instead of dma_fence_init().
>>
>> v2:
>>   * Streamlined init and added kerneldoc.
>>   * Rebase for amdgpu userq which landed since.
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
>> Reviewed-by: Christian König <christian.koenig@amd.com> # v1
>> ---
>>   drivers/dma-buf/dma-fence-chain.c             |  5 +-
>>   drivers/dma-buf/dma-fence.c                   | 69 ++++++++++++++-----
>>   .../drm/amd/amdgpu/amdgpu_eviction_fence.c    |  7 +-
>>   .../gpu/drm/amd/amdgpu/amdgpu_userq_fence.c   |  5 +-
>>   .../gpu/drm/amd/amdgpu/amdgpu_vm_tlb_fence.c  |  5 +-
>>   include/linux/dma-fence.h                     | 14 ++--
>>   6 files changed, 64 insertions(+), 41 deletions(-)
>>
>> diff --git a/drivers/dma-buf/dma-fence-chain.c b/drivers/dma-buf/dma-fence-chain.c
>> index 90424f23fd73..a8a90acf4f34 100644
>> --- a/drivers/dma-buf/dma-fence-chain.c
>> +++ b/drivers/dma-buf/dma-fence-chain.c
>> @@ -218,7 +218,6 @@ static void dma_fence_chain_set_deadline(struct dma_fence *fence,
>>   }
>>   
>>   const struct dma_fence_ops dma_fence_chain_ops = {
>> -	.use_64bit_seqno = true,
>>   	.get_driver_name = dma_fence_chain_get_driver_name,
>>   	.get_timeline_name = dma_fence_chain_get_timeline_name,
>>   	.enable_signaling = dma_fence_chain_enable_signaling,
>> @@ -262,8 +261,8 @@ void dma_fence_chain_init(struct dma_fence_chain *chain,
>>   			seqno = max(prev->seqno, seqno);
>>   	}
>>   
>> -	dma_fence_init(&chain->base, &dma_fence_chain_ops,
>> -		       &chain->lock, context, seqno);
>> +	dma_fence_init64(&chain->base, &dma_fence_chain_ops, &chain->lock,
>> +			 context, seqno);
>>   
>>   	/*
>>   	 * Chaining dma_fence_chain container together is only allowed through
>> diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
>> index f0cdd3e99d36..705b59787731 100644
>> --- a/drivers/dma-buf/dma-fence.c
>> +++ b/drivers/dma-buf/dma-fence.c
>> @@ -989,24 +989,9 @@ void dma_fence_describe(struct dma_fence *fence, struct seq_file *seq)
>>   }
>>   EXPORT_SYMBOL(dma_fence_describe);
>>   
>> -/**
>> - * 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
>> - * @context: the execution context this fence is run on
>> - * @seqno: a linear increasing sequence number for this context
>> - *
>> - * Initializes an allocated fence, the caller doesn't have to keep its
>> - * refcount after committing with this fence, but it will need to hold a
>> - * refcount again if &dma_fence_ops.enable_signaling gets called.
>> - *
>> - * context and seqno are used for easy comparison between fences, allowing
>> - * to check which fence is later by simply using dma_fence_later().
>> - */
>> -void
>> -dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops,
>> -	       spinlock_t *lock, u64 context, u64 seqno)
>> +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);
>> @@ -1017,9 +1002,55 @@ dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops,
>>   	fence->lock = lock;
>>   	fence->context = context;
>>   	fence->seqno = seqno;
>> -	fence->flags = 0UL;
>> +	fence->flags = flags;
>>   	fence->error = 0;
>>   
>>   	trace_dma_fence_init(fence);
>>   }
>> +
>> +/**
>> + * 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
>> + * @context: the execution context this fence is run on
>> + * @seqno: a linear increasing sequence number for this context
>> + *
>> + * Initializes an allocated fence, the caller doesn't have to keep its
>> + * refcount after committing with this fence, but it will need to hold a
>> + * refcount again if &dma_fence_ops.enable_signaling gets called.
>> + *
>> + * context and seqno are used for easy comparison between fences, allowing
>> + * to check which fence is later by simply using dma_fence_later().
>> + */
>> +void
>> +dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops,
>> +	       spinlock_t *lock, u64 context, u64 seqno)
>> +{
>> +	__dma_fence_init(fence, ops, lock, context, seqno, 0UL);
>> +}
>>   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
>> + * @context: the execution context this fence is run on
>> + * @seqno: a linear increasing sequence number for this context
>> + *
>> + * Initializes an allocated fence, the caller doesn't have to keep its
>> + * refcount after committing with this fence, but it will need to hold a
>> + * refcount again if &dma_fence_ops.enable_signaling gets called.
>> + *
>> + * Context and seqno are used for easy comparison between fences, allowing
>> + * to check which fence is later by simply using dma_fence_later().
>> + */
>> +void
>> +dma_fence_init64(struct dma_fence *fence, const struct dma_fence_ops *ops,
>> +		 spinlock_t *lock, u64 context, u64 seqno)
>> +{
>> +	__dma_fence_init(fence, ops, lock, context, seqno,
>> +			 BIT(DMA_FENCE_FLAG_SEQNO64_BIT));
>> +}
>> +EXPORT_SYMBOL(dma_fence_init64);
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.c
>> index 1a7469543db5..79713421bffe 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.c
>> @@ -134,7 +134,6 @@ static bool amdgpu_eviction_fence_enable_signaling(struct dma_fence *f)
>>   }
>>   
>>   static const struct dma_fence_ops amdgpu_eviction_fence_ops = {
>> -	.use_64bit_seqno = true,
>>   	.get_driver_name = amdgpu_eviction_fence_get_driver_name,
>>   	.get_timeline_name = amdgpu_eviction_fence_get_timeline_name,
>>   	.enable_signaling = amdgpu_eviction_fence_enable_signaling,
>> @@ -160,9 +159,9 @@ amdgpu_eviction_fence_create(struct amdgpu_eviction_fence_mgr *evf_mgr)
>>   	ev_fence->evf_mgr = evf_mgr;
>>   	get_task_comm(ev_fence->timeline_name, current);
>>   	spin_lock_init(&ev_fence->lock);
>> -	dma_fence_init(&ev_fence->base, &amdgpu_eviction_fence_ops,
>> -		       &ev_fence->lock, evf_mgr->ev_fence_ctx,
>> -		       atomic_inc_return(&evf_mgr->ev_fence_seq));
>> +	dma_fence_init64(&ev_fence->base, &amdgpu_eviction_fence_ops,
>> +			 &ev_fence->lock, evf_mgr->ev_fence_ctx,
>> +			 atomic_inc_return(&evf_mgr->ev_fence_seq));
>>   	return ev_fence;
>>   }
>>   
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
>> index 029cb24c28b3..5e92d00a591f 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
>> @@ -239,8 +239,8 @@ static int amdgpu_userq_fence_create(struct amdgpu_usermode_queue *userq,
>>   	fence = &userq_fence->base;
>>   	userq_fence->fence_drv = fence_drv;
>>   
>> -	dma_fence_init(fence, &amdgpu_userq_fence_ops, &userq_fence->lock,
>> -		       fence_drv->context, seq);
>> +	dma_fence_init64(fence, &amdgpu_userq_fence_ops, &userq_fence->lock,
>> +			 fence_drv->context, seq);
>>   
>>   	amdgpu_userq_fence_driver_get(fence_drv);
>>   	dma_fence_get(fence);
>> @@ -334,7 +334,6 @@ static void amdgpu_userq_fence_release(struct dma_fence *f)
>>   }
>>   
>>   static const struct dma_fence_ops amdgpu_userq_fence_ops = {
>> -	.use_64bit_seqno = true,
>>   	.get_driver_name = amdgpu_userq_fence_get_driver_name,
>>   	.get_timeline_name = amdgpu_userq_fence_get_timeline_name,
>>   	.signaled = amdgpu_userq_fence_signaled,
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_tlb_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_tlb_fence.c
>> index 51cddfa3f1e8..5d26797356a3 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_tlb_fence.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_tlb_fence.c
>> @@ -71,7 +71,6 @@ static void amdgpu_tlb_fence_work(struct work_struct *work)
>>   }
>>   
>>   static const struct dma_fence_ops amdgpu_tlb_fence_ops = {
>> -	.use_64bit_seqno = true,
>>   	.get_driver_name = amdgpu_tlb_fence_get_driver_name,
>>   	.get_timeline_name = amdgpu_tlb_fence_get_timeline_name
>>   };
>> @@ -101,8 +100,8 @@ void amdgpu_vm_tlb_fence_create(struct amdgpu_device *adev, struct amdgpu_vm *vm
>>   	INIT_WORK(&f->work, amdgpu_tlb_fence_work);
>>   	spin_lock_init(&f->lock);
>>   
>> -	dma_fence_init(&f->base, &amdgpu_tlb_fence_ops, &f->lock,
>> -		       vm->tlb_fence_context, atomic64_read(&vm->tlb_seq));
>> +	dma_fence_init64(&f->base, &amdgpu_tlb_fence_ops, &f->lock,
>> +			 vm->tlb_fence_context, atomic64_read(&vm->tlb_seq));
>>   
>>   	/* TODO: We probably need a separate wq here */
>>   	dma_fence_get(&f->base);
>> diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
>> index 48b5202c531d..a34a0dcdc446 100644
>> --- a/include/linux/dma-fence.h
>> +++ b/include/linux/dma-fence.h
>> @@ -97,6 +97,7 @@ struct dma_fence {
>>   };
>>   
>>   enum dma_fence_flag_bits {
>> +	DMA_FENCE_FLAG_SEQNO64_BIT,
>>   	DMA_FENCE_FLAG_SIGNALED_BIT,
>>   	DMA_FENCE_FLAG_TIMESTAMP_BIT,
>>   	DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT,
>> @@ -124,14 +125,6 @@ struct dma_fence_cb {
>>    *
>>    */
>>   struct dma_fence_ops {
>> -	/**
>> -	 * @use_64bit_seqno:
>> -	 *
>> -	 * True if this dma_fence implementation uses 64bit seqno, false
>> -	 * otherwise.
>> -	 */
>> -	bool use_64bit_seqno;
>> -
>>   	/**
>>   	 * @get_driver_name:
>>   	 *
>> @@ -262,6 +255,9 @@ struct dma_fence_ops {
>>   void dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops,
>>   		    spinlock_t *lock, u64 context, u64 seqno);
>>   
>> +void dma_fence_init64(struct dma_fence *fence, const struct dma_fence_ops *ops,
>> +		      spinlock_t *lock, u64 context, u64 seqno);
>> +
>>   void dma_fence_release(struct kref *kref);
>>   void dma_fence_free(struct dma_fence *fence);
>>   void dma_fence_describe(struct dma_fence *fence, struct seq_file *seq);
>> @@ -454,7 +450,7 @@ static inline bool __dma_fence_is_later(struct dma_fence *fence, u64 f1, u64 f2)
>>   	 * 32bit sequence numbers. Use a 64bit compare when the driver says to
>>   	 * do so.
>>   	 */
>> -	if (fence->ops->use_64bit_seqno)
>> +	if (test_bit(DMA_FENCE_FLAG_SEQNO64_BIT, &fence->flags))
>>   		return f1 > f2;
>>   
>>   	return (int)(lower_32_bits(f1) - lower_32_bits(f2)) > 0;
> 

Re: [PATCH v4 2/9] dma-fence: Use a flag for 64-bit seqnos

[Christian König]

On 6/2/25 15:05, Tvrtko Ursulin wrote:
> 
> Hi,
> 
> On 15/05/2025 14:15, Christian König wrote:
>> Hey drm-misc maintainers,
>>
>> can you guys please backmerge drm-next into drm-misc-next?
>>
>> I want to push this patch here but it depends on changes which are partially in drm-next and partially in drm-misc-next.
> 
> Looks like the backmerge is still pending?

Yes, @Maarten, @Maxime and @Thomas ping on this.

> In the meantime, Christian, any chance you will have some bandwith to think about the tail end of the series? Specifically patch 6 and how that is used onward.

Well the RCU grace period is quite a nifty hack. I wanted to go over it again after merging the first patches from this series.

In general looks like a good idea to me, I just don't like that we explicitely need to expose dma_fence_access_begin() and dma_fence_access_end().

Especially we can't do that while calling fence->ops->release.

Regards,
Christian.

> 
> Regards,
> 
> Tvrtko
> 
>> On 5/15/25 11:49, Tvrtko Ursulin wrote:
>>> With the goal of reducing the need for drivers to touch (and dereference)
>>> fence->ops, we move the 64-bit seqnos flag from struct dma_fence_ops to
>>> the fence->flags.
>>>
>>> Drivers which were setting this flag are changed to use new
>>> dma_fence_init64() instead of dma_fence_init().
>>>
>>> v2:
>>>   * Streamlined init and added kerneldoc.
>>>   * Rebase for amdgpu userq which landed since.
>>>
>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
>>> Reviewed-by: Christian König <christian.koenig@amd.com> # v1
>>> ---
>>>   drivers/dma-buf/dma-fence-chain.c             |  5 +-
>>>   drivers/dma-buf/dma-fence.c                   | 69 ++++++++++++++-----
>>>   .../drm/amd/amdgpu/amdgpu_eviction_fence.c    |  7 +-
>>>   .../gpu/drm/amd/amdgpu/amdgpu_userq_fence.c   |  5 +-
>>>   .../gpu/drm/amd/amdgpu/amdgpu_vm_tlb_fence.c  |  5 +-
>>>   include/linux/dma-fence.h                     | 14 ++--
>>>   6 files changed, 64 insertions(+), 41 deletions(-)
>>>
>>> diff --git a/drivers/dma-buf/dma-fence-chain.c b/drivers/dma-buf/dma-fence-chain.c
>>> index 90424f23fd73..a8a90acf4f34 100644
>>> --- a/drivers/dma-buf/dma-fence-chain.c
>>> +++ b/drivers/dma-buf/dma-fence-chain.c
>>> @@ -218,7 +218,6 @@ static void dma_fence_chain_set_deadline(struct dma_fence *fence,
>>>   }
>>>     const struct dma_fence_ops dma_fence_chain_ops = {
>>> -    .use_64bit_seqno = true,
>>>       .get_driver_name = dma_fence_chain_get_driver_name,
>>>       .get_timeline_name = dma_fence_chain_get_timeline_name,
>>>       .enable_signaling = dma_fence_chain_enable_signaling,
>>> @@ -262,8 +261,8 @@ void dma_fence_chain_init(struct dma_fence_chain *chain,
>>>               seqno = max(prev->seqno, seqno);
>>>       }
>>>   -    dma_fence_init(&chain->base, &dma_fence_chain_ops,
>>> -               &chain->lock, context, seqno);
>>> +    dma_fence_init64(&chain->base, &dma_fence_chain_ops, &chain->lock,
>>> +             context, seqno);
>>>         /*
>>>        * Chaining dma_fence_chain container together is only allowed through
>>> diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
>>> index f0cdd3e99d36..705b59787731 100644
>>> --- a/drivers/dma-buf/dma-fence.c
>>> +++ b/drivers/dma-buf/dma-fence.c
>>> @@ -989,24 +989,9 @@ void dma_fence_describe(struct dma_fence *fence, struct seq_file *seq)
>>>   }
>>>   EXPORT_SYMBOL(dma_fence_describe);
>>>   -/**
>>> - * 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
>>> - * @context: the execution context this fence is run on
>>> - * @seqno: a linear increasing sequence number for this context
>>> - *
>>> - * Initializes an allocated fence, the caller doesn't have to keep its
>>> - * refcount after committing with this fence, but it will need to hold a
>>> - * refcount again if &dma_fence_ops.enable_signaling gets called.
>>> - *
>>> - * context and seqno are used for easy comparison between fences, allowing
>>> - * to check which fence is later by simply using dma_fence_later().
>>> - */
>>> -void
>>> -dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops,
>>> -           spinlock_t *lock, u64 context, u64 seqno)
>>> +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);
>>> @@ -1017,9 +1002,55 @@ dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops,
>>>       fence->lock = lock;
>>>       fence->context = context;
>>>       fence->seqno = seqno;
>>> -    fence->flags = 0UL;
>>> +    fence->flags = flags;
>>>       fence->error = 0;
>>>         trace_dma_fence_init(fence);
>>>   }
>>> +
>>> +/**
>>> + * 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
>>> + * @context: the execution context this fence is run on
>>> + * @seqno: a linear increasing sequence number for this context
>>> + *
>>> + * Initializes an allocated fence, the caller doesn't have to keep its
>>> + * refcount after committing with this fence, but it will need to hold a
>>> + * refcount again if &dma_fence_ops.enable_signaling gets called.
>>> + *
>>> + * context and seqno are used for easy comparison between fences, allowing
>>> + * to check which fence is later by simply using dma_fence_later().
>>> + */
>>> +void
>>> +dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops,
>>> +           spinlock_t *lock, u64 context, u64 seqno)
>>> +{
>>> +    __dma_fence_init(fence, ops, lock, context, seqno, 0UL);
>>> +}
>>>   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
>>> + * @context: the execution context this fence is run on
>>> + * @seqno: a linear increasing sequence number for this context
>>> + *
>>> + * Initializes an allocated fence, the caller doesn't have to keep its
>>> + * refcount after committing with this fence, but it will need to hold a
>>> + * refcount again if &dma_fence_ops.enable_signaling gets called.
>>> + *
>>> + * Context and seqno are used for easy comparison between fences, allowing
>>> + * to check which fence is later by simply using dma_fence_later().
>>> + */
>>> +void
>>> +dma_fence_init64(struct dma_fence *fence, const struct dma_fence_ops *ops,
>>> +         spinlock_t *lock, u64 context, u64 seqno)
>>> +{
>>> +    __dma_fence_init(fence, ops, lock, context, seqno,
>>> +             BIT(DMA_FENCE_FLAG_SEQNO64_BIT));
>>> +}
>>> +EXPORT_SYMBOL(dma_fence_init64);
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.c
>>> index 1a7469543db5..79713421bffe 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.c
>>> @@ -134,7 +134,6 @@ static bool amdgpu_eviction_fence_enable_signaling(struct dma_fence *f)
>>>   }
>>>     static const struct dma_fence_ops amdgpu_eviction_fence_ops = {
>>> -    .use_64bit_seqno = true,
>>>       .get_driver_name = amdgpu_eviction_fence_get_driver_name,
>>>       .get_timeline_name = amdgpu_eviction_fence_get_timeline_name,
>>>       .enable_signaling = amdgpu_eviction_fence_enable_signaling,
>>> @@ -160,9 +159,9 @@ amdgpu_eviction_fence_create(struct amdgpu_eviction_fence_mgr *evf_mgr)
>>>       ev_fence->evf_mgr = evf_mgr;
>>>       get_task_comm(ev_fence->timeline_name, current);
>>>       spin_lock_init(&ev_fence->lock);
>>> -    dma_fence_init(&ev_fence->base, &amdgpu_eviction_fence_ops,
>>> -               &ev_fence->lock, evf_mgr->ev_fence_ctx,
>>> -               atomic_inc_return(&evf_mgr->ev_fence_seq));
>>> +    dma_fence_init64(&ev_fence->base, &amdgpu_eviction_fence_ops,
>>> +             &ev_fence->lock, evf_mgr->ev_fence_ctx,
>>> +             atomic_inc_return(&evf_mgr->ev_fence_seq));
>>>       return ev_fence;
>>>   }
>>>   diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
>>> index 029cb24c28b3..5e92d00a591f 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
>>> @@ -239,8 +239,8 @@ static int amdgpu_userq_fence_create(struct amdgpu_usermode_queue *userq,
>>>       fence = &userq_fence->base;
>>>       userq_fence->fence_drv = fence_drv;
>>>   -    dma_fence_init(fence, &amdgpu_userq_fence_ops, &userq_fence->lock,
>>> -               fence_drv->context, seq);
>>> +    dma_fence_init64(fence, &amdgpu_userq_fence_ops, &userq_fence->lock,
>>> +             fence_drv->context, seq);
>>>         amdgpu_userq_fence_driver_get(fence_drv);
>>>       dma_fence_get(fence);
>>> @@ -334,7 +334,6 @@ static void amdgpu_userq_fence_release(struct dma_fence *f)
>>>   }
>>>     static const struct dma_fence_ops amdgpu_userq_fence_ops = {
>>> -    .use_64bit_seqno = true,
>>>       .get_driver_name = amdgpu_userq_fence_get_driver_name,
>>>       .get_timeline_name = amdgpu_userq_fence_get_timeline_name,
>>>       .signaled = amdgpu_userq_fence_signaled,
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_tlb_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_tlb_fence.c
>>> index 51cddfa3f1e8..5d26797356a3 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_tlb_fence.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_tlb_fence.c
>>> @@ -71,7 +71,6 @@ static void amdgpu_tlb_fence_work(struct work_struct *work)
>>>   }
>>>     static const struct dma_fence_ops amdgpu_tlb_fence_ops = {
>>> -    .use_64bit_seqno = true,
>>>       .get_driver_name = amdgpu_tlb_fence_get_driver_name,
>>>       .get_timeline_name = amdgpu_tlb_fence_get_timeline_name
>>>   };
>>> @@ -101,8 +100,8 @@ void amdgpu_vm_tlb_fence_create(struct amdgpu_device *adev, struct amdgpu_vm *vm
>>>       INIT_WORK(&f->work, amdgpu_tlb_fence_work);
>>>       spin_lock_init(&f->lock);
>>>   -    dma_fence_init(&f->base, &amdgpu_tlb_fence_ops, &f->lock,
>>> -               vm->tlb_fence_context, atomic64_read(&vm->tlb_seq));
>>> +    dma_fence_init64(&f->base, &amdgpu_tlb_fence_ops, &f->lock,
>>> +             vm->tlb_fence_context, atomic64_read(&vm->tlb_seq));
>>>         /* TODO: We probably need a separate wq here */
>>>       dma_fence_get(&f->base);
>>> diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
>>> index 48b5202c531d..a34a0dcdc446 100644
>>> --- a/include/linux/dma-fence.h
>>> +++ b/include/linux/dma-fence.h
>>> @@ -97,6 +97,7 @@ struct dma_fence {
>>>   };
>>>     enum dma_fence_flag_bits {
>>> +    DMA_FENCE_FLAG_SEQNO64_BIT,
>>>       DMA_FENCE_FLAG_SIGNALED_BIT,
>>>       DMA_FENCE_FLAG_TIMESTAMP_BIT,
>>>       DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT,
>>> @@ -124,14 +125,6 @@ struct dma_fence_cb {
>>>    *
>>>    */
>>>   struct dma_fence_ops {
>>> -    /**
>>> -     * @use_64bit_seqno:
>>> -     *
>>> -     * True if this dma_fence implementation uses 64bit seqno, false
>>> -     * otherwise.
>>> -     */
>>> -    bool use_64bit_seqno;
>>> -
>>>       /**
>>>        * @get_driver_name:
>>>        *
>>> @@ -262,6 +255,9 @@ struct dma_fence_ops {
>>>   void dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops,
>>>               spinlock_t *lock, u64 context, u64 seqno);
>>>   +void dma_fence_init64(struct dma_fence *fence, const struct dma_fence_ops *ops,
>>> +              spinlock_t *lock, u64 context, u64 seqno);
>>> +
>>>   void dma_fence_release(struct kref *kref);
>>>   void dma_fence_free(struct dma_fence *fence);
>>>   void dma_fence_describe(struct dma_fence *fence, struct seq_file *seq);
>>> @@ -454,7 +450,7 @@ static inline bool __dma_fence_is_later(struct dma_fence *fence, u64 f1, u64 f2)
>>>        * 32bit sequence numbers. Use a 64bit compare when the driver says to
>>>        * do so.
>>>        */
>>> -    if (fence->ops->use_64bit_seqno)
>>> +    if (test_bit(DMA_FENCE_FLAG_SEQNO64_BIT, &fence->flags))
>>>           return f1 > f2;
>>>         return (int)(lower_32_bits(f1) - lower_32_bits(f2)) > 0;
>>
> 

Re: [PATCH v4 2/9] dma-fence: Use a flag for 64-bit seqnos

[Tvrtko Ursulin]

On 02/06/2025 15:42, Christian König wrote:
> On 6/2/25 15:05, Tvrtko Ursulin wrote:
>>
>> Hi,
>>
>> On 15/05/2025 14:15, Christian König wrote:
>>> Hey drm-misc maintainers,
>>>
>>> can you guys please backmerge drm-next into drm-misc-next?
>>>
>>> I want to push this patch here but it depends on changes which are partially in drm-next and partially in drm-misc-next.
>>
>> Looks like the backmerge is still pending?
> 
> Yes, @Maarten, @Maxime and @Thomas ping on this.
> 
>> In the meantime, Christian, any chance you will have some bandwith to think about the tail end of the series? Specifically patch 6 and how that is used onward.
> 
> Well the RCU grace period is quite a nifty hack. I wanted to go over it again after merging the first patches from this series.
> 
> In general looks like a good idea to me, I just don't like that we explicitely need to expose dma_fence_access_begin() and dma_fence_access_end().
> 
> Especially we can't do that while calling fence->ops->release.

Hm why not? You think something will take offence of the rcu_read_lock()?

Regards,

Tvrtko

>>> On 5/15/25 11:49, Tvrtko Ursulin wrote:
>>>> With the goal of reducing the need for drivers to touch (and dereference)
>>>> fence->ops, we move the 64-bit seqnos flag from struct dma_fence_ops to
>>>> the fence->flags.
>>>>
>>>> Drivers which were setting this flag are changed to use new
>>>> dma_fence_init64() instead of dma_fence_init().
>>>>
>>>> v2:
>>>>    * Streamlined init and added kerneldoc.
>>>>    * Rebase for amdgpu userq which landed since.
>>>>
>>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
>>>> Reviewed-by: Christian König <christian.koenig@amd.com> # v1
>>>> ---
>>>>    drivers/dma-buf/dma-fence-chain.c             |  5 +-
>>>>    drivers/dma-buf/dma-fence.c                   | 69 ++++++++++++++-----
>>>>    .../drm/amd/amdgpu/amdgpu_eviction_fence.c    |  7 +-
>>>>    .../gpu/drm/amd/amdgpu/amdgpu_userq_fence.c   |  5 +-
>>>>    .../gpu/drm/amd/amdgpu/amdgpu_vm_tlb_fence.c  |  5 +-
>>>>    include/linux/dma-fence.h                     | 14 ++--
>>>>    6 files changed, 64 insertions(+), 41 deletions(-)
>>>>
>>>> diff --git a/drivers/dma-buf/dma-fence-chain.c b/drivers/dma-buf/dma-fence-chain.c
>>>> index 90424f23fd73..a8a90acf4f34 100644
>>>> --- a/drivers/dma-buf/dma-fence-chain.c
>>>> +++ b/drivers/dma-buf/dma-fence-chain.c
>>>> @@ -218,7 +218,6 @@ static void dma_fence_chain_set_deadline(struct dma_fence *fence,
>>>>    }
>>>>      const struct dma_fence_ops dma_fence_chain_ops = {
>>>> -    .use_64bit_seqno = true,
>>>>        .get_driver_name = dma_fence_chain_get_driver_name,
>>>>        .get_timeline_name = dma_fence_chain_get_timeline_name,
>>>>        .enable_signaling = dma_fence_chain_enable_signaling,
>>>> @@ -262,8 +261,8 @@ void dma_fence_chain_init(struct dma_fence_chain *chain,
>>>>                seqno = max(prev->seqno, seqno);
>>>>        }
>>>>    -    dma_fence_init(&chain->base, &dma_fence_chain_ops,
>>>> -               &chain->lock, context, seqno);
>>>> +    dma_fence_init64(&chain->base, &dma_fence_chain_ops, &chain->lock,
>>>> +             context, seqno);
>>>>          /*
>>>>         * Chaining dma_fence_chain container together is only allowed through
>>>> diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
>>>> index f0cdd3e99d36..705b59787731 100644
>>>> --- a/drivers/dma-buf/dma-fence.c
>>>> +++ b/drivers/dma-buf/dma-fence.c
>>>> @@ -989,24 +989,9 @@ void dma_fence_describe(struct dma_fence *fence, struct seq_file *seq)
>>>>    }
>>>>    EXPORT_SYMBOL(dma_fence_describe);
>>>>    -/**
>>>> - * 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
>>>> - * @context: the execution context this fence is run on
>>>> - * @seqno: a linear increasing sequence number for this context
>>>> - *
>>>> - * Initializes an allocated fence, the caller doesn't have to keep its
>>>> - * refcount after committing with this fence, but it will need to hold a
>>>> - * refcount again if &dma_fence_ops.enable_signaling gets called.
>>>> - *
>>>> - * context and seqno are used for easy comparison between fences, allowing
>>>> - * to check which fence is later by simply using dma_fence_later().
>>>> - */
>>>> -void
>>>> -dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops,
>>>> -           spinlock_t *lock, u64 context, u64 seqno)
>>>> +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);
>>>> @@ -1017,9 +1002,55 @@ dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops,
>>>>        fence->lock = lock;
>>>>        fence->context = context;
>>>>        fence->seqno = seqno;
>>>> -    fence->flags = 0UL;
>>>> +    fence->flags = flags;
>>>>        fence->error = 0;
>>>>          trace_dma_fence_init(fence);
>>>>    }
>>>> +
>>>> +/**
>>>> + * 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
>>>> + * @context: the execution context this fence is run on
>>>> + * @seqno: a linear increasing sequence number for this context
>>>> + *
>>>> + * Initializes an allocated fence, the caller doesn't have to keep its
>>>> + * refcount after committing with this fence, but it will need to hold a
>>>> + * refcount again if &dma_fence_ops.enable_signaling gets called.
>>>> + *
>>>> + * context and seqno are used for easy comparison between fences, allowing
>>>> + * to check which fence is later by simply using dma_fence_later().
>>>> + */
>>>> +void
>>>> +dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops,
>>>> +           spinlock_t *lock, u64 context, u64 seqno)
>>>> +{
>>>> +    __dma_fence_init(fence, ops, lock, context, seqno, 0UL);
>>>> +}
>>>>    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
>>>> + * @context: the execution context this fence is run on
>>>> + * @seqno: a linear increasing sequence number for this context
>>>> + *
>>>> + * Initializes an allocated fence, the caller doesn't have to keep its
>>>> + * refcount after committing with this fence, but it will need to hold a
>>>> + * refcount again if &dma_fence_ops.enable_signaling gets called.
>>>> + *
>>>> + * Context and seqno are used for easy comparison between fences, allowing
>>>> + * to check which fence is later by simply using dma_fence_later().
>>>> + */
>>>> +void
>>>> +dma_fence_init64(struct dma_fence *fence, const struct dma_fence_ops *ops,
>>>> +         spinlock_t *lock, u64 context, u64 seqno)
>>>> +{
>>>> +    __dma_fence_init(fence, ops, lock, context, seqno,
>>>> +             BIT(DMA_FENCE_FLAG_SEQNO64_BIT));
>>>> +}
>>>> +EXPORT_SYMBOL(dma_fence_init64);
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.c
>>>> index 1a7469543db5..79713421bffe 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.c
>>>> @@ -134,7 +134,6 @@ static bool amdgpu_eviction_fence_enable_signaling(struct dma_fence *f)
>>>>    }
>>>>      static const struct dma_fence_ops amdgpu_eviction_fence_ops = {
>>>> -    .use_64bit_seqno = true,
>>>>        .get_driver_name = amdgpu_eviction_fence_get_driver_name,
>>>>        .get_timeline_name = amdgpu_eviction_fence_get_timeline_name,
>>>>        .enable_signaling = amdgpu_eviction_fence_enable_signaling,
>>>> @@ -160,9 +159,9 @@ amdgpu_eviction_fence_create(struct amdgpu_eviction_fence_mgr *evf_mgr)
>>>>        ev_fence->evf_mgr = evf_mgr;
>>>>        get_task_comm(ev_fence->timeline_name, current);
>>>>        spin_lock_init(&ev_fence->lock);
>>>> -    dma_fence_init(&ev_fence->base, &amdgpu_eviction_fence_ops,
>>>> -               &ev_fence->lock, evf_mgr->ev_fence_ctx,
>>>> -               atomic_inc_return(&evf_mgr->ev_fence_seq));
>>>> +    dma_fence_init64(&ev_fence->base, &amdgpu_eviction_fence_ops,
>>>> +             &ev_fence->lock, evf_mgr->ev_fence_ctx,
>>>> +             atomic_inc_return(&evf_mgr->ev_fence_seq));
>>>>        return ev_fence;
>>>>    }
>>>>    diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
>>>> index 029cb24c28b3..5e92d00a591f 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
>>>> @@ -239,8 +239,8 @@ static int amdgpu_userq_fence_create(struct amdgpu_usermode_queue *userq,
>>>>        fence = &userq_fence->base;
>>>>        userq_fence->fence_drv = fence_drv;
>>>>    -    dma_fence_init(fence, &amdgpu_userq_fence_ops, &userq_fence->lock,
>>>> -               fence_drv->context, seq);
>>>> +    dma_fence_init64(fence, &amdgpu_userq_fence_ops, &userq_fence->lock,
>>>> +             fence_drv->context, seq);
>>>>          amdgpu_userq_fence_driver_get(fence_drv);
>>>>        dma_fence_get(fence);
>>>> @@ -334,7 +334,6 @@ static void amdgpu_userq_fence_release(struct dma_fence *f)
>>>>    }
>>>>      static const struct dma_fence_ops amdgpu_userq_fence_ops = {
>>>> -    .use_64bit_seqno = true,
>>>>        .get_driver_name = amdgpu_userq_fence_get_driver_name,
>>>>        .get_timeline_name = amdgpu_userq_fence_get_timeline_name,
>>>>        .signaled = amdgpu_userq_fence_signaled,
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_tlb_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_tlb_fence.c
>>>> index 51cddfa3f1e8..5d26797356a3 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_tlb_fence.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_tlb_fence.c
>>>> @@ -71,7 +71,6 @@ static void amdgpu_tlb_fence_work(struct work_struct *work)
>>>>    }
>>>>      static const struct dma_fence_ops amdgpu_tlb_fence_ops = {
>>>> -    .use_64bit_seqno = true,
>>>>        .get_driver_name = amdgpu_tlb_fence_get_driver_name,
>>>>        .get_timeline_name = amdgpu_tlb_fence_get_timeline_name
>>>>    };
>>>> @@ -101,8 +100,8 @@ void amdgpu_vm_tlb_fence_create(struct amdgpu_device *adev, struct amdgpu_vm *vm
>>>>        INIT_WORK(&f->work, amdgpu_tlb_fence_work);
>>>>        spin_lock_init(&f->lock);
>>>>    -    dma_fence_init(&f->base, &amdgpu_tlb_fence_ops, &f->lock,
>>>> -               vm->tlb_fence_context, atomic64_read(&vm->tlb_seq));
>>>> +    dma_fence_init64(&f->base, &amdgpu_tlb_fence_ops, &f->lock,
>>>> +             vm->tlb_fence_context, atomic64_read(&vm->tlb_seq));
>>>>          /* TODO: We probably need a separate wq here */
>>>>        dma_fence_get(&f->base);
>>>> diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
>>>> index 48b5202c531d..a34a0dcdc446 100644
>>>> --- a/include/linux/dma-fence.h
>>>> +++ b/include/linux/dma-fence.h
>>>> @@ -97,6 +97,7 @@ struct dma_fence {
>>>>    };
>>>>      enum dma_fence_flag_bits {
>>>> +    DMA_FENCE_FLAG_SEQNO64_BIT,
>>>>        DMA_FENCE_FLAG_SIGNALED_BIT,
>>>>        DMA_FENCE_FLAG_TIMESTAMP_BIT,
>>>>        DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT,
>>>> @@ -124,14 +125,6 @@ struct dma_fence_cb {
>>>>     *
>>>>     */
>>>>    struct dma_fence_ops {
>>>> -    /**
>>>> -     * @use_64bit_seqno:
>>>> -     *
>>>> -     * True if this dma_fence implementation uses 64bit seqno, false
>>>> -     * otherwise.
>>>> -     */
>>>> -    bool use_64bit_seqno;
>>>> -
>>>>        /**
>>>>         * @get_driver_name:
>>>>         *
>>>> @@ -262,6 +255,9 @@ struct dma_fence_ops {
>>>>    void dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops,
>>>>                spinlock_t *lock, u64 context, u64 seqno);
>>>>    +void dma_fence_init64(struct dma_fence *fence, const struct dma_fence_ops *ops,
>>>> +              spinlock_t *lock, u64 context, u64 seqno);
>>>> +
>>>>    void dma_fence_release(struct kref *kref);
>>>>    void dma_fence_free(struct dma_fence *fence);
>>>>    void dma_fence_describe(struct dma_fence *fence, struct seq_file *seq);
>>>> @@ -454,7 +450,7 @@ static inline bool __dma_fence_is_later(struct dma_fence *fence, u64 f1, u64 f2)
>>>>         * 32bit sequence numbers. Use a 64bit compare when the driver says to
>>>>         * do so.
>>>>         */
>>>> -    if (fence->ops->use_64bit_seqno)
>>>> +    if (test_bit(DMA_FENCE_FLAG_SEQNO64_BIT, &fence->flags))
>>>>            return f1 > f2;
>>>>          return (int)(lower_32_bits(f1) - lower_32_bits(f2)) > 0;
>>>
>>
> 

Re: [PATCH v4 2/9] dma-fence: Use a flag for 64-bit seqnos

[Tvrtko Ursulin]

On 02/06/2025 19:00, Christian König wrote:
> On 6/2/25 17:25, Tvrtko Ursulin wrote:
>>
>> On 02/06/2025 15:42, Christian König wrote:
>>> On 6/2/25 15:05, Tvrtko Ursulin wrote:
>>>>
>>>> Hi,
>>>>
>>>> On 15/05/2025 14:15, Christian König wrote:
>>>>> Hey drm-misc maintainers,
>>>>>
>>>>> can you guys please backmerge drm-next into drm-misc-next?
>>>>>
>>>>> I want to push this patch here but it depends on changes which are partially in drm-next and partially in drm-misc-next.
>>>>
>>>> Looks like the backmerge is still pending?
>>>
>>> Yes, @Maarten, @Maxime and @Thomas ping on this.
>>>
>>>> In the meantime, Christian, any chance you will have some bandwith to think about the tail end of the series? Specifically patch 6 and how that is used onward.
>>>
>>> Well the RCU grace period is quite a nifty hack. I wanted to go over it again after merging the first patches from this series.
>>>
>>> In general looks like a good idea to me, I just don't like that we explicitely need to expose dma_fence_access_begin() and dma_fence_access_end().
>>>
>>> Especially we can't do that while calling fence->ops->release.
>>
>> Hm why not? You think something will take offence of the rcu_read_lock()?
> 
> Yes, especially it is perfectly legitimate to call synchronize_rcu() or lock semaphores/mutexes from that callback.
> 
> Either keep the RCU critical section only for the trace or even better come up with some different approach, e.g. copying the string under the RCU lock or something like that.

Hmm but the kerneldoc explicity says callback can be called from irq 
context:

	/**
	 * @release:
	 *
	 * Called on destruction of fence to release additional resources.
	 * Can be called from irq context.  This callback is optional. If it is
	 * NULL, then dma_fence_free() is instead called as the default
	 * implementation.
	 */
	void (*release)(struct dma_fence *fence);


Regards,

Tvrtko

> 
> Regards,
> Christian.
> 
>>
>> Regards,
>>
>> Tvrtko
>>
>>>>> On 5/15/25 11:49, Tvrtko Ursulin wrote:
>>>>>> With the goal of reducing the need for drivers to touch (and dereference)
>>>>>> fence->ops, we move the 64-bit seqnos flag from struct dma_fence_ops to
>>>>>> the fence->flags.
>>>>>>
>>>>>> Drivers which were setting this flag are changed to use new
>>>>>> dma_fence_init64() instead of dma_fence_init().
>>>>>>
>>>>>> v2:
>>>>>>     * Streamlined init and added kerneldoc.
>>>>>>     * Rebase for amdgpu userq which landed since.
>>>>>>
>>>>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
>>>>>> Reviewed-by: Christian König <christian.koenig@amd.com> # v1
>>>>>> ---
>>>>>>     drivers/dma-buf/dma-fence-chain.c             |  5 +-
>>>>>>     drivers/dma-buf/dma-fence.c                   | 69 ++++++++++++++-----
>>>>>>     .../drm/amd/amdgpu/amdgpu_eviction_fence.c    |  7 +-
>>>>>>     .../gpu/drm/amd/amdgpu/amdgpu_userq_fence.c   |  5 +-
>>>>>>     .../gpu/drm/amd/amdgpu/amdgpu_vm_tlb_fence.c  |  5 +-
>>>>>>     include/linux/dma-fence.h                     | 14 ++--
>>>>>>     6 files changed, 64 insertions(+), 41 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/dma-buf/dma-fence-chain.c b/drivers/dma-buf/dma-fence-chain.c
>>>>>> index 90424f23fd73..a8a90acf4f34 100644
>>>>>> --- a/drivers/dma-buf/dma-fence-chain.c
>>>>>> +++ b/drivers/dma-buf/dma-fence-chain.c
>>>>>> @@ -218,7 +218,6 @@ static void dma_fence_chain_set_deadline(struct dma_fence *fence,
>>>>>>     }
>>>>>>       const struct dma_fence_ops dma_fence_chain_ops = {
>>>>>> -    .use_64bit_seqno = true,
>>>>>>         .get_driver_name = dma_fence_chain_get_driver_name,
>>>>>>         .get_timeline_name = dma_fence_chain_get_timeline_name,
>>>>>>         .enable_signaling = dma_fence_chain_enable_signaling,
>>>>>> @@ -262,8 +261,8 @@ void dma_fence_chain_init(struct dma_fence_chain *chain,
>>>>>>                 seqno = max(prev->seqno, seqno);
>>>>>>         }
>>>>>>     -    dma_fence_init(&chain->base, &dma_fence_chain_ops,
>>>>>> -               &chain->lock, context, seqno);
>>>>>> +    dma_fence_init64(&chain->base, &dma_fence_chain_ops, &chain->lock,
>>>>>> +             context, seqno);
>>>>>>           /*
>>>>>>          * Chaining dma_fence_chain container together is only allowed through
>>>>>> diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
>>>>>> index f0cdd3e99d36..705b59787731 100644
>>>>>> --- a/drivers/dma-buf/dma-fence.c
>>>>>> +++ b/drivers/dma-buf/dma-fence.c
>>>>>> @@ -989,24 +989,9 @@ void dma_fence_describe(struct dma_fence *fence, struct seq_file *seq)
>>>>>>     }
>>>>>>     EXPORT_SYMBOL(dma_fence_describe);
>>>>>>     -/**
>>>>>> - * 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
>>>>>> - * @context: the execution context this fence is run on
>>>>>> - * @seqno: a linear increasing sequence number for this context
>>>>>> - *
>>>>>> - * Initializes an allocated fence, the caller doesn't have to keep its
>>>>>> - * refcount after committing with this fence, but it will need to hold a
>>>>>> - * refcount again if &dma_fence_ops.enable_signaling gets called.
>>>>>> - *
>>>>>> - * context and seqno are used for easy comparison between fences, allowing
>>>>>> - * to check which fence is later by simply using dma_fence_later().
>>>>>> - */
>>>>>> -void
>>>>>> -dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops,
>>>>>> -           spinlock_t *lock, u64 context, u64 seqno)
>>>>>> +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);
>>>>>> @@ -1017,9 +1002,55 @@ dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops,
>>>>>>         fence->lock = lock;
>>>>>>         fence->context = context;
>>>>>>         fence->seqno = seqno;
>>>>>> -    fence->flags = 0UL;
>>>>>> +    fence->flags = flags;
>>>>>>         fence->error = 0;
>>>>>>           trace_dma_fence_init(fence);
>>>>>>     }
>>>>>> +
>>>>>> +/**
>>>>>> + * 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
>>>>>> + * @context: the execution context this fence is run on
>>>>>> + * @seqno: a linear increasing sequence number for this context
>>>>>> + *
>>>>>> + * Initializes an allocated fence, the caller doesn't have to keep its
>>>>>> + * refcount after committing with this fence, but it will need to hold a
>>>>>> + * refcount again if &dma_fence_ops.enable_signaling gets called.
>>>>>> + *
>>>>>> + * context and seqno are used for easy comparison between fences, allowing
>>>>>> + * to check which fence is later by simply using dma_fence_later().
>>>>>> + */
>>>>>> +void
>>>>>> +dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops,
>>>>>> +           spinlock_t *lock, u64 context, u64 seqno)
>>>>>> +{
>>>>>> +    __dma_fence_init(fence, ops, lock, context, seqno, 0UL);
>>>>>> +}
>>>>>>     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
>>>>>> + * @context: the execution context this fence is run on
>>>>>> + * @seqno: a linear increasing sequence number for this context
>>>>>> + *
>>>>>> + * Initializes an allocated fence, the caller doesn't have to keep its
>>>>>> + * refcount after committing with this fence, but it will need to hold a
>>>>>> + * refcount again if &dma_fence_ops.enable_signaling gets called.
>>>>>> + *
>>>>>> + * Context and seqno are used for easy comparison between fences, allowing
>>>>>> + * to check which fence is later by simply using dma_fence_later().
>>>>>> + */
>>>>>> +void
>>>>>> +dma_fence_init64(struct dma_fence *fence, const struct dma_fence_ops *ops,
>>>>>> +         spinlock_t *lock, u64 context, u64 seqno)
>>>>>> +{
>>>>>> +    __dma_fence_init(fence, ops, lock, context, seqno,
>>>>>> +             BIT(DMA_FENCE_FLAG_SEQNO64_BIT));
>>>>>> +}
>>>>>> +EXPORT_SYMBOL(dma_fence_init64);
>>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.c
>>>>>> index 1a7469543db5..79713421bffe 100644
>>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.c
>>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.c
>>>>>> @@ -134,7 +134,6 @@ static bool amdgpu_eviction_fence_enable_signaling(struct dma_fence *f)
>>>>>>     }
>>>>>>       static const struct dma_fence_ops amdgpu_eviction_fence_ops = {
>>>>>> -    .use_64bit_seqno = true,
>>>>>>         .get_driver_name = amdgpu_eviction_fence_get_driver_name,
>>>>>>         .get_timeline_name = amdgpu_eviction_fence_get_timeline_name,
>>>>>>         .enable_signaling = amdgpu_eviction_fence_enable_signaling,
>>>>>> @@ -160,9 +159,9 @@ amdgpu_eviction_fence_create(struct amdgpu_eviction_fence_mgr *evf_mgr)
>>>>>>         ev_fence->evf_mgr = evf_mgr;
>>>>>>         get_task_comm(ev_fence->timeline_name, current);
>>>>>>         spin_lock_init(&ev_fence->lock);
>>>>>> -    dma_fence_init(&ev_fence->base, &amdgpu_eviction_fence_ops,
>>>>>> -               &ev_fence->lock, evf_mgr->ev_fence_ctx,
>>>>>> -               atomic_inc_return(&evf_mgr->ev_fence_seq));
>>>>>> +    dma_fence_init64(&ev_fence->base, &amdgpu_eviction_fence_ops,
>>>>>> +             &ev_fence->lock, evf_mgr->ev_fence_ctx,
>>>>>> +             atomic_inc_return(&evf_mgr->ev_fence_seq));
>>>>>>         return ev_fence;
>>>>>>     }
>>>>>>     diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
>>>>>> index 029cb24c28b3..5e92d00a591f 100644
>>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
>>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
>>>>>> @@ -239,8 +239,8 @@ static int amdgpu_userq_fence_create(struct amdgpu_usermode_queue *userq,
>>>>>>         fence = &userq_fence->base;
>>>>>>         userq_fence->fence_drv = fence_drv;
>>>>>>     -    dma_fence_init(fence, &amdgpu_userq_fence_ops, &userq_fence->lock,
>>>>>> -               fence_drv->context, seq);
>>>>>> +    dma_fence_init64(fence, &amdgpu_userq_fence_ops, &userq_fence->lock,
>>>>>> +             fence_drv->context, seq);
>>>>>>           amdgpu_userq_fence_driver_get(fence_drv);
>>>>>>         dma_fence_get(fence);
>>>>>> @@ -334,7 +334,6 @@ static void amdgpu_userq_fence_release(struct dma_fence *f)
>>>>>>     }
>>>>>>       static const struct dma_fence_ops amdgpu_userq_fence_ops = {
>>>>>> -    .use_64bit_seqno = true,
>>>>>>         .get_driver_name = amdgpu_userq_fence_get_driver_name,
>>>>>>         .get_timeline_name = amdgpu_userq_fence_get_timeline_name,
>>>>>>         .signaled = amdgpu_userq_fence_signaled,
>>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_tlb_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_tlb_fence.c
>>>>>> index 51cddfa3f1e8..5d26797356a3 100644
>>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_tlb_fence.c
>>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_tlb_fence.c
>>>>>> @@ -71,7 +71,6 @@ static void amdgpu_tlb_fence_work(struct work_struct *work)
>>>>>>     }
>>>>>>       static const struct dma_fence_ops amdgpu_tlb_fence_ops = {
>>>>>> -    .use_64bit_seqno = true,
>>>>>>         .get_driver_name = amdgpu_tlb_fence_get_driver_name,
>>>>>>         .get_timeline_name = amdgpu_tlb_fence_get_timeline_name
>>>>>>     };
>>>>>> @@ -101,8 +100,8 @@ void amdgpu_vm_tlb_fence_create(struct amdgpu_device *adev, struct amdgpu_vm *vm
>>>>>>         INIT_WORK(&f->work, amdgpu_tlb_fence_work);
>>>>>>         spin_lock_init(&f->lock);
>>>>>>     -    dma_fence_init(&f->base, &amdgpu_tlb_fence_ops, &f->lock,
>>>>>> -               vm->tlb_fence_context, atomic64_read(&vm->tlb_seq));
>>>>>> +    dma_fence_init64(&f->base, &amdgpu_tlb_fence_ops, &f->lock,
>>>>>> +             vm->tlb_fence_context, atomic64_read(&vm->tlb_seq));
>>>>>>           /* TODO: We probably need a separate wq here */
>>>>>>         dma_fence_get(&f->base);
>>>>>> diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
>>>>>> index 48b5202c531d..a34a0dcdc446 100644
>>>>>> --- a/include/linux/dma-fence.h
>>>>>> +++ b/include/linux/dma-fence.h
>>>>>> @@ -97,6 +97,7 @@ struct dma_fence {
>>>>>>     };
>>>>>>       enum dma_fence_flag_bits {
>>>>>> +    DMA_FENCE_FLAG_SEQNO64_BIT,
>>>>>>         DMA_FENCE_FLAG_SIGNALED_BIT,
>>>>>>         DMA_FENCE_FLAG_TIMESTAMP_BIT,
>>>>>>         DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT,
>>>>>> @@ -124,14 +125,6 @@ struct dma_fence_cb {
>>>>>>      *
>>>>>>      */
>>>>>>     struct dma_fence_ops {
>>>>>> -    /**
>>>>>> -     * @use_64bit_seqno:
>>>>>> -     *
>>>>>> -     * True if this dma_fence implementation uses 64bit seqno, false
>>>>>> -     * otherwise.
>>>>>> -     */
>>>>>> -    bool use_64bit_seqno;
>>>>>> -
>>>>>>         /**
>>>>>>          * @get_driver_name:
>>>>>>          *
>>>>>> @@ -262,6 +255,9 @@ struct dma_fence_ops {
>>>>>>     void dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops,
>>>>>>                 spinlock_t *lock, u64 context, u64 seqno);
>>>>>>     +void dma_fence_init64(struct dma_fence *fence, const struct dma_fence_ops *ops,
>>>>>> +              spinlock_t *lock, u64 context, u64 seqno);
>>>>>> +
>>>>>>     void dma_fence_release(struct kref *kref);
>>>>>>     void dma_fence_free(struct dma_fence *fence);
>>>>>>     void dma_fence_describe(struct dma_fence *fence, struct seq_file *seq);
>>>>>> @@ -454,7 +450,7 @@ static inline bool __dma_fence_is_later(struct dma_fence *fence, u64 f1, u64 f2)
>>>>>>          * 32bit sequence numbers. Use a 64bit compare when the driver says to
>>>>>>          * do so.
>>>>>>          */
>>>>>> -    if (fence->ops->use_64bit_seqno)
>>>>>> +    if (test_bit(DMA_FENCE_FLAG_SEQNO64_BIT, &fence->flags))
>>>>>>             return f1 > f2;
>>>>>>           return (int)(lower_32_bits(f1) - lower_32_bits(f2)) > 0;
>>>>>
>>>>
>>>
>>
> 

Re: [PATCH v4 2/9] dma-fence: Use a flag for 64-bit seqnos

[Christian König]

On 6/3/25 13:30, Tvrtko Ursulin wrote:
> 
> On 02/06/2025 19:00, Christian König wrote:
>> On 6/2/25 17:25, Tvrtko Ursulin wrote:
>>>
>>> On 02/06/2025 15:42, Christian König wrote:
>>>> On 6/2/25 15:05, Tvrtko Ursulin wrote:
>>>>>
>>>>> Hi,
>>>>>
>>>>> On 15/05/2025 14:15, Christian König wrote:
>>>>>> Hey drm-misc maintainers,
>>>>>>
>>>>>> can you guys please backmerge drm-next into drm-misc-next?
>>>>>>
>>>>>> I want to push this patch here but it depends on changes which are partially in drm-next and partially in drm-misc-next.
>>>>>
>>>>> Looks like the backmerge is still pending?
>>>>
>>>> Yes, @Maarten, @Maxime and @Thomas ping on this.
>>>>
>>>>> In the meantime, Christian, any chance you will have some bandwith to think about the tail end of the series? Specifically patch 6 and how that is used onward.
>>>>
>>>> Well the RCU grace period is quite a nifty hack. I wanted to go over it again after merging the first patches from this series.
>>>>
>>>> In general looks like a good idea to me, I just don't like that we explicitely need to expose dma_fence_access_begin() and dma_fence_access_end().
>>>>
>>>> Especially we can't do that while calling fence->ops->release.
>>>
>>> Hm why not? You think something will take offence of the rcu_read_lock()?
>>
>> Yes, especially it is perfectly legitimate to call synchronize_rcu() or lock semaphores/mutexes from that callback.
>>
>> Either keep the RCU critical section only for the trace or even better come up with some different approach, e.g. copying the string under the RCU lock or something like that.
> 
> Hmm but the kerneldoc explicity says callback can be called from irq context:
> 
>     /**
>      * @release:
>      *
>      * Called on destruction of fence to release additional resources.
>      * Can be called from irq context.  This callback is optional. If it is
>      * NULL, then dma_fence_free() is instead called as the default
>      * implementation.
>      */
>     void (*release)(struct dma_fence *fence);

Ah, right. I mixed that up with the dma-buf object.

Yeah in that case that is probably harmless. We delegate the final free to a work item if necessary anyway.

But I would still like to avoid having the RCU cover the release as well. Or why is there any reason why we would explicitely want to do this?

Regards,
Christian.

> 
> 
> Regards,
> 
> Tvrtko
> 
>>
>> Regards,
>> Christian.
>>
>>>
>>> Regards,
>>>
>>> Tvrtko
>>>
>>>>>> On 5/15/25 11:49, Tvrtko Ursulin wrote:
>>>>>>> With the goal of reducing the need for drivers to touch (and dereference)
>>>>>>> fence->ops, we move the 64-bit seqnos flag from struct dma_fence_ops to
>>>>>>> the fence->flags.
>>>>>>>
>>>>>>> Drivers which were setting this flag are changed to use new
>>>>>>> dma_fence_init64() instead of dma_fence_init().
>>>>>>>
>>>>>>> v2:
>>>>>>>     * Streamlined init and added kerneldoc.
>>>>>>>     * Rebase for amdgpu userq which landed since.
>>>>>>>
>>>>>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
>>>>>>> Reviewed-by: Christian König <christian.koenig@amd.com> # v1
>>>>>>> ---
>>>>>>>     drivers/dma-buf/dma-fence-chain.c             |  5 +-
>>>>>>>     drivers/dma-buf/dma-fence.c                   | 69 ++++++++++++++-----
>>>>>>>     .../drm/amd/amdgpu/amdgpu_eviction_fence.c    |  7 +-
>>>>>>>     .../gpu/drm/amd/amdgpu/amdgpu_userq_fence.c   |  5 +-
>>>>>>>     .../gpu/drm/amd/amdgpu/amdgpu_vm_tlb_fence.c  |  5 +-
>>>>>>>     include/linux/dma-fence.h                     | 14 ++--
>>>>>>>     6 files changed, 64 insertions(+), 41 deletions(-)
>>>>>>>
>>>>>>> diff --git a/drivers/dma-buf/dma-fence-chain.c b/drivers/dma-buf/dma-fence-chain.c
>>>>>>> index 90424f23fd73..a8a90acf4f34 100644
>>>>>>> --- a/drivers/dma-buf/dma-fence-chain.c
>>>>>>> +++ b/drivers/dma-buf/dma-fence-chain.c
>>>>>>> @@ -218,7 +218,6 @@ static void dma_fence_chain_set_deadline(struct dma_fence *fence,
>>>>>>>     }
>>>>>>>       const struct dma_fence_ops dma_fence_chain_ops = {
>>>>>>> -    .use_64bit_seqno = true,
>>>>>>>         .get_driver_name = dma_fence_chain_get_driver_name,
>>>>>>>         .get_timeline_name = dma_fence_chain_get_timeline_name,
>>>>>>>         .enable_signaling = dma_fence_chain_enable_signaling,
>>>>>>> @@ -262,8 +261,8 @@ void dma_fence_chain_init(struct dma_fence_chain *chain,
>>>>>>>                 seqno = max(prev->seqno, seqno);
>>>>>>>         }
>>>>>>>     -    dma_fence_init(&chain->base, &dma_fence_chain_ops,
>>>>>>> -               &chain->lock, context, seqno);
>>>>>>> +    dma_fence_init64(&chain->base, &dma_fence_chain_ops, &chain->lock,
>>>>>>> +             context, seqno);
>>>>>>>           /*
>>>>>>>          * Chaining dma_fence_chain container together is only allowed through
>>>>>>> diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
>>>>>>> index f0cdd3e99d36..705b59787731 100644
>>>>>>> --- a/drivers/dma-buf/dma-fence.c
>>>>>>> +++ b/drivers/dma-buf/dma-fence.c
>>>>>>> @@ -989,24 +989,9 @@ void dma_fence_describe(struct dma_fence *fence, struct seq_file *seq)
>>>>>>>     }
>>>>>>>     EXPORT_SYMBOL(dma_fence_describe);
>>>>>>>     -/**
>>>>>>> - * 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
>>>>>>> - * @context: the execution context this fence is run on
>>>>>>> - * @seqno: a linear increasing sequence number for this context
>>>>>>> - *
>>>>>>> - * Initializes an allocated fence, the caller doesn't have to keep its
>>>>>>> - * refcount after committing with this fence, but it will need to hold a
>>>>>>> - * refcount again if &dma_fence_ops.enable_signaling gets called.
>>>>>>> - *
>>>>>>> - * context and seqno are used for easy comparison between fences, allowing
>>>>>>> - * to check which fence is later by simply using dma_fence_later().
>>>>>>> - */
>>>>>>> -void
>>>>>>> -dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops,
>>>>>>> -           spinlock_t *lock, u64 context, u64 seqno)
>>>>>>> +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);
>>>>>>> @@ -1017,9 +1002,55 @@ dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops,
>>>>>>>         fence->lock = lock;
>>>>>>>         fence->context = context;
>>>>>>>         fence->seqno = seqno;
>>>>>>> -    fence->flags = 0UL;
>>>>>>> +    fence->flags = flags;
>>>>>>>         fence->error = 0;
>>>>>>>           trace_dma_fence_init(fence);
>>>>>>>     }
>>>>>>> +
>>>>>>> +/**
>>>>>>> + * 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
>>>>>>> + * @context: the execution context this fence is run on
>>>>>>> + * @seqno: a linear increasing sequence number for this context
>>>>>>> + *
>>>>>>> + * Initializes an allocated fence, the caller doesn't have to keep its
>>>>>>> + * refcount after committing with this fence, but it will need to hold a
>>>>>>> + * refcount again if &dma_fence_ops.enable_signaling gets called.
>>>>>>> + *
>>>>>>> + * context and seqno are used for easy comparison between fences, allowing
>>>>>>> + * to check which fence is later by simply using dma_fence_later().
>>>>>>> + */
>>>>>>> +void
>>>>>>> +dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops,
>>>>>>> +           spinlock_t *lock, u64 context, u64 seqno)
>>>>>>> +{
>>>>>>> +    __dma_fence_init(fence, ops, lock, context, seqno, 0UL);
>>>>>>> +}
>>>>>>>     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
>>>>>>> + * @context: the execution context this fence is run on
>>>>>>> + * @seqno: a linear increasing sequence number for this context
>>>>>>> + *
>>>>>>> + * Initializes an allocated fence, the caller doesn't have to keep its
>>>>>>> + * refcount after committing with this fence, but it will need to hold a
>>>>>>> + * refcount again if &dma_fence_ops.enable_signaling gets called.
>>>>>>> + *
>>>>>>> + * Context and seqno are used for easy comparison between fences, allowing
>>>>>>> + * to check which fence is later by simply using dma_fence_later().
>>>>>>> + */
>>>>>>> +void
>>>>>>> +dma_fence_init64(struct dma_fence *fence, const struct dma_fence_ops *ops,
>>>>>>> +         spinlock_t *lock, u64 context, u64 seqno)
>>>>>>> +{
>>>>>>> +    __dma_fence_init(fence, ops, lock, context, seqno,
>>>>>>> +             BIT(DMA_FENCE_FLAG_SEQNO64_BIT));
>>>>>>> +}
>>>>>>> +EXPORT_SYMBOL(dma_fence_init64);
>>>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.c
>>>>>>> index 1a7469543db5..79713421bffe 100644
>>>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.c
>>>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.c
>>>>>>> @@ -134,7 +134,6 @@ static bool amdgpu_eviction_fence_enable_signaling(struct dma_fence *f)
>>>>>>>     }
>>>>>>>       static const struct dma_fence_ops amdgpu_eviction_fence_ops = {
>>>>>>> -    .use_64bit_seqno = true,
>>>>>>>         .get_driver_name = amdgpu_eviction_fence_get_driver_name,
>>>>>>>         .get_timeline_name = amdgpu_eviction_fence_get_timeline_name,
>>>>>>>         .enable_signaling = amdgpu_eviction_fence_enable_signaling,
>>>>>>> @@ -160,9 +159,9 @@ amdgpu_eviction_fence_create(struct amdgpu_eviction_fence_mgr *evf_mgr)
>>>>>>>         ev_fence->evf_mgr = evf_mgr;
>>>>>>>         get_task_comm(ev_fence->timeline_name, current);
>>>>>>>         spin_lock_init(&ev_fence->lock);
>>>>>>> -    dma_fence_init(&ev_fence->base, &amdgpu_eviction_fence_ops,
>>>>>>> -               &ev_fence->lock, evf_mgr->ev_fence_ctx,
>>>>>>> -               atomic_inc_return(&evf_mgr->ev_fence_seq));
>>>>>>> +    dma_fence_init64(&ev_fence->base, &amdgpu_eviction_fence_ops,
>>>>>>> +             &ev_fence->lock, evf_mgr->ev_fence_ctx,
>>>>>>> +             atomic_inc_return(&evf_mgr->ev_fence_seq));
>>>>>>>         return ev_fence;
>>>>>>>     }
>>>>>>>     diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
>>>>>>> index 029cb24c28b3..5e92d00a591f 100644
>>>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
>>>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
>>>>>>> @@ -239,8 +239,8 @@ static int amdgpu_userq_fence_create(struct amdgpu_usermode_queue *userq,
>>>>>>>         fence = &userq_fence->base;
>>>>>>>         userq_fence->fence_drv = fence_drv;
>>>>>>>     -    dma_fence_init(fence, &amdgpu_userq_fence_ops, &userq_fence->lock,
>>>>>>> -               fence_drv->context, seq);
>>>>>>> +    dma_fence_init64(fence, &amdgpu_userq_fence_ops, &userq_fence->lock,
>>>>>>> +             fence_drv->context, seq);
>>>>>>>           amdgpu_userq_fence_driver_get(fence_drv);
>>>>>>>         dma_fence_get(fence);
>>>>>>> @@ -334,7 +334,6 @@ static void amdgpu_userq_fence_release(struct dma_fence *f)
>>>>>>>     }
>>>>>>>       static const struct dma_fence_ops amdgpu_userq_fence_ops = {
>>>>>>> -    .use_64bit_seqno = true,
>>>>>>>         .get_driver_name = amdgpu_userq_fence_get_driver_name,
>>>>>>>         .get_timeline_name = amdgpu_userq_fence_get_timeline_name,
>>>>>>>         .signaled = amdgpu_userq_fence_signaled,
>>>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_tlb_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_tlb_fence.c
>>>>>>> index 51cddfa3f1e8..5d26797356a3 100644
>>>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_tlb_fence.c
>>>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_tlb_fence.c
>>>>>>> @@ -71,7 +71,6 @@ static void amdgpu_tlb_fence_work(struct work_struct *work)
>>>>>>>     }
>>>>>>>       static const struct dma_fence_ops amdgpu_tlb_fence_ops = {
>>>>>>> -    .use_64bit_seqno = true,
>>>>>>>         .get_driver_name = amdgpu_tlb_fence_get_driver_name,
>>>>>>>         .get_timeline_name = amdgpu_tlb_fence_get_timeline_name
>>>>>>>     };
>>>>>>> @@ -101,8 +100,8 @@ void amdgpu_vm_tlb_fence_create(struct amdgpu_device *adev, struct amdgpu_vm *vm
>>>>>>>         INIT_WORK(&f->work, amdgpu_tlb_fence_work);
>>>>>>>         spin_lock_init(&f->lock);
>>>>>>>     -    dma_fence_init(&f->base, &amdgpu_tlb_fence_ops, &f->lock,
>>>>>>> -               vm->tlb_fence_context, atomic64_read(&vm->tlb_seq));
>>>>>>> +    dma_fence_init64(&f->base, &amdgpu_tlb_fence_ops, &f->lock,
>>>>>>> +             vm->tlb_fence_context, atomic64_read(&vm->tlb_seq));
>>>>>>>           /* TODO: We probably need a separate wq here */
>>>>>>>         dma_fence_get(&f->base);
>>>>>>> diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
>>>>>>> index 48b5202c531d..a34a0dcdc446 100644
>>>>>>> --- a/include/linux/dma-fence.h
>>>>>>> +++ b/include/linux/dma-fence.h
>>>>>>> @@ -97,6 +97,7 @@ struct dma_fence {
>>>>>>>     };
>>>>>>>       enum dma_fence_flag_bits {
>>>>>>> +    DMA_FENCE_FLAG_SEQNO64_BIT,
>>>>>>>         DMA_FENCE_FLAG_SIGNALED_BIT,
>>>>>>>         DMA_FENCE_FLAG_TIMESTAMP_BIT,
>>>>>>>         DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT,
>>>>>>> @@ -124,14 +125,6 @@ struct dma_fence_cb {
>>>>>>>      *
>>>>>>>      */
>>>>>>>     struct dma_fence_ops {
>>>>>>> -    /**
>>>>>>> -     * @use_64bit_seqno:
>>>>>>> -     *
>>>>>>> -     * True if this dma_fence implementation uses 64bit seqno, false
>>>>>>> -     * otherwise.
>>>>>>> -     */
>>>>>>> -    bool use_64bit_seqno;
>>>>>>> -
>>>>>>>         /**
>>>>>>>          * @get_driver_name:
>>>>>>>          *
>>>>>>> @@ -262,6 +255,9 @@ struct dma_fence_ops {
>>>>>>>     void dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops,
>>>>>>>                 spinlock_t *lock, u64 context, u64 seqno);
>>>>>>>     +void dma_fence_init64(struct dma_fence *fence, const struct dma_fence_ops *ops,
>>>>>>> +              spinlock_t *lock, u64 context, u64 seqno);
>>>>>>> +
>>>>>>>     void dma_fence_release(struct kref *kref);
>>>>>>>     void dma_fence_free(struct dma_fence *fence);
>>>>>>>     void dma_fence_describe(struct dma_fence *fence, struct seq_file *seq);
>>>>>>> @@ -454,7 +450,7 @@ static inline bool __dma_fence_is_later(struct dma_fence *fence, u64 f1, u64 f2)
>>>>>>>          * 32bit sequence numbers. Use a 64bit compare when the driver says to
>>>>>>>          * do so.
>>>>>>>          */
>>>>>>> -    if (fence->ops->use_64bit_seqno)
>>>>>>> +    if (test_bit(DMA_FENCE_FLAG_SEQNO64_BIT, &fence->flags))
>>>>>>>             return f1 > f2;
>>>>>>>           return (int)(lower_32_bits(f1) - lower_32_bits(f2)) > 0;
>>>>>>
>>>>>
>>>>
>>>
>>
> 

Re: [PATCH v4 2/9] dma-fence: Use a flag for 64-bit seqnos

[Tvrtko Ursulin]

On 03/06/2025 13:40, Christian König wrote:
> On 6/3/25 13:30, Tvrtko Ursulin wrote:
>>
>> On 02/06/2025 19:00, Christian König wrote:
>>> On 6/2/25 17:25, Tvrtko Ursulin wrote:
>>>>
>>>> On 02/06/2025 15:42, Christian König wrote:
>>>>> On 6/2/25 15:05, Tvrtko Ursulin wrote:
>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> On 15/05/2025 14:15, Christian König wrote:
>>>>>>> Hey drm-misc maintainers,
>>>>>>>
>>>>>>> can you guys please backmerge drm-next into drm-misc-next?
>>>>>>>
>>>>>>> I want to push this patch here but it depends on changes which are partially in drm-next and partially in drm-misc-next.
>>>>>>
>>>>>> Looks like the backmerge is still pending?
>>>>>
>>>>> Yes, @Maarten, @Maxime and @Thomas ping on this.
>>>>>
>>>>>> In the meantime, Christian, any chance you will have some bandwith to think about the tail end of the series? Specifically patch 6 and how that is used onward.
>>>>>
>>>>> Well the RCU grace period is quite a nifty hack. I wanted to go over it again after merging the first patches from this series.
>>>>>
>>>>> In general looks like a good idea to me, I just don't like that we explicitely need to expose dma_fence_access_begin() and dma_fence_access_end().
>>>>>
>>>>> Especially we can't do that while calling fence->ops->release.
>>>>
>>>> Hm why not? You think something will take offence of the rcu_read_lock()?
>>>
>>> Yes, especially it is perfectly legitimate to call synchronize_rcu() or lock semaphores/mutexes from that callback.
>>>
>>> Either keep the RCU critical section only for the trace or even better come up with some different approach, e.g. copying the string under the RCU lock or something like that.
>>
>> Hmm but the kerneldoc explicity says callback can be called from irq context:
>>
>>      /**
>>       * @release:
>>       *
>>       * Called on destruction of fence to release additional resources.
>>       * Can be called from irq context.  This callback is optional. If it is
>>       * NULL, then dma_fence_free() is instead called as the default
>>       * implementation.
>>       */
>>      void (*release)(struct dma_fence *fence);
> 
> Ah, right. I mixed that up with the dma-buf object.
> 
> Yeah in that case that is probably harmless. We delegate the final free to a work item if necessary anyway.
> 
> But I would still like to avoid having the RCU cover the release as well. Or why is there any reason why we would explicitely want to do this?

I can't remember there was a particular reason. Obviously the 
driver/timeline name vfunc access I needed a 
dma_fence_access_begin/end() block so maybe I was just sloppy and put 
the end at the end of the function instead of at the end of the block 
which can dereference them.

I will pull it earlier for the next respin, assuming no gotchas get 
discovered in the process.

Regards,

Tvrtko

> 
> Regards,
> Christian.
> 
>>
>>
>> Regards,
>>
>> Tvrtko
>>
>>>
>>> Regards,
>>> Christian.
>>>
>>>>
>>>> Regards,
>>>>
>>>> Tvrtko
>>>>
>>>>>>> On 5/15/25 11:49, Tvrtko Ursulin wrote:
>>>>>>>> With the goal of reducing the need for drivers to touch (and dereference)
>>>>>>>> fence->ops, we move the 64-bit seqnos flag from struct dma_fence_ops to
>>>>>>>> the fence->flags.
>>>>>>>>
>>>>>>>> Drivers which were setting this flag are changed to use new
>>>>>>>> dma_fence_init64() instead of dma_fence_init().
>>>>>>>>
>>>>>>>> v2:
>>>>>>>>      * Streamlined init and added kerneldoc.
>>>>>>>>      * Rebase for amdgpu userq which landed since.
>>>>>>>>
>>>>>>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
>>>>>>>> Reviewed-by: Christian König <christian.koenig@amd.com> # v1
>>>>>>>> ---
>>>>>>>>      drivers/dma-buf/dma-fence-chain.c             |  5 +-
>>>>>>>>      drivers/dma-buf/dma-fence.c                   | 69 ++++++++++++++-----
>>>>>>>>      .../drm/amd/amdgpu/amdgpu_eviction_fence.c    |  7 +-
>>>>>>>>      .../gpu/drm/amd/amdgpu/amdgpu_userq_fence.c   |  5 +-
>>>>>>>>      .../gpu/drm/amd/amdgpu/amdgpu_vm_tlb_fence.c  |  5 +-
>>>>>>>>      include/linux/dma-fence.h                     | 14 ++--
>>>>>>>>      6 files changed, 64 insertions(+), 41 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/drivers/dma-buf/dma-fence-chain.c b/drivers/dma-buf/dma-fence-chain.c
>>>>>>>> index 90424f23fd73..a8a90acf4f34 100644
>>>>>>>> --- a/drivers/dma-buf/dma-fence-chain.c
>>>>>>>> +++ b/drivers/dma-buf/dma-fence-chain.c
>>>>>>>> @@ -218,7 +218,6 @@ static void dma_fence_chain_set_deadline(struct dma_fence *fence,
>>>>>>>>      }
>>>>>>>>        const struct dma_fence_ops dma_fence_chain_ops = {
>>>>>>>> -    .use_64bit_seqno = true,
>>>>>>>>          .get_driver_name = dma_fence_chain_get_driver_name,
>>>>>>>>          .get_timeline_name = dma_fence_chain_get_timeline_name,
>>>>>>>>          .enable_signaling = dma_fence_chain_enable_signaling,
>>>>>>>> @@ -262,8 +261,8 @@ void dma_fence_chain_init(struct dma_fence_chain *chain,
>>>>>>>>                  seqno = max(prev->seqno, seqno);
>>>>>>>>          }
>>>>>>>>      -    dma_fence_init(&chain->base, &dma_fence_chain_ops,
>>>>>>>> -               &chain->lock, context, seqno);
>>>>>>>> +    dma_fence_init64(&chain->base, &dma_fence_chain_ops, &chain->lock,
>>>>>>>> +             context, seqno);
>>>>>>>>            /*
>>>>>>>>           * Chaining dma_fence_chain container together is only allowed through
>>>>>>>> diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
>>>>>>>> index f0cdd3e99d36..705b59787731 100644
>>>>>>>> --- a/drivers/dma-buf/dma-fence.c
>>>>>>>> +++ b/drivers/dma-buf/dma-fence.c
>>>>>>>> @@ -989,24 +989,9 @@ void dma_fence_describe(struct dma_fence *fence, struct seq_file *seq)
>>>>>>>>      }
>>>>>>>>      EXPORT_SYMBOL(dma_fence_describe);
>>>>>>>>      -/**
>>>>>>>> - * 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
>>>>>>>> - * @context: the execution context this fence is run on
>>>>>>>> - * @seqno: a linear increasing sequence number for this context
>>>>>>>> - *
>>>>>>>> - * Initializes an allocated fence, the caller doesn't have to keep its
>>>>>>>> - * refcount after committing with this fence, but it will need to hold a
>>>>>>>> - * refcount again if &dma_fence_ops.enable_signaling gets called.
>>>>>>>> - *
>>>>>>>> - * context and seqno are used for easy comparison between fences, allowing
>>>>>>>> - * to check which fence is later by simply using dma_fence_later().
>>>>>>>> - */
>>>>>>>> -void
>>>>>>>> -dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops,
>>>>>>>> -           spinlock_t *lock, u64 context, u64 seqno)
>>>>>>>> +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);
>>>>>>>> @@ -1017,9 +1002,55 @@ dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops,
>>>>>>>>          fence->lock = lock;
>>>>>>>>          fence->context = context;
>>>>>>>>          fence->seqno = seqno;
>>>>>>>> -    fence->flags = 0UL;
>>>>>>>> +    fence->flags = flags;
>>>>>>>>          fence->error = 0;
>>>>>>>>            trace_dma_fence_init(fence);
>>>>>>>>      }
>>>>>>>> +
>>>>>>>> +/**
>>>>>>>> + * 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
>>>>>>>> + * @context: the execution context this fence is run on
>>>>>>>> + * @seqno: a linear increasing sequence number for this context
>>>>>>>> + *
>>>>>>>> + * Initializes an allocated fence, the caller doesn't have to keep its
>>>>>>>> + * refcount after committing with this fence, but it will need to hold a
>>>>>>>> + * refcount again if &dma_fence_ops.enable_signaling gets called.
>>>>>>>> + *
>>>>>>>> + * context and seqno are used for easy comparison between fences, allowing
>>>>>>>> + * to check which fence is later by simply using dma_fence_later().
>>>>>>>> + */
>>>>>>>> +void
>>>>>>>> +dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops,
>>>>>>>> +           spinlock_t *lock, u64 context, u64 seqno)
>>>>>>>> +{
>>>>>>>> +    __dma_fence_init(fence, ops, lock, context, seqno, 0UL);
>>>>>>>> +}
>>>>>>>>      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
>>>>>>>> + * @context: the execution context this fence is run on
>>>>>>>> + * @seqno: a linear increasing sequence number for this context
>>>>>>>> + *
>>>>>>>> + * Initializes an allocated fence, the caller doesn't have to keep its
>>>>>>>> + * refcount after committing with this fence, but it will need to hold a
>>>>>>>> + * refcount again if &dma_fence_ops.enable_signaling gets called.
>>>>>>>> + *
>>>>>>>> + * Context and seqno are used for easy comparison between fences, allowing
>>>>>>>> + * to check which fence is later by simply using dma_fence_later().
>>>>>>>> + */
>>>>>>>> +void
>>>>>>>> +dma_fence_init64(struct dma_fence *fence, const struct dma_fence_ops *ops,
>>>>>>>> +         spinlock_t *lock, u64 context, u64 seqno)
>>>>>>>> +{
>>>>>>>> +    __dma_fence_init(fence, ops, lock, context, seqno,
>>>>>>>> +             BIT(DMA_FENCE_FLAG_SEQNO64_BIT));
>>>>>>>> +}
>>>>>>>> +EXPORT_SYMBOL(dma_fence_init64);
>>>>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.c
>>>>>>>> index 1a7469543db5..79713421bffe 100644
>>>>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.c
>>>>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.c
>>>>>>>> @@ -134,7 +134,6 @@ static bool amdgpu_eviction_fence_enable_signaling(struct dma_fence *f)
>>>>>>>>      }
>>>>>>>>        static const struct dma_fence_ops amdgpu_eviction_fence_ops = {
>>>>>>>> -    .use_64bit_seqno = true,
>>>>>>>>          .get_driver_name = amdgpu_eviction_fence_get_driver_name,
>>>>>>>>          .get_timeline_name = amdgpu_eviction_fence_get_timeline_name,
>>>>>>>>          .enable_signaling = amdgpu_eviction_fence_enable_signaling,
>>>>>>>> @@ -160,9 +159,9 @@ amdgpu_eviction_fence_create(struct amdgpu_eviction_fence_mgr *evf_mgr)
>>>>>>>>          ev_fence->evf_mgr = evf_mgr;
>>>>>>>>          get_task_comm(ev_fence->timeline_name, current);
>>>>>>>>          spin_lock_init(&ev_fence->lock);
>>>>>>>> -    dma_fence_init(&ev_fence->base, &amdgpu_eviction_fence_ops,
>>>>>>>> -               &ev_fence->lock, evf_mgr->ev_fence_ctx,
>>>>>>>> -               atomic_inc_return(&evf_mgr->ev_fence_seq));
>>>>>>>> +    dma_fence_init64(&ev_fence->base, &amdgpu_eviction_fence_ops,
>>>>>>>> +             &ev_fence->lock, evf_mgr->ev_fence_ctx,
>>>>>>>> +             atomic_inc_return(&evf_mgr->ev_fence_seq));
>>>>>>>>          return ev_fence;
>>>>>>>>      }
>>>>>>>>      diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
>>>>>>>> index 029cb24c28b3..5e92d00a591f 100644
>>>>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
>>>>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
>>>>>>>> @@ -239,8 +239,8 @@ static int amdgpu_userq_fence_create(struct amdgpu_usermode_queue *userq,
>>>>>>>>          fence = &userq_fence->base;
>>>>>>>>          userq_fence->fence_drv = fence_drv;
>>>>>>>>      -    dma_fence_init(fence, &amdgpu_userq_fence_ops, &userq_fence->lock,
>>>>>>>> -               fence_drv->context, seq);
>>>>>>>> +    dma_fence_init64(fence, &amdgpu_userq_fence_ops, &userq_fence->lock,
>>>>>>>> +             fence_drv->context, seq);
>>>>>>>>            amdgpu_userq_fence_driver_get(fence_drv);
>>>>>>>>          dma_fence_get(fence);
>>>>>>>> @@ -334,7 +334,6 @@ static void amdgpu_userq_fence_release(struct dma_fence *f)
>>>>>>>>      }
>>>>>>>>        static const struct dma_fence_ops amdgpu_userq_fence_ops = {
>>>>>>>> -    .use_64bit_seqno = true,
>>>>>>>>          .get_driver_name = amdgpu_userq_fence_get_driver_name,
>>>>>>>>          .get_timeline_name = amdgpu_userq_fence_get_timeline_name,
>>>>>>>>          .signaled = amdgpu_userq_fence_signaled,
>>>>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_tlb_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_tlb_fence.c
>>>>>>>> index 51cddfa3f1e8..5d26797356a3 100644
>>>>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_tlb_fence.c
>>>>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_tlb_fence.c
>>>>>>>> @@ -71,7 +71,6 @@ static void amdgpu_tlb_fence_work(struct work_struct *work)
>>>>>>>>      }
>>>>>>>>        static const struct dma_fence_ops amdgpu_tlb_fence_ops = {
>>>>>>>> -    .use_64bit_seqno = true,
>>>>>>>>          .get_driver_name = amdgpu_tlb_fence_get_driver_name,
>>>>>>>>          .get_timeline_name = amdgpu_tlb_fence_get_timeline_name
>>>>>>>>      };
>>>>>>>> @@ -101,8 +100,8 @@ void amdgpu_vm_tlb_fence_create(struct amdgpu_device *adev, struct amdgpu_vm *vm
>>>>>>>>          INIT_WORK(&f->work, amdgpu_tlb_fence_work);
>>>>>>>>          spin_lock_init(&f->lock);
>>>>>>>>      -    dma_fence_init(&f->base, &amdgpu_tlb_fence_ops, &f->lock,
>>>>>>>> -               vm->tlb_fence_context, atomic64_read(&vm->tlb_seq));
>>>>>>>> +    dma_fence_init64(&f->base, &amdgpu_tlb_fence_ops, &f->lock,
>>>>>>>> +             vm->tlb_fence_context, atomic64_read(&vm->tlb_seq));
>>>>>>>>            /* TODO: We probably need a separate wq here */
>>>>>>>>          dma_fence_get(&f->base);
>>>>>>>> diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
>>>>>>>> index 48b5202c531d..a34a0dcdc446 100644
>>>>>>>> --- a/include/linux/dma-fence.h
>>>>>>>> +++ b/include/linux/dma-fence.h
>>>>>>>> @@ -97,6 +97,7 @@ struct dma_fence {
>>>>>>>>      };
>>>>>>>>        enum dma_fence_flag_bits {
>>>>>>>> +    DMA_FENCE_FLAG_SEQNO64_BIT,
>>>>>>>>          DMA_FENCE_FLAG_SIGNALED_BIT,
>>>>>>>>          DMA_FENCE_FLAG_TIMESTAMP_BIT,
>>>>>>>>          DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT,
>>>>>>>> @@ -124,14 +125,6 @@ struct dma_fence_cb {
>>>>>>>>       *
>>>>>>>>       */
>>>>>>>>      struct dma_fence_ops {
>>>>>>>> -    /**
>>>>>>>> -     * @use_64bit_seqno:
>>>>>>>> -     *
>>>>>>>> -     * True if this dma_fence implementation uses 64bit seqno, false
>>>>>>>> -     * otherwise.
>>>>>>>> -     */
>>>>>>>> -    bool use_64bit_seqno;
>>>>>>>> -
>>>>>>>>          /**
>>>>>>>>           * @get_driver_name:
>>>>>>>>           *
>>>>>>>> @@ -262,6 +255,9 @@ struct dma_fence_ops {
>>>>>>>>      void dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops,
>>>>>>>>                  spinlock_t *lock, u64 context, u64 seqno);
>>>>>>>>      +void dma_fence_init64(struct dma_fence *fence, const struct dma_fence_ops *ops,
>>>>>>>> +              spinlock_t *lock, u64 context, u64 seqno);
>>>>>>>> +
>>>>>>>>      void dma_fence_release(struct kref *kref);
>>>>>>>>      void dma_fence_free(struct dma_fence *fence);
>>>>>>>>      void dma_fence_describe(struct dma_fence *fence, struct seq_file *seq);
>>>>>>>> @@ -454,7 +450,7 @@ static inline bool __dma_fence_is_later(struct dma_fence *fence, u64 f1, u64 f2)
>>>>>>>>           * 32bit sequence numbers. Use a 64bit compare when the driver says to
>>>>>>>>           * do so.
>>>>>>>>           */
>>>>>>>> -    if (fence->ops->use_64bit_seqno)
>>>>>>>> +    if (test_bit(DMA_FENCE_FLAG_SEQNO64_BIT, &fence->flags))
>>>>>>>>              return f1 > f2;
>>>>>>>>            return (int)(lower_32_bits(f1) - lower_32_bits(f2)) > 0;
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
> 

Re: [PATCH v4 2/9] dma-fence: Use a flag for 64-bit seqnos

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 533 bytes --]

Hi,

On Mon, Jun 02, 2025 at 04:42:27PM +0200, Christian König wrote:
> On 6/2/25 15:05, Tvrtko Ursulin wrote:
> > On 15/05/2025 14:15, Christian König wrote:
> >> Hey drm-misc maintainers,
> >>
> >> can you guys please backmerge drm-next into drm-misc-next?
> >>
> >> I want to push this patch here but it depends on changes which are partially in drm-next and partially in drm-misc-next.
> > 
> > Looks like the backmerge is still pending?
> 
> Yes, @Maarten, @Maxime and @Thomas ping on this.

It's done

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]

Re: [PATCH v4 2/9] dma-fence: Use a flag for 64-bit seqnos

[Christian König]

On 6/3/25 14:48, Tvrtko Ursulin wrote:
> 
> On 03/06/2025 13:40, Christian König wrote:
>> On 6/3/25 13:30, Tvrtko Ursulin wrote:
>>>
>>> On 02/06/2025 19:00, Christian König wrote:
>>>> On 6/2/25 17:25, Tvrtko Ursulin wrote:
>>>>>
>>>>> On 02/06/2025 15:42, Christian König wrote:
>>>>>> On 6/2/25 15:05, Tvrtko Ursulin wrote:
>>>>>>>
>>>>>>> Hi,
>>>>>>>
>>>>>>> On 15/05/2025 14:15, Christian König wrote:
>>>>>>>> Hey drm-misc maintainers,
>>>>>>>>
>>>>>>>> can you guys please backmerge drm-next into drm-misc-next?
>>>>>>>>
>>>>>>>> I want to push this patch here but it depends on changes which are partially in drm-next and partially in drm-misc-next.
>>>>>>>
>>>>>>> Looks like the backmerge is still pending?
>>>>>>
>>>>>> Yes, @Maarten, @Maxime and @Thomas ping on this.
>>>>>>
>>>>>>> In the meantime, Christian, any chance you will have some bandwith to think about the tail end of the series? Specifically patch 6 and how that is used onward.
>>>>>>
>>>>>> Well the RCU grace period is quite a nifty hack. I wanted to go over it again after merging the first patches from this series.
>>>>>>
>>>>>> In general looks like a good idea to me, I just don't like that we explicitely need to expose dma_fence_access_begin() and dma_fence_access_end().
>>>>>>
>>>>>> Especially we can't do that while calling fence->ops->release.
>>>>>
>>>>> Hm why not? You think something will take offence of the rcu_read_lock()?
>>>>
>>>> Yes, especially it is perfectly legitimate to call synchronize_rcu() or lock semaphores/mutexes from that callback.
>>>>
>>>> Either keep the RCU critical section only for the trace or even better come up with some different approach, e.g. copying the string under the RCU lock or something like that.
>>>
>>> Hmm but the kerneldoc explicity says callback can be called from irq context:
>>>
>>>      /**
>>>       * @release:
>>>       *
>>>       * Called on destruction of fence to release additional resources.
>>>       * Can be called from irq context.  This callback is optional. If it is
>>>       * NULL, then dma_fence_free() is instead called as the default
>>>       * implementation.
>>>       */
>>>      void (*release)(struct dma_fence *fence);
>>
>> Ah, right. I mixed that up with the dma-buf object.
>>
>> Yeah in that case that is probably harmless. We delegate the final free to a work item if necessary anyway.
>>
>> But I would still like to avoid having the RCU cover the release as well. Or why is there any reason why we would explicitely want to do this?
> 
> I can't remember there was a particular reason. Obviously the driver/timeline name vfunc access I needed a dma_fence_access_begin/end() block so maybe I was just sloppy and put the end at the end of the function instead of at the end of the block which can dereference them.

Yeah that's the next topic I would rather like to improve. We are kind of hiding that the returned strings are using RCU protection.

In other words it would be nicer if we could add an __rcu tag to the get_driver_name/get_timeline_name callbacks and let the automated tools complain if somebody isn't doing the proper RCU handling.

The problem is that as far as I know that is not supported by the automated tools (would be cool if somebody could double check that).

+We would need to convert the get_timeline/get_timeline_name function to something like func(struct dma_fence *fence, const char __rcu **out) to make that work.

Regards,
Christian.

> 
> I will pull it earlier for the next respin, assuming no gotchas get discovered in the process.
> 
> Regards,
> 
> Tvrtko
> 
>>
>> Regards,
>> Christian.
>>
>>>
>>>
>>> Regards,
>>>
>>> Tvrtko
>>>
>>>>
>>>> Regards,
>>>> Christian.
>>>>
>>>>>
>>>>> Regards,
>>>>>
>>>>> Tvrtko
>>>>>
>>>>>>>> On 5/15/25 11:49, Tvrtko Ursulin wrote:
>>>>>>>>> With the goal of reducing the need for drivers to touch (and dereference)
>>>>>>>>> fence->ops, we move the 64-bit seqnos flag from struct dma_fence_ops to
>>>>>>>>> the fence->flags.
>>>>>>>>>
>>>>>>>>> Drivers which were setting this flag are changed to use new
>>>>>>>>> dma_fence_init64() instead of dma_fence_init().
>>>>>>>>>
>>>>>>>>> v2:
>>>>>>>>>      * Streamlined init and added kerneldoc.
>>>>>>>>>      * Rebase for amdgpu userq which landed since.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
>>>>>>>>> Reviewed-by: Christian König <christian.koenig@amd.com> # v1
>>>>>>>>> ---
>>>>>>>>>      drivers/dma-buf/dma-fence-chain.c             |  5 +-
>>>>>>>>>      drivers/dma-buf/dma-fence.c                   | 69 ++++++++++++++-----
>>>>>>>>>      .../drm/amd/amdgpu/amdgpu_eviction_fence.c    |  7 +-
>>>>>>>>>      .../gpu/drm/amd/amdgpu/amdgpu_userq_fence.c   |  5 +-
>>>>>>>>>      .../gpu/drm/amd/amdgpu/amdgpu_vm_tlb_fence.c  |  5 +-
>>>>>>>>>      include/linux/dma-fence.h                     | 14 ++--
>>>>>>>>>      6 files changed, 64 insertions(+), 41 deletions(-)
>>>>>>>>>
>>>>>>>>> diff --git a/drivers/dma-buf/dma-fence-chain.c b/drivers/dma-buf/dma-fence-chain.c
>>>>>>>>> index 90424f23fd73..a8a90acf4f34 100644
>>>>>>>>> --- a/drivers/dma-buf/dma-fence-chain.c
>>>>>>>>> +++ b/drivers/dma-buf/dma-fence-chain.c
>>>>>>>>> @@ -218,7 +218,6 @@ static void dma_fence_chain_set_deadline(struct dma_fence *fence,
>>>>>>>>>      }
>>>>>>>>>        const struct dma_fence_ops dma_fence_chain_ops = {
>>>>>>>>> -    .use_64bit_seqno = true,
>>>>>>>>>          .get_driver_name = dma_fence_chain_get_driver_name,
>>>>>>>>>          .get_timeline_name = dma_fence_chain_get_timeline_name,
>>>>>>>>>          .enable_signaling = dma_fence_chain_enable_signaling,
>>>>>>>>> @@ -262,8 +261,8 @@ void dma_fence_chain_init(struct dma_fence_chain *chain,
>>>>>>>>>                  seqno = max(prev->seqno, seqno);
>>>>>>>>>          }
>>>>>>>>>      -    dma_fence_init(&chain->base, &dma_fence_chain_ops,
>>>>>>>>> -               &chain->lock, context, seqno);
>>>>>>>>> +    dma_fence_init64(&chain->base, &dma_fence_chain_ops, &chain->lock,
>>>>>>>>> +             context, seqno);
>>>>>>>>>            /*
>>>>>>>>>           * Chaining dma_fence_chain container together is only allowed through
>>>>>>>>> diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
>>>>>>>>> index f0cdd3e99d36..705b59787731 100644
>>>>>>>>> --- a/drivers/dma-buf/dma-fence.c
>>>>>>>>> +++ b/drivers/dma-buf/dma-fence.c
>>>>>>>>> @@ -989,24 +989,9 @@ void dma_fence_describe(struct dma_fence *fence, struct seq_file *seq)
>>>>>>>>>      }
>>>>>>>>>      EXPORT_SYMBOL(dma_fence_describe);
>>>>>>>>>      -/**
>>>>>>>>> - * 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
>>>>>>>>> - * @context: the execution context this fence is run on
>>>>>>>>> - * @seqno: a linear increasing sequence number for this context
>>>>>>>>> - *
>>>>>>>>> - * Initializes an allocated fence, the caller doesn't have to keep its
>>>>>>>>> - * refcount after committing with this fence, but it will need to hold a
>>>>>>>>> - * refcount again if &dma_fence_ops.enable_signaling gets called.
>>>>>>>>> - *
>>>>>>>>> - * context and seqno are used for easy comparison between fences, allowing
>>>>>>>>> - * to check which fence is later by simply using dma_fence_later().
>>>>>>>>> - */
>>>>>>>>> -void
>>>>>>>>> -dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops,
>>>>>>>>> -           spinlock_t *lock, u64 context, u64 seqno)
>>>>>>>>> +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);
>>>>>>>>> @@ -1017,9 +1002,55 @@ dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops,
>>>>>>>>>          fence->lock = lock;
>>>>>>>>>          fence->context = context;
>>>>>>>>>          fence->seqno = seqno;
>>>>>>>>> -    fence->flags = 0UL;
>>>>>>>>> +    fence->flags = flags;
>>>>>>>>>          fence->error = 0;
>>>>>>>>>            trace_dma_fence_init(fence);
>>>>>>>>>      }
>>>>>>>>> +
>>>>>>>>> +/**
>>>>>>>>> + * 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
>>>>>>>>> + * @context: the execution context this fence is run on
>>>>>>>>> + * @seqno: a linear increasing sequence number for this context
>>>>>>>>> + *
>>>>>>>>> + * Initializes an allocated fence, the caller doesn't have to keep its
>>>>>>>>> + * refcount after committing with this fence, but it will need to hold a
>>>>>>>>> + * refcount again if &dma_fence_ops.enable_signaling gets called.
>>>>>>>>> + *
>>>>>>>>> + * context and seqno are used for easy comparison between fences, allowing
>>>>>>>>> + * to check which fence is later by simply using dma_fence_later().
>>>>>>>>> + */
>>>>>>>>> +void
>>>>>>>>> +dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops,
>>>>>>>>> +           spinlock_t *lock, u64 context, u64 seqno)
>>>>>>>>> +{
>>>>>>>>> +    __dma_fence_init(fence, ops, lock, context, seqno, 0UL);
>>>>>>>>> +}
>>>>>>>>>      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
>>>>>>>>> + * @context: the execution context this fence is run on
>>>>>>>>> + * @seqno: a linear increasing sequence number for this context
>>>>>>>>> + *
>>>>>>>>> + * Initializes an allocated fence, the caller doesn't have to keep its
>>>>>>>>> + * refcount after committing with this fence, but it will need to hold a
>>>>>>>>> + * refcount again if &dma_fence_ops.enable_signaling gets called.
>>>>>>>>> + *
>>>>>>>>> + * Context and seqno are used for easy comparison between fences, allowing
>>>>>>>>> + * to check which fence is later by simply using dma_fence_later().
>>>>>>>>> + */
>>>>>>>>> +void
>>>>>>>>> +dma_fence_init64(struct dma_fence *fence, const struct dma_fence_ops *ops,
>>>>>>>>> +         spinlock_t *lock, u64 context, u64 seqno)
>>>>>>>>> +{
>>>>>>>>> +    __dma_fence_init(fence, ops, lock, context, seqno,
>>>>>>>>> +             BIT(DMA_FENCE_FLAG_SEQNO64_BIT));
>>>>>>>>> +}
>>>>>>>>> +EXPORT_SYMBOL(dma_fence_init64);
>>>>>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.c
>>>>>>>>> index 1a7469543db5..79713421bffe 100644
>>>>>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.c
>>>>>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.c
>>>>>>>>> @@ -134,7 +134,6 @@ static bool amdgpu_eviction_fence_enable_signaling(struct dma_fence *f)
>>>>>>>>>      }
>>>>>>>>>        static const struct dma_fence_ops amdgpu_eviction_fence_ops = {
>>>>>>>>> -    .use_64bit_seqno = true,
>>>>>>>>>          .get_driver_name = amdgpu_eviction_fence_get_driver_name,
>>>>>>>>>          .get_timeline_name = amdgpu_eviction_fence_get_timeline_name,
>>>>>>>>>          .enable_signaling = amdgpu_eviction_fence_enable_signaling,
>>>>>>>>> @@ -160,9 +159,9 @@ amdgpu_eviction_fence_create(struct amdgpu_eviction_fence_mgr *evf_mgr)
>>>>>>>>>          ev_fence->evf_mgr = evf_mgr;
>>>>>>>>>          get_task_comm(ev_fence->timeline_name, current);
>>>>>>>>>          spin_lock_init(&ev_fence->lock);
>>>>>>>>> -    dma_fence_init(&ev_fence->base, &amdgpu_eviction_fence_ops,
>>>>>>>>> -               &ev_fence->lock, evf_mgr->ev_fence_ctx,
>>>>>>>>> -               atomic_inc_return(&evf_mgr->ev_fence_seq));
>>>>>>>>> +    dma_fence_init64(&ev_fence->base, &amdgpu_eviction_fence_ops,
>>>>>>>>> +             &ev_fence->lock, evf_mgr->ev_fence_ctx,
>>>>>>>>> +             atomic_inc_return(&evf_mgr->ev_fence_seq));
>>>>>>>>>          return ev_fence;
>>>>>>>>>      }
>>>>>>>>>      diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
>>>>>>>>> index 029cb24c28b3..5e92d00a591f 100644
>>>>>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
>>>>>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
>>>>>>>>> @@ -239,8 +239,8 @@ static int amdgpu_userq_fence_create(struct amdgpu_usermode_queue *userq,
>>>>>>>>>          fence = &userq_fence->base;
>>>>>>>>>          userq_fence->fence_drv = fence_drv;
>>>>>>>>>      -    dma_fence_init(fence, &amdgpu_userq_fence_ops, &userq_fence->lock,
>>>>>>>>> -               fence_drv->context, seq);
>>>>>>>>> +    dma_fence_init64(fence, &amdgpu_userq_fence_ops, &userq_fence->lock,
>>>>>>>>> +             fence_drv->context, seq);
>>>>>>>>>            amdgpu_userq_fence_driver_get(fence_drv);
>>>>>>>>>          dma_fence_get(fence);
>>>>>>>>> @@ -334,7 +334,6 @@ static void amdgpu_userq_fence_release(struct dma_fence *f)
>>>>>>>>>      }
>>>>>>>>>        static const struct dma_fence_ops amdgpu_userq_fence_ops = {
>>>>>>>>> -    .use_64bit_seqno = true,
>>>>>>>>>          .get_driver_name = amdgpu_userq_fence_get_driver_name,
>>>>>>>>>          .get_timeline_name = amdgpu_userq_fence_get_timeline_name,
>>>>>>>>>          .signaled = amdgpu_userq_fence_signaled,
>>>>>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_tlb_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_tlb_fence.c
>>>>>>>>> index 51cddfa3f1e8..5d26797356a3 100644
>>>>>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_tlb_fence.c
>>>>>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_tlb_fence.c
>>>>>>>>> @@ -71,7 +71,6 @@ static void amdgpu_tlb_fence_work(struct work_struct *work)
>>>>>>>>>      }
>>>>>>>>>        static const struct dma_fence_ops amdgpu_tlb_fence_ops = {
>>>>>>>>> -    .use_64bit_seqno = true,
>>>>>>>>>          .get_driver_name = amdgpu_tlb_fence_get_driver_name,
>>>>>>>>>          .get_timeline_name = amdgpu_tlb_fence_get_timeline_name
>>>>>>>>>      };
>>>>>>>>> @@ -101,8 +100,8 @@ void amdgpu_vm_tlb_fence_create(struct amdgpu_device *adev, struct amdgpu_vm *vm
>>>>>>>>>          INIT_WORK(&f->work, amdgpu_tlb_fence_work);
>>>>>>>>>          spin_lock_init(&f->lock);
>>>>>>>>>      -    dma_fence_init(&f->base, &amdgpu_tlb_fence_ops, &f->lock,
>>>>>>>>> -               vm->tlb_fence_context, atomic64_read(&vm->tlb_seq));
>>>>>>>>> +    dma_fence_init64(&f->base, &amdgpu_tlb_fence_ops, &f->lock,
>>>>>>>>> +             vm->tlb_fence_context, atomic64_read(&vm->tlb_seq));
>>>>>>>>>            /* TODO: We probably need a separate wq here */
>>>>>>>>>          dma_fence_get(&f->base);
>>>>>>>>> diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
>>>>>>>>> index 48b5202c531d..a34a0dcdc446 100644
>>>>>>>>> --- a/include/linux/dma-fence.h
>>>>>>>>> +++ b/include/linux/dma-fence.h
>>>>>>>>> @@ -97,6 +97,7 @@ struct dma_fence {
>>>>>>>>>      };
>>>>>>>>>        enum dma_fence_flag_bits {
>>>>>>>>> +    DMA_FENCE_FLAG_SEQNO64_BIT,
>>>>>>>>>          DMA_FENCE_FLAG_SIGNALED_BIT,
>>>>>>>>>          DMA_FENCE_FLAG_TIMESTAMP_BIT,
>>>>>>>>>          DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT,
>>>>>>>>> @@ -124,14 +125,6 @@ struct dma_fence_cb {
>>>>>>>>>       *
>>>>>>>>>       */
>>>>>>>>>      struct dma_fence_ops {
>>>>>>>>> -    /**
>>>>>>>>> -     * @use_64bit_seqno:
>>>>>>>>> -     *
>>>>>>>>> -     * True if this dma_fence implementation uses 64bit seqno, false
>>>>>>>>> -     * otherwise.
>>>>>>>>> -     */
>>>>>>>>> -    bool use_64bit_seqno;
>>>>>>>>> -
>>>>>>>>>          /**
>>>>>>>>>           * @get_driver_name:
>>>>>>>>>           *
>>>>>>>>> @@ -262,6 +255,9 @@ struct dma_fence_ops {
>>>>>>>>>      void dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops,
>>>>>>>>>                  spinlock_t *lock, u64 context, u64 seqno);
>>>>>>>>>      +void dma_fence_init64(struct dma_fence *fence, const struct dma_fence_ops *ops,
>>>>>>>>> +              spinlock_t *lock, u64 context, u64 seqno);
>>>>>>>>> +
>>>>>>>>>      void dma_fence_release(struct kref *kref);
>>>>>>>>>      void dma_fence_free(struct dma_fence *fence);
>>>>>>>>>      void dma_fence_describe(struct dma_fence *fence, struct seq_file *seq);
>>>>>>>>> @@ -454,7 +450,7 @@ static inline bool __dma_fence_is_later(struct dma_fence *fence, u64 f1, u64 f2)
>>>>>>>>>           * 32bit sequence numbers. Use a 64bit compare when the driver says to
>>>>>>>>>           * do so.
>>>>>>>>>           */
>>>>>>>>> -    if (fence->ops->use_64bit_seqno)
>>>>>>>>> +    if (test_bit(DMA_FENCE_FLAG_SEQNO64_BIT, &fence->flags))
>>>>>>>>>              return f1 > f2;
>>>>>>>>>            return (int)(lower_32_bits(f1) - lower_32_bits(f2)) > 0;
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
> 

Re: [PATCH v4 2/9] dma-fence: Use a flag for 64-bit seqnos

[Tvrtko Ursulin]

On 03/06/2025 14:13, Maxime Ripard wrote:
> Hi,
> 
> On Mon, Jun 02, 2025 at 04:42:27PM +0200, Christian König wrote:
>> On 6/2/25 15:05, Tvrtko Ursulin wrote:
>>> On 15/05/2025 14:15, Christian König wrote:
>>>> Hey drm-misc maintainers,
>>>>
>>>> can you guys please backmerge drm-next into drm-misc-next?
>>>>
>>>> I want to push this patch here but it depends on changes which are partially in drm-next and partially in drm-misc-next.
>>>
>>> Looks like the backmerge is still pending?
>>
>> Yes, @Maarten, @Maxime and @Thomas ping on this.
> 
> It's done

Thanks Maxime!

Christian, I can merge 2-5 to take some load off you if you want?

Regards,

Tvrtko

Re: [PATCH v4 2/9] dma-fence: Use a flag for 64-bit seqnos

[Christian König]

On 6/3/25 15:13, Maxime Ripard wrote:
> Hi,
> 
> On Mon, Jun 02, 2025 at 04:42:27PM +0200, Christian König wrote:
>> On 6/2/25 15:05, Tvrtko Ursulin wrote:
>>> On 15/05/2025 14:15, Christian König wrote:
>>>> Hey drm-misc maintainers,
>>>>
>>>> can you guys please backmerge drm-next into drm-misc-next?
>>>>
>>>> I want to push this patch here but it depends on changes which are partially in drm-next and partially in drm-misc-next.
>>>
>>> Looks like the backmerge is still pending?
>>
>> Yes, @Maarten, @Maxime and @Thomas ping on this.
> 
> It's done

Thanks, I will start merge things tomorrow.

Regards,
Christian.

> 
> Maxime

Re: [PATCH v4 2/9] dma-fence: Use a flag for 64-bit seqnos

[Christian König]

On 6/3/25 17:00, Tvrtko Ursulin wrote:
> 
> On 03/06/2025 14:13, Maxime Ripard wrote:
>> Hi,
>>
>> On Mon, Jun 02, 2025 at 04:42:27PM +0200, Christian König wrote:
>>> On 6/2/25 15:05, Tvrtko Ursulin wrote:
>>>> On 15/05/2025 14:15, Christian König wrote:
>>>>> Hey drm-misc maintainers,
>>>>>
>>>>> can you guys please backmerge drm-next into drm-misc-next?
>>>>>
>>>>> I want to push this patch here but it depends on changes which are partially in drm-next and partially in drm-misc-next.
>>>>
>>>> Looks like the backmerge is still pending?
>>>
>>> Yes, @Maarten, @Maxime and @Thomas ping on this.
>>
>> It's done
> 
> Thanks Maxime!
> 
> Christian, I can merge 2-5 to take some load off you if you want?

Sure, go ahead.

Then I can call it a day for today :)

Cheers,
Christian.

> 
> Regards,
> 
> Tvrtko
> 

Re: [PATCH v4 2/9] dma-fence: Use a flag for 64-bit seqnos

[Tvrtko Ursulin]

On 03/06/2025 17:27, Christian König wrote:
> On 6/3/25 17:00, Tvrtko Ursulin wrote:
>>
>> On 03/06/2025 14:13, Maxime Ripard wrote:
>>> Hi,
>>>
>>> On Mon, Jun 02, 2025 at 04:42:27PM +0200, Christian König wrote:
>>>> On 6/2/25 15:05, Tvrtko Ursulin wrote:
>>>>> On 15/05/2025 14:15, Christian König wrote:
>>>>>> Hey drm-misc maintainers,
>>>>>>
>>>>>> can you guys please backmerge drm-next into drm-misc-next?
>>>>>>
>>>>>> I want to push this patch here but it depends on changes which are partially in drm-next and partially in drm-misc-next.
>>>>>
>>>>> Looks like the backmerge is still pending?
>>>>
>>>> Yes, @Maarten, @Maxime and @Thomas ping on this.
>>>
>>> It's done
>>
>> Thanks Maxime!
>>
>> Christian, I can merge 2-5 to take some load off you if you want?
> 
> Sure, go ahead.
> 
> Then I can call it a day for today :)

Pushed. Thanks for the reviews once more!

I will aim to respin the rest of the series ASAP but may be delayed a 
few days.

Regards,

Tvrtko