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();
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),
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),
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),
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),
linaro-mm-sig@lists.linaro.org
-
Christian König -
Tvrtko Ursulin