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

[Christian König <christian.koenig@xxxxxxx>]

Am 16.02.24 um 17:32 schrieb Daniel Vetter:
> On Tue, Feb 13, 2024 at 04:50:26PM +0100, Pierre-Eric Pelloux-Prayer wrote:
> > This new event can be used to trace where a given dma_fence is added
> > as a dependency of some other work.
> How?
>
> What I'd expected here is that you add a dependency chain from one fence
> to another, but this only has one fence.

That's what I though initially as well, but at the point we add the 
dependency fences to the scheduler job we don't have the scheduler fence 
initialized yet.

We could change this so that we only trace all the fences after we have 
initialized the scheduler fence, but then we loose the information where 
the dependency comes from.

> How do you figure out what's the
> next dma_fence that will stall on this dependency?

I'm not fully sure on that either. Pierre?

Christian.


>   Like in the gpu
> scheduler we do know what will be the fence that userspace gets back, so
> we can make that connection. And same for the atomic code (although you
> don't wire that up at all).
>
> I'm very confused on how this works and rather worried it's a brittle
> amdgpu-only solution ...
> -Sima
>
> > I plan to use it in amdgpu.
> >
> > Signed-off-by: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@xxxxxxx>
> > ---
> >   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)
> > +		__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,
> > +
> > +	TP_PROTO(struct dma_fence *fence, const char *reason),
> > +
> > +	TP_ARGS(fence, reason)
> > +);
> > +
> >   #endif /*  _TRACE_DMA_FENCE_H */
> >   
> >   /* This part must be outside protection */
> > --
> > 2.40.1