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

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

Hi

On Tue, 2024-02-20 at 10:17 +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.
> 
> v2: Modify trace_xe_bo_move(), it takes the integer mem_type
> rather than a string.
> Make mem_type_to_name() extern, it will be used by trace.(Thomas)
> 
> v3: Move mem_type_to_name() to xe_bo.[ch] (Thomas, Matt)
> 
> v4: Add device details to reduce ambiquity related to vram0/vram1.
> (Oak)
> 
> v5: Rename mem_type_to_name to xe_mem_type_to_name. (Thomas)
> 
> v6: Optimised code to use xe_bo_device(__entry->bo). (Thomas)
> 
> Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> 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>
> Reviewed-by: Oak Zeng <oak.zeng@intel.com>
> Reviewed-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>

I pushed this to drm-xe-next now, changing the title from
xe/xe_bo_move: ... to drm/xe/xe_bo_move:

Thanks,
Thomas



> ---
>  drivers/gpu/drm/xe/xe_bo.c         | 11 +++++++++--
>  drivers/gpu/drm/xe/xe_bo.h         |  1 +
>  drivers/gpu/drm/xe/xe_drm_client.c | 12 ++----------
>  drivers/gpu/drm/xe/xe_trace.h      | 27 +++++++++++++++++++++++----
>  4 files changed, 35 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c
> index 686d716c5581..d59b67d43c25 100644
> --- a/drivers/gpu/drm/xe/xe_bo.c
> +++ b/drivers/gpu/drm/xe/xe_bo.c
> @@ -28,6 +28,14 @@
>  #include "xe_ttm_stolen_mgr.h"
>  #include "xe_vm.h"
>  
> +const char *const xe_mem_type_to_name[TTM_NUM_MEM_TYPES]  = {
> +	[XE_PL_SYSTEM] = "system",
> +	[XE_PL_TT] = "gtt",
> +	[XE_PL_VRAM0] = "vram0",
> +	[XE_PL_VRAM1] = "vram1",
> +	[XE_PL_STOLEN] = "stolen"
> +};
> +
>  static const struct ttm_place sys_placement_flags = {
>  	.fpfn = 0,
>  	.lpfn = 0,
> @@ -727,8 +735,7 @@ 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, new_mem->mem_type, 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_bo.h b/drivers/gpu/drm/xe/xe_bo.h
> index db4b2db6b073..7e49e670d91e 100644
> --- a/drivers/gpu/drm/xe/xe_bo.h
> +++ b/drivers/gpu/drm/xe/xe_bo.h
> @@ -244,6 +244,7 @@ int xe_bo_evict_pinned(struct xe_bo *bo);
>  int xe_bo_restore_pinned(struct xe_bo *bo);
>  
>  extern const struct ttm_device_funcs xe_ttm_funcs;
> +extern const char *const xe_mem_type_to_name[];
>  
>  int xe_gem_create_ioctl(struct drm_device *dev, void *data,
>  			struct drm_file *file);
> diff --git a/drivers/gpu/drm/xe/xe_drm_client.c
> b/drivers/gpu/drm/xe/xe_drm_client.c
> index 82d1305e831f..6040e4d22b28 100644
> --- a/drivers/gpu/drm/xe/xe_drm_client.c
> +++ b/drivers/gpu/drm/xe/xe_drm_client.c
> @@ -131,14 +131,6 @@ static void bo_meminfo(struct xe_bo *bo,
>  
>  static void show_meminfo(struct drm_printer *p, struct drm_file
> *file)
>  {
> -	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,
> -		[XE_PL_STOLEN] = "stolen"
> -	};
>  	struct drm_memory_stats stats[TTM_NUM_MEM_TYPES] = {};
>  	struct xe_file *xef = file->driver_priv;
>  	struct ttm_device *bdev = &xef->xe->ttm;
> @@ -171,7 +163,7 @@ static void show_meminfo(struct drm_printer *p,
> struct drm_file *file)
>  	spin_unlock(&client->bos_lock);
>  
>  	for (mem_type = XE_PL_SYSTEM; mem_type < TTM_NUM_MEM_TYPES;
> ++mem_type) {
> -		if (!mem_type_to_name[mem_type])
> +		if (!xe_mem_type_to_name[mem_type])
>  			continue;
>  
>  		man = ttm_manager_type(bdev, mem_type);
> @@ -182,7 +174,7 @@ static void show_meminfo(struct drm_printer *p,
> struct drm_file *file)
>  					      
> DRM_GEM_OBJECT_RESIDENT |
>  					       (mem_type !=
> XE_PL_SYSTEM ? 0 :
>  					      
> DRM_GEM_OBJECT_PURGEABLE),
> -					      
> mem_type_to_name[mem_type]);
> +					      
> xe_mem_type_to_name[mem_type]);
>  		}
>  	}
>  }
> diff --git a/drivers/gpu/drm/xe/xe_trace.h
> b/drivers/gpu/drm/xe/xe_trace.h
> index e4e7262191ad..0cce98a6b14b 100644
> --- a/drivers/gpu/drm/xe/xe_trace.h
> +++ b/drivers/gpu/drm/xe/xe_trace.h
> @@ -12,6 +12,7 @@
>  #include <linux/tracepoint.h>
>  #include <linux/types.h>
>  
> +#include "xe_bo.h"
>  #include "xe_bo_types.h"
>  #include "xe_exec_queue_types.h"
>  #include "xe_gpu_scheduler_types.h"
> @@ -31,7 +32,7 @@ DECLARE_EVENT_CLASS(xe_gt_tlb_invalidation_fence,
>  			     ),
>  
>  		    TP_fast_assign(
> -			   __entry->fence = (unsigned long)fence;
> +			   __entry->fence = (u64)fence;
>  			   __entry->seqno = fence->seqno;
>  			   ),
>  
> @@ -100,9 +101,27 @@ 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, uint32_t new_placement,
> uint32_t old_placement),
> +	    TP_ARGS(bo, new_placement, old_placement),
> +	    TP_STRUCT__entry(
> +		     __field(struct xe_bo *, bo)
> +		     __field(size_t, size)
> +		     __field(u32, new_placement)
> +		     __field(u32, old_placement)
> +		     __array(char, device_id, 12)
> +			),
> +
> +	    TP_fast_assign(
> +		   __entry->bo      = bo;
> +		   __entry->size = bo->size;
> +		   __entry->new_placement = new_placement;
> +		   __entry->old_placement = old_placement;
> +		   strscpy(__entry->device_id,
> dev_name(xe_bo_device(__entry->bo)->drm.dev), 12);
> +		   ),
> +	    TP_printk("migrate object %p [size %zu] from %s to %s
> device_id:%s",
> +		      __entry->bo, __entry->size,
> xe_mem_type_to_name[__entry->old_placement],
> +		      xe_mem_type_to_name[__entry->new_placement],
> __entry->device_id)
>  );
>  
>  DECLARE_EVENT_CLASS(xe_exec_queue,