[PATCH] xe/xe_bo_move: Enhance xe_bo_move trace
Dandamudi, Priyanka
priyanka.dandamudi at intel.com
Fri Jan 12 04:48:22 UTC 2024
> -----Original Message-----
> From: Jani Nikula <jani.nikula at linux.intel.com>
> Sent: Monday, January 8, 2024 9:20 PM
> To: Thomas Hellström <thomas.hellstrom at linux.intel.com>; Dandamudi,
> Priyanka <priyanka.dandamudi at intel.com>; intel-xe at lists.freedesktop.org;
> Kempczynski, Zbigniew <zbigniew.kempczynski at intel.com>; Zeng, Oak
> <oak.zeng at intel.com>; Welty, Brian <brian.welty at intel.com>; Brost,
> Matthew <matthew.brost at intel.com>
> Subject: Re: [PATCH] xe/xe_bo_move: Enhance xe_bo_move trace
>
> On Mon, 08 Jan 2024, Thomas Hellström
> <thomas.hellstrom at linux.intel.com> wrote:
> > On Fri, 2024-01-05 at 10:54 +0530, priyanka.dandamudi at intel.com wrote:
> >> From: Priyanka Dandamudi <priyanka.dandamudi at 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)
> >>
> >> Cc: Thomas Hellström <thomas.hellstrom at linux.intel.com>
> >> Cc: Oak Zeng <oak.zeng at intel.com>
> >> Cc: Kempczynski Zbigniew <Zbigniew.Kempczynski at intel.com>
> >> Cc: Matthew Brost <matthew.brost at intel.com>
> >> Cc: Brian Welty <brian.welty at intel.com>
> >> Signed-off-by: Priyanka Dandamudi <priyanka.dandamudi at intel.com>
> >> ---
> >> drivers/gpu/drm/xe/xe_bo.c | 11 +++++++++--
> >> drivers/gpu/drm/xe/xe_bo.h | 1 +
> >> drivers/gpu/drm/xe/xe_drm_client.c | 8 --------
> >> drivers/gpu/drm/xe/xe_trace.h | 24 +++++++++++++++++++++---
> >> 4 files changed, 31 insertions(+), 13 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c
> >> index 8e4a3b1f6b93..1154d8f7b3b1 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 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,
> >> @@ -713,8 +721,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
> >> 97b32528c600..bf77bf17e519 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 struct ttm_device_funcs xe_ttm_funcs;
> >> +extern const char *const mem_type_to_name[];
> >
> > NIT: We should name this xe_mem_type_to_name[] now that it's extern.
> >
> > With that,
> > Reviewed-by: Thomas Hellström <thomas.hellstrom at linux.intel.com>
>
> FWIW, in i915 land I would've required converting that to a proper function,
> with bounds checks etc.
>
> Data is not an interface, and all that.
>
> BR,
> Jani.
I think we can make this as an enhancement on top of this.
Regards,
Priyanka
>
> >
> >
> >>
> >> 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..1ac14c910491 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; diff --git
> >> a/drivers/gpu/drm/xe/xe_trace.h b/drivers/gpu/drm/xe/xe_trace.h index
> >> 95163c303f3e..c66a91df7d8b 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"
> >> @@ -100,9 +101,26 @@ 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)
> >> + ),
> >> +
> >> + TP_fast_assign(
> >> + __entry->bo = bo;
> >> + __entry->size = bo->size;
> >> + __entry->new_placement = new_placement;
> >> + __entry->old_placement = old_placement;
> >> +
> >> + ),
> >> + TP_printk("migrate object %p [size %zu] from %s to %s",
> >> + __entry->bo, __entry->size,
> >> mem_type_to_name[__entry->old_placement],
> >> + mem_type_to_name[__entry->new_placement])
> >> );
> >>
> >> DECLARE_EVENT_CLASS(xe_exec_queue,
> >
>
> --
> Jani Nikula, Intel
More information about the Intel-xe
mailing list