[PATCH 0/6] dma-fence: Remove return code of dma_fence_signal() et al.

[PATCH 0/6] dma-fence: Remove return code of dma_fence_signal() et al.

[Philipp Stanner]

Tested this with dma_buf selftests and drm_sched tests.

Changes in v2:
  - Fix bug and don't turn the kernel into a smoking pile of ashes by
    not setting the signaled-bit…
  - Add functions (dma_fence_check_and_signal()) preserving the old
    behavior of dma_fence_signal() & Co. (Felix)
  - Use those new functions in amdkfd, xe, ttm and st-dma-fence.
  - Be a bit less aggressive and keep the git-diff smaller.
  - Add a patch using the flag-helper in Xe. (Matthew)

Barely anyone uses dma_fence_signal()'s (and similar functions') return
code. Checking it is pretty much useless anyways, because what are you
going to do if a fence was already signal it? Unsignal it and signal it
again? ;p

Removing the return code simplifies the API and makes it easier for me
to sit on top with Rust DmaFence.

Philipp Stanner (8):
  dma-buf/dma-fence: Add dma_fence_test_signaled_flag()
  dma-buf/dma-fence: Add dma_fence_check_and_signal()
  amd/amdkfd: Use dma_fence_check_and_signal()
  drm/xe: Use dma_fence_check_and_signal_locked()
  dma-buf: Don't misuse dma_fence_signal()
  drm/ttm: Use dma_fence_check_and_signal()
  dma-buf/dma-fence: Remove return code of signaling-functions
  drm/xe: Use dma_fence_test_signaled_flag()

 drivers/dma-buf/dma-fence.c                   | 100 +++++++++++-------
 drivers/dma-buf/st-dma-fence.c                |   4 +-
 drivers/gpu/drm/amd/amdkfd/kfd_process.c      |   6 +-
 .../gpu/drm/ttm/tests/ttm_bo_validate_test.c  |   2 +-
 drivers/gpu/drm/xe/xe_exec_queue.c            |   9 +-
 drivers/gpu/drm/xe/xe_hw_fence.c              |   4 +-
 drivers/gpu/drm/xe/xe_pt.c                    |   3 +-
 drivers/gpu/drm/xe/xe_sched_job.c             |   2 +-
 include/linux/dma-fence.h                     |  35 ++++--
 9 files changed, 102 insertions(+), 63 deletions(-)

-- 
2.49.0

[PATCH v2 1/8] dma-buf/dma-fence: Add dma_fence_test_signaled_flag()

[Philipp Stanner]

The dma_fence framework checks at many places whether the signaled flag
of a fence is already set. The code can be simplified and made more
readable by providing a helper function for that.

Add dma_fence_test_signaled_flag(), which only checks whether a fence is
signaled. Use it internally.

Suggested-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Signed-off-by: Philipp Stanner <phasta@kernel.org>
---
 drivers/dma-buf/dma-fence.c | 16 ++++++++--------
 include/linux/dma-fence.h   | 24 ++++++++++++++++++++++--
 2 files changed, 30 insertions(+), 10 deletions(-)

diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
index 39e6f93dc310..96d72ffc0750 100644
--- a/drivers/dma-buf/dma-fence.c
+++ b/drivers/dma-buf/dma-fence.c
@@ -545,7 +545,7 @@ void dma_fence_release(struct kref *kref)
 	trace_dma_fence_destroy(fence);
 
 	if (!list_empty(&fence->cb_list) &&
-	    !test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags)) {
+	    !dma_fence_test_signaled_flag(fence)) {
 		const char __rcu *timeline;
 		const char __rcu *driver;
 		unsigned long flags;
@@ -602,7 +602,7 @@ static bool __dma_fence_enable_signaling(struct dma_fence *fence)
 	was_set = test_and_set_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT,
 				   &fence->flags);
 
-	if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
+	if (dma_fence_test_signaled_flag(fence))
 		return false;
 
 	if (!was_set && fence->ops->enable_signaling) {
@@ -666,7 +666,7 @@ int dma_fence_add_callback(struct dma_fence *fence, struct dma_fence_cb *cb,
 	if (WARN_ON(!fence || !func))
 		return -EINVAL;
 
-	if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags)) {
+	if (dma_fence_test_signaled_flag(fence)) {
 		INIT_LIST_HEAD(&cb->node);
 		return -ENOENT;
 	}
@@ -783,7 +783,7 @@ dma_fence_default_wait(struct dma_fence *fence, bool intr, signed long timeout)
 
 	spin_lock_irqsave(fence->lock, flags);
 
-	if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
+	if (dma_fence_test_signaled_flag(fence))
 		goto out;
 
 	if (intr && signal_pending(current)) {
@@ -800,7 +800,7 @@ dma_fence_default_wait(struct dma_fence *fence, bool intr, signed long timeout)
 	cb.task = current;
 	list_add(&cb.base.node, &fence->cb_list);
 
-	while (!test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags) && ret > 0) {
+	while (!dma_fence_test_signaled_flag(fence) && ret > 0) {
 		if (intr)
 			__set_current_state(TASK_INTERRUPTIBLE);
 		else
@@ -832,7 +832,7 @@ dma_fence_test_signaled_any(struct dma_fence **fences, uint32_t count,
 
 	for (i = 0; i < count; ++i) {
 		struct dma_fence *fence = fences[i];
-		if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags)) {
+		if (dma_fence_test_signaled_flag(fence)) {
 			if (idx)
 				*idx = i;
 			return true;
@@ -1108,7 +1108,7 @@ const char __rcu *dma_fence_driver_name(struct dma_fence *fence)
 	RCU_LOCKDEP_WARN(!rcu_read_lock_held(),
 			 "RCU protection is required for safe access to returned string");
 
-	if (!test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
+	if (!dma_fence_test_signaled_flag(fence))
 		return fence->ops->get_driver_name(fence);
 	else
 		return "detached-driver";
@@ -1140,7 +1140,7 @@ const char __rcu *dma_fence_timeline_name(struct dma_fence *fence)
 	RCU_LOCKDEP_WARN(!rcu_read_lock_held(),
 			 "RCU protection is required for safe access to returned string");
 
-	if (!test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
+	if (!dma_fence_test_signaled_flag(fence))
 		return fence->ops->get_timeline_name(fence);
 	else
 		return "signaled-timeline";
diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
index 64639e104110..19972f5d176f 100644
--- a/include/linux/dma-fence.h
+++ b/include/linux/dma-fence.h
@@ -401,6 +401,26 @@ void dma_fence_enable_sw_signaling(struct dma_fence *fence);
 const char __rcu *dma_fence_driver_name(struct dma_fence *fence);
 const char __rcu *dma_fence_timeline_name(struct dma_fence *fence);
 
+/*
+ * dma_fence_test_signaled_flag - Only check whether a fence is signaled yet.
+ * @fence: the fence to check
+ *
+ * This function just checks whether @fence is signaled, without interacting
+ * with the fence in any way. The user must, therefore, ensure through other
+ * means that fences get signaled eventually.
+ *
+ * This function uses test_bit(), which is thread-safe. Naturally, this function
+ * should be used opportunistically; a fence could get signaled at any moment
+ * after the check is done.
+ *
+ * Return: true if signaled, false otherwise.
+ */
+static inline bool
+dma_fence_test_signaled_flag(struct dma_fence *fence)
+{
+	return test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags);
+}
+
 /**
  * dma_fence_is_signaled_locked - Return an indication if the fence
  *                                is signaled yet.
@@ -418,7 +438,7 @@ const char __rcu *dma_fence_timeline_name(struct dma_fence *fence);
 static inline bool
 dma_fence_is_signaled_locked(struct dma_fence *fence)
 {
-	if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
+	if (dma_fence_test_signaled_flag(fence))
 		return true;
 
 	if (fence->ops->signaled && fence->ops->signaled(fence)) {
@@ -448,7 +468,7 @@ dma_fence_is_signaled_locked(struct dma_fence *fence)
 static inline bool
 dma_fence_is_signaled(struct dma_fence *fence)
 {
-	if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
+	if (dma_fence_test_signaled_flag(fence))
 		return true;
 
 	if (fence->ops->signaled && fence->ops->signaled(fence)) {
-- 
2.49.0

Re: [PATCH v2 1/8] dma-buf/dma-fence: Add dma_fence_test_signaled_flag()

[Christian König]

On 12/1/25 11:50, Philipp Stanner wrote:
> The dma_fence framework checks at many places whether the signaled flag
> of a fence is already set. The code can be simplified and made more
> readable by providing a helper function for that.
> 
> Add dma_fence_test_signaled_flag(), which only checks whether a fence is
> signaled. Use it internally.
> 
> Suggested-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
> Signed-off-by: Philipp Stanner <phasta@kernel.org>

Reviewed-by: Christian König <christian.koenig@amd.com>

> ---
>  drivers/dma-buf/dma-fence.c | 16 ++++++++--------
>  include/linux/dma-fence.h   | 24 ++++++++++++++++++++++--
>  2 files changed, 30 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
> index 39e6f93dc310..96d72ffc0750 100644
> --- a/drivers/dma-buf/dma-fence.c
> +++ b/drivers/dma-buf/dma-fence.c
> @@ -545,7 +545,7 @@ void dma_fence_release(struct kref *kref)
>  	trace_dma_fence_destroy(fence);
>  
>  	if (!list_empty(&fence->cb_list) &&
> -	    !test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags)) {
> +	    !dma_fence_test_signaled_flag(fence)) {
>  		const char __rcu *timeline;
>  		const char __rcu *driver;
>  		unsigned long flags;
> @@ -602,7 +602,7 @@ static bool __dma_fence_enable_signaling(struct dma_fence *fence)
>  	was_set = test_and_set_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT,
>  				   &fence->flags);
>  
> -	if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
> +	if (dma_fence_test_signaled_flag(fence))
>  		return false;
>  
>  	if (!was_set && fence->ops->enable_signaling) {
> @@ -666,7 +666,7 @@ int dma_fence_add_callback(struct dma_fence *fence, struct dma_fence_cb *cb,
>  	if (WARN_ON(!fence || !func))
>  		return -EINVAL;
>  
> -	if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags)) {
> +	if (dma_fence_test_signaled_flag(fence)) {
>  		INIT_LIST_HEAD(&cb->node);
>  		return -ENOENT;
>  	}
> @@ -783,7 +783,7 @@ dma_fence_default_wait(struct dma_fence *fence, bool intr, signed long timeout)
>  
>  	spin_lock_irqsave(fence->lock, flags);
>  
> -	if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
> +	if (dma_fence_test_signaled_flag(fence))
>  		goto out;
>  
>  	if (intr && signal_pending(current)) {
> @@ -800,7 +800,7 @@ dma_fence_default_wait(struct dma_fence *fence, bool intr, signed long timeout)
>  	cb.task = current;
>  	list_add(&cb.base.node, &fence->cb_list);
>  
> -	while (!test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags) && ret > 0) {
> +	while (!dma_fence_test_signaled_flag(fence) && ret > 0) {
>  		if (intr)
>  			__set_current_state(TASK_INTERRUPTIBLE);
>  		else
> @@ -832,7 +832,7 @@ dma_fence_test_signaled_any(struct dma_fence **fences, uint32_t count,
>  
>  	for (i = 0; i < count; ++i) {
>  		struct dma_fence *fence = fences[i];
> -		if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags)) {
> +		if (dma_fence_test_signaled_flag(fence)) {
>  			if (idx)
>  				*idx = i;
>  			return true;
> @@ -1108,7 +1108,7 @@ const char __rcu *dma_fence_driver_name(struct dma_fence *fence)
>  	RCU_LOCKDEP_WARN(!rcu_read_lock_held(),
>  			 "RCU protection is required for safe access to returned string");
>  
> -	if (!test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
> +	if (!dma_fence_test_signaled_flag(fence))
>  		return fence->ops->get_driver_name(fence);
>  	else
>  		return "detached-driver";
> @@ -1140,7 +1140,7 @@ const char __rcu *dma_fence_timeline_name(struct dma_fence *fence)
>  	RCU_LOCKDEP_WARN(!rcu_read_lock_held(),
>  			 "RCU protection is required for safe access to returned string");
>  
> -	if (!test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
> +	if (!dma_fence_test_signaled_flag(fence))
>  		return fence->ops->get_timeline_name(fence);
>  	else
>  		return "signaled-timeline";
> diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
> index 64639e104110..19972f5d176f 100644
> --- a/include/linux/dma-fence.h
> +++ b/include/linux/dma-fence.h
> @@ -401,6 +401,26 @@ void dma_fence_enable_sw_signaling(struct dma_fence *fence);
>  const char __rcu *dma_fence_driver_name(struct dma_fence *fence);
>  const char __rcu *dma_fence_timeline_name(struct dma_fence *fence);
>  
> +/*
> + * dma_fence_test_signaled_flag - Only check whether a fence is signaled yet.
> + * @fence: the fence to check
> + *
> + * This function just checks whether @fence is signaled, without interacting
> + * with the fence in any way. The user must, therefore, ensure through other
> + * means that fences get signaled eventually.
> + *
> + * This function uses test_bit(), which is thread-safe. Naturally, this function
> + * should be used opportunistically; a fence could get signaled at any moment
> + * after the check is done.
> + *
> + * Return: true if signaled, false otherwise.
> + */
> +static inline bool
> +dma_fence_test_signaled_flag(struct dma_fence *fence)
> +{
> +	return test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags);
> +}
> +
>  /**
>   * dma_fence_is_signaled_locked - Return an indication if the fence
>   *                                is signaled yet.
> @@ -418,7 +438,7 @@ const char __rcu *dma_fence_timeline_name(struct dma_fence *fence);
>  static inline bool
>  dma_fence_is_signaled_locked(struct dma_fence *fence)
>  {
> -	if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
> +	if (dma_fence_test_signaled_flag(fence))
>  		return true;
>  
>  	if (fence->ops->signaled && fence->ops->signaled(fence)) {
> @@ -448,7 +468,7 @@ dma_fence_is_signaled_locked(struct dma_fence *fence)
>  static inline bool
>  dma_fence_is_signaled(struct dma_fence *fence)
>  {
> -	if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
> +	if (dma_fence_test_signaled_flag(fence))
>  		return true;
>  
>  	if (fence->ops->signaled && fence->ops->signaled(fence)) {

[PATCH v2 2/8] dma-buf/dma-fence: Add dma_fence_check_and_signal()

[Philipp Stanner]

The overwhelming majority of users of dma_fence signaling functions
don't care about whether the fence had already been signaled by someone
else. Therefore, the return code shall be removed from those functions.

For the few users who rely on the check, a new, specialized function
shall be provided.

Add dma_fence_check_and_signal(), which signals a fence if it had not
yet been signaled, and informs the user about that.

Add a counter part, dma_fence_check_and_signal_locked(), which doesn't
take the spinlock.

Signed-off-by: Philipp Stanner <phasta@kernel.org>
---
 drivers/dma-buf/dma-fence.c | 44 +++++++++++++++++++++++++++++++++++++
 include/linux/dma-fence.h   |  2 ++
 2 files changed, 46 insertions(+)

diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
index 96d72ffc0750..146de62887cf 100644
--- a/drivers/dma-buf/dma-fence.c
+++ b/drivers/dma-buf/dma-fence.c
@@ -445,6 +445,50 @@ int dma_fence_signal_locked(struct dma_fence *fence)
 }
 EXPORT_SYMBOL(dma_fence_signal_locked);
 
+/**
+ * dma_fence_check_and_signal_locked - signal the fence if it's not yet signaled
+ * @fence: the fence to check and signal
+ *
+ * Checks whether a fence was signaled and signals it if it was not yet signaled.
+ *
+ * Unlike dma_fence_check_and_signal(), this function must be called with
+ * &struct dma_fence.lock being held.
+ *
+ * Return: true if fence has been signaled already, false otherwise.
+ */
+bool dma_fence_check_and_signal_locked(struct dma_fence *fence)
+{
+	bool ret;
+
+	ret = dma_fence_test_signaled_flag(fence);
+	dma_fence_signal_locked(fence);
+
+	return ret;
+}
+EXPORT_SYMBOL(dma_fence_check_and_signal_locked);
+
+/**
+ * dma_fence_check_and_signal - signal the fence if it's not yet signaled
+ * @fence: the fence to check and signal
+ *
+ * Checks whether a fence was signaled and signals it if it was not yet signaled.
+ * All this is done in a race-free manner.
+ *
+ * Return: true if fence has been signaled already, false otherwise.
+ */
+bool dma_fence_check_and_signal(struct dma_fence *fence)
+{
+	unsigned long flags;
+	bool ret;
+
+	spin_lock_irqsave(fence->lock, flags);
+	ret = dma_fence_check_and_signal_locked(fence);
+	spin_unlock_irqrestore(fence->lock, flags);
+
+	return ret;
+}
+EXPORT_SYMBOL(dma_fence_check_and_signal);
+
 /**
  * dma_fence_signal - signal completion of a fence
  * @fence: the fence to signal
diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
index 19972f5d176f..0504afe52c2a 100644
--- a/include/linux/dma-fence.h
+++ b/include/linux/dma-fence.h
@@ -365,6 +365,8 @@ static inline void __dma_fence_might_wait(void) {}
 #endif
 
 int dma_fence_signal(struct dma_fence *fence);
+bool dma_fence_check_and_signal(struct dma_fence *fence);
+bool dma_fence_check_and_signal_locked(struct dma_fence *fence);
 int dma_fence_signal_locked(struct dma_fence *fence);
 int dma_fence_signal_timestamp(struct dma_fence *fence, ktime_t timestamp);
 int dma_fence_signal_timestamp_locked(struct dma_fence *fence,
-- 
2.49.0

Re: [PATCH v2 2/8] dma-buf/dma-fence: Add dma_fence_check_and_signal()

[Christian König]

On 12/1/25 11:50, Philipp Stanner wrote:
> The overwhelming majority of users of dma_fence signaling functions
> don't care about whether the fence had already been signaled by someone
> else. Therefore, the return code shall be removed from those functions.
> 
> For the few users who rely on the check, a new, specialized function
> shall be provided.
> 
> Add dma_fence_check_and_signal(), which signals a fence if it had not
> yet been signaled, and informs the user about that.
> 
> Add a counter part, dma_fence_check_and_signal_locked(), which doesn't
> take the spinlock.
> 
> Signed-off-by: Philipp Stanner <phasta@kernel.org>
> ---
>  drivers/dma-buf/dma-fence.c | 44 +++++++++++++++++++++++++++++++++++++
>  include/linux/dma-fence.h   |  2 ++
>  2 files changed, 46 insertions(+)
> 
> diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
> index 96d72ffc0750..146de62887cf 100644
> --- a/drivers/dma-buf/dma-fence.c
> +++ b/drivers/dma-buf/dma-fence.c
> @@ -445,6 +445,50 @@ int dma_fence_signal_locked(struct dma_fence *fence)
>  }
>  EXPORT_SYMBOL(dma_fence_signal_locked);
>  
> +/**
> + * dma_fence_check_and_signal_locked - signal the fence if it's not yet signaled
> + * @fence: the fence to check and signal
> + *
> + * Checks whether a fence was signaled and signals it if it was not yet signaled.
> + *
> + * Unlike dma_fence_check_and_signal(), this function must be called with
> + * &struct dma_fence.lock being held.
> + *
> + * Return: true if fence has been signaled already, false otherwise.
> + */
> +bool dma_fence_check_and_signal_locked(struct dma_fence *fence)

I'm seriously considering to nuke all the unlocked variants of dma_fence functions and just make it mandatory for callers to grab the lock manually.

> +{
> +	bool ret;
> +
> +	ret = dma_fence_test_signaled_flag(fence);
> +	dma_fence_signal_locked(fence);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL(dma_fence_check_and_signal_locked);
> +
> +/**
> + * dma_fence_check_and_signal - signal the fence if it's not yet signaled
> + * @fence: the fence to check and signal
> + *
> + * Checks whether a fence was signaled and signals it if it was not yet signaled.
> + * All this is done in a race-free manner.
> + *
> + * Return: true if fence has been signaled already, false otherwise.
> + */
> +bool dma_fence_check_and_signal(struct dma_fence *fence)

So I think we should name this one here dma_fence_check_and_signal_unlocked() and drop the postfix from the locked variant.

> +{
> +	unsigned long flags;
> +	bool ret;
> +
> +	spin_lock_irqsave(fence->lock, flags);
> +	ret = dma_fence_check_and_signal_locked(fence);
> +	spin_unlock_irqrestore(fence->lock, flags);

Could this use guard(fence->lock, flags) ?

Regards,
Christian.

> +
> +	return ret;
> +}
> +EXPORT_SYMBOL(dma_fence_check_and_signal);
> +
>  /**
>   * dma_fence_signal - signal completion of a fence
>   * @fence: the fence to signal
> diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
> index 19972f5d176f..0504afe52c2a 100644
> --- a/include/linux/dma-fence.h
> +++ b/include/linux/dma-fence.h
> @@ -365,6 +365,8 @@ static inline void __dma_fence_might_wait(void) {}
>  #endif
>  
>  int dma_fence_signal(struct dma_fence *fence);
> +bool dma_fence_check_and_signal(struct dma_fence *fence);
> +bool dma_fence_check_and_signal_locked(struct dma_fence *fence);
>  int dma_fence_signal_locked(struct dma_fence *fence);
>  int dma_fence_signal_timestamp(struct dma_fence *fence, ktime_t timestamp);
>  int dma_fence_signal_timestamp_locked(struct dma_fence *fence,

Re: [PATCH v2 2/8] dma-buf/dma-fence: Add dma_fence_check_and_signal()

[Philipp Stanner]

On Mon, 2025-12-01 at 14:23 +0100, Christian König wrote:
> On 12/1/25 11:50, Philipp Stanner wrote:
> > The overwhelming majority of users of dma_fence signaling functions
> > don't care about whether the fence had already been signaled by someone
> > else. Therefore, the return code shall be removed from those functions.
> > 
> > For the few users who rely on the check, a new, specialized function
> > shall be provided.
> > 
> > Add dma_fence_check_and_signal(), which signals a fence if it had not
> > yet been signaled, and informs the user about that.
> > 
> > Add a counter part, dma_fence_check_and_signal_locked(), which doesn't
> > take the spinlock.
> > 
> > Signed-off-by: Philipp Stanner <phasta@kernel.org>
> > ---
> >  drivers/dma-buf/dma-fence.c | 44 +++++++++++++++++++++++++++++++++++++
> >  include/linux/dma-fence.h   |  2 ++
> >  2 files changed, 46 insertions(+)
> > 
> > diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
> > index 96d72ffc0750..146de62887cf 100644
> > --- a/drivers/dma-buf/dma-fence.c
> > +++ b/drivers/dma-buf/dma-fence.c
> > @@ -445,6 +445,50 @@ int dma_fence_signal_locked(struct dma_fence *fence)
> >  }
> >  EXPORT_SYMBOL(dma_fence_signal_locked);
> >  
> > +/**
> > + * dma_fence_check_and_signal_locked - signal the fence if it's not yet signaled
> > + * @fence: the fence to check and signal
> > + *
> > + * Checks whether a fence was signaled and signals it if it was not yet signaled.
> > + *
> > + * Unlike dma_fence_check_and_signal(), this function must be called with
> > + * &struct dma_fence.lock being held.
> > + *
> > + * Return: true if fence has been signaled already, false otherwise.
> > + */
> > +bool dma_fence_check_and_signal_locked(struct dma_fence *fence)
> 
> I'm seriously considering to nuke all the unlocked variants of dma_fence functions and just make it mandatory for callers to grab the lock manually.
> 

You mean "nuke the *locked* variants.

Why, though? Aren't they enough for most users?
I suppose you have all those subtle races in mind..

> > +{
> > +	bool ret;
> > +
> > +	ret = dma_fence_test_signaled_flag(fence);
> > +	dma_fence_signal_locked(fence);
> > +
> > +	return ret;
> > +}
> > +EXPORT_SYMBOL(dma_fence_check_and_signal_locked);
> > +
> > +/**
> > + * dma_fence_check_and_signal - signal the fence if it's not yet signaled
> > + * @fence: the fence to check and signal
> > + *
> > + * Checks whether a fence was signaled and signals it if it was not yet signaled.
> > + * All this is done in a race-free manner.
> > + *
> > + * Return: true if fence has been signaled already, false otherwise.
> > + */
> > +bool dma_fence_check_and_signal(struct dma_fence *fence)
> 
> So I think we should name this one here dma_fence_check_and_signal_unlocked() and drop the postfix from the locked variant.

postfix?

Well, now, IDK. Can't we, for this series, keep the _locked() variant
so that it's congruent with all the other dma_fence code?

And then later if you want to force manual locking you can add that
kernel-wide in a separate series, since it'll be a discussion-worthy,
bigger chunk of work.

That's cleaner, and my series here won't prevent that once merged.

> 
> > +{
> > +	unsigned long flags;
> > +	bool ret;
> > +
> > +	spin_lock_irqsave(fence->lock, flags);
> > +	ret = dma_fence_check_and_signal_locked(fence);
> > +	spin_unlock_irqrestore(fence->lock, flags);
> 
> Could this use guard(fence->lock, flags) ?

guard? You mean a lockdep guard? Do you have a pointer to someplace in
dma_fence who does what you mean / want?


P.

> 
> Regards,
> Christian.
> 
> > +
> > +	return ret;
> > +}
> > +EXPORT_SYMBOL(dma_fence_check_and_signal);
> > +
> >  /**
> >   * dma_fence_signal - signal completion of a fence
> >   * @fence: the fence to signal
> > diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
> > index 19972f5d176f..0504afe52c2a 100644
> > --- a/include/linux/dma-fence.h
> > +++ b/include/linux/dma-fence.h
> > @@ -365,6 +365,8 @@ static inline void __dma_fence_might_wait(void) {}
> >  #endif
> >  
> >  int dma_fence_signal(struct dma_fence *fence);
> > +bool dma_fence_check_and_signal(struct dma_fence *fence);
> > +bool dma_fence_check_and_signal_locked(struct dma_fence *fence);
> >  int dma_fence_signal_locked(struct dma_fence *fence);
> >  int dma_fence_signal_timestamp(struct dma_fence *fence, ktime_t timestamp);
> >  int dma_fence_signal_timestamp_locked(struct dma_fence *fence,
> 

Re: [PATCH v2 2/8] dma-buf/dma-fence: Add dma_fence_check_and_signal()

[Christian König]

On 12/1/25 14:55, Philipp Stanner wrote:
> On Mon, 2025-12-01 at 14:23 +0100, Christian König wrote:
>> On 12/1/25 11:50, Philipp Stanner wrote:
>>> The overwhelming majority of users of dma_fence signaling functions
>>> don't care about whether the fence had already been signaled by someone
>>> else. Therefore, the return code shall be removed from those functions.
>>>
>>> For the few users who rely on the check, a new, specialized function
>>> shall be provided.
>>>
>>> Add dma_fence_check_and_signal(), which signals a fence if it had not
>>> yet been signaled, and informs the user about that.
>>>
>>> Add a counter part, dma_fence_check_and_signal_locked(), which doesn't
>>> take the spinlock.
>>>
>>> Signed-off-by: Philipp Stanner <phasta@kernel.org>
>>> ---
>>>  drivers/dma-buf/dma-fence.c | 44 +++++++++++++++++++++++++++++++++++++
>>>  include/linux/dma-fence.h   |  2 ++
>>>  2 files changed, 46 insertions(+)
>>>
>>> diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
>>> index 96d72ffc0750..146de62887cf 100644
>>> --- a/drivers/dma-buf/dma-fence.c
>>> +++ b/drivers/dma-buf/dma-fence.c
>>> @@ -445,6 +445,50 @@ int dma_fence_signal_locked(struct dma_fence *fence)
>>>  }
>>>  EXPORT_SYMBOL(dma_fence_signal_locked);
>>>  
>>> +/**
>>> + * dma_fence_check_and_signal_locked - signal the fence if it's not yet signaled
>>> + * @fence: the fence to check and signal
>>> + *
>>> + * Checks whether a fence was signaled and signals it if it was not yet signaled.
>>> + *
>>> + * Unlike dma_fence_check_and_signal(), this function must be called with
>>> + * &struct dma_fence.lock being held.
>>> + *
>>> + * Return: true if fence has been signaled already, false otherwise.
>>> + */
>>> +bool dma_fence_check_and_signal_locked(struct dma_fence *fence)
>>
>> I'm seriously considering to nuke all the unlocked variants of dma_fence functions and just make it mandatory for callers to grab the lock manually.
>>
> 
> You mean "nuke the *locked* variants.

Sorry, that wasn't specific enough.

What I meant was making the locked variants the default instead of the unlocked ones.

> 
> Why, though? Aren't they enough for most users?
> I suppose you have all those subtle races in mind..

Yeah, exactly that.

> 
>>> +{
>>> +	bool ret;
>>> +
>>> +	ret = dma_fence_test_signaled_flag(fence);
>>> +	dma_fence_signal_locked(fence);
>>> +
>>> +	return ret;
>>> +}
>>> +EXPORT_SYMBOL(dma_fence_check_and_signal_locked);
>>> +
>>> +/**
>>> + * dma_fence_check_and_signal - signal the fence if it's not yet signaled
>>> + * @fence: the fence to check and signal
>>> + *
>>> + * Checks whether a fence was signaled and signals it if it was not yet signaled.
>>> + * All this is done in a race-free manner.
>>> + *
>>> + * Return: true if fence has been signaled already, false otherwise.
>>> + */
>>> +bool dma_fence_check_and_signal(struct dma_fence *fence)
>>
>> So I think we should name this one here dma_fence_check_and_signal_unlocked() and drop the postfix from the locked variant.
> 
> postfix?
> 
> Well, now, IDK. Can't we, for this series, keep the _locked() variant
> so that it's congruent with all the other dma_fence code?

Good point. That thought was not really related to this series here.

> 
> And then later if you want to force manual locking you can add that
> kernel-wide in a separate series, since it'll be a discussion-worthy,
> bigger chunk of work.
> 
> That's cleaner, and my series here won't prevent that once merged.
> 
>>
>>> +{
>>> +	unsigned long flags;
>>> +	bool ret;
>>> +
>>> +	spin_lock_irqsave(fence->lock, flags);
>>> +	ret = dma_fence_check_and_signal_locked(fence);
>>> +	spin_unlock_irqrestore(fence->lock, flags);
>>
>> Could this use guard(fence->lock, flags) ?
> 
> guard? You mean a lockdep guard? Do you have a pointer to someplace in
> dma_fence who does what you mean / want?

E.g. like guard(spinlock_irqsave)(&fence->lock);

Regards,
Christian.

> 
> 
> P.
> 
>>
>> Regards,
>> Christian.
>>
>>> +
>>> +	return ret;
>>> +}
>>> +EXPORT_SYMBOL(dma_fence_check_and_signal);
>>> +
>>>  /**
>>>   * dma_fence_signal - signal completion of a fence
>>>   * @fence: the fence to signal
>>> diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
>>> index 19972f5d176f..0504afe52c2a 100644
>>> --- a/include/linux/dma-fence.h
>>> +++ b/include/linux/dma-fence.h
>>> @@ -365,6 +365,8 @@ static inline void __dma_fence_might_wait(void) {}
>>>  #endif
>>>  
>>>  int dma_fence_signal(struct dma_fence *fence);
>>> +bool dma_fence_check_and_signal(struct dma_fence *fence);
>>> +bool dma_fence_check_and_signal_locked(struct dma_fence *fence);
>>>  int dma_fence_signal_locked(struct dma_fence *fence);
>>>  int dma_fence_signal_timestamp(struct dma_fence *fence, ktime_t timestamp);
>>>  int dma_fence_signal_timestamp_locked(struct dma_fence *fence,
>>
> 

Re: [PATCH v2 2/8] dma-buf/dma-fence: Add dma_fence_check_and_signal()

[Philipp Stanner]

On Mon, 2025-12-01 at 16:20 +0100, Christian König wrote:
> On 12/1/25 14:55, Philipp Stanner wrote:
> > On Mon, 2025-12-01 at 14:23 +0100, Christian König wrote:
> > > On 12/1/25 11:50, Philipp Stanner wrote:
> > > > The overwhelming majority of users of dma_fence signaling functions
> > > > don't care about whether the fence had already been signaled by someone
> > > > else. Therefore, the return code shall be removed from those functions.
> > > > 
> > > > For the few users who rely on the check, a new, specialized function
> > > > shall be provided.
> > > > 
> > > > Add dma_fence_check_and_signal(), which signals a fence if it had not
> > > > yet been signaled, and informs the user about that.
> > > > 
> > > > Add a counter part, dma_fence_check_and_signal_locked(), which doesn't
> > > > take the spinlock.
> > > > 
> > > > Signed-off-by: Philipp Stanner <phasta@kernel.org>
> > > > ---
> > > >  drivers/dma-buf/dma-fence.c | 44 +++++++++++++++++++++++++++++++++++++
> > > >  include/linux/dma-fence.h   |  2 ++
> > > >  2 files changed, 46 insertions(+)
> > > > 
> > > > diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
> > > > index 96d72ffc0750..146de62887cf 100644
> > > > --- a/drivers/dma-buf/dma-fence.c
> > > > +++ b/drivers/dma-buf/dma-fence.c
> > > > @@ -445,6 +445,50 @@ int dma_fence_signal_locked(struct dma_fence *fence)
> > > >  }
> > > >  EXPORT_SYMBOL(dma_fence_signal_locked);
> > > >  
> > > > +/**
> > > > + * dma_fence_check_and_signal_locked - signal the fence if it's not yet signaled
> > > > + * @fence: the fence to check and signal
> > > > + *
> > > > + * Checks whether a fence was signaled and signals it if it was not yet signaled.
> > > > + *
> > > > + * Unlike dma_fence_check_and_signal(), this function must be called with
> > > > + * &struct dma_fence.lock being held.
> > > > + *
> > > > + * Return: true if fence has been signaled already, false otherwise.
> > > > + */
> > > > +bool dma_fence_check_and_signal_locked(struct dma_fence *fence)
> > > 
> > > I'm seriously considering to nuke all the unlocked variants of dma_fence functions and just make it mandatory for callers to grab the lock manually.
> > > 
> > 
> > You mean "nuke the *locked* variants.
> 
> Sorry, that wasn't specific enough.
> 
> What I meant was making the locked variants the default instead of the unlocked ones.

Well, no :D

What you want to do is:
- Delete / deprecate the *locked* variants
- Force all users to take the fence lock manually, then use the (now
all unlocked) dma fence functions.

ACK?

> 
> > 
> > Why, though? Aren't they enough for most users?
> > I suppose you have all those subtle races in mind..
> 
> Yeah, exactly that.
> 
> > 
> > > > +{
> > > > +	bool ret;
> > > > +
> > > > +	ret = dma_fence_test_signaled_flag(fence);
> > > > +	dma_fence_signal_locked(fence);
> > > > +
> > > > +	return ret;
> > > > +}
> > > > +EXPORT_SYMBOL(dma_fence_check_and_signal_locked);
> > > > +
> > > > +/**
> > > > + * dma_fence_check_and_signal - signal the fence if it's not yet signaled
> > > > + * @fence: the fence to check and signal
> > > > + *
> > > > + * Checks whether a fence was signaled and signals it if it was not yet signaled.
> > > > + * All this is done in a race-free manner.
> > > > + *
> > > > + * Return: true if fence has been signaled already, false otherwise.
> > > > + */
> > > > +bool dma_fence_check_and_signal(struct dma_fence *fence)
> > > 
> > > So I think we should name this one here dma_fence_check_and_signal_unlocked() and drop the postfix from the locked variant.
> > 
> > postfix?
> > 
> > Well, now, IDK. Can't we, for this series, keep the _locked() variant
> > so that it's congruent with all the other dma_fence code?
> 
> Good point. That thought was not really related to this series here.

OK, then let's progress with this here for now.


P.

> 
> > 
> > And then later if you want to force manual locking you can add that
> > kernel-wide in a separate series, since it'll be a discussion-worthy,
> > bigger chunk of work.
> > 
> > That's cleaner, and my series here won't prevent that once merged.
> > 
> > > 
> > > > +{
> > > > +	unsigned long flags;
> > > > +	bool ret;
> > > > +
> > > > +	spin_lock_irqsave(fence->lock, flags);
> > > > +	ret = dma_fence_check_and_signal_locked(fence);
> > > > +	spin_unlock_irqrestore(fence->lock, flags);
> > > 
> > > Could this use guard(fence->lock, flags) ?
> > 
> > guard? You mean a lockdep guard? Do you have a pointer to someplace in
> > dma_fence who does what you mean / want?
> 
> E.g. like guard(spinlock_irqsave)(&fence->lock);
> 
> Regards,
> Christian.
> 
> > 
> > 
> > P.
> > 
> > > 
> > > Regards,
> > > Christian.
> > > 
> > > > +
> > > > +	return ret;
> > > > +}
> > > > +EXPORT_SYMBOL(dma_fence_check_and_signal);
> > > > +
> > > >  /**
> > > >   * dma_fence_signal - signal completion of a fence
> > > >   * @fence: the fence to signal
> > > > diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
> > > > index 19972f5d176f..0504afe52c2a 100644
> > > > --- a/include/linux/dma-fence.h
> > > > +++ b/include/linux/dma-fence.h
> > > > @@ -365,6 +365,8 @@ static inline void __dma_fence_might_wait(void) {}
> > > >  #endif
> > > >  
> > > >  int dma_fence_signal(struct dma_fence *fence);
> > > > +bool dma_fence_check_and_signal(struct dma_fence *fence);
> > > > +bool dma_fence_check_and_signal_locked(struct dma_fence *fence);
> > > >  int dma_fence_signal_locked(struct dma_fence *fence);
> > > >  int dma_fence_signal_timestamp(struct dma_fence *fence, ktime_t timestamp);
> > > >  int dma_fence_signal_timestamp_locked(struct dma_fence *fence,
> > > 
> > 
> 

Re: [PATCH v2 2/8] dma-buf/dma-fence: Add dma_fence_check_and_signal()

[Christian König]

On 12/1/25 16:34, Philipp Stanner wrote:
> On Mon, 2025-12-01 at 16:20 +0100, Christian König wrote:
>> On 12/1/25 14:55, Philipp Stanner wrote:
>>> On Mon, 2025-12-01 at 14:23 +0100, Christian König wrote:
>>>> On 12/1/25 11:50, Philipp Stanner wrote:
>>>>> The overwhelming majority of users of dma_fence signaling functions
>>>>> don't care about whether the fence had already been signaled by someone
>>>>> else. Therefore, the return code shall be removed from those functions.
>>>>>
>>>>> For the few users who rely on the check, a new, specialized function
>>>>> shall be provided.
>>>>>
>>>>> Add dma_fence_check_and_signal(), which signals a fence if it had not
>>>>> yet been signaled, and informs the user about that.
>>>>>
>>>>> Add a counter part, dma_fence_check_and_signal_locked(), which doesn't
>>>>> take the spinlock.
>>>>>
>>>>> Signed-off-by: Philipp Stanner <phasta@kernel.org>
>>>>> ---
>>>>>  drivers/dma-buf/dma-fence.c | 44 +++++++++++++++++++++++++++++++++++++
>>>>>  include/linux/dma-fence.h   |  2 ++
>>>>>  2 files changed, 46 insertions(+)
>>>>>
>>>>> diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
>>>>> index 96d72ffc0750..146de62887cf 100644
>>>>> --- a/drivers/dma-buf/dma-fence.c
>>>>> +++ b/drivers/dma-buf/dma-fence.c
>>>>> @@ -445,6 +445,50 @@ int dma_fence_signal_locked(struct dma_fence *fence)
>>>>>  }
>>>>>  EXPORT_SYMBOL(dma_fence_signal_locked);
>>>>>  
>>>>> +/**
>>>>> + * dma_fence_check_and_signal_locked - signal the fence if it's not yet signaled
>>>>> + * @fence: the fence to check and signal
>>>>> + *
>>>>> + * Checks whether a fence was signaled and signals it if it was not yet signaled.
>>>>> + *
>>>>> + * Unlike dma_fence_check_and_signal(), this function must be called with
>>>>> + * &struct dma_fence.lock being held.
>>>>> + *
>>>>> + * Return: true if fence has been signaled already, false otherwise.
>>>>> + */
>>>>> +bool dma_fence_check_and_signal_locked(struct dma_fence *fence)
>>>>
>>>> I'm seriously considering to nuke all the unlocked variants of dma_fence functions and just make it mandatory for callers to grab the lock manually.
>>>>
>>>
>>> You mean "nuke the *locked* variants.
>>
>> Sorry, that wasn't specific enough.
>>
>> What I meant was making the locked variants the default instead of the unlocked ones.
> 
> Well, no :D
> 
> What you want to do is:
> - Delete / deprecate the *locked* variants
> - Force all users to take the fence lock manually, then use the (now
> all unlocked) dma fence functions.
> 
> ACK?

I'm sick with cold/flu like symptoms at the moment, but that sounds mixed up to me (but maybe I should get a bit sleep first).

>>
>>>
>>> Why, though? Aren't they enough for most users?
>>> I suppose you have all those subtle races in mind..
>>
>> Yeah, exactly that.
>>
>>>
>>>>> +{
>>>>> +	bool ret;
>>>>> +
>>>>> +	ret = dma_fence_test_signaled_flag(fence);
>>>>> +	dma_fence_signal_locked(fence);
>>>>> +
>>>>> +	return ret;
>>>>> +}
>>>>> +EXPORT_SYMBOL(dma_fence_check_and_signal_locked);
>>>>> +
>>>>> +/**
>>>>> + * dma_fence_check_and_signal - signal the fence if it's not yet signaled
>>>>> + * @fence: the fence to check and signal
>>>>> + *
>>>>> + * Checks whether a fence was signaled and signals it if it was not yet signaled.
>>>>> + * All this is done in a race-free manner.
>>>>> + *
>>>>> + * Return: true if fence has been signaled already, false otherwise.
>>>>> + */
>>>>> +bool dma_fence_check_and_signal(struct dma_fence *fence)
>>>>
>>>> So I think we should name this one here dma_fence_check_and_signal_unlocked() and drop the postfix from the locked variant.
>>>
>>> postfix?
>>>
>>> Well, now, IDK. Can't we, for this series, keep the _locked() variant
>>> so that it's congruent with all the other dma_fence code?
>>
>> Good point. That thought was not really related to this series here.
> 
> OK, then let's progress with this here for now.

Works for me, give me a day to go over it again and review it.

Regards,
Christian.

> 
> 
> P.
> 
>>
>>>
>>> And then later if you want to force manual locking you can add that
>>> kernel-wide in a separate series, since it'll be a discussion-worthy,
>>> bigger chunk of work.
>>>
>>> That's cleaner, and my series here won't prevent that once merged.
>>>
>>>>
>>>>> +{
>>>>> +	unsigned long flags;
>>>>> +	bool ret;
>>>>> +
>>>>> +	spin_lock_irqsave(fence->lock, flags);
>>>>> +	ret = dma_fence_check_and_signal_locked(fence);
>>>>> +	spin_unlock_irqrestore(fence->lock, flags);
>>>>
>>>> Could this use guard(fence->lock, flags) ?
>>>
>>> guard? You mean a lockdep guard? Do you have a pointer to someplace in
>>> dma_fence who does what you mean / want?
>>
>> E.g. like guard(spinlock_irqsave)(&fence->lock);
>>
>> Regards,
>> Christian.
>>
>>>
>>>
>>> P.
>>>
>>>>
>>>> Regards,
>>>> Christian.
>>>>
>>>>> +
>>>>> +	return ret;
>>>>> +}
>>>>> +EXPORT_SYMBOL(dma_fence_check_and_signal);
>>>>> +
>>>>>  /**
>>>>>   * dma_fence_signal - signal completion of a fence
>>>>>   * @fence: the fence to signal
>>>>> diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
>>>>> index 19972f5d176f..0504afe52c2a 100644
>>>>> --- a/include/linux/dma-fence.h
>>>>> +++ b/include/linux/dma-fence.h
>>>>> @@ -365,6 +365,8 @@ static inline void __dma_fence_might_wait(void) {}
>>>>>  #endif
>>>>>  
>>>>>  int dma_fence_signal(struct dma_fence *fence);
>>>>> +bool dma_fence_check_and_signal(struct dma_fence *fence);
>>>>> +bool dma_fence_check_and_signal_locked(struct dma_fence *fence);
>>>>>  int dma_fence_signal_locked(struct dma_fence *fence);
>>>>>  int dma_fence_signal_timestamp(struct dma_fence *fence, ktime_t timestamp);
>>>>>  int dma_fence_signal_timestamp_locked(struct dma_fence *fence,
>>>>
>>>
>>
> 

Re: [PATCH v2 2/8] dma-buf/dma-fence: Add dma_fence_check_and_signal()

[Philipp Stanner]

On Mon, 2025-12-01 at 16:20 +0100, Christian König wrote:
> On 12/1/25 14:55, Philipp Stanner wrote:
> > On Mon, 2025-12-01 at 14:23 +0100, Christian König wrote:
> > > On 12/1/25 11:50, Philipp Stanner wrote:
> > > > The overwhelming majority of users of dma_fence signaling functions
> > > > don't care about whether the fence had already been signaled by someone
> > > > 
> > 

[…]

> > > 
> > > > +{
> > > > +	unsigned long flags;
> > > > +	bool ret;
> > > > +
> > > > +	spin_lock_irqsave(fence->lock, flags);
> > > > +	ret = dma_fence_check_and_signal_locked(fence);
> > > > +	spin_unlock_irqrestore(fence->lock, flags);
> > > 
> > > Could this use guard(fence->lock, flags) ?
> > 
> > guard? You mean a lockdep guard? Do you have a pointer to someplace in
> > dma_fence who does what you mean / want?
> 
> E.g. like guard(spinlock_irqsave)(&fence->lock);


Hmm, but why?
It's obvious to all readers that I do spin_unlock_irqrestore() here.
It's very simple code, lock, 1 line, unlock. What would the guard
improve?


P.

Re: [PATCH v2 2/8] dma-buf/dma-fence: Add dma_fence_check_and_signal()

[Christian König]

On 12/1/25 16:53, Philipp Stanner wrote:
> On Mon, 2025-12-01 at 16:20 +0100, Christian König wrote:
>> On 12/1/25 14:55, Philipp Stanner wrote:
>>> On Mon, 2025-12-01 at 14:23 +0100, Christian König wrote:
>>>> On 12/1/25 11:50, Philipp Stanner wrote:
>>>>> The overwhelming majority of users of dma_fence signaling functions
>>>>> don't care about whether the fence had already been signaled by someone
>>>>>
>>>
> 
> […]
> 
>>>>
>>>>> +{
>>>>> +	unsigned long flags;
>>>>> +	bool ret;
>>>>> +
>>>>> +	spin_lock_irqsave(fence->lock, flags);
>>>>> +	ret = dma_fence_check_and_signal_locked(fence);
>>>>> +	spin_unlock_irqrestore(fence->lock, flags);
>>>>
>>>> Could this use guard(fence->lock, flags) ?
>>>
>>> guard? You mean a lockdep guard? Do you have a pointer to someplace in
>>> dma_fence who does what you mean / want?
>>
>> E.g. like guard(spinlock_irqsave)(&fence->lock);
> 
> 
> Hmm, but why?
> It's obvious to all readers that I do spin_unlock_irqrestore() here.
> It's very simple code, lock, 1 line, unlock. What would the guard
> improve?

Well you can save using the local variables.

So this:

	unsigned long flags;
	bool ret;

	spin_lock_irqsave(fence->lock, flags);
	ret = dma_fence_check_and_signal_locked(fence);
	spin_unlock_irqrestore(fence->lock, flags);

	return ret;

Becomes just:

	guard(spinlock_irqsave)(&fence->lock);
	return dma_fence_check_and_signal_locked(fence);

Regards,
Christian.

> 
> 
> P.

Re: [PATCH v2 2/8] dma-buf/dma-fence: Add dma_fence_check_and_signal()

[Philipp Stanner]

On Mon, 2025-12-01 at 17:08 +0100, Christian König wrote:
> On 12/1/25 16:53, Philipp Stanner wrote:
> > On Mon, 2025-12-01 at 16:20 +0100, Christian König wrote:
> > > On 12/1/25 14:55, Philipp Stanner wrote:
> > > > On Mon, 2025-12-01 at 14:23 +0100, Christian König wrote:
> > > > > On 12/1/25 11:50, Philipp Stanner wrote:
> > > > > > The overwhelming majority of users of dma_fence signaling functions
> > > > > > don't care about whether the fence had already been signaled by someone
> > > > > > 
> > > > 
> > 
> > […]
> > 
> > > > > 
> > > > > > +{
> > > > > > +	unsigned long flags;
> > > > > > +	bool ret;
> > > > > > +
> > > > > > +	spin_lock_irqsave(fence->lock, flags);
> > > > > > +	ret = dma_fence_check_and_signal_locked(fence);
> > > > > > +	spin_unlock_irqrestore(fence->lock, flags);
> > > > > 
> > > > > Could this use guard(fence->lock, flags) ?
> > > > 
> > > > guard? You mean a lockdep guard? Do you have a pointer to someplace in
> > > > dma_fence who does what you mean / want?
> > > 
> > > E.g. like guard(spinlock_irqsave)(&fence->lock);
> > 
> > 
> > Hmm, but why?
> > It's obvious to all readers that I do spin_unlock_irqrestore() here.
> > It's very simple code, lock, 1 line, unlock. What would the guard
> > improve?
> 
> Well you can save using the local variables.
> 
> So this:
> 
> 	unsigned long flags;
> 	bool ret;
> 
> 	spin_lock_irqsave(fence->lock, flags);
> 	ret = dma_fence_check_and_signal_locked(fence);
> 	spin_unlock_irqrestore(fence->lock, flags);
> 
> 	return ret;
> 
> Becomes just:
> 
> 	guard(spinlock_irqsave)(&fence->lock);
> 	return dma_fence_check_and_signal_locked(fence);

Mhm, I guess I agree that __cleanup is a cool new feature that can be
useful at many places. But in this case I think it's actually less
readable and doesn't really give lots of advantages. And don't I have
to use a DEFINE_GUARD or DEFINE_FREE in the first place?

If it's your maintainer preference, I can look into that though..

P.

Re: [PATCH v2 2/8] dma-buf/dma-fence: Add dma_fence_check_and_signal()

[Christian König]

On 12/1/25 11:50, Philipp Stanner wrote:
> The overwhelming majority of users of dma_fence signaling functions
> don't care about whether the fence had already been signaled by someone
> else. Therefore, the return code shall be removed from those functions.
> 
> For the few users who rely on the check, a new, specialized function
> shall be provided.
> 
> Add dma_fence_check_and_signal(), which signals a fence if it had not
> yet been signaled, and informs the user about that.
> 
> Add a counter part, dma_fence_check_and_signal_locked(), which doesn't
> take the spinlock.
> 
> Signed-off-by: Philipp Stanner <phasta@kernel.org>

As discussed let's stick with this version for now, it's technical correct and all cleanups can come later as general coding style changes.

Reviewed-by: Christian König <christian.koenig@amd.com>

> ---
>  drivers/dma-buf/dma-fence.c | 44 +++++++++++++++++++++++++++++++++++++
>  include/linux/dma-fence.h   |  2 ++
>  2 files changed, 46 insertions(+)
> 
> diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
> index 96d72ffc0750..146de62887cf 100644
> --- a/drivers/dma-buf/dma-fence.c
> +++ b/drivers/dma-buf/dma-fence.c
> @@ -445,6 +445,50 @@ int dma_fence_signal_locked(struct dma_fence *fence)
>  }
>  EXPORT_SYMBOL(dma_fence_signal_locked);
>  
> +/**
> + * dma_fence_check_and_signal_locked - signal the fence if it's not yet signaled
> + * @fence: the fence to check and signal
> + *
> + * Checks whether a fence was signaled and signals it if it was not yet signaled.
> + *
> + * Unlike dma_fence_check_and_signal(), this function must be called with
> + * &struct dma_fence.lock being held.
> + *
> + * Return: true if fence has been signaled already, false otherwise.
> + */
> +bool dma_fence_check_and_signal_locked(struct dma_fence *fence)
> +{
> +	bool ret;
> +
> +	ret = dma_fence_test_signaled_flag(fence);
> +	dma_fence_signal_locked(fence);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL(dma_fence_check_and_signal_locked);
> +
> +/**
> + * dma_fence_check_and_signal - signal the fence if it's not yet signaled
> + * @fence: the fence to check and signal
> + *
> + * Checks whether a fence was signaled and signals it if it was not yet signaled.
> + * All this is done in a race-free manner.
> + *
> + * Return: true if fence has been signaled already, false otherwise.
> + */
> +bool dma_fence_check_and_signal(struct dma_fence *fence)
> +{
> +	unsigned long flags;
> +	bool ret;
> +
> +	spin_lock_irqsave(fence->lock, flags);
> +	ret = dma_fence_check_and_signal_locked(fence);
> +	spin_unlock_irqrestore(fence->lock, flags);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL(dma_fence_check_and_signal);
> +
>  /**
>   * dma_fence_signal - signal completion of a fence
>   * @fence: the fence to signal
> diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
> index 19972f5d176f..0504afe52c2a 100644
> --- a/include/linux/dma-fence.h
> +++ b/include/linux/dma-fence.h
> @@ -365,6 +365,8 @@ static inline void __dma_fence_might_wait(void) {}
>  #endif
>  
>  int dma_fence_signal(struct dma_fence *fence);
> +bool dma_fence_check_and_signal(struct dma_fence *fence);
> +bool dma_fence_check_and_signal_locked(struct dma_fence *fence);
>  int dma_fence_signal_locked(struct dma_fence *fence);
>  int dma_fence_signal_timestamp(struct dma_fence *fence, ktime_t timestamp);
>  int dma_fence_signal_timestamp_locked(struct dma_fence *fence,

[PATCH v2 3/8] amd/amdkfd: Use dma_fence_check_and_signal()

[Philipp Stanner]

amdkfd is one of the few users which relies on the return code of
dma_fence_signal(), which, so far, informs the caller whether the fence
had already been signaled.

As there are barely any users, dma_fence signaling functions shall get
the return value void. To do so, the few users must be ported to a
function which preserves the old behavior.

Replace the call to dma_fence_signal() with one to
dma_fence_check_and_signal().

Suggested-by: Christian König <christian.koenig@amd.com>
Signed-off-by: Philipp Stanner <phasta@kernel.org>
---
 drivers/gpu/drm/amd/amdkfd/kfd_process.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
index ddfe30c13e9d..4dc46ac6a65e 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
@@ -1983,10 +1983,10 @@ kfd_process_gpuid_from_node(struct kfd_process *p, struct kfd_node *node,
 	return -EINVAL;
 }
 
-static int signal_eviction_fence(struct kfd_process *p)
+static bool signal_eviction_fence(struct kfd_process *p)
 {
 	struct dma_fence *ef;
-	int ret;
+	bool ret;
 
 	rcu_read_lock();
 	ef = dma_fence_get_rcu_safe(&p->ef);
@@ -1994,7 +1994,7 @@ static int signal_eviction_fence(struct kfd_process *p)
 	if (!ef)
 		return -EINVAL;
 
-	ret = dma_fence_signal(ef);
+	ret = dma_fence_check_and_signal(ef);
 	dma_fence_put(ef);
 
 	return ret;
-- 
2.49.0

Re: [PATCH v2 3/8] amd/amdkfd: Use dma_fence_check_and_signal()

[Felix Kuehling]

On 2025-12-01 05:50, Philipp Stanner wrote:
> amdkfd is one of the few users which relies on the return code of
> dma_fence_signal(), which, so far, informs the caller whether the fence
> had already been signaled.
>
> As there are barely any users, dma_fence signaling functions shall get
> the return value void. To do so, the few users must be ported to a
> function which preserves the old behavior.
>
> Replace the call to dma_fence_signal() with one to
> dma_fence_check_and_signal().
>
> Suggested-by: Christian König <christian.koenig@amd.com>
> Signed-off-by: Philipp Stanner <phasta@kernel.org>

This patch is

Reviewed-by: Felix Kuehling <felix.kuehling@amd.com>


> ---
>   drivers/gpu/drm/amd/amdkfd/kfd_process.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> index ddfe30c13e9d..4dc46ac6a65e 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> @@ -1983,10 +1983,10 @@ kfd_process_gpuid_from_node(struct kfd_process *p, struct kfd_node *node,
>   	return -EINVAL;
>   }
>   
> -static int signal_eviction_fence(struct kfd_process *p)
> +static bool signal_eviction_fence(struct kfd_process *p)
>   {
>   	struct dma_fence *ef;
> -	int ret;
> +	bool ret;
>   
>   	rcu_read_lock();
>   	ef = dma_fence_get_rcu_safe(&p->ef);
> @@ -1994,7 +1994,7 @@ static int signal_eviction_fence(struct kfd_process *p)
>   	if (!ef)
>   		return -EINVAL;
>   
> -	ret = dma_fence_signal(ef);
> +	ret = dma_fence_check_and_signal(ef);
>   	dma_fence_put(ef);
>   
>   	return ret;

Re: [PATCH v2 3/8] amd/amdkfd: Use dma_fence_check_and_signal()

[Christian König]

On 12/1/25 11:50, Philipp Stanner wrote:
> amdkfd is one of the few users which relies on the return code of
> dma_fence_signal(), which, so far, informs the caller whether the fence
> had already been signaled.
> 
> As there are barely any users, dma_fence signaling functions shall get
> the return value void. To do so, the few users must be ported to a
> function which preserves the old behavior.
> 
> Replace the call to dma_fence_signal() with one to
> dma_fence_check_and_signal().
> 
> Suggested-by: Christian König <christian.koenig@amd.com>
> Signed-off-by: Philipp Stanner <phasta@kernel.org>

Reviewed-by: Christian König <christian.koenig@amd.com>

> ---
>  drivers/gpu/drm/amd/amdkfd/kfd_process.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> index ddfe30c13e9d..4dc46ac6a65e 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> @@ -1983,10 +1983,10 @@ kfd_process_gpuid_from_node(struct kfd_process *p, struct kfd_node *node,
>  	return -EINVAL;
>  }
>  
> -static int signal_eviction_fence(struct kfd_process *p)
> +static bool signal_eviction_fence(struct kfd_process *p)
>  {
>  	struct dma_fence *ef;
> -	int ret;
> +	bool ret;
>  
>  	rcu_read_lock();
>  	ef = dma_fence_get_rcu_safe(&p->ef);
> @@ -1994,7 +1994,7 @@ static int signal_eviction_fence(struct kfd_process *p)
>  	if (!ef)
>  		return -EINVAL;
>  
> -	ret = dma_fence_signal(ef);
> +	ret = dma_fence_check_and_signal(ef);
>  	dma_fence_put(ef);
>  
>  	return ret;

[PATCH v2 4/8] drm/xe: Use dma_fence_check_and_signal_locked()

[Philipp Stanner]

Xe is one of the few users utilizing the return code of
dma_fence_signal() to check whether a fence had already been signaled by
someone else.

To clean up and simplify the dma_fence API, the few kernel users relying
on that behavior shall be ported to an alternative function.

Replace dma_fence_signal_locked() with
dma_fence_check_and_signal_locked().

Signed-off-by: Philipp Stanner <phasta@kernel.org>
---
 drivers/gpu/drm/xe/xe_hw_fence.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/xe/xe_hw_fence.c b/drivers/gpu/drm/xe/xe_hw_fence.c
index b2a0c46dfcd4..f6057456e460 100644
--- a/drivers/gpu/drm/xe/xe_hw_fence.c
+++ b/drivers/gpu/drm/xe/xe_hw_fence.c
@@ -85,7 +85,6 @@ void xe_hw_fence_irq_finish(struct xe_hw_fence_irq *irq)
 {
 	struct xe_hw_fence *fence, *next;
 	unsigned long flags;
-	int err;
 	bool tmp;
 
 	if (XE_WARN_ON(!list_empty(&irq->pending))) {
@@ -93,9 +92,8 @@ void xe_hw_fence_irq_finish(struct xe_hw_fence_irq *irq)
 		spin_lock_irqsave(&irq->lock, flags);
 		list_for_each_entry_safe(fence, next, &irq->pending, irq_link) {
 			list_del_init(&fence->irq_link);
-			err = dma_fence_signal_locked(&fence->dma);
+			XE_WARN_ON(dma_fence_check_and_signal_locked(&fence->dma));
 			dma_fence_put(&fence->dma);
-			XE_WARN_ON(err);
 		}
 		spin_unlock_irqrestore(&irq->lock, flags);
 		dma_fence_end_signalling(tmp);
-- 
2.49.0

Re: [PATCH v2 4/8] drm/xe: Use dma_fence_check_and_signal_locked()

[Matthew Brost]

On Mon, Dec 01, 2025 at 11:50:08AM +0100, Philipp Stanner wrote:
> Xe is one of the few users utilizing the return code of
> dma_fence_signal() to check whether a fence had already been signaled by
> someone else.
> 
> To clean up and simplify the dma_fence API, the few kernel users relying
> on that behavior shall be ported to an alternative function.
> 
> Replace dma_fence_signal_locked() with
> dma_fence_check_and_signal_locked().
> 
> Signed-off-by: Philipp Stanner <phasta@kernel.org>
> ---
>  drivers/gpu/drm/xe/xe_hw_fence.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_hw_fence.c b/drivers/gpu/drm/xe/xe_hw_fence.c
> index b2a0c46dfcd4..f6057456e460 100644
> --- a/drivers/gpu/drm/xe/xe_hw_fence.c
> +++ b/drivers/gpu/drm/xe/xe_hw_fence.c
> @@ -85,7 +85,6 @@ void xe_hw_fence_irq_finish(struct xe_hw_fence_irq *irq)
>  {
>  	struct xe_hw_fence *fence, *next;
>  	unsigned long flags;
> -	int err;
>  	bool tmp;
>  
>  	if (XE_WARN_ON(!list_empty(&irq->pending))) {
> @@ -93,9 +92,8 @@ void xe_hw_fence_irq_finish(struct xe_hw_fence_irq *irq)
>  		spin_lock_irqsave(&irq->lock, flags);
>  		list_for_each_entry_safe(fence, next, &irq->pending, irq_link) {
>  			list_del_init(&fence->irq_link);
> -			err = dma_fence_signal_locked(&fence->dma);
> +			XE_WARN_ON(dma_fence_check_and_signal_locked(&fence->dma));

I think XE_WARN_ON can compile out in certain builds. Best to leave warn on logic as is.

Also a little confused by this new helper... Doesn't
dma_fence_signal_locked already check if a fence is already signaled and
bail? Running out the door so I don't have time dig in here, but can you
explain?

Matt

>  			dma_fence_put(&fence->dma);
> -			XE_WARN_ON(err);
>  		}
>  		spin_unlock_irqrestore(&irq->lock, flags);
>  		dma_fence_end_signalling(tmp);
> -- 
> 2.49.0
> 

Re: [PATCH v2 4/8] drm/xe: Use dma_fence_check_and_signal_locked()

[Philipp Stanner]

On Mon, 2025-12-01 at 11:38 -0800, Matthew Brost wrote:
> On Mon, Dec 01, 2025 at 11:50:08AM +0100, Philipp Stanner wrote:
> > Xe is one of the few users utilizing the return code of
> > dma_fence_signal() to check whether a fence had already been signaled by
> > someone else.
> > 
> > To clean up and simplify the dma_fence API, the few kernel users relying
> > on that behavior shall be ported to an alternative function.
> > 
> > Replace dma_fence_signal_locked() with
> > dma_fence_check_and_signal_locked().
> > 
> > Signed-off-by: Philipp Stanner <phasta@kernel.org>
> > ---
> >  drivers/gpu/drm/xe/xe_hw_fence.c | 4 +---
> >  1 file changed, 1 insertion(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/xe/xe_hw_fence.c b/drivers/gpu/drm/xe/xe_hw_fence.c
> > index b2a0c46dfcd4..f6057456e460 100644
> > --- a/drivers/gpu/drm/xe/xe_hw_fence.c
> > +++ b/drivers/gpu/drm/xe/xe_hw_fence.c
> > @@ -85,7 +85,6 @@ void xe_hw_fence_irq_finish(struct xe_hw_fence_irq *irq)
> >  {
> >  	struct xe_hw_fence *fence, *next;
> >  	unsigned long flags;
> > -	int err;
> >  	bool tmp;
> >  
> >  	if (XE_WARN_ON(!list_empty(&irq->pending))) {
> > @@ -93,9 +92,8 @@ void xe_hw_fence_irq_finish(struct xe_hw_fence_irq *irq)
> >  		spin_lock_irqsave(&irq->lock, flags);
> >  		list_for_each_entry_safe(fence, next, &irq->pending, irq_link) {
> >  			list_del_init(&fence->irq_link);
> > -			err = dma_fence_signal_locked(&fence->dma);
> > +			XE_WARN_ON(dma_fence_check_and_signal_locked(&fence->dma));
> 
> I think XE_WARN_ON can compile out in certain builds. Best to leave warn on logic as is.

OK, will adjust.

> 
> Also a little confused by this new helper... Doesn't
> dma_fence_signal_locked already check if a fence is already signaled and
> bail? Running out the door so I don't have time dig in here, but can you
> explain?

Yes, that is what dma_fence_signal_locked() *currently* does. The
series, however, is about removing that check from the default
interfaces because barely anyone uses dma_fence_signal() et.al.'s
return code. To simplify the interfaces.
The 2-3 users who need the code get this new function. See cover
letter.


P.

> 
> Matt
> 
> >  			dma_fence_put(&fence->dma);
> > -			XE_WARN_ON(err);
> >  		}
> >  		spin_unlock_irqrestore(&irq->lock, flags);
> >  		dma_fence_end_signalling(tmp);
> > -- 
> > 2.49.0
> > 

Re: [PATCH v2 4/8] drm/xe: Use dma_fence_check_and_signal_locked()

[Matthew Brost]

On Tue, Dec 02, 2025 at 08:17:17AM +0100, Philipp Stanner wrote:
> On Mon, 2025-12-01 at 11:38 -0800, Matthew Brost wrote:
> > On Mon, Dec 01, 2025 at 11:50:08AM +0100, Philipp Stanner wrote:
> > > Xe is one of the few users utilizing the return code of
> > > dma_fence_signal() to check whether a fence had already been signaled by
> > > someone else.
> > > 
> > > To clean up and simplify the dma_fence API, the few kernel users relying
> > > on that behavior shall be ported to an alternative function.
> > > 
> > > Replace dma_fence_signal_locked() with
> > > dma_fence_check_and_signal_locked().
> > > 
> > > Signed-off-by: Philipp Stanner <phasta@kernel.org>
> > > ---
> > >  drivers/gpu/drm/xe/xe_hw_fence.c | 4 +---
> > >  1 file changed, 1 insertion(+), 3 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/xe/xe_hw_fence.c b/drivers/gpu/drm/xe/xe_hw_fence.c
> > > index b2a0c46dfcd4..f6057456e460 100644
> > > --- a/drivers/gpu/drm/xe/xe_hw_fence.c
> > > +++ b/drivers/gpu/drm/xe/xe_hw_fence.c
> > > @@ -85,7 +85,6 @@ void xe_hw_fence_irq_finish(struct xe_hw_fence_irq *irq)
> > >  {
> > >  	struct xe_hw_fence *fence, *next;
> > >  	unsigned long flags;
> > > -	int err;
> > >  	bool tmp;
> > >  
> > >  	if (XE_WARN_ON(!list_empty(&irq->pending))) {
> > > @@ -93,9 +92,8 @@ void xe_hw_fence_irq_finish(struct xe_hw_fence_irq *irq)
> > >  		spin_lock_irqsave(&irq->lock, flags);
> > >  		list_for_each_entry_safe(fence, next, &irq->pending, irq_link) {
> > >  			list_del_init(&fence->irq_link);
> > > -			err = dma_fence_signal_locked(&fence->dma);
> > > +			XE_WARN_ON(dma_fence_check_and_signal_locked(&fence->dma));
> > 
> > I think XE_WARN_ON can compile out in certain builds. Best to leave warn on logic as is.
> 
> OK, will adjust.
> 
> > 
> > Also a little confused by this new helper... Doesn't
> > dma_fence_signal_locked already check if a fence is already signaled and
> > bail? Running out the door so I don't have time dig in here, but can you
> > explain?
> 
> Yes, that is what dma_fence_signal_locked() *currently* does. The
> series, however, is about removing that check from the default
> interfaces because barely anyone uses dma_fence_signal() et.al.'s
> return code. To simplify the interfaces.
> The 2-3 users who need the code get this new function. See cover
> letter.
> 

Thanks for explaination. This should work then with XE_WARN_ON refactor.

Matt

> 
> P.
> 
> > 
> > Matt
> > 
> > >  			dma_fence_put(&fence->dma);
> > > -			XE_WARN_ON(err);
> > >  		}
> > >  		spin_unlock_irqrestore(&irq->lock, flags);
> > >  		dma_fence_end_signalling(tmp);
> > > -- 
> > > 2.49.0
> > > 
> 

Re: [PATCH v2 4/8] drm/xe: Use dma_fence_check_and_signal_locked()

[Andi Shyti]

Hi Matt,

> > @@ -93,9 +92,8 @@ void xe_hw_fence_irq_finish(struct xe_hw_fence_irq *irq)
> >  		spin_lock_irqsave(&irq->lock, flags);
> >  		list_for_each_entry_safe(fence, next, &irq->pending, irq_link) {
> >  			list_del_init(&fence->irq_link);
> > -			err = dma_fence_signal_locked(&fence->dma);
> > +			XE_WARN_ON(dma_fence_check_and_signal_locked(&fence->dma));
> 
> I think XE_WARN_ON can compile out in certain builds. Best to leave warn on logic as is.

I don't think XE_WARN_ON compiles out. It should always evaluate
the content, then, depending on the build, it prints debug logs.

Andi

Re: [PATCH v2 4/8] drm/xe: Use dma_fence_check_and_signal_locked()

[Matthew Brost]

On Tue, Dec 02, 2025 at 09:47:39PM +0100, Andi Shyti wrote:
> Hi Matt,
> 
> > > @@ -93,9 +92,8 @@ void xe_hw_fence_irq_finish(struct xe_hw_fence_irq *irq)
> > >  		spin_lock_irqsave(&irq->lock, flags);
> > >  		list_for_each_entry_safe(fence, next, &irq->pending, irq_link) {
> > >  			list_del_init(&fence->irq_link);
> > > -			err = dma_fence_signal_locked(&fence->dma);
> > > +			XE_WARN_ON(dma_fence_check_and_signal_locked(&fence->dma));
> > 
> > I think XE_WARN_ON can compile out in certain builds. Best to leave warn on logic as is.
> 
> I don't think XE_WARN_ON compiles out. It should always evaluate
> the content, then, depending on the build, it prints debug logs.

Ah, ok. I thought XE_WARN_ON was tied to a Kconfig to compile out or
WARN_ON (this is what XE_WARN_ON resolves) compiled out in some kernel
builds. Upon more looking, I guess neither of these is the case.

So I guess this patch is actually:
Reviewed-by: Matthew Brost <matthew.brost@intel.com>

Sorry for the noise. Side note, we should probably just XE_WARN_ON too
since this is just WARN_ON.

Matt

> 
> Andi

Re: [PATCH v2 4/8] drm/xe: Use dma_fence_check_and_signal_locked()

[Rodrigo Vivi]

On Tue, Dec 02, 2025 at 01:04:24PM -0800, Matthew Brost wrote:
> On Tue, Dec 02, 2025 at 09:47:39PM +0100, Andi Shyti wrote:
> > Hi Matt,
> > 
> > > > @@ -93,9 +92,8 @@ void xe_hw_fence_irq_finish(struct xe_hw_fence_irq *irq)
> > > >  		spin_lock_irqsave(&irq->lock, flags);
> > > >  		list_for_each_entry_safe(fence, next, &irq->pending, irq_link) {
> > > >  			list_del_init(&fence->irq_link);
> > > > -			err = dma_fence_signal_locked(&fence->dma);
> > > > +			XE_WARN_ON(dma_fence_check_and_signal_locked(&fence->dma));
> > > 
> > > I think XE_WARN_ON can compile out in certain builds. Best to leave warn on logic as is.
> > 
> > I don't think XE_WARN_ON compiles out. It should always evaluate
> > the content, then, depending on the build, it prints debug logs.
> 
> Ah, ok. I thought XE_WARN_ON was tied to a Kconfig to compile out or
> WARN_ON (this is what XE_WARN_ON resolves) compiled out in some kernel
> builds. Upon more looking, I guess neither of these is the case.
> 
> So I guess this patch is actually:
> Reviewed-by: Matthew Brost <matthew.brost@intel.com>

and

Acked-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

if needed to merge through any other tree...

> 
> Sorry for the noise. Side note, we should probably just XE_WARN_ON too
> since this is just WARN_ON.
> 
> Matt
> 
> > 
> > Andi

Re: [PATCH v2 4/8] drm/xe: Use dma_fence_check_and_signal_locked()

[Christian König]

On 12/1/25 11:50, Philipp Stanner wrote:
> Xe is one of the few users utilizing the return code of
> dma_fence_signal() to check whether a fence had already been signaled by
> someone else.
> 
> To clean up and simplify the dma_fence API, the few kernel users relying
> on that behavior shall be ported to an alternative function.
> 
> Replace dma_fence_signal_locked() with
> dma_fence_check_and_signal_locked().
> 
> Signed-off-by: Philipp Stanner <phasta@kernel.org>

Acked-by: Christian König <christian.koenig@amd.com>

> ---
>  drivers/gpu/drm/xe/xe_hw_fence.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_hw_fence.c b/drivers/gpu/drm/xe/xe_hw_fence.c
> index b2a0c46dfcd4..f6057456e460 100644
> --- a/drivers/gpu/drm/xe/xe_hw_fence.c
> +++ b/drivers/gpu/drm/xe/xe_hw_fence.c
> @@ -85,7 +85,6 @@ void xe_hw_fence_irq_finish(struct xe_hw_fence_irq *irq)
>  {
>  	struct xe_hw_fence *fence, *next;
>  	unsigned long flags;
> -	int err;
>  	bool tmp;
>  
>  	if (XE_WARN_ON(!list_empty(&irq->pending))) {
> @@ -93,9 +92,8 @@ void xe_hw_fence_irq_finish(struct xe_hw_fence_irq *irq)
>  		spin_lock_irqsave(&irq->lock, flags);
>  		list_for_each_entry_safe(fence, next, &irq->pending, irq_link) {
>  			list_del_init(&fence->irq_link);
> -			err = dma_fence_signal_locked(&fence->dma);
> +			XE_WARN_ON(dma_fence_check_and_signal_locked(&fence->dma));
>  			dma_fence_put(&fence->dma);
> -			XE_WARN_ON(err);
>  		}
>  		spin_unlock_irqrestore(&irq->lock, flags);
>  		dma_fence_end_signalling(tmp);

[PATCH v2 5/8] dma-buf: Don't misuse dma_fence_signal()

[Philipp Stanner]

The return code of dma_fence_signal() is not really useful as there is
nothing reasonable to do if a fence was already signaled. That return
code shall be removed from the kernel.

Moreover, dma_fence_signal() should not be used to check whether fences
are signaled. That's what dma_fence_is_signaled() and
dma_fence_test_signaled_flag() exist for.

Replace the non-canonical usage of dma_fence_signal().

Suggested-by: Christian König <christian.koenig@amd.com>
Signed-off-by: Philipp Stanner <phasta@kernel.org>
---
 drivers/dma-buf/st-dma-fence.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/dma-buf/st-dma-fence.c b/drivers/dma-buf/st-dma-fence.c
index 27a36045410b..4dbe39c58bfb 100644
--- a/drivers/dma-buf/st-dma-fence.c
+++ b/drivers/dma-buf/st-dma-fence.c
@@ -126,7 +126,7 @@ static int test_signaling(void *arg)
 		goto err_free;
 	}
 
-	if (dma_fence_signal(f)) {
+	if (dma_fence_check_and_signal(f)) {
 		pr_err("Fence reported being already signaled\n");
 		goto err_free;
 	}
@@ -136,7 +136,7 @@ static int test_signaling(void *arg)
 		goto err_free;
 	}
 
-	if (!dma_fence_signal(f)) {
+	if (!dma_fence_test_signaled_flag(f)) {
 		pr_err("Fence reported not being already signaled\n");
 		goto err_free;
 	}
-- 
2.49.0

Re: [PATCH v2 5/8] dma-buf: Don't misuse dma_fence_signal()

[Christian König]

On 12/1/25 11:50, Philipp Stanner wrote:
> The return code of dma_fence_signal() is not really useful as there is
> nothing reasonable to do if a fence was already signaled. That return
> code shall be removed from the kernel.
> 
> Moreover, dma_fence_signal() should not be used to check whether fences
> are signaled. That's what dma_fence_is_signaled() and
> dma_fence_test_signaled_flag() exist for.
> 
> Replace the non-canonical usage of dma_fence_signal().
> 
> Suggested-by: Christian König <christian.koenig@amd.com>
> Signed-off-by: Philipp Stanner <phasta@kernel.org>

Reviewed-by: Christian König <christian.koenig@amd.com>

> ---
>  drivers/dma-buf/st-dma-fence.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/dma-buf/st-dma-fence.c b/drivers/dma-buf/st-dma-fence.c
> index 27a36045410b..4dbe39c58bfb 100644
> --- a/drivers/dma-buf/st-dma-fence.c
> +++ b/drivers/dma-buf/st-dma-fence.c
> @@ -126,7 +126,7 @@ static int test_signaling(void *arg)
>  		goto err_free;
>  	}
>  
> -	if (dma_fence_signal(f)) {
> +	if (dma_fence_check_and_signal(f)) {
>  		pr_err("Fence reported being already signaled\n");
>  		goto err_free;
>  	}
> @@ -136,7 +136,7 @@ static int test_signaling(void *arg)
>  		goto err_free;
>  	}
>  
> -	if (!dma_fence_signal(f)) {
> +	if (!dma_fence_test_signaled_flag(f)) {
>  		pr_err("Fence reported not being already signaled\n");
>  		goto err_free;
>  	}

[PATCH v2 6/8] drm/ttm: Use dma_fence_check_and_signal()

[Philipp Stanner]

The return code of dma_fence_signal() is not useful and shall be removed
from the kernel. To do so, the few users who rely on the return code
must be ported.

Use dma_fence_check_and_signal() and mapp its boolean return code to
dma_fence_signal()'s former value for already-signaled fences.

Suggested-by: Christian König <christian.koenig@amd.com>
Signed-off-by: Philipp Stanner <phasta@kernel.org>
---
 drivers/gpu/drm/ttm/tests/ttm_bo_validate_test.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/ttm/tests/ttm_bo_validate_test.c b/drivers/gpu/drm/ttm/tests/ttm_bo_validate_test.c
index 1bcc67977f48..eaf2e91f8e97 100644
--- a/drivers/gpu/drm/ttm/tests/ttm_bo_validate_test.c
+++ b/drivers/gpu/drm/ttm/tests/ttm_bo_validate_test.c
@@ -692,7 +692,7 @@ static int threaded_fence_signal(void *arg)
 
 	msleep(20);
 
-	return dma_fence_signal(fence);
+	return dma_fence_check_and_signal(fence) ? -EINVAL : 0;
 }
 
 static void ttm_bo_validate_move_fence_not_signaled(struct kunit *test)
-- 
2.49.0

Re: [PATCH v2 6/8] drm/ttm: Use dma_fence_check_and_signal()

[Christian König]

On 12/1/25 11:50, Philipp Stanner wrote:
> The return code of dma_fence_signal() is not useful and shall be removed
> from the kernel. To do so, the few users who rely on the return code
> must be ported.
> 
> Use dma_fence_check_and_signal() and mapp its boolean return code to
> dma_fence_signal()'s former value for already-signaled fences.
> 
> Suggested-by: Christian König <christian.koenig@amd.com>
> Signed-off-by: Philipp Stanner <phasta@kernel.org>

Reviewed-by: Christian König <christian.koenig@amd.com>

> ---
>  drivers/gpu/drm/ttm/tests/ttm_bo_validate_test.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/ttm/tests/ttm_bo_validate_test.c b/drivers/gpu/drm/ttm/tests/ttm_bo_validate_test.c
> index 1bcc67977f48..eaf2e91f8e97 100644
> --- a/drivers/gpu/drm/ttm/tests/ttm_bo_validate_test.c
> +++ b/drivers/gpu/drm/ttm/tests/ttm_bo_validate_test.c
> @@ -692,7 +692,7 @@ static int threaded_fence_signal(void *arg)
>  
>  	msleep(20);
>  
> -	return dma_fence_signal(fence);
> +	return dma_fence_check_and_signal(fence) ? -EINVAL : 0;
>  }
>  
>  static void ttm_bo_validate_move_fence_not_signaled(struct kunit *test)

[PATCH v2 7/8] dma-buf/dma-fence: Remove return code of signaling-functions

[Philipp Stanner]

All functions used for signaling a fence return an error code whose sole
purpose is to tell whether a fence was already signaled.

This is racy and has been used by almost no party in the kernel, and the
few users have been removed in preceding cleanup commits.

Turn all signaling-functions into void-functions.

Suggested-by: Christian König <christian.koenig@amd.com>
Signed-off-by: Philipp Stanner <phasta@kernel.org>
---
 drivers/dma-buf/dma-fence.c | 40 ++++++++++---------------------------
 include/linux/dma-fence.h   |  9 ++++-----
 2 files changed, 14 insertions(+), 35 deletions(-)

diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
index 146de62887cf..529a44371b35 100644
--- a/drivers/dma-buf/dma-fence.c
+++ b/drivers/dma-buf/dma-fence.c
@@ -360,11 +360,8 @@ void __dma_fence_might_wait(void)
  *
  * Unlike dma_fence_signal_timestamp(), 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.
  */
-int dma_fence_signal_timestamp_locked(struct dma_fence *fence,
+void dma_fence_signal_timestamp_locked(struct dma_fence *fence,
 				      ktime_t timestamp)
 {
 	struct dma_fence_cb *cur, *tmp;
@@ -374,7 +371,7 @@ int dma_fence_signal_timestamp_locked(struct dma_fence *fence,
 
 	if (unlikely(test_and_set_bit(DMA_FENCE_FLAG_SIGNALED_BIT,
 				      &fence->flags)))
-		return -EINVAL;
+		return;
 
 	/* Stash the cb_list before replacing it with the timestamp */
 	list_replace(&fence->cb_list, &cb_list);
@@ -387,8 +384,6 @@ int dma_fence_signal_timestamp_locked(struct dma_fence *fence,
 		INIT_LIST_HEAD(&cur->node);
 		cur->func(fence, cur);
 	}
-
-	return 0;
 }
 EXPORT_SYMBOL(dma_fence_signal_timestamp_locked);
 
@@ -403,23 +398,17 @@ EXPORT_SYMBOL(dma_fence_signal_timestamp_locked);
  * can only go from the unsignaled to the signaled state and not back, it will
  * only be effective the first time. Set the timestamp provided as the fence
  * signal timestamp.
- *
- * Returns 0 on success and a negative error value when @fence has been
- * signalled already.
  */
-int dma_fence_signal_timestamp(struct dma_fence *fence, ktime_t timestamp)
+void dma_fence_signal_timestamp(struct dma_fence *fence, ktime_t timestamp)
 {
 	unsigned long flags;
-	int ret;
 
 	if (WARN_ON(!fence))
-		return -EINVAL;
+		return;
 
 	spin_lock_irqsave(fence->lock, flags);
-	ret = dma_fence_signal_timestamp_locked(fence, timestamp);
+	dma_fence_signal_timestamp_locked(fence, timestamp);
 	spin_unlock_irqrestore(fence->lock, flags);
-
-	return ret;
 }
 EXPORT_SYMBOL(dma_fence_signal_timestamp);
 
@@ -435,13 +424,10 @@ EXPORT_SYMBOL(dma_fence_signal_timestamp);
  *
  * 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.
  */
-int dma_fence_signal_locked(struct dma_fence *fence)
+void dma_fence_signal_locked(struct dma_fence *fence)
 {
-	return dma_fence_signal_timestamp_locked(fence, ktime_get());
+	dma_fence_signal_timestamp_locked(fence, ktime_get());
 }
 EXPORT_SYMBOL(dma_fence_signal_locked);
 
@@ -498,28 +484,22 @@ EXPORT_SYMBOL(dma_fence_check_and_signal);
  * 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.
- *
- * Returns 0 on success and a negative error value when @fence has been
- * signalled already.
  */
-int dma_fence_signal(struct dma_fence *fence)
+void dma_fence_signal(struct dma_fence *fence)
 {
 	unsigned long flags;
-	int ret;
 	bool tmp;
 
 	if (WARN_ON(!fence))
-		return -EINVAL;
+		return;
 
 	tmp = dma_fence_begin_signalling();
 
 	spin_lock_irqsave(fence->lock, flags);
-	ret = dma_fence_signal_timestamp_locked(fence, ktime_get());
+	dma_fence_signal_timestamp_locked(fence, ktime_get());
 	spin_unlock_irqrestore(fence->lock, flags);
 
 	dma_fence_end_signalling(tmp);
-
-	return ret;
 }
 EXPORT_SYMBOL(dma_fence_signal);
 
diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
index 0504afe52c2a..d4c92fd35092 100644
--- a/include/linux/dma-fence.h
+++ b/include/linux/dma-fence.h
@@ -364,13 +364,12 @@ static inline void dma_fence_end_signalling(bool cookie) {}
 static inline void __dma_fence_might_wait(void) {}
 #endif
 
-int dma_fence_signal(struct dma_fence *fence);
+void dma_fence_signal(struct dma_fence *fence);
 bool dma_fence_check_and_signal(struct dma_fence *fence);
 bool dma_fence_check_and_signal_locked(struct dma_fence *fence);
-int dma_fence_signal_locked(struct dma_fence *fence);
-int dma_fence_signal_timestamp(struct dma_fence *fence, ktime_t timestamp);
-int dma_fence_signal_timestamp_locked(struct dma_fence *fence,
-				      ktime_t timestamp);
+void dma_fence_signal_locked(struct dma_fence *fence);
+void dma_fence_signal_timestamp(struct dma_fence *fence, ktime_t timestamp);
+void dma_fence_signal_timestamp_locked(struct dma_fence *fence, ktime_t timestamp);
 signed long dma_fence_default_wait(struct dma_fence *fence,
 				   bool intr, signed long timeout);
 int dma_fence_add_callback(struct dma_fence *fence,
-- 
2.49.0

Re: [PATCH v2 7/8] dma-buf/dma-fence: Remove return code of signaling-functions

[Christian König]

On 12/1/25 11:50, Philipp Stanner wrote:
> All functions used for signaling a fence return an error code whose sole
> purpose is to tell whether a fence was already signaled.
> 
> This is racy and has been used by almost no party in the kernel, and the
> few users have been removed in preceding cleanup commits.
> 
> Turn all signaling-functions into void-functions.
> 
> Suggested-by: Christian König <christian.koenig@amd.com>
> Signed-off-by: Philipp Stanner <phasta@kernel.org>

Reviewed-by: Christian König <christian.koenig@amd.com>

> ---
>  drivers/dma-buf/dma-fence.c | 40 ++++++++++---------------------------
>  include/linux/dma-fence.h   |  9 ++++-----
>  2 files changed, 14 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
> index 146de62887cf..529a44371b35 100644
> --- a/drivers/dma-buf/dma-fence.c
> +++ b/drivers/dma-buf/dma-fence.c
> @@ -360,11 +360,8 @@ void __dma_fence_might_wait(void)
>   *
>   * Unlike dma_fence_signal_timestamp(), 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.
>   */
> -int dma_fence_signal_timestamp_locked(struct dma_fence *fence,
> +void dma_fence_signal_timestamp_locked(struct dma_fence *fence,
>  				      ktime_t timestamp)
>  {
>  	struct dma_fence_cb *cur, *tmp;
> @@ -374,7 +371,7 @@ int dma_fence_signal_timestamp_locked(struct dma_fence *fence,
>  
>  	if (unlikely(test_and_set_bit(DMA_FENCE_FLAG_SIGNALED_BIT,
>  				      &fence->flags)))
> -		return -EINVAL;
> +		return;
>  
>  	/* Stash the cb_list before replacing it with the timestamp */
>  	list_replace(&fence->cb_list, &cb_list);
> @@ -387,8 +384,6 @@ int dma_fence_signal_timestamp_locked(struct dma_fence *fence,
>  		INIT_LIST_HEAD(&cur->node);
>  		cur->func(fence, cur);
>  	}
> -
> -	return 0;
>  }
>  EXPORT_SYMBOL(dma_fence_signal_timestamp_locked);
>  
> @@ -403,23 +398,17 @@ EXPORT_SYMBOL(dma_fence_signal_timestamp_locked);
>   * can only go from the unsignaled to the signaled state and not back, it will
>   * only be effective the first time. Set the timestamp provided as the fence
>   * signal timestamp.
> - *
> - * Returns 0 on success and a negative error value when @fence has been
> - * signalled already.
>   */
> -int dma_fence_signal_timestamp(struct dma_fence *fence, ktime_t timestamp)
> +void dma_fence_signal_timestamp(struct dma_fence *fence, ktime_t timestamp)
>  {
>  	unsigned long flags;
> -	int ret;
>  
>  	if (WARN_ON(!fence))
> -		return -EINVAL;
> +		return;
>  
>  	spin_lock_irqsave(fence->lock, flags);
> -	ret = dma_fence_signal_timestamp_locked(fence, timestamp);
> +	dma_fence_signal_timestamp_locked(fence, timestamp);
>  	spin_unlock_irqrestore(fence->lock, flags);
> -
> -	return ret;
>  }
>  EXPORT_SYMBOL(dma_fence_signal_timestamp);
>  
> @@ -435,13 +424,10 @@ EXPORT_SYMBOL(dma_fence_signal_timestamp);
>   *
>   * 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.
>   */
> -int dma_fence_signal_locked(struct dma_fence *fence)
> +void dma_fence_signal_locked(struct dma_fence *fence)
>  {
> -	return dma_fence_signal_timestamp_locked(fence, ktime_get());
> +	dma_fence_signal_timestamp_locked(fence, ktime_get());
>  }
>  EXPORT_SYMBOL(dma_fence_signal_locked);
>  
> @@ -498,28 +484,22 @@ EXPORT_SYMBOL(dma_fence_check_and_signal);
>   * 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.
> - *
> - * Returns 0 on success and a negative error value when @fence has been
> - * signalled already.
>   */
> -int dma_fence_signal(struct dma_fence *fence)
> +void dma_fence_signal(struct dma_fence *fence)
>  {
>  	unsigned long flags;
> -	int ret;
>  	bool tmp;
>  
>  	if (WARN_ON(!fence))
> -		return -EINVAL;
> +		return;
>  
>  	tmp = dma_fence_begin_signalling();
>  
>  	spin_lock_irqsave(fence->lock, flags);
> -	ret = dma_fence_signal_timestamp_locked(fence, ktime_get());
> +	dma_fence_signal_timestamp_locked(fence, ktime_get());
>  	spin_unlock_irqrestore(fence->lock, flags);
>  
>  	dma_fence_end_signalling(tmp);
> -
> -	return ret;
>  }
>  EXPORT_SYMBOL(dma_fence_signal);
>  
> diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
> index 0504afe52c2a..d4c92fd35092 100644
> --- a/include/linux/dma-fence.h
> +++ b/include/linux/dma-fence.h
> @@ -364,13 +364,12 @@ static inline void dma_fence_end_signalling(bool cookie) {}
>  static inline void __dma_fence_might_wait(void) {}
>  #endif
>  
> -int dma_fence_signal(struct dma_fence *fence);
> +void dma_fence_signal(struct dma_fence *fence);
>  bool dma_fence_check_and_signal(struct dma_fence *fence);
>  bool dma_fence_check_and_signal_locked(struct dma_fence *fence);
> -int dma_fence_signal_locked(struct dma_fence *fence);
> -int dma_fence_signal_timestamp(struct dma_fence *fence, ktime_t timestamp);
> -int dma_fence_signal_timestamp_locked(struct dma_fence *fence,
> -				      ktime_t timestamp);
> +void dma_fence_signal_locked(struct dma_fence *fence);
> +void dma_fence_signal_timestamp(struct dma_fence *fence, ktime_t timestamp);
> +void dma_fence_signal_timestamp_locked(struct dma_fence *fence, ktime_t timestamp);
>  signed long dma_fence_default_wait(struct dma_fence *fence,
>  				   bool intr, signed long timeout);
>  int dma_fence_add_callback(struct dma_fence *fence,

[PATCH v2 8/8] drm/xe: Use dma_fence_test_signaled_flag()

[Philipp Stanner]

There is a new dma_fence helper which simplifies testing for a fence's
signaled_flag. Use it in xe.

Signed-off-by: Philipp Stanner <phasta@kernel.org>
---
 drivers/gpu/drm/xe/xe_exec_queue.c | 9 +++------
 drivers/gpu/drm/xe/xe_pt.c         | 3 +--
 drivers/gpu/drm/xe/xe_sched_job.c  | 2 +-
 3 files changed, 5 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/xe/xe_exec_queue.c b/drivers/gpu/drm/xe/xe_exec_queue.c
index cb5f204c08ed..06736f52fbaa 100644
--- a/drivers/gpu/drm/xe/xe_exec_queue.c
+++ b/drivers/gpu/drm/xe/xe_exec_queue.c
@@ -1037,8 +1037,7 @@ struct dma_fence *xe_exec_queue_last_fence_get(struct xe_exec_queue *q,
 
 	xe_exec_queue_last_fence_lockdep_assert(q, vm);
 
-	if (q->last_fence &&
-	    test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &q->last_fence->flags))
+	if (q->last_fence && dma_fence_test_signaled_flag(q->last_fence))
 		xe_exec_queue_last_fence_put(q, vm);
 
 	fence = q->last_fence ? q->last_fence : dma_fence_get_stub();
@@ -1064,8 +1063,7 @@ struct dma_fence *xe_exec_queue_last_fence_get_for_resume(struct xe_exec_queue *
 
 	lockdep_assert_held_write(&q->hwe->hw_engine_group->mode_sem);
 
-	if (q->last_fence &&
-	    test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &q->last_fence->flags))
+	if (q->last_fence && dma_fence_test_signaled_flag(q->last_fence))
 		xe_exec_queue_last_fence_put_unlocked(q);
 
 	fence = q->last_fence ? q->last_fence : dma_fence_get_stub();
@@ -1106,8 +1104,7 @@ int xe_exec_queue_last_fence_test_dep(struct xe_exec_queue *q, struct xe_vm *vm)
 
 	fence = xe_exec_queue_last_fence_get(q, vm);
 	if (fence) {
-		err = test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags) ?
-			0 : -ETIME;
+		err = dma_fence_test_signaled_flag(fence) ? 0 : -ETIME;
 		dma_fence_put(fence);
 	}
 
diff --git a/drivers/gpu/drm/xe/xe_pt.c b/drivers/gpu/drm/xe/xe_pt.c
index 07f96bda638a..1ca2dec18e51 100644
--- a/drivers/gpu/drm/xe/xe_pt.c
+++ b/drivers/gpu/drm/xe/xe_pt.c
@@ -1208,8 +1208,7 @@ static bool no_in_syncs(struct xe_sync_entry *syncs, u32 num_syncs)
 	for (i = 0; i < num_syncs; i++) {
 		struct dma_fence *fence = syncs[i].fence;
 
-		if (fence && !test_bit(DMA_FENCE_FLAG_SIGNALED_BIT,
-				       &fence->flags))
+		if (fence && !dma_fence_test_signaled_flag(fence))
 			return false;
 	}
 
diff --git a/drivers/gpu/drm/xe/xe_sched_job.c b/drivers/gpu/drm/xe/xe_sched_job.c
index d21bf8f26964..1c9ba49a325b 100644
--- a/drivers/gpu/drm/xe/xe_sched_job.c
+++ b/drivers/gpu/drm/xe/xe_sched_job.c
@@ -188,7 +188,7 @@ static bool xe_fence_set_error(struct dma_fence *fence, int error)
 	bool signaled;
 
 	spin_lock_irqsave(fence->lock, irq_flags);
-	signaled = test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags);
+	signaled = dma_fence_test_signaled_flag(fence);
 	if (!signaled)
 		dma_fence_set_error(fence, error);
 	spin_unlock_irqrestore(fence->lock, irq_flags);
-- 
2.49.0

Re: [PATCH v2 8/8] drm/xe: Use dma_fence_test_signaled_flag()

[Matthew Brost]

On Mon, Dec 01, 2025 at 11:50:12AM +0100, Philipp Stanner wrote:
> There is a new dma_fence helper which simplifies testing for a fence's
> signaled_flag. Use it in xe.
> 
> Signed-off-by: Philipp Stanner <phasta@kernel.org>

Reviewed-by: Matthew Brost <matthew.brost@intel.com>

> ---
>  drivers/gpu/drm/xe/xe_exec_queue.c | 9 +++------
>  drivers/gpu/drm/xe/xe_pt.c         | 3 +--
>  drivers/gpu/drm/xe/xe_sched_job.c  | 2 +-
>  3 files changed, 5 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_exec_queue.c b/drivers/gpu/drm/xe/xe_exec_queue.c
> index cb5f204c08ed..06736f52fbaa 100644
> --- a/drivers/gpu/drm/xe/xe_exec_queue.c
> +++ b/drivers/gpu/drm/xe/xe_exec_queue.c
> @@ -1037,8 +1037,7 @@ struct dma_fence *xe_exec_queue_last_fence_get(struct xe_exec_queue *q,
>  
>  	xe_exec_queue_last_fence_lockdep_assert(q, vm);
>  
> -	if (q->last_fence &&
> -	    test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &q->last_fence->flags))
> +	if (q->last_fence && dma_fence_test_signaled_flag(q->last_fence))
>  		xe_exec_queue_last_fence_put(q, vm);
>  
>  	fence = q->last_fence ? q->last_fence : dma_fence_get_stub();
> @@ -1064,8 +1063,7 @@ struct dma_fence *xe_exec_queue_last_fence_get_for_resume(struct xe_exec_queue *
>  
>  	lockdep_assert_held_write(&q->hwe->hw_engine_group->mode_sem);
>  
> -	if (q->last_fence &&
> -	    test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &q->last_fence->flags))
> +	if (q->last_fence && dma_fence_test_signaled_flag(q->last_fence))
>  		xe_exec_queue_last_fence_put_unlocked(q);
>  
>  	fence = q->last_fence ? q->last_fence : dma_fence_get_stub();
> @@ -1106,8 +1104,7 @@ int xe_exec_queue_last_fence_test_dep(struct xe_exec_queue *q, struct xe_vm *vm)
>  
>  	fence = xe_exec_queue_last_fence_get(q, vm);
>  	if (fence) {
> -		err = test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags) ?
> -			0 : -ETIME;
> +		err = dma_fence_test_signaled_flag(fence) ? 0 : -ETIME;
>  		dma_fence_put(fence);
>  	}
>  
> diff --git a/drivers/gpu/drm/xe/xe_pt.c b/drivers/gpu/drm/xe/xe_pt.c
> index 07f96bda638a..1ca2dec18e51 100644
> --- a/drivers/gpu/drm/xe/xe_pt.c
> +++ b/drivers/gpu/drm/xe/xe_pt.c
> @@ -1208,8 +1208,7 @@ static bool no_in_syncs(struct xe_sync_entry *syncs, u32 num_syncs)
>  	for (i = 0; i < num_syncs; i++) {
>  		struct dma_fence *fence = syncs[i].fence;
>  
> -		if (fence && !test_bit(DMA_FENCE_FLAG_SIGNALED_BIT,
> -				       &fence->flags))
> +		if (fence && !dma_fence_test_signaled_flag(fence))
>  			return false;
>  	}
>  
> diff --git a/drivers/gpu/drm/xe/xe_sched_job.c b/drivers/gpu/drm/xe/xe_sched_job.c
> index d21bf8f26964..1c9ba49a325b 100644
> --- a/drivers/gpu/drm/xe/xe_sched_job.c
> +++ b/drivers/gpu/drm/xe/xe_sched_job.c
> @@ -188,7 +188,7 @@ static bool xe_fence_set_error(struct dma_fence *fence, int error)
>  	bool signaled;
>  
>  	spin_lock_irqsave(fence->lock, irq_flags);
> -	signaled = test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags);
> +	signaled = dma_fence_test_signaled_flag(fence);
>  	if (!signaled)
>  		dma_fence_set_error(fence, error);
>  	spin_unlock_irqrestore(fence->lock, irq_flags);
> -- 
> 2.49.0
> 

Re: [PATCH v2 8/8] drm/xe: Use dma_fence_test_signaled_flag()

[Philipp Stanner]

On Mon, 2025-12-01 at 11:33 -0800, Matthew Brost wrote:
> On Mon, Dec 01, 2025 at 11:50:12AM +0100, Philipp Stanner wrote:
> > There is a new dma_fence helper which simplifies testing for a fence's
> > signaled_flag. Use it in xe.
> > 
> > Signed-off-by: Philipp Stanner <phasta@kernel.org>
> 
> Reviewed-by: Matthew Brost <matthew.brost@intel.com>

Doesn't apply on drm-misc-next. Suggestions about what to do about
that?


P.


> 
> > ---
> >  drivers/gpu/drm/xe/xe_exec_queue.c | 9 +++------
> >  drivers/gpu/drm/xe/xe_pt.c         | 3 +--
> >  drivers/gpu/drm/xe/xe_sched_job.c  | 2 +-
> >  3 files changed, 5 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/xe/xe_exec_queue.c b/drivers/gpu/drm/xe/xe_exec_queue.c
> > index cb5f204c08ed..06736f52fbaa 100644
> > --- a/drivers/gpu/drm/xe/xe_exec_queue.c
> > +++ b/drivers/gpu/drm/xe/xe_exec_queue.c
> > @@ -1037,8 +1037,7 @@ struct dma_fence *xe_exec_queue_last_fence_get(struct xe_exec_queue *q,
> >  
> >  	xe_exec_queue_last_fence_lockdep_assert(q, vm);
> >  
> > -	if (q->last_fence &&
> > -	    test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &q->last_fence->flags))
> > +	if (q->last_fence && dma_fence_test_signaled_flag(q->last_fence))
> >  		xe_exec_queue_last_fence_put(q, vm);
> >  
> >  	fence = q->last_fence ? q->last_fence : dma_fence_get_stub();
> > @@ -1064,8 +1063,7 @@ struct dma_fence *xe_exec_queue_last_fence_get_for_resume(struct xe_exec_queue *
> >  
> >  	lockdep_assert_held_write(&q->hwe->hw_engine_group->mode_sem);
> >  
> > -	if (q->last_fence &&
> > -	    test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &q->last_fence->flags))
> > +	if (q->last_fence && dma_fence_test_signaled_flag(q->last_fence))
> >  		xe_exec_queue_last_fence_put_unlocked(q);
> >  
> >  	fence = q->last_fence ? q->last_fence : dma_fence_get_stub();
> > @@ -1106,8 +1104,7 @@ int xe_exec_queue_last_fence_test_dep(struct xe_exec_queue *q, struct xe_vm *vm)
> >  
> >  	fence = xe_exec_queue_last_fence_get(q, vm);
> >  	if (fence) {
> > -		err = test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags) ?
> > -			0 : -ETIME;
> > +		err = dma_fence_test_signaled_flag(fence) ? 0 : -ETIME;
> >  		dma_fence_put(fence);
> >  	}
> >  
> > diff --git a/drivers/gpu/drm/xe/xe_pt.c b/drivers/gpu/drm/xe/xe_pt.c
> > index 07f96bda638a..1ca2dec18e51 100644
> > --- a/drivers/gpu/drm/xe/xe_pt.c
> > +++ b/drivers/gpu/drm/xe/xe_pt.c
> > @@ -1208,8 +1208,7 @@ static bool no_in_syncs(struct xe_sync_entry *syncs, u32 num_syncs)
> >  	for (i = 0; i < num_syncs; i++) {
> >  		struct dma_fence *fence = syncs[i].fence;
> >  
> > -		if (fence && !test_bit(DMA_FENCE_FLAG_SIGNALED_BIT,
> > -				       &fence->flags))
> > +		if (fence && !dma_fence_test_signaled_flag(fence))
> >  			return false;
> >  	}
> >  
> > diff --git a/drivers/gpu/drm/xe/xe_sched_job.c b/drivers/gpu/drm/xe/xe_sched_job.c
> > index d21bf8f26964..1c9ba49a325b 100644
> > --- a/drivers/gpu/drm/xe/xe_sched_job.c
> > +++ b/drivers/gpu/drm/xe/xe_sched_job.c
> > @@ -188,7 +188,7 @@ static bool xe_fence_set_error(struct dma_fence *fence, int error)
> >  	bool signaled;
> >  
> >  	spin_lock_irqsave(fence->lock, irq_flags);
> > -	signaled = test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags);
> > +	signaled = dma_fence_test_signaled_flag(fence);
> >  	if (!signaled)
> >  		dma_fence_set_error(fence, error);
> >  	spin_unlock_irqrestore(fence->lock, irq_flags);
> > -- 
> > 2.49.0
> > 

Re: [PATCH v2 8/8] drm/xe: Use dma_fence_test_signaled_flag()

[Christian König]

On 12/1/25 11:50, Philipp Stanner wrote:
> There is a new dma_fence helper which simplifies testing for a fence's
> signaled_flag. Use it in xe.
> 
> Signed-off-by: Philipp Stanner <phasta@kernel.org>

Acked-by: Christian König <christian.koenig@amd.com>

> ---
>  drivers/gpu/drm/xe/xe_exec_queue.c | 9 +++------
>  drivers/gpu/drm/xe/xe_pt.c         | 3 +--
>  drivers/gpu/drm/xe/xe_sched_job.c  | 2 +-
>  3 files changed, 5 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_exec_queue.c b/drivers/gpu/drm/xe/xe_exec_queue.c
> index cb5f204c08ed..06736f52fbaa 100644
> --- a/drivers/gpu/drm/xe/xe_exec_queue.c
> +++ b/drivers/gpu/drm/xe/xe_exec_queue.c
> @@ -1037,8 +1037,7 @@ struct dma_fence *xe_exec_queue_last_fence_get(struct xe_exec_queue *q,
>  
>  	xe_exec_queue_last_fence_lockdep_assert(q, vm);
>  
> -	if (q->last_fence &&
> -	    test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &q->last_fence->flags))
> +	if (q->last_fence && dma_fence_test_signaled_flag(q->last_fence))
>  		xe_exec_queue_last_fence_put(q, vm);
>  
>  	fence = q->last_fence ? q->last_fence : dma_fence_get_stub();
> @@ -1064,8 +1063,7 @@ struct dma_fence *xe_exec_queue_last_fence_get_for_resume(struct xe_exec_queue *
>  
>  	lockdep_assert_held_write(&q->hwe->hw_engine_group->mode_sem);
>  
> -	if (q->last_fence &&
> -	    test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &q->last_fence->flags))
> +	if (q->last_fence && dma_fence_test_signaled_flag(q->last_fence))
>  		xe_exec_queue_last_fence_put_unlocked(q);
>  
>  	fence = q->last_fence ? q->last_fence : dma_fence_get_stub();
> @@ -1106,8 +1104,7 @@ int xe_exec_queue_last_fence_test_dep(struct xe_exec_queue *q, struct xe_vm *vm)
>  
>  	fence = xe_exec_queue_last_fence_get(q, vm);
>  	if (fence) {
> -		err = test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags) ?
> -			0 : -ETIME;
> +		err = dma_fence_test_signaled_flag(fence) ? 0 : -ETIME;
>  		dma_fence_put(fence);
>  	}
>  
> diff --git a/drivers/gpu/drm/xe/xe_pt.c b/drivers/gpu/drm/xe/xe_pt.c
> index 07f96bda638a..1ca2dec18e51 100644
> --- a/drivers/gpu/drm/xe/xe_pt.c
> +++ b/drivers/gpu/drm/xe/xe_pt.c
> @@ -1208,8 +1208,7 @@ static bool no_in_syncs(struct xe_sync_entry *syncs, u32 num_syncs)
>  	for (i = 0; i < num_syncs; i++) {
>  		struct dma_fence *fence = syncs[i].fence;
>  
> -		if (fence && !test_bit(DMA_FENCE_FLAG_SIGNALED_BIT,
> -				       &fence->flags))
> +		if (fence && !dma_fence_test_signaled_flag(fence))
>  			return false;
>  	}
>  
> diff --git a/drivers/gpu/drm/xe/xe_sched_job.c b/drivers/gpu/drm/xe/xe_sched_job.c
> index d21bf8f26964..1c9ba49a325b 100644
> --- a/drivers/gpu/drm/xe/xe_sched_job.c
> +++ b/drivers/gpu/drm/xe/xe_sched_job.c
> @@ -188,7 +188,7 @@ static bool xe_fence_set_error(struct dma_fence *fence, int error)
>  	bool signaled;
>  
>  	spin_lock_irqsave(fence->lock, irq_flags);
> -	signaled = test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags);
> +	signaled = dma_fence_test_signaled_flag(fence);
>  	if (!signaled)
>  		dma_fence_set_error(fence, error);
>  	spin_unlock_irqrestore(fence->lock, irq_flags);

Re: [PATCH v2 8/8] drm/xe: Use dma_fence_test_signaled_flag()

[Philipp Stanner]

On Wed, 2025-12-03 at 14:15 +0100, Christian König wrote:
> On 12/1/25 11:50, Philipp Stanner wrote:
> > There is a new dma_fence helper which simplifies testing for a fence's
> > signaled_flag. Use it in xe.
> > 
> > Signed-off-by: Philipp Stanner <phasta@kernel.org>
> 
> Acked-by: Christian König <christian.koenig@amd.com>

This series would then be completely reviewed, it seems. So one could
push it. Question is just who and where, and what to do about the merge
conflict with intel.

Matthew?


P.

> 
> > ---
> >  drivers/gpu/drm/xe/xe_exec_queue.c | 9 +++------
> >  drivers/gpu/drm/xe/xe_pt.c         | 3 +--
> >  drivers/gpu/drm/xe/xe_sched_job.c  | 2 +-
> >  3 files changed, 5 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/xe/xe_exec_queue.c b/drivers/gpu/drm/xe/xe_exec_queue.c
> > index cb5f204c08ed..06736f52fbaa 100644
> > --- a/drivers/gpu/drm/xe/xe_exec_queue.c
> > +++ b/drivers/gpu/drm/xe/xe_exec_queue.c
> > @@ -1037,8 +1037,7 @@ struct dma_fence *xe_exec_queue_last_fence_get(struct xe_exec_queue *q,
> >  
> >  	xe_exec_queue_last_fence_lockdep_assert(q, vm);
> >  
> > -	if (q->last_fence &&
> > -	    test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &q->last_fence->flags))
> > +	if (q->last_fence && dma_fence_test_signaled_flag(q->last_fence))
> >  		xe_exec_queue_last_fence_put(q, vm);
> >  
> >  	fence = q->last_fence ? q->last_fence : dma_fence_get_stub();
> > @@ -1064,8 +1063,7 @@ struct dma_fence *xe_exec_queue_last_fence_get_for_resume(struct xe_exec_queue *
> >  
> >  	lockdep_assert_held_write(&q->hwe->hw_engine_group->mode_sem);
> >  
> > -	if (q->last_fence &&
> > -	    test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &q->last_fence->flags))
> > +	if (q->last_fence && dma_fence_test_signaled_flag(q->last_fence))
> >  		xe_exec_queue_last_fence_put_unlocked(q);
> >  
> >  	fence = q->last_fence ? q->last_fence : dma_fence_get_stub();
> > @@ -1106,8 +1104,7 @@ int xe_exec_queue_last_fence_test_dep(struct xe_exec_queue *q, struct xe_vm *vm)
> >  
> >  	fence = xe_exec_queue_last_fence_get(q, vm);
> >  	if (fence) {
> > -		err = test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags) ?
> > -			0 : -ETIME;
> > +		err = dma_fence_test_signaled_flag(fence) ? 0 : -ETIME;
> >  		dma_fence_put(fence);
> >  	}
> >  
> > diff --git a/drivers/gpu/drm/xe/xe_pt.c b/drivers/gpu/drm/xe/xe_pt.c
> > index 07f96bda638a..1ca2dec18e51 100644
> > --- a/drivers/gpu/drm/xe/xe_pt.c
> > +++ b/drivers/gpu/drm/xe/xe_pt.c
> > @@ -1208,8 +1208,7 @@ static bool no_in_syncs(struct xe_sync_entry *syncs, u32 num_syncs)
> >  	for (i = 0; i < num_syncs; i++) {
> >  		struct dma_fence *fence = syncs[i].fence;
> >  
> > -		if (fence && !test_bit(DMA_FENCE_FLAG_SIGNALED_BIT,
> > -				       &fence->flags))
> > +		if (fence && !dma_fence_test_signaled_flag(fence))
> >  			return false;
> >  	}
> >  
> > diff --git a/drivers/gpu/drm/xe/xe_sched_job.c b/drivers/gpu/drm/xe/xe_sched_job.c
> > index d21bf8f26964..1c9ba49a325b 100644
> > --- a/drivers/gpu/drm/xe/xe_sched_job.c
> > +++ b/drivers/gpu/drm/xe/xe_sched_job.c
> > @@ -188,7 +188,7 @@ static bool xe_fence_set_error(struct dma_fence *fence, int error)
> >  	bool signaled;
> >  
> >  	spin_lock_irqsave(fence->lock, irq_flags);
> > -	signaled = test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags);
> > +	signaled = dma_fence_test_signaled_flag(fence);
> >  	if (!signaled)
> >  		dma_fence_set_error(fence, error);
> >  	spin_unlock_irqrestore(fence->lock, irq_flags);
> 

Re: [PATCH v2 8/8] drm/xe: Use dma_fence_test_signaled_flag()

[Christian König]

On 12/3/25 16:18, Philipp Stanner wrote:
> On Wed, 2025-12-03 at 14:15 +0100, Christian König wrote:
>> On 12/1/25 11:50, Philipp Stanner wrote:
>>> There is a new dma_fence helper which simplifies testing for a fence's
>>> signaled_flag. Use it in xe.
>>>
>>> Signed-off-by: Philipp Stanner <phasta@kernel.org>
>>
>> Acked-by: Christian König <christian.koenig@amd.com>
> 
> This series would then be completely reviewed, it seems. So one could
> push it. Question is just who and where, and what to do about the merge
> conflict with intel.

I think as long as it isn't a major merge conflict push it to drm-misc-next and make sure that drm-tip had the correct conflict resolution.

> Matthew?

Should have the last word on this when it's an XE merge conflict.

Christian.

> 
> 
> P.
> 
>>
>>> ---
>>>  drivers/gpu/drm/xe/xe_exec_queue.c | 9 +++------
>>>  drivers/gpu/drm/xe/xe_pt.c         | 3 +--
>>>  drivers/gpu/drm/xe/xe_sched_job.c  | 2 +-
>>>  3 files changed, 5 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/xe/xe_exec_queue.c b/drivers/gpu/drm/xe/xe_exec_queue.c
>>> index cb5f204c08ed..06736f52fbaa 100644
>>> --- a/drivers/gpu/drm/xe/xe_exec_queue.c
>>> +++ b/drivers/gpu/drm/xe/xe_exec_queue.c
>>> @@ -1037,8 +1037,7 @@ struct dma_fence *xe_exec_queue_last_fence_get(struct xe_exec_queue *q,
>>>  
>>>  	xe_exec_queue_last_fence_lockdep_assert(q, vm);
>>>  
>>> -	if (q->last_fence &&
>>> -	    test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &q->last_fence->flags))
>>> +	if (q->last_fence && dma_fence_test_signaled_flag(q->last_fence))
>>>  		xe_exec_queue_last_fence_put(q, vm);
>>>  
>>>  	fence = q->last_fence ? q->last_fence : dma_fence_get_stub();
>>> @@ -1064,8 +1063,7 @@ struct dma_fence *xe_exec_queue_last_fence_get_for_resume(struct xe_exec_queue *
>>>  
>>>  	lockdep_assert_held_write(&q->hwe->hw_engine_group->mode_sem);
>>>  
>>> -	if (q->last_fence &&
>>> -	    test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &q->last_fence->flags))
>>> +	if (q->last_fence && dma_fence_test_signaled_flag(q->last_fence))
>>>  		xe_exec_queue_last_fence_put_unlocked(q);
>>>  
>>>  	fence = q->last_fence ? q->last_fence : dma_fence_get_stub();
>>> @@ -1106,8 +1104,7 @@ int xe_exec_queue_last_fence_test_dep(struct xe_exec_queue *q, struct xe_vm *vm)
>>>  
>>>  	fence = xe_exec_queue_last_fence_get(q, vm);
>>>  	if (fence) {
>>> -		err = test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags) ?
>>> -			0 : -ETIME;
>>> +		err = dma_fence_test_signaled_flag(fence) ? 0 : -ETIME;
>>>  		dma_fence_put(fence);
>>>  	}
>>>  
>>> diff --git a/drivers/gpu/drm/xe/xe_pt.c b/drivers/gpu/drm/xe/xe_pt.c
>>> index 07f96bda638a..1ca2dec18e51 100644
>>> --- a/drivers/gpu/drm/xe/xe_pt.c
>>> +++ b/drivers/gpu/drm/xe/xe_pt.c
>>> @@ -1208,8 +1208,7 @@ static bool no_in_syncs(struct xe_sync_entry *syncs, u32 num_syncs)
>>>  	for (i = 0; i < num_syncs; i++) {
>>>  		struct dma_fence *fence = syncs[i].fence;
>>>  
>>> -		if (fence && !test_bit(DMA_FENCE_FLAG_SIGNALED_BIT,
>>> -				       &fence->flags))
>>> +		if (fence && !dma_fence_test_signaled_flag(fence))
>>>  			return false;
>>>  	}
>>>  
>>> diff --git a/drivers/gpu/drm/xe/xe_sched_job.c b/drivers/gpu/drm/xe/xe_sched_job.c
>>> index d21bf8f26964..1c9ba49a325b 100644
>>> --- a/drivers/gpu/drm/xe/xe_sched_job.c
>>> +++ b/drivers/gpu/drm/xe/xe_sched_job.c
>>> @@ -188,7 +188,7 @@ static bool xe_fence_set_error(struct dma_fence *fence, int error)
>>>  	bool signaled;
>>>  
>>>  	spin_lock_irqsave(fence->lock, irq_flags);
>>> -	signaled = test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags);
>>> +	signaled = dma_fence_test_signaled_flag(fence);
>>>  	if (!signaled)
>>>  		dma_fence_set_error(fence, error);
>>>  	spin_unlock_irqrestore(fence->lock, irq_flags);
>>
> 

Re: [PATCH v2 8/8] drm/xe: Use dma_fence_test_signaled_flag()

[Matthew Brost]

On Wed, Dec 03, 2025 at 04:24:26PM +0100, Christian König wrote:
> On 12/3/25 16:18, Philipp Stanner wrote:
> > On Wed, 2025-12-03 at 14:15 +0100, Christian König wrote:
> >> On 12/1/25 11:50, Philipp Stanner wrote:
> >>> There is a new dma_fence helper which simplifies testing for a fence's
> >>> signaled_flag. Use it in xe.
> >>>
> >>> Signed-off-by: Philipp Stanner <phasta@kernel.org>
> >>
> >> Acked-by: Christian König <christian.koenig@amd.com>
> > 
> > This series would then be completely reviewed, it seems. So one could
> > push it. Question is just who and where, and what to do about the merge
> > conflict with intel.
> 
> I think as long as it isn't a major merge conflict push it to drm-misc-next and make sure that drm-tip had the correct conflict resolution.
> 
> > Matthew?
> 
> Should have the last word on this when it's an XE merge conflict.
> 

It isn't pressing to get this patch into Xe as it is just an addition
compared to patch 4 which I believe is actually required for
functionality in Xe. So to avoid conflicts maybe just push the first 7
and for patch 8 once these patches propagate to drm-xe-next, I'll rebase
the last patch and push it into our tree.

Matt

> Christian.
> 
> > 
> > 
> > P.
> > 
> >>
> >>> ---
> >>>  drivers/gpu/drm/xe/xe_exec_queue.c | 9 +++------
> >>>  drivers/gpu/drm/xe/xe_pt.c         | 3 +--
> >>>  drivers/gpu/drm/xe/xe_sched_job.c  | 2 +-
> >>>  3 files changed, 5 insertions(+), 9 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/xe/xe_exec_queue.c b/drivers/gpu/drm/xe/xe_exec_queue.c
> >>> index cb5f204c08ed..06736f52fbaa 100644
> >>> --- a/drivers/gpu/drm/xe/xe_exec_queue.c
> >>> +++ b/drivers/gpu/drm/xe/xe_exec_queue.c
> >>> @@ -1037,8 +1037,7 @@ struct dma_fence *xe_exec_queue_last_fence_get(struct xe_exec_queue *q,
> >>>  
> >>>  	xe_exec_queue_last_fence_lockdep_assert(q, vm);
> >>>  
> >>> -	if (q->last_fence &&
> >>> -	    test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &q->last_fence->flags))
> >>> +	if (q->last_fence && dma_fence_test_signaled_flag(q->last_fence))
> >>>  		xe_exec_queue_last_fence_put(q, vm);
> >>>  
> >>>  	fence = q->last_fence ? q->last_fence : dma_fence_get_stub();
> >>> @@ -1064,8 +1063,7 @@ struct dma_fence *xe_exec_queue_last_fence_get_for_resume(struct xe_exec_queue *
> >>>  
> >>>  	lockdep_assert_held_write(&q->hwe->hw_engine_group->mode_sem);
> >>>  
> >>> -	if (q->last_fence &&
> >>> -	    test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &q->last_fence->flags))
> >>> +	if (q->last_fence && dma_fence_test_signaled_flag(q->last_fence))
> >>>  		xe_exec_queue_last_fence_put_unlocked(q);
> >>>  
> >>>  	fence = q->last_fence ? q->last_fence : dma_fence_get_stub();
> >>> @@ -1106,8 +1104,7 @@ int xe_exec_queue_last_fence_test_dep(struct xe_exec_queue *q, struct xe_vm *vm)
> >>>  
> >>>  	fence = xe_exec_queue_last_fence_get(q, vm);
> >>>  	if (fence) {
> >>> -		err = test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags) ?
> >>> -			0 : -ETIME;
> >>> +		err = dma_fence_test_signaled_flag(fence) ? 0 : -ETIME;
> >>>  		dma_fence_put(fence);
> >>>  	}
> >>>  
> >>> diff --git a/drivers/gpu/drm/xe/xe_pt.c b/drivers/gpu/drm/xe/xe_pt.c
> >>> index 07f96bda638a..1ca2dec18e51 100644
> >>> --- a/drivers/gpu/drm/xe/xe_pt.c
> >>> +++ b/drivers/gpu/drm/xe/xe_pt.c
> >>> @@ -1208,8 +1208,7 @@ static bool no_in_syncs(struct xe_sync_entry *syncs, u32 num_syncs)
> >>>  	for (i = 0; i < num_syncs; i++) {
> >>>  		struct dma_fence *fence = syncs[i].fence;
> >>>  
> >>> -		if (fence && !test_bit(DMA_FENCE_FLAG_SIGNALED_BIT,
> >>> -				       &fence->flags))
> >>> +		if (fence && !dma_fence_test_signaled_flag(fence))
> >>>  			return false;
> >>>  	}
> >>>  
> >>> diff --git a/drivers/gpu/drm/xe/xe_sched_job.c b/drivers/gpu/drm/xe/xe_sched_job.c
> >>> index d21bf8f26964..1c9ba49a325b 100644
> >>> --- a/drivers/gpu/drm/xe/xe_sched_job.c
> >>> +++ b/drivers/gpu/drm/xe/xe_sched_job.c
> >>> @@ -188,7 +188,7 @@ static bool xe_fence_set_error(struct dma_fence *fence, int error)
> >>>  	bool signaled;
> >>>  
> >>>  	spin_lock_irqsave(fence->lock, irq_flags);
> >>> -	signaled = test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags);
> >>> +	signaled = dma_fence_test_signaled_flag(fence);
> >>>  	if (!signaled)
> >>>  		dma_fence_set_error(fence, error);
> >>>  	spin_unlock_irqrestore(fence->lock, irq_flags);
> >>
> > 
> 

Re: [PATCH v2 8/8] drm/xe: Use dma_fence_test_signaled_flag()

[Rodrigo Vivi]

On Wed, Dec 03, 2025 at 09:31:25AM -0800, Matthew Brost wrote:
> On Wed, Dec 03, 2025 at 04:24:26PM +0100, Christian König wrote:
> > On 12/3/25 16:18, Philipp Stanner wrote:
> > > On Wed, 2025-12-03 at 14:15 +0100, Christian König wrote:
> > >> On 12/1/25 11:50, Philipp Stanner wrote:
> > >>> There is a new dma_fence helper which simplifies testing for a fence's
> > >>> signaled_flag. Use it in xe.
> > >>>
> > >>> Signed-off-by: Philipp Stanner <phasta@kernel.org>
> > >>
> > >> Acked-by: Christian König <christian.koenig@amd.com>
> > > 
> > > This series would then be completely reviewed, it seems. So one could
> > > push it. Question is just who and where, and what to do about the merge
> > > conflict with intel.
> > 
> > I think as long as it isn't a major merge conflict push it to drm-misc-next and make sure that drm-tip had the correct conflict resolution.
> > 
> > > Matthew?
> > 
> > Should have the last word on this when it's an XE merge conflict.
> > 
> 
> It isn't pressing to get this patch into Xe as it is just an addition
> compared to patch 4 which I believe is actually required for
> functionality in Xe. So to avoid conflicts maybe just push the first 7
> and for patch 8 once these patches propagate to drm-xe-next, I'll rebase
> the last patch and push it into our tree.

If that can wait good, but if not I don't believe it will be hard conflicts.

in case it is needed to ge to other trees:
Acked-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> 
> Matt
> 
> > Christian.
> > 
> > > 
> > > 
> > > P.
> > > 
> > >>
> > >>> ---
> > >>>  drivers/gpu/drm/xe/xe_exec_queue.c | 9 +++------
> > >>>  drivers/gpu/drm/xe/xe_pt.c         | 3 +--
> > >>>  drivers/gpu/drm/xe/xe_sched_job.c  | 2 +-
> > >>>  3 files changed, 5 insertions(+), 9 deletions(-)
> > >>>
> > >>> diff --git a/drivers/gpu/drm/xe/xe_exec_queue.c b/drivers/gpu/drm/xe/xe_exec_queue.c
> > >>> index cb5f204c08ed..06736f52fbaa 100644
> > >>> --- a/drivers/gpu/drm/xe/xe_exec_queue.c
> > >>> +++ b/drivers/gpu/drm/xe/xe_exec_queue.c
> > >>> @@ -1037,8 +1037,7 @@ struct dma_fence *xe_exec_queue_last_fence_get(struct xe_exec_queue *q,
> > >>>  
> > >>>  	xe_exec_queue_last_fence_lockdep_assert(q, vm);
> > >>>  
> > >>> -	if (q->last_fence &&
> > >>> -	    test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &q->last_fence->flags))
> > >>> +	if (q->last_fence && dma_fence_test_signaled_flag(q->last_fence))
> > >>>  		xe_exec_queue_last_fence_put(q, vm);
> > >>>  
> > >>>  	fence = q->last_fence ? q->last_fence : dma_fence_get_stub();
> > >>> @@ -1064,8 +1063,7 @@ struct dma_fence *xe_exec_queue_last_fence_get_for_resume(struct xe_exec_queue *
> > >>>  
> > >>>  	lockdep_assert_held_write(&q->hwe->hw_engine_group->mode_sem);
> > >>>  
> > >>> -	if (q->last_fence &&
> > >>> -	    test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &q->last_fence->flags))
> > >>> +	if (q->last_fence && dma_fence_test_signaled_flag(q->last_fence))
> > >>>  		xe_exec_queue_last_fence_put_unlocked(q);
> > >>>  
> > >>>  	fence = q->last_fence ? q->last_fence : dma_fence_get_stub();
> > >>> @@ -1106,8 +1104,7 @@ int xe_exec_queue_last_fence_test_dep(struct xe_exec_queue *q, struct xe_vm *vm)
> > >>>  
> > >>>  	fence = xe_exec_queue_last_fence_get(q, vm);
> > >>>  	if (fence) {
> > >>> -		err = test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags) ?
> > >>> -			0 : -ETIME;
> > >>> +		err = dma_fence_test_signaled_flag(fence) ? 0 : -ETIME;
> > >>>  		dma_fence_put(fence);
> > >>>  	}
> > >>>  
> > >>> diff --git a/drivers/gpu/drm/xe/xe_pt.c b/drivers/gpu/drm/xe/xe_pt.c
> > >>> index 07f96bda638a..1ca2dec18e51 100644
> > >>> --- a/drivers/gpu/drm/xe/xe_pt.c
> > >>> +++ b/drivers/gpu/drm/xe/xe_pt.c
> > >>> @@ -1208,8 +1208,7 @@ static bool no_in_syncs(struct xe_sync_entry *syncs, u32 num_syncs)
> > >>>  	for (i = 0; i < num_syncs; i++) {
> > >>>  		struct dma_fence *fence = syncs[i].fence;
> > >>>  
> > >>> -		if (fence && !test_bit(DMA_FENCE_FLAG_SIGNALED_BIT,
> > >>> -				       &fence->flags))
> > >>> +		if (fence && !dma_fence_test_signaled_flag(fence))
> > >>>  			return false;
> > >>>  	}
> > >>>  
> > >>> diff --git a/drivers/gpu/drm/xe/xe_sched_job.c b/drivers/gpu/drm/xe/xe_sched_job.c
> > >>> index d21bf8f26964..1c9ba49a325b 100644
> > >>> --- a/drivers/gpu/drm/xe/xe_sched_job.c
> > >>> +++ b/drivers/gpu/drm/xe/xe_sched_job.c
> > >>> @@ -188,7 +188,7 @@ static bool xe_fence_set_error(struct dma_fence *fence, int error)
> > >>>  	bool signaled;
> > >>>  
> > >>>  	spin_lock_irqsave(fence->lock, irq_flags);
> > >>> -	signaled = test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags);
> > >>> +	signaled = dma_fence_test_signaled_flag(fence);
> > >>>  	if (!signaled)
> > >>>  		dma_fence_set_error(fence, error);
> > >>>  	spin_unlock_irqrestore(fence->lock, irq_flags);
> > >>
> > > 
> > 

Re: [PATCH 0/6] dma-fence: Remove return code of dma_fence_signal() et al.

[Philipp Stanner]

On Mon, 2025-12-01 at 11:50 +0100, Philipp Stanner wrote:
> Tested this with dma_buf selftests and drm_sched tests.
> 
> Changes in v2:
>   - Fix bug and don't turn the kernel into a smoking pile of ashes by
>     not setting the signaled-bit…
>   - Add functions (dma_fence_check_and_signal()) preserving the old
>     behavior of dma_fence_signal() & Co. (Felix)
>   - Use those new functions in amdkfd, xe, ttm and st-dma-fence.
>   - Be a bit less aggressive and keep the git-diff smaller.
>   - Add a patch using the flag-helper in Xe. (Matthew)
> 
> Barely anyone uses dma_fence_signal()'s (and similar functions') return
> code. Checking it is pretty much useless anyways, because what are you
> going to do if a fence was already signal it? Unsignal it and signal it
> again? ;p
> 
> Removing the return code simplifies the API and makes it easier for me
> to sit on top with Rust DmaFence.
> 
> Philipp Stanner (8):
>   dma-buf/dma-fence: Add dma_fence_test_signaled_flag()
>   dma-buf/dma-fence: Add dma_fence_check_and_signal()
>   amd/amdkfd: Use dma_fence_check_and_signal()
>   drm/xe: Use dma_fence_check_and_signal_locked()
>   dma-buf: Don't misuse dma_fence_signal()
>   drm/ttm: Use dma_fence_check_and_signal()
>   dma-buf/dma-fence: Remove return code of signaling-functions

Applied those 7 patches to drm-misc-next. Had to do a tiny rebase
because 033559473dd3b55558b535aa37b8848c207b5cbb is not yet in drm-
misc-next (dma-fence series was based on master at first, which
contains that commit).

>   drm/xe: Use dma_fence_test_signaled_flag()

Left for Matthow to pick up whenever he wishes.


P.

> 
>  drivers/dma-buf/dma-fence.c                   | 100 +++++++++++-------
>  drivers/dma-buf/st-dma-fence.c                |   4 +-
>  drivers/gpu/drm/amd/amdkfd/kfd_process.c      |   6 +-
>  .../gpu/drm/ttm/tests/ttm_bo_validate_test.c  |   2 +-
>  drivers/gpu/drm/xe/xe_exec_queue.c            |   9 +-
>  drivers/gpu/drm/xe/xe_hw_fence.c              |   4 +-
>  drivers/gpu/drm/xe/xe_pt.c                    |   3 +-
>  drivers/gpu/drm/xe/xe_sched_job.c             |   2 +-
>  include/linux/dma-fence.h                     |  35 ++++--
>  9 files changed, 102 insertions(+), 63 deletions(-)
>