[PATCH 1/2] dma-buf: Add __dma_fence_is_signaled()

[PATCH 1/2] dma-buf: Add __dma_fence_is_signaled()

[Philipp Stanner]

Some parties want to check whether a function is already signaled
without actually signaling the fence, which is what
dma_fence_is_signaled() might due if the fence ops 'signaled' callback
is implemented.

Add __dma_fence_is_signaled(), 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>
---
 include/linux/dma-fence.h | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
index 48b5202c531d..ac951a54a007 100644
--- a/include/linux/dma-fence.h
+++ b/include/linux/dma-fence.h
@@ -381,6 +381,26 @@ bool dma_fence_remove_callback(struct dma_fence *fence,
 			       struct dma_fence_cb *cb);
 void dma_fence_enable_sw_signaling(struct dma_fence *fence);
 
+/**
+ * __dma_fence_is_signaled - 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 does not require locking.
+ *
+ * See also dma_fence_is_signaled().
+ *
+ * Return: true if signaled, false otherwise.
+ */
+static inline bool
+__dma_fence_is_signaled(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.
@@ -398,7 +418,7 @@ void dma_fence_enable_sw_signaling(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_is_signaled(fence))
 		return true;
 
 	if (fence->ops->signaled && fence->ops->signaled(fence)) {
@@ -428,7 +448,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_is_signaled(fence))
 		return true;
 
 	if (fence->ops->signaled && fence->ops->signaled(fence)) {
-- 
2.49.0

[PATCH 2/2] drm/nouveau: Don't signal when killing the fence context

[Philipp Stanner]

dma_fence_is_signaled_locked(), which is used in
nouveau_fence_context_kill(), can signal fences below the surface
through a callback.

There is neither need for nor use in doing that when killing a fence
context.

Replace dma_fence_is_signaled_locked() with __dma_fence_is_signaled(), a
function which only checks, never signals.

Signed-off-by: Philipp Stanner <phasta@kernel.org>
---
 drivers/gpu/drm/nouveau/nouveau_fence.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_fence.c b/drivers/gpu/drm/nouveau/nouveau_fence.c
index d5654e26d5bc..993b3dcb5db0 100644
--- a/drivers/gpu/drm/nouveau/nouveau_fence.c
+++ b/drivers/gpu/drm/nouveau/nouveau_fence.c
@@ -88,7 +88,7 @@ nouveau_fence_context_kill(struct nouveau_fence_chan *fctx, int error)
 
 	spin_lock_irqsave(&fctx->lock, flags);
 	list_for_each_entry_safe(fence, tmp, &fctx->pending, head) {
-		if (error && !dma_fence_is_signaled_locked(&fence->base))
+		if (error && !__dma_fence_is_signaled(&fence->base))
 			dma_fence_set_error(&fence->base, error);
 
 		if (nouveau_fence_signal(fence))
-- 
2.49.0

Re: [PATCH 2/2] drm/nouveau: Don't signal when killing the fence context

[Christian König]

On 5/22/25 13:25, Philipp Stanner wrote:
> dma_fence_is_signaled_locked(), which is used in
> nouveau_fence_context_kill(), can signal fences below the surface
> through a callback.
> 
> There is neither need for nor use in doing that when killing a fence
> context.
> 
> Replace dma_fence_is_signaled_locked() with __dma_fence_is_signaled(), a
> function which only checks, never signals.

That is not a good approach.

Having the __dma_fence_is_signaled() means that other would be allowed to call it as well.

But nouveau can do that here only because it knows that the fence was issued by nouveau.

What nouveau can to is to test the signaled flag directly, but that's what you try to avoid as well.

Regards,
Christian.

> 
> Signed-off-by: Philipp Stanner <phasta@kernel.org>
> ---
>  drivers/gpu/drm/nouveau/nouveau_fence.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/nouveau/nouveau_fence.c b/drivers/gpu/drm/nouveau/nouveau_fence.c
> index d5654e26d5bc..993b3dcb5db0 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_fence.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_fence.c
> @@ -88,7 +88,7 @@ nouveau_fence_context_kill(struct nouveau_fence_chan *fctx, int error)
>  
>  	spin_lock_irqsave(&fctx->lock, flags);
>  	list_for_each_entry_safe(fence, tmp, &fctx->pending, head) {
> -		if (error && !dma_fence_is_signaled_locked(&fence->base))
> +		if (error && !__dma_fence_is_signaled(&fence->base))
>  			dma_fence_set_error(&fence->base, error);
>  
>  		if (nouveau_fence_signal(fence))

Re: [PATCH 2/2] drm/nouveau: Don't signal when killing the fence context

[Philipp Stanner]

On Thu, 2025-05-22 at 14:06 +0200, Christian König wrote:
> On 5/22/25 13:25, Philipp Stanner wrote:
> > dma_fence_is_signaled_locked(), which is used in
> > nouveau_fence_context_kill(), can signal fences below the surface
> > through a callback.
> > 
> > There is neither need for nor use in doing that when killing a
> > fence
> > context.
> > 
> > Replace dma_fence_is_signaled_locked() with
> > __dma_fence_is_signaled(), a
> > function which only checks, never signals.
> 
> That is not a good approach.
> 
> Having the __dma_fence_is_signaled() means that other would be
> allowed to call it as well.
> 
> But nouveau can do that here only because it knows that the fence was
> issued by nouveau.
> 
> What nouveau can to is to test the signaled flag directly, but that's
> what you try to avoid as well.

There's many parties who check the bit already.

And if Nouveau is allowed to do that, one can just as well provide a
wrapper for it.

That has the advantage of centralizing the responsibility and
documenting it.

P.

> 
> Regards,
> Christian.
> 
> > 
> > Signed-off-by: Philipp Stanner <phasta@kernel.org>
> > ---
> >  drivers/gpu/drm/nouveau/nouveau_fence.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/nouveau/nouveau_fence.c
> > b/drivers/gpu/drm/nouveau/nouveau_fence.c
> > index d5654e26d5bc..993b3dcb5db0 100644
> > --- a/drivers/gpu/drm/nouveau/nouveau_fence.c
> > +++ b/drivers/gpu/drm/nouveau/nouveau_fence.c
> > @@ -88,7 +88,7 @@ nouveau_fence_context_kill(struct
> > nouveau_fence_chan *fctx, int error)
> >  
> >  	spin_lock_irqsave(&fctx->lock, flags);
> >  	list_for_each_entry_safe(fence, tmp, &fctx->pending, head)
> > {
> > -		if (error && !dma_fence_is_signaled_locked(&fence-
> > >base))
> > +		if (error && !__dma_fence_is_signaled(&fence-
> > >base))
> >  			dma_fence_set_error(&fence->base, error);
> >  
> >  		if (nouveau_fence_signal(fence))
> 

Re: [PATCH 2/2] drm/nouveau: Don't signal when killing the fence context

[Christian König]

On 5/22/25 14:20, Philipp Stanner wrote:
> On Thu, 2025-05-22 at 14:06 +0200, Christian König wrote:
>> On 5/22/25 13:25, Philipp Stanner wrote:
>>> dma_fence_is_signaled_locked(), which is used in
>>> nouveau_fence_context_kill(), can signal fences below the surface
>>> through a callback.
>>>
>>> There is neither need for nor use in doing that when killing a
>>> fence
>>> context.
>>>
>>> Replace dma_fence_is_signaled_locked() with
>>> __dma_fence_is_signaled(), a
>>> function which only checks, never signals.
>>
>> That is not a good approach.
>>
>> Having the __dma_fence_is_signaled() means that other would be
>> allowed to call it as well.
>>
>> But nouveau can do that here only because it knows that the fence was
>> issued by nouveau.
>>
>> What nouveau can to is to test the signaled flag directly, but that's
>> what you try to avoid as well.
> 
> There's many parties who check the bit already.
> 
> And if Nouveau is allowed to do that, one can just as well provide a
> wrapper for it.

No, exactly that's what is usually avoided in cases like this here.

See all the functions inside include/linux/dma-fence.h can be used by everybody. It's basically the public interface of the dma_fence object.

So testing if a fence is signaled without calling the callback is only allowed by whoever implemented the fence.

In other words nouveau can test nouveau fences, i915 can test i915 fences, amdgpu can test amdgpu fences etc... But if you have the wrapper that makes it officially allowed that nouveau starts testing i915 fences and that would be problematic.

Regards,
Christian.

> 
> That has the advantage of centralizing the responsibility and
> documenting it.
> 
> P.
> 
>>
>> Regards,
>> Christian.
>>
>>>
>>> Signed-off-by: Philipp Stanner <phasta@kernel.org>
>>> ---
>>>  drivers/gpu/drm/nouveau/nouveau_fence.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/nouveau/nouveau_fence.c
>>> b/drivers/gpu/drm/nouveau/nouveau_fence.c
>>> index d5654e26d5bc..993b3dcb5db0 100644
>>> --- a/drivers/gpu/drm/nouveau/nouveau_fence.c
>>> +++ b/drivers/gpu/drm/nouveau/nouveau_fence.c
>>> @@ -88,7 +88,7 @@ nouveau_fence_context_kill(struct
>>> nouveau_fence_chan *fctx, int error)
>>>  
>>>  	spin_lock_irqsave(&fctx->lock, flags);
>>>  	list_for_each_entry_safe(fence, tmp, &fctx->pending, head)
>>> {
>>> -		if (error && !dma_fence_is_signaled_locked(&fence-
>>>> base))
>>> +		if (error && !__dma_fence_is_signaled(&fence-
>>>> base))
>>>  			dma_fence_set_error(&fence->base, error);
>>>  
>>>  		if (nouveau_fence_signal(fence))
>>
> 

Re: [PATCH 2/2] drm/nouveau: Don't signal when killing the fence context

[Philipp Stanner]

On Thu, 2025-05-22 at 14:34 +0200, Christian König wrote:
> On 5/22/25 14:20, Philipp Stanner wrote:
> > On Thu, 2025-05-22 at 14:06 +0200, Christian König wrote:
> > > On 5/22/25 13:25, Philipp Stanner wrote:
> > > > dma_fence_is_signaled_locked(), which is used in
> > > > nouveau_fence_context_kill(), can signal fences below the
> > > > surface
> > > > through a callback.
> > > > 
> > > > There is neither need for nor use in doing that when killing a
> > > > fence
> > > > context.
> > > > 
> > > > Replace dma_fence_is_signaled_locked() with
> > > > __dma_fence_is_signaled(), a
> > > > function which only checks, never signals.
> > > 
> > > That is not a good approach.
> > > 
> > > Having the __dma_fence_is_signaled() means that other would be
> > > allowed to call it as well.
> > > 
> > > But nouveau can do that here only because it knows that the fence
> > > was
> > > issued by nouveau.
> > > 
> > > What nouveau can to is to test the signaled flag directly, but
> > > that's
> > > what you try to avoid as well.
> > 
> > There's many parties who check the bit already.
> > 
> > And if Nouveau is allowed to do that, one can just as well provide
> > a
> > wrapper for it.
> 
> No, exactly that's what is usually avoided in cases like this here.
> 
> See all the functions inside include/linux/dma-fence.h can be used by
> everybody. It's basically the public interface of the dma_fence
> object.
> 
> So testing if a fence is signaled without calling the callback is
> only allowed by whoever implemented the fence.

Why?

See, who owns the callback? -> the driver which emitted the fence. If
the driver doesn't guarantee that all fences will be signaled, the
callback (always returning false) doesn't help you in any way.

I think the issue you're seeing is more that a party that only ever
checks a fence's state through callbacks (and doesn't signal them
through interrupts for example) would run danger of fences never
getting signaled.

But that's already the case if someone doesn't implement the callback.

The fundamental basis is always the same: The driver must guarantee
that all fences get signaled. Independently from other users checking
the fence this or that way, independently from the callback being
implemented.

> 
> In other words nouveau can test nouveau fences, i915 can test i915
> fences, amdgpu can test amdgpu fences etc... But if you have the
> wrapper that makes it officially allowed that nouveau starts testing
> i915 fences and that would be problematic.

I don't see the context here. That applies to the other functions as
well.


P.

> 
> Regards,
> Christian.
> 
> > 
> > That has the advantage of centralizing the responsibility and
> > documenting it.
> > 
> > P.
> > 
> > > 
> > > Regards,
> > > Christian.
> > > 
> > > > 
> > > > Signed-off-by: Philipp Stanner <phasta@kernel.org>
> > > > ---
> > > >  drivers/gpu/drm/nouveau/nouveau_fence.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/nouveau/nouveau_fence.c
> > > > b/drivers/gpu/drm/nouveau/nouveau_fence.c
> > > > index d5654e26d5bc..993b3dcb5db0 100644
> > > > --- a/drivers/gpu/drm/nouveau/nouveau_fence.c
> > > > +++ b/drivers/gpu/drm/nouveau/nouveau_fence.c
> > > > @@ -88,7 +88,7 @@ nouveau_fence_context_kill(struct
> > > > nouveau_fence_chan *fctx, int error)
> > > >  
> > > >  	spin_lock_irqsave(&fctx->lock, flags);
> > > >  	list_for_each_entry_safe(fence, tmp, &fctx->pending,
> > > > head)
> > > > {
> > > > -		if (error &&
> > > > !dma_fence_is_signaled_locked(&fence-
> > > > > base))
> > > > +		if (error && !__dma_fence_is_signaled(&fence-
> > > > > base))
> > > >  			dma_fence_set_error(&fence->base,
> > > > error);
> > > >  
> > > >  		if (nouveau_fence_signal(fence))
> > > 
> > 
> 

Re: [PATCH 2/2] drm/nouveau: Don't signal when killing the fence context

[Tvrtko Ursulin]

On 22/05/2025 13:34, Christian König wrote:
> On 5/22/25 14:20, Philipp Stanner wrote:
>> On Thu, 2025-05-22 at 14:06 +0200, Christian König wrote:
>>> On 5/22/25 13:25, Philipp Stanner wrote:
>>>> dma_fence_is_signaled_locked(), which is used in
>>>> nouveau_fence_context_kill(), can signal fences below the surface
>>>> through a callback.
>>>>
>>>> There is neither need for nor use in doing that when killing a
>>>> fence
>>>> context.
>>>>
>>>> Replace dma_fence_is_signaled_locked() with
>>>> __dma_fence_is_signaled(), a
>>>> function which only checks, never signals.
>>>
>>> That is not a good approach.
>>>
>>> Having the __dma_fence_is_signaled() means that other would be
>>> allowed to call it as well.
>>>
>>> But nouveau can do that here only because it knows that the fence was
>>> issued by nouveau.
>>>
>>> What nouveau can to is to test the signaled flag directly, but that's
>>> what you try to avoid as well.
>>
>> There's many parties who check the bit already.
>>
>> And if Nouveau is allowed to do that, one can just as well provide a
>> wrapper for it.
> 
> No, exactly that's what is usually avoided in cases like this here.
> 
> See all the functions inside include/linux/dma-fence.h can be used by everybody. It's basically the public interface of the dma_fence object.
> 
> So testing if a fence is signaled without calling the callback is only allowed by whoever implemented the fence.
> 
> In other words nouveau can test nouveau fences, i915 can test i915 fences, amdgpu can test amdgpu fences etc... But if you have the wrapper that makes it officially allowed that nouveau starts testing i915 fences and that would be problematic.

But why? Say for example scheduler dependencies - why the scheduler 
couldn't ignore them at add time, but it can before trying to install a 
callback on them, and instead has to opportunistically signal someone 
else's fences?

I don't see it. But even if there is a reason, advantage of the helper 
is that it can document this at a centralised place.

Regards,

Tvrtko

>> That has the advantage of centralizing the responsibility and
>> documenting it.
>>
>> P.
>>
>>>
>>> Regards,
>>> Christian.
>>>
>>>>
>>>> Signed-off-by: Philipp Stanner <phasta@kernel.org>
>>>> ---
>>>>   drivers/gpu/drm/nouveau/nouveau_fence.c | 2 +-
>>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/nouveau/nouveau_fence.c
>>>> b/drivers/gpu/drm/nouveau/nouveau_fence.c
>>>> index d5654e26d5bc..993b3dcb5db0 100644
>>>> --- a/drivers/gpu/drm/nouveau/nouveau_fence.c
>>>> +++ b/drivers/gpu/drm/nouveau/nouveau_fence.c
>>>> @@ -88,7 +88,7 @@ nouveau_fence_context_kill(struct
>>>> nouveau_fence_chan *fctx, int error)
>>>>   
>>>>   	spin_lock_irqsave(&fctx->lock, flags);
>>>>   	list_for_each_entry_safe(fence, tmp, &fctx->pending, head)
>>>> {
>>>> -		if (error && !dma_fence_is_signaled_locked(&fence-
>>>>> base))
>>>> +		if (error && !__dma_fence_is_signaled(&fence-
>>>>> base))
>>>>   			dma_fence_set_error(&fence->base, error);
>>>>   
>>>>   		if (nouveau_fence_signal(fence))
>>>
>>
> 

Re: [PATCH 2/2] drm/nouveau: Don't signal when killing the fence context

[Danilo Krummrich]

On Thu, May 22, 2025 at 02:34:33PM +0200, Christian König wrote:
> See all the functions inside include/linux/dma-fence.h can be used by everybody. It's basically the public interface of the dma_fence object.

As you write below, in certain cases it is valid to call this from drivers, so
it's not unreasonable to have it as part of the public API.

> So testing if a fence is signaled without calling the callback is only allowed by whoever implemented the fence.
> 
> In other words nouveau can test nouveau fences, i915 can test i915 fences, amdgpu can test amdgpu fences etc... But if you have the wrapper that makes it officially allowed that nouveau starts testing i915 fences and that would be problematic.

In general, I like the  __dma_fence_is_signaled() helper, because this way we
can document in which cases it is allowed to be used, i.e. the ones you descibe
above.

test_bit() can be called by anyone and there is no documentation comment
explaining that it is only allowed under certain conditions.

Having the __dma_fence_is_signaled() helper properly documented could get you
rid of having to explain in which case the test_bit() dance is allowed to do
over and over again. :-)

I also think the name is good, since the '__' prefix already implies that there
are some restrictions on the use of this helper.

Re: [PATCH 2/2] drm/nouveau: Don't signal when killing the fence context

[Christian König]

On 5/22/25 14:42, Philipp Stanner wrote:
> On Thu, 2025-05-22 at 14:34 +0200, Christian König wrote:
>> On 5/22/25 14:20, Philipp Stanner wrote:
>>> On Thu, 2025-05-22 at 14:06 +0200, Christian König wrote:
>>>> On 5/22/25 13:25, Philipp Stanner wrote:
>>>>> dma_fence_is_signaled_locked(), which is used in
>>>>> nouveau_fence_context_kill(), can signal fences below the
>>>>> surface
>>>>> through a callback.
>>>>>
>>>>> There is neither need for nor use in doing that when killing a
>>>>> fence
>>>>> context.
>>>>>
>>>>> Replace dma_fence_is_signaled_locked() with
>>>>> __dma_fence_is_signaled(), a
>>>>> function which only checks, never signals.
>>>>
>>>> That is not a good approach.
>>>>
>>>> Having the __dma_fence_is_signaled() means that other would be
>>>> allowed to call it as well.
>>>>
>>>> But nouveau can do that here only because it knows that the fence
>>>> was
>>>> issued by nouveau.
>>>>
>>>> What nouveau can to is to test the signaled flag directly, but
>>>> that's
>>>> what you try to avoid as well.
>>>
>>> There's many parties who check the bit already.
>>>
>>> And if Nouveau is allowed to do that, one can just as well provide
>>> a
>>> wrapper for it.
>>
>> No, exactly that's what is usually avoided in cases like this here.
>>
>> See all the functions inside include/linux/dma-fence.h can be used by
>> everybody. It's basically the public interface of the dma_fence
>> object.
>>
>> So testing if a fence is signaled without calling the callback is
>> only allowed by whoever implemented the fence.
> 
> Why?
> 
> See, who owns the callback? -> the driver which emitted the fence. If
> the driver doesn't guarantee that all fences will be signaled, the
> callback (always returning false) doesn't help you in any way.
> 
> I think the issue you're seeing is more that a party that only ever
> checks a fence's state through callbacks (and doesn't signal them
> through interrupts for example) would run danger of fences never
> getting signaled.

Partially correct, yes.

> But that's already the case if someone doesn't implement the callback.

But than this implementation must always signal somehow else.

> The fundamental basis is always the same: The driver must guarantee
> that all fences get signaled. Independently from other users checking
> the fence this or that way, independently from the callback being
> implemented.

Yeah, but it is invalid for a caller to not ask the implementation if it's signaled or not. See the rational behind that is to avoid abuse of the interface.

E.g. when you don't know the implementation side use the defined API and don't mess with the internals. If you do know the implementation side then it's valid that you check the internals.

Regards,
Christian.

> 
>>
>> In other words nouveau can test nouveau fences, i915 can test i915
>> fences, amdgpu can test amdgpu fences etc... But if you have the
>> wrapper that makes it officially allowed that nouveau starts testing
>> i915 fences and that would be problematic.
> 
> I don't see the context here. That applies to the other functions as
> well.
> 
> 
> P.
> 
>>
>> Regards,
>> Christian.
>>
>>>
>>> That has the advantage of centralizing the responsibility and
>>> documenting it.
>>>
>>> P.
>>>
>>>>
>>>> Regards,
>>>> Christian.
>>>>
>>>>>
>>>>> Signed-off-by: Philipp Stanner <phasta@kernel.org>
>>>>> ---
>>>>>  drivers/gpu/drm/nouveau/nouveau_fence.c | 2 +-
>>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/nouveau/nouveau_fence.c
>>>>> b/drivers/gpu/drm/nouveau/nouveau_fence.c
>>>>> index d5654e26d5bc..993b3dcb5db0 100644
>>>>> --- a/drivers/gpu/drm/nouveau/nouveau_fence.c
>>>>> +++ b/drivers/gpu/drm/nouveau/nouveau_fence.c
>>>>> @@ -88,7 +88,7 @@ nouveau_fence_context_kill(struct
>>>>> nouveau_fence_chan *fctx, int error)
>>>>>  
>>>>>  	spin_lock_irqsave(&fctx->lock, flags);
>>>>>  	list_for_each_entry_safe(fence, tmp, &fctx->pending,
>>>>> head)
>>>>> {
>>>>> -		if (error &&
>>>>> !dma_fence_is_signaled_locked(&fence-
>>>>>> base))
>>>>> +		if (error && !__dma_fence_is_signaled(&fence-
>>>>>> base))
>>>>>  			dma_fence_set_error(&fence->base,
>>>>> error);
>>>>>  
>>>>>  		if (nouveau_fence_signal(fence))
>>>>
>>>
>>
> 

Re: [PATCH 2/2] drm/nouveau: Don't signal when killing the fence context

[Christian König]

On 5/22/25 14:59, Danilo Krummrich wrote:
> On Thu, May 22, 2025 at 02:34:33PM +0200, Christian König wrote:
>> See all the functions inside include/linux/dma-fence.h can be used by everybody. It's basically the public interface of the dma_fence object.
> 
> As you write below, in certain cases it is valid to call this from drivers, so
> it's not unreasonable to have it as part of the public API.

The question is from which drivers?

>> So testing if a fence is signaled without calling the callback is only allowed by whoever implemented the fence.
>>
>> In other words nouveau can test nouveau fences, i915 can test i915 fences, amdgpu can test amdgpu fences etc... But if you have the wrapper that makes it officially allowed that nouveau starts testing i915 fences and that would be problematic.
> 
> In general, I like the  __dma_fence_is_signaled() helper, because this way we
> can document in which cases it is allowed to be used, i.e. the ones you descibe
> above.
> 
> test_bit() can be called by anyone and there is no documentation comment
> explaining that it is only allowed under certain conditions.

That's a rather good argument.

> Having the __dma_fence_is_signaled() helper properly documented could get you
> rid of having to explain in which case the test_bit() dance is allowed to do
> over and over again. :-)

That's an even better argument. 

> I also think the name is good, since the '__' prefix already implies that there
> are some restrictions on the use of this helper.

I'm still hesitating. Adding something to the API always made it usable by everybody.

Now suddenly saying we add that to the include/linux/dma-fence.h header but only certainly code can use it still sounds questionable to me.

Regards,
Christian.

Re: [PATCH 2/2] drm/nouveau: Don't signal when killing the fence context

[Christian König]

On 5/22/25 14:57, Tvrtko Ursulin wrote:
> 
> On 22/05/2025 13:34, Christian König wrote:
>> On 5/22/25 14:20, Philipp Stanner wrote:
>>> On Thu, 2025-05-22 at 14:06 +0200, Christian König wrote:
>>>> On 5/22/25 13:25, Philipp Stanner wrote:
>>>>> dma_fence_is_signaled_locked(), which is used in
>>>>> nouveau_fence_context_kill(), can signal fences below the surface
>>>>> through a callback.
>>>>>
>>>>> There is neither need for nor use in doing that when killing a
>>>>> fence
>>>>> context.
>>>>>
>>>>> Replace dma_fence_is_signaled_locked() with
>>>>> __dma_fence_is_signaled(), a
>>>>> function which only checks, never signals.
>>>>
>>>> That is not a good approach.
>>>>
>>>> Having the __dma_fence_is_signaled() means that other would be
>>>> allowed to call it as well.
>>>>
>>>> But nouveau can do that here only because it knows that the fence was
>>>> issued by nouveau.
>>>>
>>>> What nouveau can to is to test the signaled flag directly, but that's
>>>> what you try to avoid as well.
>>>
>>> There's many parties who check the bit already.
>>>
>>> And if Nouveau is allowed to do that, one can just as well provide a
>>> wrapper for it.
>>
>> No, exactly that's what is usually avoided in cases like this here.
>>
>> See all the functions inside include/linux/dma-fence.h can be used by everybody. It's basically the public interface of the dma_fence object.
>>
>> So testing if a fence is signaled without calling the callback is only allowed by whoever implemented the fence.
>>
>> In other words nouveau can test nouveau fences, i915 can test i915 fences, amdgpu can test amdgpu fences etc... But if you have the wrapper that makes it officially allowed that nouveau starts testing i915 fences and that would be problematic.
> 
> But why? Say for example scheduler dependencies - why the scheduler couldn't ignore them at add time, but it can before trying to install a callback on them, and instead has to opportunistically signal someone else's fences?

We had cases where people tested the signaling status from time to time and were then surprised that the fence never signaled.

> I don't see it. But even if there is a reason, advantage of the helper is that it can document this at a centralised place.

Yeah, that is basically the only argument I can see which speaks in favor of that approach.

Regards,
Christian. 

> 
> Regards,
> 
> Tvrtko
> 
>>> That has the advantage of centralizing the responsibility and
>>> documenting it.
>>>
>>> P.
>>>
>>>>
>>>> Regards,
>>>> Christian.
>>>>
>>>>>
>>>>> Signed-off-by: Philipp Stanner <phasta@kernel.org>
>>>>> ---
>>>>>   drivers/gpu/drm/nouveau/nouveau_fence.c | 2 +-
>>>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/nouveau/nouveau_fence.c
>>>>> b/drivers/gpu/drm/nouveau/nouveau_fence.c
>>>>> index d5654e26d5bc..993b3dcb5db0 100644
>>>>> --- a/drivers/gpu/drm/nouveau/nouveau_fence.c
>>>>> +++ b/drivers/gpu/drm/nouveau/nouveau_fence.c
>>>>> @@ -88,7 +88,7 @@ nouveau_fence_context_kill(struct
>>>>> nouveau_fence_chan *fctx, int error)
>>>>>         spin_lock_irqsave(&fctx->lock, flags);
>>>>>       list_for_each_entry_safe(fence, tmp, &fctx->pending, head)
>>>>> {
>>>>> -        if (error && !dma_fence_is_signaled_locked(&fence-
>>>>>> base))
>>>>> +        if (error && !__dma_fence_is_signaled(&fence-
>>>>>> base))
>>>>>               dma_fence_set_error(&fence->base, error);
>>>>>             if (nouveau_fence_signal(fence))
>>>>
>>>
>>
> 

Re: [PATCH 2/2] drm/nouveau: Don't signal when killing the fence context

[Philipp Stanner]

On Thu, 2025-05-22 at 15:09 +0200, Christian König wrote:
> On 5/22/25 14:59, Danilo Krummrich wrote:
> > On Thu, May 22, 2025 at 02:34:33PM +0200, Christian König wrote:
> > > See all the functions inside include/linux/dma-fence.h can be
> > > used by everybody. It's basically the public interface of the
> > > dma_fence object.
> > 
> > As you write below, in certain cases it is valid to call this from
> > drivers, so
> > it's not unreasonable to have it as part of the public API.
> 
> The question is from which drivers?
> 
> > > So testing if a fence is signaled without calling the callback is
> > > only allowed by whoever implemented the fence.
> > > 
> > > In other words nouveau can test nouveau fences, i915 can test
> > > i915 fences, amdgpu can test amdgpu fences etc... But if you have
> > > the wrapper that makes it officially allowed that nouveau starts
> > > testing i915 fences and that would be problematic.
> > 
> > In general, I like the  __dma_fence_is_signaled() helper, because
> > this way we
> > can document in which cases it is allowed to be used, i.e. the ones
> > you descibe
> > above.
> > 
> > test_bit() can be called by anyone and there is no documentation
> > comment
> > explaining that it is only allowed under certain conditions.
> 
> That's a rather good argument.
> 
> > Having the __dma_fence_is_signaled() helper properly documented
> > could get you
> > rid of having to explain in which case the test_bit() dance is
> > allowed to do
> > over and over again. :-)
> 
> That's an even better argument. 
> 
> > I also think the name is good, since the '__' prefix already
> > implies that there
> > are some restrictions on the use of this helper.
> 
> I'm still hesitating. Adding something to the API always made it
> usable by everybody.
> 
> Now suddenly saying we add that to the include/linux/dma-fence.h
> header but only certainly code can use it still sounds questionable
> to me.

If I understand the current code correctly, the documentation state and
the question "which driver is allowed to do it?" is the same, because
the documentation for the signaled callback doesn't specify that:


	/**
	 * @signaled:
	 *
	 * Peek whether the fence is signaled, as a fastpath optimization for
	 * e.g. dma_fence_wait() or dma_fence_add_callback(). Note that this
	 * callback does not need to make any guarantees beyond that a fence
	 * once indicates as signalled must always return true from this
	 * callback. This callback may return false even if the fence has
	 * completed already, in this case information hasn't propogated throug
	 * the system yet. See also dma_fence_is_signaled().
	 *
	 * May set &dma_fence.error if returning true.
	 *
	 * This callback is optional.
	 */
	bool (*signaled)(struct dma_fence *fence);


"optional". What if I don't ipmlement it? Who should implement it?

If the callback is optional, then dma_fence_is_signaled() is the same
as __dma_fence_is_signaled().

IOW, it already needs to be better documented who needs to implement
the callback and who doesn't. If we get clarity on that, we also get
clarity on who may use __dma_fence_is_signaled().


P.

> 
> Regards,
> Christian.

Re: [PATCH 2/2] drm/nouveau: Don't signal when killing the fence context

[Christian König]

On 5/22/25 15:16, Philipp Stanner wrote:
> On Thu, 2025-05-22 at 15:09 +0200, Christian König wrote:
>> On 5/22/25 14:59, Danilo Krummrich wrote:
>>> On Thu, May 22, 2025 at 02:34:33PM +0200, Christian König wrote:
>>>> See all the functions inside include/linux/dma-fence.h can be
>>>> used by everybody. It's basically the public interface of the
>>>> dma_fence object.
>>>
>>> As you write below, in certain cases it is valid to call this from
>>> drivers, so
>>> it's not unreasonable to have it as part of the public API.
>>
>> The question is from which drivers?
>>
>>>> So testing if a fence is signaled without calling the callback is
>>>> only allowed by whoever implemented the fence.
>>>>
>>>> In other words nouveau can test nouveau fences, i915 can test
>>>> i915 fences, amdgpu can test amdgpu fences etc... But if you have
>>>> the wrapper that makes it officially allowed that nouveau starts
>>>> testing i915 fences and that would be problematic.
>>>
>>> In general, I like the  __dma_fence_is_signaled() helper, because
>>> this way we
>>> can document in which cases it is allowed to be used, i.e. the ones
>>> you descibe
>>> above.
>>>
>>> test_bit() can be called by anyone and there is no documentation
>>> comment
>>> explaining that it is only allowed under certain conditions.
>>
>> That's a rather good argument.
>>
>>> Having the __dma_fence_is_signaled() helper properly documented
>>> could get you
>>> rid of having to explain in which case the test_bit() dance is
>>> allowed to do
>>> over and over again. :-)
>>
>> That's an even better argument. 
>>
>>> I also think the name is good, since the '__' prefix already
>>> implies that there
>>> are some restrictions on the use of this helper.
>>
>> I'm still hesitating. Adding something to the API always made it
>> usable by everybody.
>>
>> Now suddenly saying we add that to the include/linux/dma-fence.h
>> header but only certainly code can use it still sounds questionable
>> to me.
> 
> If I understand the current code correctly, the documentation state and
> the question "which driver is allowed to do it?" is the same, because
> the documentation for the signaled callback doesn't specify that:
> 
> 
> 	/**
> 	 * @signaled:
> 	 *
> 	 * Peek whether the fence is signaled, as a fastpath optimization for
> 	 * e.g. dma_fence_wait() or dma_fence_add_callback(). Note that this
> 	 * callback does not need to make any guarantees beyond that a fence
> 	 * once indicates as signalled must always return true from this
> 	 * callback. This callback may return false even if the fence has
> 	 * completed already, in this case information hasn't propogated throug
> 	 * the system yet. See also dma_fence_is_signaled().
> 	 *
> 	 * May set &dma_fence.error if returning true.
> 	 *
> 	 * This callback is optional.
> 	 */
> 	bool (*signaled)(struct dma_fence *fence);
> 
> 
> "optional". What if I don't ipmlement it? Who should implement it?
> 
> If the callback is optional, then dma_fence_is_signaled() is the same
> as __dma_fence_is_signaled().
> 
> IOW, it already needs to be better documented who needs to implement
> the callback and who doesn't. If we get clarity on that, we also get
> clarity on who may use __dma_fence_is_signaled().

Well there is no need to implement it, but when it is implemented the caller *must* call it when polling.

IIRC the background that we didn't allowed this was that we already had the case that users only looked at the signaling bit and then where surprised that it never changed.

Regards,
Christian.

> 
> 
> P.
> 
>>
>> Regards,
>> Christian.
> 

Re: [PATCH 2/2] drm/nouveau: Don't signal when killing the fence context

[Danilo Krummrich]

On Thu, May 22, 2025 at 03:09:49PM +0200, Christian König wrote:
> On 5/22/25 14:59, Danilo Krummrich wrote:
> > On Thu, May 22, 2025 at 02:34:33PM +0200, Christian König wrote:
> >> See all the functions inside include/linux/dma-fence.h can be used by everybody. It's basically the public interface of the dma_fence object.
> > 
> > As you write below, in certain cases it is valid to call this from drivers, so
> > it's not unreasonable to have it as part of the public API.
> 
> The question is from which drivers?

Well, any driver that uses it to check its own fences, as you say below.

> >> So testing if a fence is signaled without calling the callback is only allowed by whoever implemented the fence.
> >>
> >> In other words nouveau can test nouveau fences, i915 can test i915 fences, amdgpu can test amdgpu fences etc... But if you have the wrapper that makes it officially allowed that nouveau starts testing i915 fences and that would be problematic.
> > 
> > In general, I like the  __dma_fence_is_signaled() helper, because this way we
> > can document in which cases it is allowed to be used, i.e. the ones you descibe
> > above.
> > 
> > test_bit() can be called by anyone and there is no documentation comment
> > explaining that it is only allowed under certain conditions.
> 
> That's a rather good argument.
> 
> > Having the __dma_fence_is_signaled() helper properly documented could get you
> > rid of having to explain in which case the test_bit() dance is allowed to do
> > over and over again. :-)
> 
> That's an even better argument. 
> 
> > I also think the name is good, since the '__' prefix already implies that there
> > are some restrictions on the use of this helper.
> 
> I'm still hesitating. Adding something to the API always made it usable by everybody.

You can't prevent that, the test_bit() dance can be done by anyone, but you can
document it properly with this helper. :-)

Re: [PATCH 2/2] drm/nouveau: Don't signal when killing the fence context

[Philipp Stanner]

On Thu, 2025-05-22 at 15:24 +0200, Christian König wrote:
> On 5/22/25 15:16, Philipp Stanner wrote:
> > On Thu, 2025-05-22 at 15:09 +0200, Christian König wrote:
> > > On 5/22/25 14:59, Danilo Krummrich wrote:
> > > > On Thu, May 22, 2025 at 02:34:33PM +0200, Christian König
> > > > wrote:
> > > > > See all the functions inside include/linux/dma-fence.h can be
> > > > > used by everybody. It's basically the public interface of the
> > > > > dma_fence object.
> > > > 
> > > > As you write below, in certain cases it is valid to call this
> > > > from
> > > > drivers, so
> > > > it's not unreasonable to have it as part of the public API.
> > > 
> > > The question is from which drivers?
> > > 
> > > > > So testing if a fence is signaled without calling the
> > > > > callback is
> > > > > only allowed by whoever implemented the fence.
> > > > > 
> > > > > In other words nouveau can test nouveau fences, i915 can test
> > > > > i915 fences, amdgpu can test amdgpu fences etc... But if you
> > > > > have
> > > > > the wrapper that makes it officially allowed that nouveau
> > > > > starts
> > > > > testing i915 fences and that would be problematic.
> > > > 
> > > > In general, I like the  __dma_fence_is_signaled() helper,
> > > > because
> > > > this way we
> > > > can document in which cases it is allowed to be used, i.e. the
> > > > ones
> > > > you descibe
> > > > above.
> > > > 
> > > > test_bit() can be called by anyone and there is no
> > > > documentation
> > > > comment
> > > > explaining that it is only allowed under certain conditions.
> > > 
> > > That's a rather good argument.
> > > 
> > > > Having the __dma_fence_is_signaled() helper properly documented
> > > > could get you
> > > > rid of having to explain in which case the test_bit() dance is
> > > > allowed to do
> > > > over and over again. :-)
> > > 
> > > That's an even better argument. 
> > > 
> > > > I also think the name is good, since the '__' prefix already
> > > > implies that there
> > > > are some restrictions on the use of this helper.
> > > 
> > > I'm still hesitating. Adding something to the API always made it
> > > usable by everybody.
> > > 
> > > Now suddenly saying we add that to the include/linux/dma-fence.h
> > > header but only certainly code can use it still sounds
> > > questionable
> > > to me.
> > 
> > If I understand the current code correctly, the documentation state
> > and
> > the question "which driver is allowed to do it?" is the same,
> > because
> > the documentation for the signaled callback doesn't specify that:
> > 
> > 
> > 	/**
> > 	 * @signaled:
> > 	 *
> > 	 * Peek whether the fence is signaled, as a fastpath
> > optimization for
> > 	 * e.g. dma_fence_wait() or dma_fence_add_callback(). Note
> > that this
> > 	 * callback does not need to make any guarantees beyond
> > that a fence
> > 	 * once indicates as signalled must always return true
> > from this
> > 	 * callback. This callback may return false even if the
> > fence has
> > 	 * completed already, in this case information hasn't
> > propogated throug
> > 	 * the system yet. See also dma_fence_is_signaled().
> > 	 *
> > 	 * May set &dma_fence.error if returning true.
> > 	 *
> > 	 * This callback is optional.
> > 	 */
> > 	bool (*signaled)(struct dma_fence *fence);
> > 
> > 
> > "optional". What if I don't ipmlement it? Who should implement it?
> > 
> > If the callback is optional, then dma_fence_is_signaled() is the
> > same
> > as __dma_fence_is_signaled().
> > 
> > IOW, it already needs to be better documented who needs to
> > implement
> > the callback and who doesn't. If we get clarity on that, we also
> > get
> > clarity on who may use __dma_fence_is_signaled().
> 
> Well there is no need to implement it, but when it is implemented the
> caller *must* call it when polling.

I don't understand. Please elaborate on that a bit more. If there's no
need to implement it, then why can't one have a
__dma_fence_is_signaled(), which is then identical?

> 
> IIRC the background that we didn't allowed this was that we already
> had the case that users only looked at the signaling bit and then
> where surprised that it never changed.

Why would anyone expect that a fence gets signaled by calling a
function with the name "dma_fence_is_signaled()"? :-)

That was my original point, the name is not intuitive at all.

For example, if a driver doesn't implement that callback but signals
fences in interrupt handlers, and then forgets to (re-)activate the
interrupt, fences will never get signaled and callers to
dma_fence_is_signaled() will never read 'true', which isn't surprising.

Again, the point remains the same: the driver must guarantee that
fences will get signaled. Independently from how consumers of the fence
check it. Consumers could just stop calling dma_fence_is_signaled()
after the point in time T alltogether and then the driver would still
have to signal everything.


P.



> 
> Regards,
> Christian.
> 
> > 
> > 
> > P.
> > 
> > > 
> > > Regards,
> > > Christian.
> > 
> 

Re: [PATCH 2/2] drm/nouveau: Don't signal when killing the fence context

[Danilo Krummrich]

On Thu, May 22, 2025 at 03:05:02PM +0200, Christian König wrote:
> E.g. when you don't know the implementation side use the defined API and don't mess with the internals. If you do know the implementation side then it's valid that you check the internals.

I assume you meant this as "bothering with the internals of you *own* fence is
fine, but not with foreign ones".

And if the driver messes with the internals of its own fence code that's fine,
but in this case we talk about the generic dma_fence implementation, i.e. an
internal flag of the dma_fence implementation.

In general, a driver should *never* bother with implementation details of a
generic component, regardless whether the author knows the internal details.
Things are *always* prone to change and then this may result into subtle bugs.

Re: [PATCH 2/2] drm/nouveau: Don't signal when killing the fence context

[Christian König]

On 5/22/25 15:43, Philipp Stanner wrote:
>>
>> Well there is no need to implement it, but when it is implemented the
>> caller *must* call it when polling.
> 
> I don't understand. Please elaborate on that a bit more. If there's no
> need to implement it, then why can't one have a
> __dma_fence_is_signaled(), which is then identical?

Because the caller then doesn't call it when it is implemented.

E.g. you give people a function which ignores the callback.

>>
>> IIRC the background that we didn't allowed this was that we already
>> had the case that users only looked at the signaling bit and then
>> where surprised that it never changed.
> 
> Why would anyone expect that a fence gets signaled by calling a
> function with the name "dma_fence_is_signaled()"? :-)

Because when that function returns true the fence is considered signaled.

> That was my original point, the name is not intuitive at all.
> 
> For example, if a driver doesn't implement that callback but signals
> fences in interrupt handlers, and then forgets to (re-)activate the
> interrupt, fences will never get signaled and callers to
> dma_fence_is_signaled() will never read 'true', which isn't surprising.
> 
> Again, the point remains the same: the driver must guarantee that
> fences will get signaled. Independently from how consumers of the fence
> check it. Consumers could just stop calling dma_fence_is_signaled()
> after the point in time T alltogether and then the driver would still
> have to signal everything.

No it doesn't. You need to call dma_fence_enable_signaling for that.

Regards,
Christian.

> 
> 
> P.
> 
> 
> 
>>
>> Regards,
>> Christian.
>>
>>>
>>>
>>> P.
>>>
>>>>
>>>> Regards,
>>>> Christian.
>>>
>>
> 

Re: [PATCH 2/2] drm/nouveau: Don't signal when killing the fence context

[Christian König]

On 5/22/25 15:50, Danilo Krummrich wrote:
> On Thu, May 22, 2025 at 03:05:02PM +0200, Christian König wrote:
>> E.g. when you don't know the implementation side use the defined API and don't mess with the internals. If you do know the implementation side then it's valid that you check the internals.
> 
> I assume you meant this as "bothering with the internals of you *own* fence is
> fine, but not with foreign ones".

Yes, exactly that.

> And if the driver messes with the internals of its own fence code that's fine,
> but in this case we talk about the generic dma_fence implementation, i.e. an
> internal flag of the dma_fence implementation.

Well the flag is under the control of the fence implementation.

> In general, a driver should *never* bother with implementation details of a
> generic component, regardless whether the author knows the internal details.
> Things are *always* prone to change and then this may result into subtle bugs.

Yeah, I know what you mean. The implementation kind of sub-classes the dma_fence component to use it to implement it's specific function.

In C++ we would distinct the function between into private: protected: and public: sections, but we don't have that luxury here.

But you already convinced me with the argument that this needs to be better documented, I'm just not sure if adding the function documentation would do it.

Regards,
Christian.

Re: [PATCH 1/2] dma-buf: Add __dma_fence_is_signaled()

[Tvrtko Ursulin]

On 22/05/2025 12:25, Philipp Stanner wrote:
> Some parties want to check whether a function is already signaled
> without actually signaling the fence, which is what
> dma_fence_is_signaled() might due if the fence ops 'signaled' callback

s/due/do/

> is implemented.
> 
> Add __dma_fence_is_signaled(), 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>
> ---
>   include/linux/dma-fence.h | 24 ++++++++++++++++++++++--
>   1 file changed, 22 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
> index 48b5202c531d..ac951a54a007 100644
> --- a/include/linux/dma-fence.h
> +++ b/include/linux/dma-fence.h
> @@ -381,6 +381,26 @@ bool dma_fence_remove_callback(struct dma_fence *fence,
>   			       struct dma_fence_cb *cb);
>   void dma_fence_enable_sw_signaling(struct dma_fence *fence);
>   
> +/**
> + * __dma_fence_is_signaled - 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

s/user/caller/ ?

Otherwise looks good to me. For if/when Christian approves you can add my:

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>

Regards,

Tvrtko

> + * means that fences get signaled eventually.
> + *
> + * This function does not require locking.
> + *
> + * See also dma_fence_is_signaled().
> + *
> + * Return: true if signaled, false otherwise.
> + */
> +static inline bool
> +__dma_fence_is_signaled(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.
> @@ -398,7 +418,7 @@ void dma_fence_enable_sw_signaling(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_is_signaled(fence))
>   		return true;
>   
>   	if (fence->ops->signaled && fence->ops->signaled(fence)) {
> @@ -428,7 +448,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_is_signaled(fence))
>   		return true;
>   
>   	if (fence->ops->signaled && fence->ops->signaled(fence)) {