[PATCH 3/3] dma-fence: Set the timestamp after the notifying the cb_list

Chris Wilson chris at chris-wilson.co.uk
Mon Aug 19 09:59:28 UTC 2019


Now that the timestamp and cb_list share the same slot in the fence,
with the current order of setting the timestamp before notifying the
cb_list, we have to take a temporary copy of the list. If we don't set
the timestamp, we can simply process the list in situ. This also gives
us the advantage that we get a signal when the cb_list is complete,
useful in a few cases that need to serialise against the cb_list.

Suggested-by: Christian König <christian.koenig at amd.com>
Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
Cc: Christian König <christian.koenig at amd.com>
---
 drivers/dma-buf/dma-fence.c                 | 41 +++++++-----------
 drivers/dma-buf/st-dma-fence.c              |  8 +---
 drivers/dma-buf/sync_file.c                 |  5 +--
 drivers/gpu/drm/i915/gt/intel_breadcrumbs.c | 28 +-----------
 include/linux/dma-fence.h                   | 47 ++++++++++++++++++++-
 5 files changed, 65 insertions(+), 64 deletions(-)

diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
index 2c136aee3e79..972a4b90b820 100644
--- a/drivers/dma-buf/dma-fence.c
+++ b/drivers/dma-buf/dma-fence.c
@@ -111,47 +111,36 @@ u64 dma_fence_context_alloc(unsigned num)
 EXPORT_SYMBOL(dma_fence_context_alloc);
 
 /**
- * dma_fence_signal_locked - signal completion of a fence
+ * __dma_fence_signal_locked - Perform signaling of a fence
  * @fence: the fence to signal
+ * @timestamp: the timsetamp of fence completion
  *
- * Signal completion for software callbacks on a fence, this will unblock
- * dma_fence_wait() calls and run all the callbacks added with
- * dma_fence_add_callback(). Can be called multiple times, but since a fence
- * can only go from the unsignaled to the signaled state and not back, it will
- * only be effective the first time.
+ * See dma_fence_signal() for the typical interface.
  *
- * Unlike dma_fence_signal(), this function must be called with &dma_fence.lock
- * held.
+ * This provides the low-level operations required upon signaling a fence,
+ * such as processing the callback list and setting the timestamp.
  *
- * Returns 0 on success and a negative error value when @fence has been
- * signalled already.
+ * Requires the caller to hold the fence->lock and already have marked the
+ * fence as signaled in an exclusive manner.
+ *
+ * Great care must be taken in calling this function!
  */
-int dma_fence_signal_locked(struct dma_fence *fence)
+void __dma_fence_signal_locked(struct dma_fence *fence, ktime_t timestamp)
 {
 	struct dma_fence_cb *cur, *tmp;
-	struct list_head cb_list;
 
 	lockdep_assert_held(fence->lock);
 
-	if (unlikely(test_and_set_bit(DMA_FENCE_FLAG_SIGNALED_BIT,
-				      &fence->flags)))
-		return -EINVAL;
-
-	/* Stash the cb_list before replacing it with the timestamp */
-	list_replace(&fence->cb_list, &cb_list);
-
-	fence->timestamp = ktime_get();
-	set_bit(DMA_FENCE_FLAG_TIMESTAMP_BIT, &fence->flags);
-	trace_dma_fence_signaled(fence);
-
-	list_for_each_entry_safe(cur, tmp, &cb_list, node) {
+	list_for_each_entry_safe(cur, tmp, &fence->cb_list, node) {
 		INIT_LIST_HEAD(&cur->node);
 		cur->func(fence, cur);
 	}
 
-	return 0;
+	fence->timestamp = timestamp; /* overwrites fence->cb_list */
+	set_bit(DMA_FENCE_FLAG_TIMESTAMP_BIT, &fence->flags);
+	trace_dma_fence_signaled(fence);
 }
-EXPORT_SYMBOL(dma_fence_signal_locked);
+EXPORT_SYMBOL(__dma_fence_signal_locked);
 
 /**
  * dma_fence_signal - signal completion of a fence
diff --git a/drivers/dma-buf/st-dma-fence.c b/drivers/dma-buf/st-dma-fence.c
index a365dc7440e5..1fba51a5a46b 100644
--- a/drivers/dma-buf/st-dma-fence.c
+++ b/drivers/dma-buf/st-dma-fence.c
@@ -434,12 +434,6 @@ struct race_thread {
 	int id;
 };
 
-static void __wait_for_callbacks(struct dma_fence *f)
-{
-	spin_lock_irq(f->lock);
-	spin_unlock_irq(f->lock);
-}
-
 static int thread_signal_callback(void *arg)
 {
 	const struct race_thread *t = arg;
@@ -478,7 +472,7 @@ static int thread_signal_callback(void *arg)
 
 		if (!cb.seen) {
 			dma_fence_wait(f2, false);
-			__wait_for_callbacks(f2);
+			dma_fence_wait_for_notify(f2);
 		}
 
 		if (!READ_ONCE(cb.seen)) {
diff --git a/drivers/dma-buf/sync_file.c b/drivers/dma-buf/sync_file.c
index 25c5c071645b..f801dabb33a4 100644
--- a/drivers/dma-buf/sync_file.c
+++ b/drivers/dma-buf/sync_file.c
@@ -384,9 +384,8 @@ static int sync_fill_fence_info(struct dma_fence *fence,
 		sizeof(info->driver_name));
 
 	info->status = dma_fence_get_status(fence);
-	while (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags) &&
-	       !test_bit(DMA_FENCE_FLAG_TIMESTAMP_BIT, &fence->flags))
-		cpu_relax();
+	if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
+		dma_fence_wait_for_notify(fence);
 	info->timestamp_ns =
 		test_bit(DMA_FENCE_FLAG_TIMESTAMP_BIT, &fence->flags) ?
 		ktime_to_ns(fence->timestamp) :
diff --git a/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c b/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c
index 09c68dda2098..dbfb3b5348c4 100644
--- a/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c
+++ b/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c
@@ -105,29 +105,6 @@ __dma_fence_signal(struct dma_fence *fence)
 	return !test_and_set_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags);
 }
 
-static void
-__dma_fence_signal__timestamp(struct dma_fence *fence, ktime_t timestamp)
-{
-	fence->timestamp = timestamp;
-	set_bit(DMA_FENCE_FLAG_TIMESTAMP_BIT, &fence->flags);
-	trace_dma_fence_signaled(fence);
-}
-
-static void
-__dma_fence_signal__notify(struct dma_fence *fence,
-			   const struct list_head *list)
-{
-	struct dma_fence_cb *cur, *tmp;
-
-	lockdep_assert_held(fence->lock);
-	lockdep_assert_irqs_disabled();
-
-	list_for_each_entry_safe(cur, tmp, list, node) {
-		INIT_LIST_HEAD(&cur->node);
-		cur->func(fence, cur);
-	}
-}
-
 void intel_engine_breadcrumbs_irq(struct intel_engine_cs *engine)
 {
 	struct intel_breadcrumbs *b = &engine->breadcrumbs;
@@ -187,12 +164,9 @@ void intel_engine_breadcrumbs_irq(struct intel_engine_cs *engine)
 	list_for_each_safe(pos, next, &signal) {
 		struct i915_request *rq =
 			list_entry(pos, typeof(*rq), signal_link);
-		struct list_head cb_list;
 
 		spin_lock(&rq->lock);
-		list_replace(&rq->fence.cb_list, &cb_list);
-		__dma_fence_signal__timestamp(&rq->fence, timestamp);
-		__dma_fence_signal__notify(&rq->fence, &cb_list);
+		__dma_fence_signal_locked(&rq->fence, timestamp);
 		spin_unlock(&rq->lock);
 
 		i915_request_put(rq);
diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
index 3347c54f3a87..b93c83f240c2 100644
--- a/include/linux/dma-fence.h
+++ b/include/linux/dma-fence.h
@@ -357,8 +357,36 @@ dma_fence_get_rcu_safe(struct dma_fence __rcu **fencep)
 	} while (1);
 }
 
+void __dma_fence_signal_locked(struct dma_fence *fence, ktime_t timestamp);
+
+/**
+ * dma_fence_signal_locked - signal completion of a fence
+ * @fence: the fence to signal
+ *
+ * Signal completion for software callbacks on a fence, this will unblock
+ * dma_fence_wait() calls and run all the callbacks added with
+ * dma_fence_add_callback(). Can be called multiple times, but since a fence
+ * can only go from the unsignaled to the signaled state and not back, it will
+ * only be effective the first time.
+ *
+ * Unlike dma_fence_signal(), this function must be called with &dma_fence.lock
+ * held.
+ *
+ * Returns 0 on success and a negative error value when @fence has been
+ * signalled already.
+ */
+static inline int dma_fence_signal_locked(struct dma_fence *fence)
+{
+	if (unlikely(test_and_set_bit(DMA_FENCE_FLAG_SIGNALED_BIT,
+				      &fence->flags)))
+		return -EINVAL;
+
+	__dma_fence_signal_locked(fence, ktime_get());
+	return 0;
+}
+
 int dma_fence_signal(struct dma_fence *fence);
-int dma_fence_signal_locked(struct dma_fence *fence);
+
 signed long dma_fence_default_wait(struct dma_fence *fence,
 				   bool intr, signed long timeout);
 int dma_fence_add_callback(struct dma_fence *fence,
@@ -426,6 +454,23 @@ dma_fence_is_signaled(struct dma_fence *fence)
 	return false;
 }
 
+/**
+ * dma_fence_wait_for_notify - Wait until the notifications are complete
+ * @fence: the fence to wait on
+ *
+ * After marking the fence as signaled, a number of operations but we
+ * are completely done, from notifying all the listeners culminating
+ * in setting the timestamp on the fence. This signals the completion
+ * of all the callbacks and the end of the siganling operation.
+ */
+static inline void
+dma_fence_wait_for_notify(struct dma_fence *fence)
+{
+	WARN_ON(!test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags));
+	while (!test_bit(DMA_FENCE_FLAG_TIMESTAMP_BIT, &fence->flags))
+		cpu_relax();
+}
+
 /**
  * __dma_fence_is_later - return if f1 is chronologically later than f2
  * @f1: the first fence's seqno
-- 
2.23.0.rc1



More information about the dri-devel mailing list