On Fri, Feb 16, 2024 at 05:51:59PM +0100, Christian König wrote:
> 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.
Hm right, I thought we'd dump the hashed pointe value into the fence
events too, then you could make the connection. But we don't, so this is a
bit annoying ...
And walking the entire scheduler dependency chain at trace_dma_fence_emit
time (or something similar) maybe?
-Sima
> > 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
> > >
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
