Re: [PATCH] xe/xe_bo_move: Enhance xe_bo_move trace

["Thomas Hellström" <thomas.hellstrom@linux.intel.com>]

Hi,

On Wed, 2024-01-03 at 15:59 +0530, priyanka.dandamudi@intel.com wrote:
> From: Priyanka Dandamudi <priyanka.dandamudi@intel.com>
> 
> Enhanced xe_bo_move trace to be more readable.
> It will help to show the migration details.
> Src and dst details.
> 
> Cc: Oak Zeng <oak.zeng@intel.com>
> Cc: Kempczynski Zbigniew <Zbigniew.Kempczynski@intel.com>
> Cc: Matthew Brost <matthew.brost@intel.com>
> Cc: Brian Welty <brian.welty@intel.com>
> Signed-off-by: Priyanka Dandamudi <priyanka.dandamudi@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_bo.c    | 12 ++++++++++--
>  drivers/gpu/drm/xe/xe_trace.h | 22 +++++++++++++++++++---
>  2 files changed, 29 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c
> index 8e4a3b1f6b93..3fa7ca99e85b 100644
> --- a/drivers/gpu/drm/xe/xe_bo.c
> +++ b/drivers/gpu/drm/xe/xe_bo.c
> @@ -613,6 +613,14 @@ static int xe_bo_move(struct ttm_buffer_object
> *ttm_bo, bool evict,
>  		      struct ttm_resource *new_mem,
>  		      struct ttm_place *hop)
>  {
> +	static const char *const
> mem_type_to_name[TTM_NUM_MEM_TYPES]  = {
> +		[XE_PL_SYSTEM] = "system",
> +		[XE_PL_TT] = "gtt",
> +		[XE_PL_VRAM0] = "vram0",
> +		[XE_PL_VRAM1] = "vram1",
> +		[4 ... 6] = NULL,

Can we avoid this line ^^^ and have these entries automatically
initialized to zero.

> +		[XE_PL_STOLEN] = "stolen"
> +	};

Also perhaps make this extern and use it from the tracing code? That
would mean trace_xe_bo_move() taking the integer mem_type rather than a
string? 

Thanks,
Thomas


>  	struct xe_device *xe = ttm_to_xe_device(ttm_bo->bdev);
>  	struct xe_bo *bo = ttm_to_xe_bo(ttm_bo);
>  	struct ttm_resource *old_mem = ttm_bo->resource;
> @@ -713,8 +721,8 @@ static int xe_bo_move(struct ttm_buffer_object
> *ttm_bo, bool evict,
>  		migrate = xe->tiles[0].migrate;
>  
>  	xe_assert(xe, migrate);
> -
> -	trace_xe_bo_move(bo);
> +	trace_xe_bo_move(bo, mem_type_to_name[new_mem->mem_type],
> +			 mem_type_to_name[old_mem_type]);
>  	xe_device_mem_access_get(xe);
>  
>  	if (xe_bo_is_pinned(bo) && !xe_bo_is_user(bo)) {
> diff --git a/drivers/gpu/drm/xe/xe_trace.h
> b/drivers/gpu/drm/xe/xe_trace.h
> index 95163c303f3e..bfa7b2391378 100644
> --- a/drivers/gpu/drm/xe/xe_trace.h
> +++ b/drivers/gpu/drm/xe/xe_trace.h
> @@ -100,9 +100,25 @@ DEFINE_EVENT(xe_bo, xe_bo_cpu_fault,
>  	     TP_ARGS(bo)
>  );
>  
> -DEFINE_EVENT(xe_bo, xe_bo_move,
> -	     TP_PROTO(struct xe_bo *bo),
> -	     TP_ARGS(bo)
> +TRACE_EVENT(xe_bo_move,
> +	    TP_PROTO(struct xe_bo *bo, const char *new_placement,
> const char *old_placement),
> +	    TP_ARGS(bo, new_placement, old_placement),
> +	    TP_STRUCT__entry(
> +		     __field(struct xe_bo *, bo)
> +		     __field(size_t, size)
> +		     __array(char, new_placement, 16)
> +		     __array(char, old_placement, 16)
> +			),
> +
> +	    TP_fast_assign(
> +		   __entry->bo      = bo;
> +		   __entry->size = bo->size;
> +		   strscpy(__entry->new_placement, new_placement,
> 16);
> +		   strscpy(__entry->old_placement, old_placement,
> 16)
> +		   ),
> +	    TP_printk("migrate object %p [size %zu] from %s to %s",
> +		      __entry->bo, __entry->size, __entry-
> >old_placement,
> +		      __entry->new_placement)
>  );
>  
>  DECLARE_EVENT_CLASS(xe_exec_queue,