Re: [PATCH 1/6] drm/xe: Add additional trace points for LRCs

["Summers, Stuart" <stuart.summers@intel.com>]

On Tue, 2025-10-28 at 12:46 -0700, Matt Atwood wrote:
> On Mon, Oct 27, 2025 at 06:04:07PM +0000, Stuart Summers wrote:
> > Add trace points to indicate when an LRC has been
> > created and destroyed or get and put.
> > 
> > Signed-off-by: Stuart Summers <stuart.summers@intel.com>
> > ---
> >  drivers/gpu/drm/xe/xe_lrc.c       |  4 +++
> >  drivers/gpu/drm/xe/xe_lrc.h       |  3 +++
> >  drivers/gpu/drm/xe/xe_trace_lrc.h | 42
> > ++++++++++++++++++++++++++++++-
> >  3 files changed, 48 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/xe/xe_lrc.c
> > b/drivers/gpu/drm/xe/xe_lrc.c
> > index b5083c99dd50..42d1c861fe18 100644
> > --- a/drivers/gpu/drm/xe/xe_lrc.c
> > +++ b/drivers/gpu/drm/xe/xe_lrc.c
> > @@ -1575,6 +1575,8 @@ struct xe_lrc *xe_lrc_create(struct
> > xe_hw_engine *hwe, struct xe_vm *vm,
> >                 return ERR_PTR(err);
> >         }
> >  
> > +       trace_xe_lrc_create(lrc);
> > +
> >         return lrc;
> >  }
> >  
> > @@ -1589,6 +1591,8 @@ void xe_lrc_destroy(struct kref *ref)
> >  {
> >         struct xe_lrc *lrc = container_of(ref, struct xe_lrc,
> > refcount);
> >  
> > +       trace_xe_lrc_destroy(lrc);
> > +
> >         xe_lrc_finish(lrc);
> >         kfree(lrc);
> >  }
> > diff --git a/drivers/gpu/drm/xe/xe_lrc.h
> > b/drivers/gpu/drm/xe/xe_lrc.h
> > index 2fb628da5c43..fd67810f9812 100644
> > --- a/drivers/gpu/drm/xe/xe_lrc.h
> > +++ b/drivers/gpu/drm/xe/xe_lrc.h
> > @@ -8,6 +8,7 @@
> >  #include <linux/types.h>
> >  
> >  #include "xe_lrc_types.h"
> > +#include "xe_trace_lrc.h"
> >  
> >  struct drm_printer;
> >  struct xe_bb;
> > @@ -61,6 +62,7 @@ void xe_lrc_destroy(struct kref *ref);
> >  static inline struct xe_lrc *xe_lrc_get(struct xe_lrc *lrc)
> >  {
> >         kref_get(&lrc->refcount);
> > +       trace_xe_lrc_get(lrc);
> >         return lrc;
> >  }
> >  
> > @@ -73,6 +75,7 @@ static inline struct xe_lrc *xe_lrc_get(struct
> > xe_lrc *lrc)
> >   */
> >  static inline void xe_lrc_put(struct xe_lrc *lrc)
> >  {
> > +       trace_xe_lrc_put(lrc);
> >         kref_put(&lrc->refcount, xe_lrc_destroy);
> >  }
> >  
> > diff --git a/drivers/gpu/drm/xe/xe_trace_lrc.h
> > b/drivers/gpu/drm/xe/xe_trace_lrc.h
> > index d525cbee1e34..e8daa5d323e7 100644
> > --- a/drivers/gpu/drm/xe/xe_trace_lrc.h
> > +++ b/drivers/gpu/drm/xe/xe_trace_lrc.h
> > @@ -13,7 +13,6 @@
> >  #include <linux/types.h>
> >  
> >  #include "xe_gt_types.h"
> > -#include "xe_lrc.h"
> >  #include "xe_lrc_types.h"
> >  
> >  #define __dev_name_lrc(lrc)    dev_name(gt_to_xe((lrc)-
> > >fence_ctx.gt)->drm.dev)
> > @@ -42,6 +41,47 @@ TRACE_EVENT(xe_lrc_update_timestamp,
> >                       __get_str(device_id))
> >  );
> >  
> > +DECLARE_EVENT_CLASS(xe_lrc,
> > +           TP_PROTO(struct xe_lrc *lrc),
> > +           TP_ARGS(lrc),
> > +
> > +           TP_STRUCT__entry(
> This looks wrong style wise, but is done in every instance we do
> anything like this. Maybe Matt Brost can chime in on why the style is
> this way, and if a change is suggested it can be done in a follow up
> patch.

Yeah I had taken this from one of the existing tracefs entries. I agree
it doesn't look right. I'd like to pull that out separately though if
possible.

> 
> Reviewed-by: Matt Atwood <matthew.s.atwood@intel.com>

Thanks for the review!

-Stuart

> > +                    __field(struct xe_lrc *, lrc)
> > +                    __string(name, lrc->fence_ctx.name)
> > +                    __string(device_id, __dev_name_lrc(lrc))
> > +           ),
> > +
> > +           TP_fast_assign(
> > +                  __entry->lrc = lrc;
> > +                  __assign_str(name);
> > +                  __assign_str(device_id);
> > +                  ),
> > +
> > +           TP_printk("lrc=:%p lrc->name=%s device_id:%s",
> > +                     __entry->lrc, __get_str(name),
> > +                     __get_str(device_id))
> > +);
> > +
> > +DEFINE_EVENT(xe_lrc, xe_lrc_create,
> > +            TP_PROTO(struct xe_lrc *lrc),
> > +            TP_ARGS(lrc)
> > +);
> > +
> > +DEFINE_EVENT(xe_lrc, xe_lrc_destroy,
> > +            TP_PROTO(struct xe_lrc *lrc),
> > +            TP_ARGS(lrc)
> > +);
> > +
> > +DEFINE_EVENT(xe_lrc, xe_lrc_get,
> > +            TP_PROTO(struct xe_lrc *lrc),
> > +            TP_ARGS(lrc)
> > +);
> > +
> > +DEFINE_EVENT(xe_lrc, xe_lrc_put,
> > +            TP_PROTO(struct xe_lrc *lrc),
> > +            TP_ARGS(lrc)
> > +);
> > +
> >  #endif
> >  
> >  /* This part must be outside protection */
> > -- 
> > 2.34.1
> >