[PATCH] dma-buf/dma-fence: Document why signaled-bit is set early

[PATCH] dma-buf/dma-fence: Document why signaled-bit is set early

[Philipp Stanner]

There have been repeated discussions about why the signaled-bit is set
before a fence is actually done signaling. Repeatedly, people try to
change that.

As long as the bit can be read locklessly, for example by
dma_fence_is_signaled(), the bit cannot be set at any other place.

Document that to reduce further noise about the topic.

Signed-off-by: Philipp Stanner <phasta@kernel.org>
---
I thought that if people repeatedly try to make a change, and it
repeatedly gets rejected, then we can all save ourselves time and work
by quickly documenting it :-)

This way, we also remember in 5 years *why* something was or wasn't
done.

Philipp
---
 drivers/dma-buf/dma-fence.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
index bd58688b81a7..a71e18a9e346 100644
--- a/drivers/dma-buf/dma-fence.c
+++ b/drivers/dma-buf/dma-fence.c
@@ -359,6 +359,16 @@ void dma_fence_signal_timestamp_locked(struct dma_fence *fence,
 
 	dma_fence_assert_held(fence);
 
+	/*
+	 * It would be better to set the bit after the callbacks ran so that the
+	 * fast-path-check in dma_fence_is_signaled() cannot return true while
+	 * they are running.
+	 *
+	 * That is, however, impossible because many parties now check for the
+	 * signaled-state of a fence in their callbacks (or in the work_items
+	 * kicked by them). So we have to keep setting it here as long as the
+	 * signaled-bit is read locklessly by anyone.
+	 */
 	if (unlikely(test_and_set_bit(DMA_FENCE_FLAG_SIGNALED_BIT,
 				      &fence->flags)))
 		return;

base-commit: cf5d8e390e2ac2cfa24a50645efdbb9f0eebe65f
-- 
2.56.0

Re: [PATCH] dma-buf/dma-fence: Document why signaled-bit is set early

[Christian König]

On 10/23/26 12:00, Philipp Stanner wrote:
> There have been repeated discussions about why the signaled-bit is set
> before a fence is actually done signaling. Repeatedly, people try to
> change that.
> 
> As long as the bit can be read locklessly, for example by
> dma_fence_is_signaled(), the bit cannot be set at any other place.
> 
> Document that to reduce further noise about the topic.
> 
> Signed-off-by: Philipp Stanner <phasta@kernel.org>

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

> ---
> I thought that if people repeatedly try to make a change, and it
> repeatedly gets rejected, then we can all save ourselves time and work
> by quickly documenting it :-)
> 
> This way, we also remember in 5 years *why* something was or wasn't
> done.
> 
> Philipp
> ---
>  drivers/dma-buf/dma-fence.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
> index bd58688b81a7..a71e18a9e346 100644
> --- a/drivers/dma-buf/dma-fence.c
> +++ b/drivers/dma-buf/dma-fence.c
> @@ -359,6 +359,16 @@ void dma_fence_signal_timestamp_locked(struct dma_fence *fence,
>  
>  	dma_fence_assert_held(fence);
>  
> +	/*
> +	 * It would be better to set the bit after the callbacks ran so that the
> +	 * fast-path-check in dma_fence_is_signaled() cannot return true while
> +	 * they are running.
> +	 *
> +	 * That is, however, impossible because many parties now check for the
> +	 * signaled-state of a fence in their callbacks (or in the work_items
> +	 * kicked by them). So we have to keep setting it here as long as the
> +	 * signaled-bit is read locklessly by anyone.
> +	 */
>  	if (unlikely(test_and_set_bit(DMA_FENCE_FLAG_SIGNALED_BIT,
>  				      &fence->flags)))
>  		return;
> 
> base-commit: cf5d8e390e2ac2cfa24a50645efdbb9f0eebe65f