Re: [RFC PATCH] dma-fence: Remove 64-bit flag

["Christian König" <christian.koenig@amd.com>]

On 10/20/25 13:18, Matthew Brost wrote:
> On Mon, Oct 20, 2025 at 10:16:23AM +0200, Philipp Stanner wrote:
>> On Fri, 2025-10-17 at 14:28 -0700, Matthew Brost wrote:
>>> On Fri, Oct 17, 2025 at 11:31:47AM +0200, Philipp Stanner wrote:
>>>> It seems that DMA_FENCE_FLAG_SEQNO64_BIT has no real effects anymore,
>>>> since seqno is a u64 everywhere.
>>>>
>>>> Remove the unneeded flag.
>>>>
>>>> Signed-off-by: Philipp Stanner <phasta@kernel.org>
>>>> ---
>>>> Seems to me that this flag doesn't really do anything anymore?
>>>>
>>>> I *suspect* that it could be that some drivers pass a u32 to
>>>> dma_fence_init()? I guess they could be ported, couldn't they.
>>>>
>>>
>>> Xe uses 32-bit hardware fence sequence numbers—see [1] and [2]. We could
>>> switch to 64-bit hardware fence sequence numbers, but that would require
>>> changes on the driver side. If you sent this to our CI, I’m fairly
>>> certain we’d see a bunch of failures. I suspect this would also break
>>> several other drivers.
>>
>> What exactly breaks? Help me out here; if you pass a u32 for a u64,
> 
> Seqno wraps.
> 
>> doesn't the C standard guarantee that the higher, unused 32 bits will
>> be 0?
> 
> 	return (int)(lower_32_bits(f1) - lower_32_bits(f2)) > 0;
> 
> Look at the above logic.
> 
> f1 = 0x0;
> f2 = 0xffffffff; /* -1 */
> 
> The above statement will correctly return true.
> 
> Compared to the below statement which returns false.
> 
> 	return f1 > f2;
> 
> We test seqno wraps in Xe by setting our initial seqno to -127, again if
> you send this patch to our CI any test which sends more than 127 job on
> queue will likely fail.

Yeah, exactly that's why this flag is needed for quite a lot of things.

Question is what is missing in the documentation to make that clear?

Regards,
Christian.

> 
> Matt
> 
>>
>> Because the only thing the flag still does is do this lower_32 check in
>> fence_is_later.
>>
>> P.
>>
>>>
>>> As I mentioned, all Xe-supported platforms could be updated since their
>>> rings support 64-bit store instructions. However, I suspect that very
>>> old i915 platforms don’t support such instructions in the ring. I agree
>>> this is a legacy issue, and we should probably use 64-bit sequence
>>> numbers in Xe. But again, platforms and drivers that are decades old
>>> might break as a result.
>>>
>>> Matt
>>>
>>> [1] https://elixir.bootlin.com/linux/v6.17.1/source/drivers/gpu/drm/xe/xe_hw_fence.c#L264
>>> [2] https://elixir.bootlin.com/linux/v6.17.1/source/drivers/gpu/drm/xe/xe_hw_fence_types.h#L51
>>>
>>>> P.
>>>> ---
>>>>  drivers/dma-buf/dma-fence.c |  3 +--
>>>>  include/linux/dma-fence.h   | 10 +---------
>>>>  2 files changed, 2 insertions(+), 11 deletions(-)
>>>>
>>>> diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
>>>> index 3f78c56b58dc..24794c027813 100644
>>>> --- a/drivers/dma-buf/dma-fence.c
>>>> +++ b/drivers/dma-buf/dma-fence.c
>>>> @@ -1078,8 +1078,7 @@ void
>>>>  dma_fence_init64(struct dma_fence *fence, const struct dma_fence_ops *ops,
>>>>  		 spinlock_t *lock, u64 context, u64 seqno)
>>>>  {
>>>> -	__dma_fence_init(fence, ops, lock, context, seqno,
>>>> -			 BIT(DMA_FENCE_FLAG_SEQNO64_BIT));
>>>> +	__dma_fence_init(fence, ops, lock, context, seqno, 0);
>>>>  }
>>>>  EXPORT_SYMBOL(dma_fence_init64);
>>>>  
>>>> diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
>>>> index 64639e104110..4eca2db28625 100644
>>>> --- a/include/linux/dma-fence.h
>>>> +++ b/include/linux/dma-fence.h
>>>> @@ -98,7 +98,6 @@ struct dma_fence {
>>>>  };
>>>>  
>>>>  enum dma_fence_flag_bits {
>>>> -	DMA_FENCE_FLAG_SEQNO64_BIT,
>>>>  	DMA_FENCE_FLAG_SIGNALED_BIT,
>>>>  	DMA_FENCE_FLAG_TIMESTAMP_BIT,
>>>>  	DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT,
>>>> @@ -470,14 +469,7 @@ dma_fence_is_signaled(struct dma_fence *fence)
>>>>   */
>>>>  static inline bool __dma_fence_is_later(struct dma_fence *fence, u64 f1, u64 f2)
>>>>  {
>>>> -	/* This is for backward compatibility with drivers which can only handle
>>>> -	 * 32bit sequence numbers. Use a 64bit compare when the driver says to
>>>> -	 * do so.
>>>> -	 */
>>>> -	if (test_bit(DMA_FENCE_FLAG_SEQNO64_BIT, &fence->flags))
>>>> -		return f1 > f2;
>>>> -
>>>> -	return (int)(lower_32_bits(f1) - lower_32_bits(f2)) > 0;
>>>> +	return f1 > f2;
>>>>  }
>>>>  
>>>>  /**
>>>> -- 
>>>> 2.49.0
>>>>
>>