[PATCH 1/3] dma-fence: Silce sparse warning in dma_fence_describe

[PATCH 1/3] dma-fence: Silce sparse warning in dma_fence_describe

[Tvrtko Ursulin]

Sparse complains about assigning a string to a __rcu annotated local
variable:

drivers/dma-buf/dma-fence.c:1040:38: warning: incorrect type in initializer (different address spaces)
drivers/dma-buf/dma-fence.c:1040:38:    expected char const [noderef] __rcu *timeline
drivers/dma-buf/dma-fence.c:1040:38:    got char *
drivers/dma-buf/dma-fence.c:1041:36: warning: incorrect type in initializer (different address spaces)
drivers/dma-buf/dma-fence.c:1041:36:    expected char const [noderef] __rcu *driver
drivers/dma-buf/dma-fence.c:1041:36:    got char *

It is harmless but lets silence it.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Fixes: ac364014fd81 ("dma-buf: cleanup dma_fence_describe v3")
Cc: Christian König <christian.koenig@amd.com>
Cc: linux-media@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Cc: linaro-mm-sig@lists.linaro.org
---
 drivers/dma-buf/dma-fence.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
index 1826ba73094c..a2aa82f4eedd 100644
--- a/drivers/dma-buf/dma-fence.c
+++ b/drivers/dma-buf/dma-fence.c
@@ -1037,8 +1037,8 @@ EXPORT_SYMBOL(dma_fence_set_deadline);
  */
 void dma_fence_describe(struct dma_fence *fence, struct seq_file *seq)
 {
-	const char __rcu *timeline = "";
-	const char __rcu *driver = "";
+	const char __rcu *timeline = (const char __rcu *)"";
+	const char __rcu *driver = (const char __rcu *)"";
 	const char *signaled = "";
 
 	rcu_read_lock();
-- 
2.52.0

[PATCH 2/3] dma-fence: Fix potential tracepoint null pointer dereferences

[Tvrtko Ursulin]

Trace_dma_fence_signaled, trace_dma_fence_wait_end and
trace_dma_fence_destroy can all dereference a null fence->ops pointer
after it has been reset on fence signalling.

Lets use the safe string getters for most tracepoints to a void this.

But for the signalling tracepoint, we move it to before the fence->ops
is reset and special case its definition in order to avoid losing the
driver and timeline information.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Fixes: 541c8f2468b9 ("dma-buf: detach fence ops on signal v3")
Cc: Christian König <christian.koenig@amd.com>
Cc: Philipp Stanner <phasta@kernel.org>
Cc: Boris Brezillon <boris.brezillon@collabora.com>
Cc: linux-media@vger.kernel.org
Cc: linaro-mm-sig@lists.linaro.org
---
 drivers/dma-buf/dma-fence.c      |  3 ++-
 include/trace/events/dma_fence.h | 29 +++++++++++++++++++++++++++--
 2 files changed, 29 insertions(+), 3 deletions(-)

diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
index a2aa82f4eedd..b3bfa6943a8e 100644
--- a/drivers/dma-buf/dma-fence.c
+++ b/drivers/dma-buf/dma-fence.c
@@ -363,6 +363,8 @@ void dma_fence_signal_timestamp_locked(struct dma_fence *fence,
 				      &fence->flags)))
 		return;
 
+	trace_dma_fence_signaled(fence);
+
 	/*
 	 * When neither a release nor a wait operation is specified set the ops
 	 * pointer to NULL to allow the fence structure to become independent
@@ -377,7 +379,6 @@ void dma_fence_signal_timestamp_locked(struct dma_fence *fence,
 
 	fence->timestamp = timestamp;
 	set_bit(DMA_FENCE_FLAG_TIMESTAMP_BIT, &fence->flags);
-	trace_dma_fence_signaled(fence);
 
 	list_for_each_entry_safe(cur, tmp, &cb_list, node) {
 		INIT_LIST_HEAD(&cur->node);
diff --git a/include/trace/events/dma_fence.h b/include/trace/events/dma_fence.h
index 3abba45c0601..220bf71446e8 100644
--- a/include/trace/events/dma_fence.h
+++ b/include/trace/events/dma_fence.h
@@ -9,12 +9,37 @@
 
 struct dma_fence;
 
+DECLARE_EVENT_CLASS(dma_fence,
+
+	TP_PROTO(struct dma_fence *fence),
+
+	TP_ARGS(fence),
+
+	TP_STRUCT__entry(
+		__string(driver, dma_fence_driver_name(fence))
+		__string(timeline, dma_fence_timeline_name(fence))
+		__field(unsigned int, context)
+		__field(unsigned int, seqno)
+	),
+
+	TP_fast_assign(
+		__assign_str(driver);
+		__assign_str(timeline);
+		__entry->context = fence->context;
+		__entry->seqno = fence->seqno;
+	),
+
+	TP_printk("driver=%s timeline=%s context=%u seqno=%u",
+		  __get_str(driver), __get_str(timeline), __entry->context,
+		  __entry->seqno)
+);
+
 /*
  * Safe only for call sites which are guaranteed to not race with fence
  * signaling,holding the fence->lock and having checked for not signaled, or the
  * signaling path itself.
  */
-DECLARE_EVENT_CLASS(dma_fence,
+DECLARE_EVENT_CLASS(dma_fence_ops,
 
 	TP_PROTO(struct dma_fence *fence),
 
@@ -67,7 +92,7 @@ DEFINE_EVENT(dma_fence, dma_fence_enable_signal,
 	TP_ARGS(fence)
 );
 
-DEFINE_EVENT(dma_fence, dma_fence_signaled,
+DEFINE_EVENT(dma_fence_ops, dma_fence_signaled,
 
 	TP_PROTO(struct dma_fence *fence),
 
-- 
2.52.0

Re: [PATCH 2/3] dma-fence: Fix potential tracepoint null pointer dereferences

[Christian König]

On 4/13/26 12:05, Tvrtko Ursulin wrote:
> Trace_dma_fence_signaled, trace_dma_fence_wait_end and
> trace_dma_fence_destroy can all dereference a null fence->ops pointer
> after it has been reset on fence signalling.
> 
> Lets use the safe string getters for most tracepoints to a void this.
> 
> But for the signalling tracepoint, we move it to before the fence->ops
> is reset and special case its definition in order to avoid losing the
> driver and timeline information.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
> Fixes: 541c8f2468b9 ("dma-buf: detach fence ops on signal v3")
> Cc: Christian König <christian.koenig@amd.com>
> Cc: Philipp Stanner <phasta@kernel.org>
> Cc: Boris Brezillon <boris.brezillon@collabora.com>
> Cc: linux-media@vger.kernel.org
> Cc: linaro-mm-sig@lists.linaro.org
> ---
>  drivers/dma-buf/dma-fence.c      |  3 ++-
>  include/trace/events/dma_fence.h | 29 +++++++++++++++++++++++++++--
>  2 files changed, 29 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
> index a2aa82f4eedd..b3bfa6943a8e 100644
> --- a/drivers/dma-buf/dma-fence.c
> +++ b/drivers/dma-buf/dma-fence.c
> @@ -363,6 +363,8 @@ void dma_fence_signal_timestamp_locked(struct dma_fence *fence,
>  				      &fence->flags)))
>  		return;
>  
> +	trace_dma_fence_signaled(fence);
> +
>  	/*
>  	 * When neither a release nor a wait operation is specified set the ops
>  	 * pointer to NULL to allow the fence structure to become independent
> @@ -377,7 +379,6 @@ void dma_fence_signal_timestamp_locked(struct dma_fence *fence,
>  
>  	fence->timestamp = timestamp;
>  	set_bit(DMA_FENCE_FLAG_TIMESTAMP_BIT, &fence->flags);
> -	trace_dma_fence_signaled(fence);
>  
>  	list_for_each_entry_safe(cur, tmp, &cb_list, node) {
>  		INIT_LIST_HEAD(&cur->node);
> diff --git a/include/trace/events/dma_fence.h b/include/trace/events/dma_fence.h
> index 3abba45c0601..220bf71446e8 100644
> --- a/include/trace/events/dma_fence.h
> +++ b/include/trace/events/dma_fence.h
> @@ -9,12 +9,37 @@
>  
>  struct dma_fence;
>  
> +DECLARE_EVENT_CLASS(dma_fence,
> +
> +	TP_PROTO(struct dma_fence *fence),
> +
> +	TP_ARGS(fence),
> +
> +	TP_STRUCT__entry(
> +		__string(driver, dma_fence_driver_name(fence))
> +		__string(timeline, dma_fence_timeline_name(fence))


That requires that we hold the RCU read side lock while doing the trace.

Not sure if that can be done inside the DECLARE_EVENT_CLASS() macro.

> +		__field(unsigned int, context)
> +		__field(unsigned int, seqno)
> +	),
> +
> +	TP_fast_assign(
> +		__assign_str(driver);
> +		__assign_str(timeline);
> +		__entry->context = fence->context;
> +		__entry->seqno = fence->seqno;
> +	),
> +
> +	TP_printk("driver=%s timeline=%s context=%u seqno=%u",
> +		  __get_str(driver), __get_str(timeline), __entry->context,
> +		  __entry->seqno)
> +);
> +
>  /*
>   * Safe only for call sites which are guaranteed to not race with fence
>   * signaling,holding the fence->lock and having checked for not signaled, or the
>   * signaling path itself.
>   */
> -DECLARE_EVENT_CLASS(dma_fence,
> +DECLARE_EVENT_CLASS(dma_fence_ops,
>  
>  	TP_PROTO(struct dma_fence *fence),
>  
> @@ -67,7 +92,7 @@ DEFINE_EVENT(dma_fence, dma_fence_enable_signal,
>  	TP_ARGS(fence)
>  );
>  
> -DEFINE_EVENT(dma_fence, dma_fence_signaled,
> +DEFINE_EVENT(dma_fence_ops, dma_fence_signaled,

The signal trace event is actually unproblematic. The question is more what to do with the release event.

Regards,
Christian.

>  
>  	TP_PROTO(struct dma_fence *fence),
>  

Re: [PATCH 2/3] dma-fence: Fix potential tracepoint null pointer dereferences

[Tvrtko Ursulin]

On 13/04/2026 12:32, Christian König wrote:
> On 4/13/26 12:05, Tvrtko Ursulin wrote:
>> Trace_dma_fence_signaled, trace_dma_fence_wait_end and
>> trace_dma_fence_destroy can all dereference a null fence->ops pointer
>> after it has been reset on fence signalling.
>>
>> Lets use the safe string getters for most tracepoints to a void this.
>>
>> But for the signalling tracepoint, we move it to before the fence->ops
>> is reset and special case its definition in order to avoid losing the
>> driver and timeline information.
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
>> Fixes: 541c8f2468b9 ("dma-buf: detach fence ops on signal v3")
>> Cc: Christian König <christian.koenig@amd.com>
>> Cc: Philipp Stanner <phasta@kernel.org>
>> Cc: Boris Brezillon <boris.brezillon@collabora.com>
>> Cc: linux-media@vger.kernel.org
>> Cc: linaro-mm-sig@lists.linaro.org
>> ---
>>   drivers/dma-buf/dma-fence.c      |  3 ++-
>>   include/trace/events/dma_fence.h | 29 +++++++++++++++++++++++++++--
>>   2 files changed, 29 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
>> index a2aa82f4eedd..b3bfa6943a8e 100644
>> --- a/drivers/dma-buf/dma-fence.c
>> +++ b/drivers/dma-buf/dma-fence.c
>> @@ -363,6 +363,8 @@ void dma_fence_signal_timestamp_locked(struct dma_fence *fence,
>>   				      &fence->flags)))
>>   		return;
>>   
>> +	trace_dma_fence_signaled(fence);
>> +
>>   	/*
>>   	 * When neither a release nor a wait operation is specified set the ops
>>   	 * pointer to NULL to allow the fence structure to become independent
>> @@ -377,7 +379,6 @@ void dma_fence_signal_timestamp_locked(struct dma_fence *fence,
>>   
>>   	fence->timestamp = timestamp;
>>   	set_bit(DMA_FENCE_FLAG_TIMESTAMP_BIT, &fence->flags);
>> -	trace_dma_fence_signaled(fence);
>>   
>>   	list_for_each_entry_safe(cur, tmp, &cb_list, node) {
>>   		INIT_LIST_HEAD(&cur->node);
>> diff --git a/include/trace/events/dma_fence.h b/include/trace/events/dma_fence.h
>> index 3abba45c0601..220bf71446e8 100644
>> --- a/include/trace/events/dma_fence.h
>> +++ b/include/trace/events/dma_fence.h
>> @@ -9,12 +9,37 @@
>>   
>>   struct dma_fence;
>>   
>> +DECLARE_EVENT_CLASS(dma_fence,
>> +
>> +	TP_PROTO(struct dma_fence *fence),
>> +
>> +	TP_ARGS(fence),
>> +
>> +	TP_STRUCT__entry(
>> +		__string(driver, dma_fence_driver_name(fence))
>> +		__string(timeline, dma_fence_timeline_name(fence))
> 
> 
> That requires that we hold the RCU read side lock while doing the trace.

True, and actually 3/3 needs to be squashed in 2/3 and then it is all 
fine. Then all tracepoints which use dma_fence_*_name() helpers are 
inside rcu_read_lock, and ones which do not use the dma_fence_ops class 
where they are guaranteed to be be safe.

> Not sure if that can be done inside the DECLARE_EVENT_CLASS() macro.

No idea.

>> +		__field(unsigned int, context)
>> +		__field(unsigned int, seqno)
>> +	),
>> +
>> +	TP_fast_assign(
>> +		__assign_str(driver);
>> +		__assign_str(timeline);
>> +		__entry->context = fence->context;
>> +		__entry->seqno = fence->seqno;
>> +	),
>> +
>> +	TP_printk("driver=%s timeline=%s context=%u seqno=%u",
>> +		  __get_str(driver), __get_str(timeline), __entry->context,
>> +		  __entry->seqno)
>> +);
>> +
>>   /*
>>    * Safe only for call sites which are guaranteed to not race with fence
>>    * signaling,holding the fence->lock and having checked for not signaled, or the
>>    * signaling path itself.
>>    */
>> -DECLARE_EVENT_CLASS(dma_fence,
>> +DECLARE_EVENT_CLASS(dma_fence_ops,
>>   
>>   	TP_PROTO(struct dma_fence *fence),
>>   
>> @@ -67,7 +92,7 @@ DEFINE_EVENT(dma_fence, dma_fence_enable_signal,
>>   	TP_ARGS(fence)
>>   );
>>   
>> -DEFINE_EVENT(dma_fence, dma_fence_signaled,
>> +DEFINE_EVENT(dma_fence_ops, dma_fence_signaled,
> 
> The signal trace event is actually unproblematic. The question is more what to do with the release event.

Yes, unproblematic after fixed in this patch, or you mean something else?

trace_dma_fence_destroy is fine after this patch AFAICS and I did test it.

Regards,

Tvrtko

>>   	TP_PROTO(struct dma_fence *fence),
>>   
> 

[PATCH 3/3] dma-fence: Init and enable signalling can use the fast path tracepoint

[Tvrtko Ursulin]

From dma_fence_init (cannot signal yet) and dma_fence_enable_signaling
(fence->lock and rcu_read_lock held, ops checked manually), it is safe to
use the marginally faster tracepoint class.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Cc: Christian König <christian.koenig@amd.com>
Cc: Philipp Stanner <phasta@kernel.org>
Cc: Boris Brezillon <boris.brezillon@collabora.com>
Cc: linux-media@vger.kernel.org
Cc: linaro-mm-sig@lists.linaro.org
---
 include/trace/events/dma_fence.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/trace/events/dma_fence.h b/include/trace/events/dma_fence.h
index 220bf71446e8..9e0cb9ce2388 100644
--- a/include/trace/events/dma_fence.h
+++ b/include/trace/events/dma_fence.h
@@ -71,7 +71,7 @@ DEFINE_EVENT(dma_fence, dma_fence_emit,
 	TP_ARGS(fence)
 );
 
-DEFINE_EVENT(dma_fence, dma_fence_init,
+DEFINE_EVENT(dma_fence_ops, dma_fence_init,
 
 	TP_PROTO(struct dma_fence *fence),
 
@@ -85,7 +85,7 @@ DEFINE_EVENT(dma_fence, dma_fence_destroy,
 	TP_ARGS(fence)
 );
 
-DEFINE_EVENT(dma_fence, dma_fence_enable_signal,
+DEFINE_EVENT(dma_fence_ops, dma_fence_enable_signal,
 
 	TP_PROTO(struct dma_fence *fence),
 
-- 
2.52.0