The overwhelming majority of users of dma_fence signaling functions
don't care about whether the fence had already been signaled by someone
else. Therefore, the return code shall be removed from those functions.

For the few users who rely on the check, a new, specialized function
shall be provided.

Add dma_fence_check_and_signal(), which signals a fence if it had not
yet been signaled, and informs the user about that.

Add a counter part, dma_fence_check_and_signal_locked(), which doesn't
take the spinlock.

Signed-off-by: Philipp Stanner <phasta@kernel.org>
---
 drivers/dma-buf/dma-fence.c | 44 +++++++++++++++++++++++++++++++++++++
 include/linux/dma-fence.h   |  2 ++
 2 files changed, 46 insertions(+)

diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
index 96d72ffc0750..146de62887cf 100644
--- a/drivers/dma-buf/dma-fence.c
+++ b/drivers/dma-buf/dma-fence.c
@@ -445,6 +445,50 @@ int dma_fence_signal_locked(struct dma_fence *fence)
 }
 EXPORT_SYMBOL(dma_fence_signal_locked);
 
+/**
+ * dma_fence_check_and_signal_locked - signal the fence if it's not yet signaled
+ * @fence: the fence to check and signal
+ *
+ * Checks whether a fence was signaled and signals it if it was not yet signaled.
+ *
+ * Unlike dma_fence_check_and_signal(), this function must be called with
+ * &struct dma_fence.lock being held.
+ *
+ * Return: true if fence has been signaled already, false otherwise.
+ */
+bool dma_fence_check_and_signal_locked(struct dma_fence *fence)
+{
+	bool ret;
+
+	ret = dma_fence_test_signaled_flag(fence);
+	dma_fence_signal_locked(fence);
+
+	return ret;
+}
+EXPORT_SYMBOL(dma_fence_check_and_signal_locked);
+
+/**
+ * dma_fence_check_and_signal - signal the fence if it's not yet signaled
+ * @fence: the fence to check and signal
+ *
+ * Checks whether a fence was signaled and signals it if it was not yet signaled.
+ * All this is done in a race-free manner.
+ *
+ * Return: true if fence has been signaled already, false otherwise.
+ */
+bool dma_fence_check_and_signal(struct dma_fence *fence)
+{
+	unsigned long flags;
+	bool ret;
+
+	spin_lock_irqsave(fence->lock, flags);
+	ret = dma_fence_check_and_signal_locked(fence);
+	spin_unlock_irqrestore(fence->lock, flags);
+
+	return ret;
+}
+EXPORT_SYMBOL(dma_fence_check_and_signal);
+
 /**
  * dma_fence_signal - signal completion of a fence
  * @fence: the fence to signal
diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
index 19972f5d176f..0504afe52c2a 100644
--- a/include/linux/dma-fence.h
+++ b/include/linux/dma-fence.h
@@ -365,6 +365,8 @@ static inline void __dma_fence_might_wait(void) {}
 #endif
 
 int 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,
-- 
2.49.0

On 12/1/25 11:50, Philipp Stanner wrote:
> The overwhelming majority of users of dma_fence signaling functions
> don't care about whether the fence had already been signaled by someone
> else. Therefore, the return code shall be removed from those functions.
> 
> For the few users who rely on the check, a new, specialized function
> shall be provided.
> 
> Add dma_fence_check_and_signal(), which signals a fence if it had not
> yet been signaled, and informs the user about that.
> 
> Add a counter part, dma_fence_check_and_signal_locked(), which doesn't
> take the spinlock.
> 
> Signed-off-by: Philipp Stanner <phasta@kernel.org>
> ---
>  drivers/dma-buf/dma-fence.c | 44 +++++++++++++++++++++++++++++++++++++
>  include/linux/dma-fence.h   |  2 ++
>  2 files changed, 46 insertions(+)
> 
> diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
> index 96d72ffc0750..146de62887cf 100644
> --- a/drivers/dma-buf/dma-fence.c
> +++ b/drivers/dma-buf/dma-fence.c
> @@ -445,6 +445,50 @@ int dma_fence_signal_locked(struct dma_fence *fence)
>  }
>  EXPORT_SYMBOL(dma_fence_signal_locked);
>  
> +/**
> + * dma_fence_check_and_signal_locked - signal the fence if it's not yet signaled
> + * @fence: the fence to check and signal
> + *
> + * Checks whether a fence was signaled and signals it if it was not yet signaled.
> + *
> + * Unlike dma_fence_check_and_signal(), this function must be called with
> + * &struct dma_fence.lock being held.
> + *
> + * Return: true if fence has been signaled already, false otherwise.
> + */
> +bool dma_fence_check_and_signal_locked(struct dma_fence *fence)

I'm seriously considering to nuke all the unlocked variants of dma_fence functions and just make it mandatory for callers to grab the lock manually.

> +{
> +	bool ret;
> +
> +	ret = dma_fence_test_signaled_flag(fence);
> +	dma_fence_signal_locked(fence);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL(dma_fence_check_and_signal_locked);
> +
> +/**
> + * dma_fence_check_and_signal - signal the fence if it's not yet signaled
> + * @fence: the fence to check and signal
> + *
> + * Checks whether a fence was signaled and signals it if it was not yet signaled.
> + * All this is done in a race-free manner.
> + *
> + * Return: true if fence has been signaled already, false otherwise.
> + */
> +bool dma_fence_check_and_signal(struct dma_fence *fence)

So I think we should name this one here dma_fence_check_and_signal_unlocked() and drop the postfix from the locked variant.

> +{
> +	unsigned long flags;
> +	bool ret;
> +
> +	spin_lock_irqsave(fence->lock, flags);
> +	ret = dma_fence_check_and_signal_locked(fence);
> +	spin_unlock_irqrestore(fence->lock, flags);

Could this use guard(fence->lock, flags) ?

Regards,
Christian.

> +
> +	return ret;
> +}
> +EXPORT_SYMBOL(dma_fence_check_and_signal);
> +
>  /**
>   * dma_fence_signal - signal completion of a fence
>   * @fence: the fence to signal
> diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
> index 19972f5d176f..0504afe52c2a 100644
> --- a/include/linux/dma-fence.h
> +++ b/include/linux/dma-fence.h
> @@ -365,6 +365,8 @@ static inline void __dma_fence_might_wait(void) {}
>  #endif
>  
>  int 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,

On Mon, 2025-12-01 at 14:23 +0100, Christian König wrote:
> On 12/1/25 11:50, Philipp Stanner wrote:
> > The overwhelming majority of users of dma_fence signaling functions
> > don't care about whether the fence had already been signaled by someone
> > else. Therefore, the return code shall be removed from those functions.
> > 
> > For the few users who rely on the check, a new, specialized function
> > shall be provided.
> > 
> > Add dma_fence_check_and_signal(), which signals a fence if it had not
> > yet been signaled, and informs the user about that.
> > 
> > Add a counter part, dma_fence_check_and_signal_locked(), which doesn't
> > take the spinlock.
> > 
> > Signed-off-by: Philipp Stanner <phasta@kernel.org>
> > ---
> >  drivers/dma-buf/dma-fence.c | 44 +++++++++++++++++++++++++++++++++++++
> >  include/linux/dma-fence.h   |  2 ++
> >  2 files changed, 46 insertions(+)
> > 
> > diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
> > index 96d72ffc0750..146de62887cf 100644
> > --- a/drivers/dma-buf/dma-fence.c
> > +++ b/drivers/dma-buf/dma-fence.c
> > @@ -445,6 +445,50 @@ int dma_fence_signal_locked(struct dma_fence *fence)
> >  }
> >  EXPORT_SYMBOL(dma_fence_signal_locked);
> >  
> > +/**
> > + * dma_fence_check_and_signal_locked - signal the fence if it's not yet signaled
> > + * @fence: the fence to check and signal
> > + *
> > + * Checks whether a fence was signaled and signals it if it was not yet signaled.
> > + *
> > + * Unlike dma_fence_check_and_signal(), this function must be called with
> > + * &struct dma_fence.lock being held.
> > + *
> > + * Return: true if fence has been signaled already, false otherwise.
> > + */
> > +bool dma_fence_check_and_signal_locked(struct dma_fence *fence)
> 
> I'm seriously considering to nuke all the unlocked variants of dma_fence functions and just make it mandatory for callers to grab the lock manually.
> 

You mean "nuke the *locked* variants.

Why, though? Aren't they enough for most users?
I suppose you have all those subtle races in mind..

> > +{
> > +	bool ret;
> > +
> > +	ret = dma_fence_test_signaled_flag(fence);
> > +	dma_fence_signal_locked(fence);
> > +
> > +	return ret;
> > +}
> > +EXPORT_SYMBOL(dma_fence_check_and_signal_locked);
> > +
> > +/**
> > + * dma_fence_check_and_signal - signal the fence if it's not yet signaled
> > + * @fence: the fence to check and signal
> > + *
> > + * Checks whether a fence was signaled and signals it if it was not yet signaled.
> > + * All this is done in a race-free manner.
> > + *
> > + * Return: true if fence has been signaled already, false otherwise.
> > + */
> > +bool dma_fence_check_and_signal(struct dma_fence *fence)
> 
> So I think we should name this one here dma_fence_check_and_signal_unlocked() and drop the postfix from the locked variant.

postfix?

Well, now, IDK. Can't we, for this series, keep the _locked() variant
so that it's congruent with all the other dma_fence code?

And then later if you want to force manual locking you can add that
kernel-wide in a separate series, since it'll be a discussion-worthy,
bigger chunk of work.

That's cleaner, and my series here won't prevent that once merged.

> 
> > +{
> > +	unsigned long flags;
> > +	bool ret;
> > +
> > +	spin_lock_irqsave(fence->lock, flags);
> > +	ret = dma_fence_check_and_signal_locked(fence);
> > +	spin_unlock_irqrestore(fence->lock, flags);
> 
> Could this use guard(fence->lock, flags) ?

guard? You mean a lockdep guard? Do you have a pointer to someplace in
dma_fence who does what you mean / want?


P.

> 
> Regards,
> Christian.
> 
> > +
> > +	return ret;
> > +}
> > +EXPORT_SYMBOL(dma_fence_check_and_signal);
> > +
> >  /**
> >   * dma_fence_signal - signal completion of a fence
> >   * @fence: the fence to signal
> > diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
> > index 19972f5d176f..0504afe52c2a 100644
> > --- a/include/linux/dma-fence.h
> > +++ b/include/linux/dma-fence.h
> > @@ -365,6 +365,8 @@ static inline void __dma_fence_might_wait(void) {}
> >  #endif
> >  
> >  int 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,
> 

On 12/1/25 14:55, Philipp Stanner wrote:
> On Mon, 2025-12-01 at 14:23 +0100, Christian König wrote:
>> On 12/1/25 11:50, Philipp Stanner wrote:
>>> The overwhelming majority of users of dma_fence signaling functions
>>> don't care about whether the fence had already been signaled by someone
>>> else. Therefore, the return code shall be removed from those functions.
>>>
>>> For the few users who rely on the check, a new, specialized function
>>> shall be provided.
>>>
>>> Add dma_fence_check_and_signal(), which signals a fence if it had not
>>> yet been signaled, and informs the user about that.
>>>
>>> Add a counter part, dma_fence_check_and_signal_locked(), which doesn't
>>> take the spinlock.
>>>
>>> Signed-off-by: Philipp Stanner <phasta@kernel.org>
>>> ---
>>>  drivers/dma-buf/dma-fence.c | 44 +++++++++++++++++++++++++++++++++++++
>>>  include/linux/dma-fence.h   |  2 ++
>>>  2 files changed, 46 insertions(+)
>>>
>>> diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
>>> index 96d72ffc0750..146de62887cf 100644
>>> --- a/drivers/dma-buf/dma-fence.c
>>> +++ b/drivers/dma-buf/dma-fence.c
>>> @@ -445,6 +445,50 @@ int dma_fence_signal_locked(struct dma_fence *fence)
>>>  }
>>>  EXPORT_SYMBOL(dma_fence_signal_locked);
>>>  
>>> +/**
>>> + * dma_fence_check_and_signal_locked - signal the fence if it's not yet signaled
>>> + * @fence: the fence to check and signal
>>> + *
>>> + * Checks whether a fence was signaled and signals it if it was not yet signaled.
>>> + *
>>> + * Unlike dma_fence_check_and_signal(), this function must be called with
>>> + * &struct dma_fence.lock being held.
>>> + *
>>> + * Return: true if fence has been signaled already, false otherwise.
>>> + */
>>> +bool dma_fence_check_and_signal_locked(struct dma_fence *fence)
>>
>> I'm seriously considering to nuke all the unlocked variants of dma_fence functions and just make it mandatory for callers to grab the lock manually.
>>
> 
> You mean "nuke the *locked* variants.

Sorry, that wasn't specific enough.

What I meant was making the locked variants the default instead of the unlocked ones.

> 
> Why, though? Aren't they enough for most users?
> I suppose you have all those subtle races in mind..

Yeah, exactly that.

> 
>>> +{
>>> +	bool ret;
>>> +
>>> +	ret = dma_fence_test_signaled_flag(fence);
>>> +	dma_fence_signal_locked(fence);
>>> +
>>> +	return ret;
>>> +}
>>> +EXPORT_SYMBOL(dma_fence_check_and_signal_locked);
>>> +
>>> +/**
>>> + * dma_fence_check_and_signal - signal the fence if it's not yet signaled
>>> + * @fence: the fence to check and signal
>>> + *
>>> + * Checks whether a fence was signaled and signals it if it was not yet signaled.
>>> + * All this is done in a race-free manner.
>>> + *
>>> + * Return: true if fence has been signaled already, false otherwise.
>>> + */
>>> +bool dma_fence_check_and_signal(struct dma_fence *fence)
>>
>> So I think we should name this one here dma_fence_check_and_signal_unlocked() and drop the postfix from the locked variant.
> 
> postfix?
> 
> Well, now, IDK. Can't we, for this series, keep the _locked() variant
> so that it's congruent with all the other dma_fence code?

Good point. That thought was not really related to this series here.

> 
> And then later if you want to force manual locking you can add that
> kernel-wide in a separate series, since it'll be a discussion-worthy,
> bigger chunk of work.
> 
> That's cleaner, and my series here won't prevent that once merged.
> 
>>
>>> +{
>>> +	unsigned long flags;
>>> +	bool ret;
>>> +
>>> +	spin_lock_irqsave(fence->lock, flags);
>>> +	ret = dma_fence_check_and_signal_locked(fence);
>>> +	spin_unlock_irqrestore(fence->lock, flags);
>>
>> Could this use guard(fence->lock, flags) ?
> 
> guard? You mean a lockdep guard? Do you have a pointer to someplace in
> dma_fence who does what you mean / want?

E.g. like guard(spinlock_irqsave)(&fence->lock);

Regards,
Christian.

> 
> 
> P.
> 
>>
>> Regards,
>> Christian.
>>
>>> +
>>> +	return ret;
>>> +}
>>> +EXPORT_SYMBOL(dma_fence_check_and_signal);
>>> +
>>>  /**
>>>   * dma_fence_signal - signal completion of a fence
>>>   * @fence: the fence to signal
>>> diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
>>> index 19972f5d176f..0504afe52c2a 100644
>>> --- a/include/linux/dma-fence.h
>>> +++ b/include/linux/dma-fence.h
>>> @@ -365,6 +365,8 @@ static inline void __dma_fence_might_wait(void) {}
>>>  #endif
>>>  
>>>  int 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,
>>
> 

On Mon, 2025-12-01 at 16:20 +0100, Christian König wrote:
> On 12/1/25 14:55, Philipp Stanner wrote:
> > On Mon, 2025-12-01 at 14:23 +0100, Christian König wrote:
> > > On 12/1/25 11:50, Philipp Stanner wrote:
> > > > The overwhelming majority of users of dma_fence signaling functions
> > > > don't care about whether the fence had already been signaled by someone
> > > > 
> > 

[…]

> > > 
> > > > +{
> > > > +	unsigned long flags;
> > > > +	bool ret;
> > > > +
> > > > +	spin_lock_irqsave(fence->lock, flags);
> > > > +	ret = dma_fence_check_and_signal_locked(fence);
> > > > +	spin_unlock_irqrestore(fence->lock, flags);
> > > 
> > > Could this use guard(fence->lock, flags) ?
> > 
> > guard? You mean a lockdep guard? Do you have a pointer to someplace in
> > dma_fence who does what you mean / want?
> 
> E.g. like guard(spinlock_irqsave)(&fence->lock);


Hmm, but why?
It's obvious to all readers that I do spin_unlock_irqrestore() here.
It's very simple code, lock, 1 line, unlock. What would the guard
improve?


P.

On 12/1/25 16:53, Philipp Stanner wrote:
> On Mon, 2025-12-01 at 16:20 +0100, Christian König wrote:
>> On 12/1/25 14:55, Philipp Stanner wrote:
>>> On Mon, 2025-12-01 at 14:23 +0100, Christian König wrote:
>>>> On 12/1/25 11:50, Philipp Stanner wrote:
>>>>> The overwhelming majority of users of dma_fence signaling functions
>>>>> don't care about whether the fence had already been signaled by someone
>>>>>
>>>
> 
> […]
> 
>>>>
>>>>> +{
>>>>> +	unsigned long flags;
>>>>> +	bool ret;
>>>>> +
>>>>> +	spin_lock_irqsave(fence->lock, flags);
>>>>> +	ret = dma_fence_check_and_signal_locked(fence);
>>>>> +	spin_unlock_irqrestore(fence->lock, flags);
>>>>
>>>> Could this use guard(fence->lock, flags) ?
>>>
>>> guard? You mean a lockdep guard? Do you have a pointer to someplace in
>>> dma_fence who does what you mean / want?
>>
>> E.g. like guard(spinlock_irqsave)(&fence->lock);
> 
> 
> Hmm, but why?
> It's obvious to all readers that I do spin_unlock_irqrestore() here.
> It's very simple code, lock, 1 line, unlock. What would the guard
> improve?

Well you can save using the local variables.

So this:

	unsigned long flags;
	bool ret;

	spin_lock_irqsave(fence->lock, flags);
	ret = dma_fence_check_and_signal_locked(fence);
	spin_unlock_irqrestore(fence->lock, flags);

	return ret;

Becomes just:

	guard(spinlock_irqsave)(&fence->lock);
	return dma_fence_check_and_signal_locked(fence);

Regards,
Christian.

> 
> 
> P.

On Mon, 2025-12-01 at 16:20 +0100, Christian König wrote:
> On 12/1/25 14:55, Philipp Stanner wrote:
> > On Mon, 2025-12-01 at 14:23 +0100, Christian König wrote:
> > > On 12/1/25 11:50, Philipp Stanner wrote:
> > > > The overwhelming majority of users of dma_fence signaling functions
> > > > don't care about whether the fence had already been signaled by someone
> > > > else. Therefore, the return code shall be removed from those functions.
> > > > 
> > > > For the few users who rely on the check, a new, specialized function
> > > > shall be provided.
> > > > 
> > > > Add dma_fence_check_and_signal(), which signals a fence if it had not
> > > > yet been signaled, and informs the user about that.
> > > > 
> > > > Add a counter part, dma_fence_check_and_signal_locked(), which doesn't
> > > > take the spinlock.
> > > > 
> > > > Signed-off-by: Philipp Stanner <phasta@kernel.org>
> > > > ---
> > > >  drivers/dma-buf/dma-fence.c | 44 +++++++++++++++++++++++++++++++++++++
> > > >  include/linux/dma-fence.h   |  2 ++
> > > >  2 files changed, 46 insertions(+)
> > > > 
> > > > diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
> > > > index 96d72ffc0750..146de62887cf 100644
> > > > --- a/drivers/dma-buf/dma-fence.c
> > > > +++ b/drivers/dma-buf/dma-fence.c
> > > > @@ -445,6 +445,50 @@ int dma_fence_signal_locked(struct dma_fence *fence)
> > > >  }
> > > >  EXPORT_SYMBOL(dma_fence_signal_locked);
> > > >  
> > > > +/**
> > > > + * dma_fence_check_and_signal_locked - signal the fence if it's not yet signaled
> > > > + * @fence: the fence to check and signal
> > > > + *
> > > > + * Checks whether a fence was signaled and signals it if it was not yet signaled.
> > > > + *
> > > > + * Unlike dma_fence_check_and_signal(), this function must be called with
> > > > + * &struct dma_fence.lock being held.
> > > > + *
> > > > + * Return: true if fence has been signaled already, false otherwise.
> > > > + */
> > > > +bool dma_fence_check_and_signal_locked(struct dma_fence *fence)
> > > 
> > > I'm seriously considering to nuke all the unlocked variants of dma_fence functions and just make it mandatory for callers to grab the lock manually.
> > > 
> > 
> > You mean "nuke the *locked* variants.
> 
> Sorry, that wasn't specific enough.
> 
> What I meant was making the locked variants the default instead of the unlocked ones.

Well, no :D

What you want to do is:
- Delete / deprecate the *locked* variants
- Force all users to take the fence lock manually, then use the (now
all unlocked) dma fence functions.

ACK?

> 
> > 
> > Why, though? Aren't they enough for most users?
> > I suppose you have all those subtle races in mind..
> 
> Yeah, exactly that.
> 
> > 
> > > > +{
> > > > +	bool ret;
> > > > +
> > > > +	ret = dma_fence_test_signaled_flag(fence);
> > > > +	dma_fence_signal_locked(fence);
> > > > +
> > > > +	return ret;
> > > > +}
> > > > +EXPORT_SYMBOL(dma_fence_check_and_signal_locked);
> > > > +
> > > > +/**
> > > > + * dma_fence_check_and_signal - signal the fence if it's not yet signaled
> > > > + * @fence: the fence to check and signal
> > > > + *
> > > > + * Checks whether a fence was signaled and signals it if it was not yet signaled.
> > > > + * All this is done in a race-free manner.
> > > > + *
> > > > + * Return: true if fence has been signaled already, false otherwise.
> > > > + */
> > > > +bool dma_fence_check_and_signal(struct dma_fence *fence)
> > > 
> > > So I think we should name this one here dma_fence_check_and_signal_unlocked() and drop the postfix from the locked variant.
> > 
> > postfix?
> > 
> > Well, now, IDK. Can't we, for this series, keep the _locked() variant
> > so that it's congruent with all the other dma_fence code?
> 
> Good point. That thought was not really related to this series here.

OK, then let's progress with this here for now.


P.

> 
> > 
> > And then later if you want to force manual locking you can add that
> > kernel-wide in a separate series, since it'll be a discussion-worthy,
> > bigger chunk of work.
> > 
> > That's cleaner, and my series here won't prevent that once merged.
> > 
> > > 
> > > > +{
> > > > +	unsigned long flags;
> > > > +	bool ret;
> > > > +
> > > > +	spin_lock_irqsave(fence->lock, flags);
> > > > +	ret = dma_fence_check_and_signal_locked(fence);
> > > > +	spin_unlock_irqrestore(fence->lock, flags);
> > > 
> > > Could this use guard(fence->lock, flags) ?
> > 
> > guard? You mean a lockdep guard? Do you have a pointer to someplace in
> > dma_fence who does what you mean / want?
> 
> E.g. like guard(spinlock_irqsave)(&fence->lock);
> 
> Regards,
> Christian.
> 
> > 
> > 
> > P.
> > 
> > > 
> > > Regards,
> > > Christian.
> > > 
> > > > +
> > > > +	return ret;
> > > > +}
> > > > +EXPORT_SYMBOL(dma_fence_check_and_signal);
> > > > +
> > > >  /**
> > > >   * dma_fence_signal - signal completion of a fence
> > > >   * @fence: the fence to signal
> > > > diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
> > > > index 19972f5d176f..0504afe52c2a 100644
> > > > --- a/include/linux/dma-fence.h
> > > > +++ b/include/linux/dma-fence.h
> > > > @@ -365,6 +365,8 @@ static inline void __dma_fence_might_wait(void) {}
> > > >  #endif
> > > >  
> > > >  int 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,
> > > 
> > 
> 

On 12/1/25 16:34, Philipp Stanner wrote:
> On Mon, 2025-12-01 at 16:20 +0100, Christian König wrote:
>> On 12/1/25 14:55, Philipp Stanner wrote:
>>> On Mon, 2025-12-01 at 14:23 +0100, Christian König wrote:
>>>> On 12/1/25 11:50, Philipp Stanner wrote:
>>>>> The overwhelming majority of users of dma_fence signaling functions
>>>>> don't care about whether the fence had already been signaled by someone
>>>>> else. Therefore, the return code shall be removed from those functions.
>>>>>
>>>>> For the few users who rely on the check, a new, specialized function
>>>>> shall be provided.
>>>>>
>>>>> Add dma_fence_check_and_signal(), which signals a fence if it had not
>>>>> yet been signaled, and informs the user about that.
>>>>>
>>>>> Add a counter part, dma_fence_check_and_signal_locked(), which doesn't
>>>>> take the spinlock.
>>>>>
>>>>> Signed-off-by: Philipp Stanner <phasta@kernel.org>
>>>>> ---
>>>>>  drivers/dma-buf/dma-fence.c | 44 +++++++++++++++++++++++++++++++++++++
>>>>>  include/linux/dma-fence.h   |  2 ++
>>>>>  2 files changed, 46 insertions(+)
>>>>>
>>>>> diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
>>>>> index 96d72ffc0750..146de62887cf 100644
>>>>> --- a/drivers/dma-buf/dma-fence.c
>>>>> +++ b/drivers/dma-buf/dma-fence.c
>>>>> @@ -445,6 +445,50 @@ int dma_fence_signal_locked(struct dma_fence *fence)
>>>>>  }
>>>>>  EXPORT_SYMBOL(dma_fence_signal_locked);
>>>>>  
>>>>> +/**
>>>>> + * dma_fence_check_and_signal_locked - signal the fence if it's not yet signaled
>>>>> + * @fence: the fence to check and signal
>>>>> + *
>>>>> + * Checks whether a fence was signaled and signals it if it was not yet signaled.
>>>>> + *
>>>>> + * Unlike dma_fence_check_and_signal(), this function must be called with
>>>>> + * &struct dma_fence.lock being held.
>>>>> + *
>>>>> + * Return: true if fence has been signaled already, false otherwise.
>>>>> + */
>>>>> +bool dma_fence_check_and_signal_locked(struct dma_fence *fence)
>>>>
>>>> I'm seriously considering to nuke all the unlocked variants of dma_fence functions and just make it mandatory for callers to grab the lock manually.
>>>>
>>>
>>> You mean "nuke the *locked* variants.
>>
>> Sorry, that wasn't specific enough.
>>
>> What I meant was making the locked variants the default instead of the unlocked ones.
> 
> Well, no :D
> 
> What you want to do is:
> - Delete / deprecate the *locked* variants
> - Force all users to take the fence lock manually, then use the (now
> all unlocked) dma fence functions.
> 
> ACK?

I'm sick with cold/flu like symptoms at the moment, but that sounds mixed up to me (but maybe I should get a bit sleep first).

>>
>>>
>>> Why, though? Aren't they enough for most users?
>>> I suppose you have all those subtle races in mind..
>>
>> Yeah, exactly that.
>>
>>>
>>>>> +{
>>>>> +	bool ret;
>>>>> +
>>>>> +	ret = dma_fence_test_signaled_flag(fence);
>>>>> +	dma_fence_signal_locked(fence);
>>>>> +
>>>>> +	return ret;
>>>>> +}
>>>>> +EXPORT_SYMBOL(dma_fence_check_and_signal_locked);
>>>>> +
>>>>> +/**
>>>>> + * dma_fence_check_and_signal - signal the fence if it's not yet signaled
>>>>> + * @fence: the fence to check and signal
>>>>> + *
>>>>> + * Checks whether a fence was signaled and signals it if it was not yet signaled.
>>>>> + * All this is done in a race-free manner.
>>>>> + *
>>>>> + * Return: true if fence has been signaled already, false otherwise.
>>>>> + */
>>>>> +bool dma_fence_check_and_signal(struct dma_fence *fence)
>>>>
>>>> So I think we should name this one here dma_fence_check_and_signal_unlocked() and drop the postfix from the locked variant.
>>>
>>> postfix?
>>>
>>> Well, now, IDK. Can't we, for this series, keep the _locked() variant
>>> so that it's congruent with all the other dma_fence code?
>>
>> Good point. That thought was not really related to this series here.
> 
> OK, then let's progress with this here for now.

Works for me, give me a day to go over it again and review it.

Regards,
Christian.

> 
> 
> P.
> 
>>
>>>
>>> And then later if you want to force manual locking you can add that
>>> kernel-wide in a separate series, since it'll be a discussion-worthy,
>>> bigger chunk of work.
>>>
>>> That's cleaner, and my series here won't prevent that once merged.
>>>
>>>>
>>>>> +{
>>>>> +	unsigned long flags;
>>>>> +	bool ret;
>>>>> +
>>>>> +	spin_lock_irqsave(fence->lock, flags);
>>>>> +	ret = dma_fence_check_and_signal_locked(fence);
>>>>> +	spin_unlock_irqrestore(fence->lock, flags);
>>>>
>>>> Could this use guard(fence->lock, flags) ?
>>>
>>> guard? You mean a lockdep guard? Do you have a pointer to someplace in
>>> dma_fence who does what you mean / want?
>>
>> E.g. like guard(spinlock_irqsave)(&fence->lock);
>>
>> Regards,
>> Christian.
>>
>>>
>>>
>>> P.
>>>
>>>>
>>>> Regards,
>>>> Christian.
>>>>
>>>>> +
>>>>> +	return ret;
>>>>> +}
>>>>> +EXPORT_SYMBOL(dma_fence_check_and_signal);
>>>>> +
>>>>>  /**
>>>>>   * dma_fence_signal - signal completion of a fence
>>>>>   * @fence: the fence to signal
>>>>> diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
>>>>> index 19972f5d176f..0504afe52c2a 100644
>>>>> --- a/include/linux/dma-fence.h
>>>>> +++ b/include/linux/dma-fence.h
>>>>> @@ -365,6 +365,8 @@ static inline void __dma_fence_might_wait(void) {}
>>>>>  #endif
>>>>>  
>>>>>  int 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,
>>>>
>>>
>>
>