Re: [PATCH v2 1/6] tracing, dma-buf: add a trace_dma_fence_sync_to event

[Christian König]

Am 13.02.24 um 16:50 schrieb Pierre-Eric Pelloux-Prayer:
This new event can be used to trace where a given dma_fence is added
as a dependency of some other work.
I plan to use it in amdgpu.

Signed-off-by: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-pra...@amd.com>
---
  drivers/dma-buf/dma-fence.c      |  1 +
  include/trace/events/dma_fence.h | 34 ++++++++++++++++++++++++++++++++
  2 files changed, 35 insertions(+)

diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
index e0fd99e61a2d..671a499a5ccd 100644
--- a/drivers/dma-buf/dma-fence.c
+++ b/drivers/dma-buf/dma-fence.c
@@ -23,6 +23,7 @@
  EXPORT_TRACEPOINT_SYMBOL(dma_fence_emit);
  EXPORT_TRACEPOINT_SYMBOL(dma_fence_enable_signal);
  EXPORT_TRACEPOINT_SYMBOL(dma_fence_signaled);
+EXPORT_TRACEPOINT_SYMBOL(dma_fence_sync_to);
  
  static DEFINE_SPINLOCK(dma_fence_stub_lock);
  static struct dma_fence dma_fence_stub;
diff --git a/include/trace/events/dma_fence.h b/include/trace/events/dma_fence.h
index 3963e79ca7b4..9b3875f7aa79 100644
--- a/include/trace/events/dma_fence.h
+++ b/include/trace/events/dma_fence.h
@@ -83,6 +83,40 @@ DEFINE_EVENT(dma_fence, dma_fence_wait_end,
        TP_ARGS(fence)
  );
  
+DECLARE_EVENT_CLASS(dma_fence_from,
+
+       TP_PROTO(struct dma_fence *fence, const char *reason),
+
+       TP_ARGS(fence, reason),
+
+       TP_STRUCT__entry(
+               __string(driver, fence->ops->get_driver_name(fence))
+               __string(timeline, fence->ops->get_timeline_name(fence))
+               __field(unsigned int, context)
+               __field(unsigned int, seqno)

Those are 64bit numbers, only recording the lower 32bits isn't enough.

+               __string(reason, reason)
+       ),
+
+       TP_fast_assign(
+               __assign_str(driver, fence->ops->get_driver_name(fence));
+               __assign_str(timeline, fence->ops->get_timeline_name(fence));
+               __entry->context = fence->context;
+               __entry->seqno = fence->seqno;
+               __assign_str(reason, reason);
+       ),
+
+       TP_printk("driver=%s timeline=%s context=%u seqno=%u reason=%s",
+                 __get_str(driver), __get_str(timeline), __entry->context,
+                 __entry->seqno, __get_str(reason))
+);
+
+DEFINE_EVENT(dma_fence_from, dma_fence_sync_to,

For a single event you should probably use TRACE_EVENT() instead of 
declaring a class. A class is only used if you have multiple events with 
the same parameters.

Then the name dma_fence_sync_to is not that descriptive. Maybe 
dma_fence_used_as_dependency() is better.

Then we should probably wire this up in the DRM scheduler as well. See 
functions drm_sched_job_add_dependency(), 
drm_sched_job_add_resv_dependencies() and 
drm_sched_job_add_syncobj_dependency().

Should be trivial to add the new trace point there as well.

Thanks,
Christian.

+
+       TP_PROTO(struct dma_fence *fence, const char *reason),
+
+       TP_ARGS(fence, reason)
+);
+
  #endif /*  _TRACE_DMA_FENCE_H */
  
  /* This part must be outside protection */