dma-fence: Remove 64-bit flag

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

Posted by Philipp Stanner 3 months, 3 weeks ago

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.

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

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

Posted by Matthew Brost 3 months, 3 weeks ago

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.

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
>

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

Posted by Philipp Stanner 3 months, 2 weeks ago

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,
doesn't the C standard guarantee that the higher, unused 32 bits will
be 0?

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

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

Posted by Matthew Brost 3 months, 2 weeks ago

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.

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

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

Posted by Christian König 3 months, 1 week ago

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