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@xxxxxxx>
> Signed-off-by: Philipp Stanner <phasta@xxxxxxxxxx>

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

> ---
>  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,