[PATCH v3 3/6] drm/xe/trace: Print device_id in xe_trace_bo events

[Gustavo Sousa]

Quoting Radhakrishna Sripada (2024-05-30 12:13:10-03:00)
>In multi-gpu environments it is important to know the device
>bo/vm belongs to. The tracing information includes the device_id
>to indicate the device the event is associated with.
>
>Suggested-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
>Cc: Lucas De Marchi <lucas.demarchi at intel.com>
>Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada at intel.com>
>---
> drivers/gpu/drm/xe/xe_trace_bo.h | 20 ++++++++++++++------
> 1 file changed, 14 insertions(+), 6 deletions(-)
>
>diff --git a/drivers/gpu/drm/xe/xe_trace_bo.h b/drivers/gpu/drm/xe/xe_trace_bo.h
>index 6d90f3f42d54..88204d4988fd 100644
>--- a/drivers/gpu/drm/xe/xe_trace_bo.h
>+++ b/drivers/gpu/drm/xe/xe_trace_bo.h
>@@ -24,16 +24,20 @@ DECLARE_EVENT_CLASS(xe_bo,
>                              __field(size_t, size)
>                              __field(u32, flags)
>                              __field(struct xe_vm *, vm)
>+                             __array(char, device_id, 12)

Two questions:

- Are we certain that 12 is always a suitable size? I see that i915
  display (in intel_display_trace.h) uses the variable sized variant for
  displaying dev name (e.g. the event intel_pipe_enable).

- Are we required to have additions of fields at the end? Putting that
  at the beginning feels more natural to me.

>                              ),
> 
>                     TP_fast_assign(
>                            __entry->size = bo->size;
>                            __entry->flags = bo->flags;
>                            __entry->vm = bo->vm;
>+                           strscpy(__entry->device_id,
>+                                   dev_name(xe_bo_device(bo)->drm.dev), 12);
>                            ),
> 
>-                    TP_printk("size=%zu, flags=0x%02x, vm=%p",
>-                              __entry->size, __entry->flags, __entry->vm)
>+                    TP_printk("dev: %s: size=%zu, flags=0x%02x, vm=%p",
>+                              __entry->device_id, __entry->size,
>+                              __entry->flags, __entry->vm)
> );
> 
> DEFINE_EVENT(xe_bo, xe_bo_cpu_fault,
>@@ -78,6 +82,7 @@ DECLARE_EVENT_CLASS(xe_vma,
>                              __field(u64, start)
>                              __field(u64, end)
>                              __field(u64, ptr)
>+                             __array(char, device_id, 12)
>                              ),
> 
>                     TP_fast_assign(
>@@ -86,10 +91,11 @@ DECLARE_EVENT_CLASS(xe_vma,
>                            __entry->start = xe_vma_start(vma);
>                            __entry->end = xe_vma_end(vma) - 1;
>                            __entry->ptr = xe_vma_userptr(vma);
>+                           strscpy(__entry->device_id, dev_name(xe_vma_vm(vma)->xe->drm.dev), 12);
>                            ),
> 
>-                    TP_printk("vma=%p, asid=0x%05x, start=0x%012llx, end=0x%012llx, userptr=0x%012llx,",
>-                              __entry->vma, __entry->asid, __entry->start,
>+                    TP_printk("dev: %s: vma=%p, asid=0x%05x, start=0x%012llx, end=0x%012llx, userptr=0x%012llx,",

I would go with "dev=%s, " to conform with the rest of the line. If you
prefer to keep the above, I would remove the ":" after "dev".

--
Gustavo Sousa

>+                              __entry->device_id, __entry->vma, __entry->asid, __entry->start,
>                               __entry->end, __entry->ptr)
> )
> 
>@@ -175,15 +181,17 @@ DECLARE_EVENT_CLASS(xe_vm,
>                     TP_STRUCT__entry(
>                              __field(struct xe_vm *, vm)
>                              __field(u32, asid)
>+                             __array(char, device_id, 12)
>                              ),
> 
>                     TP_fast_assign(
>                            __entry->vm = vm;
>                            __entry->asid = vm->usm.asid;
>+                           strscpy(__entry->device_id, dev_name(vm->xe->drm.dev), 12);
>                            ),
> 
>-                    TP_printk("vm=%p, asid=0x%05x",  __entry->vm,
>-                              __entry->asid)
>+                    TP_printk("dev: %s: vm=%p, asid=0x%05x", __entry->device_id,
>+                              __entry->vm,  __entry->asid)
> );
> 
> DEFINE_EVENT(xe_vm, xe_vm_kill,
>-- 
>2.34.1
>