[PATCH] drm/xe: Fix cast on trace variable again

Lucas De Marchi lucas.demarchi at intel.com
Wed Feb 21 20:20:32 UTC 2024


On Wed, Feb 21, 2024 at 07:58:40PM +0200, Ville Syrjälä wrote:
>On Tue, Feb 20, 2024 at 12:56:07PM -0800, Lucas De Marchi wrote:
>> Same fix as commit 8d038f49c1f3 ("drm/xe: Fix cast on trace variable")
>> that has been inadvertently reverted by a later commit.
>>
>> Fixes: a0df2cc858c3 ("drm/xe/xe_bo_move: Enhance xe_bo_move trace")
>> Cc: Matt Roper <matthew.d.roper at intel.com>
>> Cc: Priyanka Dandamudi <priyanka.dandamudi at intel.com>
>> Cc: Oak Zeng <oak.zeng at intel.com>
>> Cc: Thomas Hellström <thomas.hellstrom at linux.intel.com>
>> Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>
>> ---
>>  drivers/gpu/drm/xe/xe_trace.h | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_trace.h b/drivers/gpu/drm/xe/xe_trace.h
>> index 0cce98a6b14b..e43c3bde3202 100644
>> --- a/drivers/gpu/drm/xe/xe_trace.h
>> +++ b/drivers/gpu/drm/xe/xe_trace.h
>> @@ -32,7 +32,7 @@ DECLARE_EVENT_CLASS(xe_gt_tlb_invalidation_fence,
>>  			     ),
>>
>>  		    TP_fast_assign(
>> -			   __entry->fence = (u64)fence;
>> +			   __entry->fence = (unsigned long)fence;
>
>What's the point of these weird gymnastics?
>Ie. why isn't __entry->fence just a pointer?

Indeed... apparently for no good reason. The following patch should work
as well, while converting all the trace events to use pointers. I didn't
bother with a 0x prefix since it's already used as %p in other places
and I don't think we want to leak the kernel address too, so not using
%px.  Does that look good to you?

-------------8<-----------------
Subject: [PATCH] drm/xe: Use pointers in trace events

Commit a0df2cc858c3 ("drm/xe/xe_bo_move: Enhance xe_bo_move trace")
inadvertently reverted commit 8d038f49c1f3 ("drm/xe: Fix cast on trace
variable"), breaking the build on 32bits.

As noted by Ville, there's no point in converting the pointers to u64
and add casts everywhere. In fact, it's better to just use %p and let
the address be hashed. Convert all the cases in xe_trace.h to use
pointers.

Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
Cc: Matt Roper <matthew.d.roper at intel.com>, 
Cc: Priyanka Dandamudi <priyanka.dandamudi at intel.com>
Cc: Oak Zeng <oak.zeng at intel.com>, 
Cc: Thomas Hellström <thomas.hellstrom at linux.intel.com>
Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>

diff --git a/drivers/gpu/drm/xe/xe_trace.h b/drivers/gpu/drm/xe/xe_trace.h
index 0cce98a6b14b..3b97633d81d8 100644
--- a/drivers/gpu/drm/xe/xe_trace.h
+++ b/drivers/gpu/drm/xe/xe_trace.h
@@ -27,16 +27,16 @@ DECLARE_EVENT_CLASS(xe_gt_tlb_invalidation_fence,
  		    TP_ARGS(fence),
  
  		    TP_STRUCT__entry(
-			     __field(u64, fence)
+			     __field(struct xe_gt_tlb_invalidation_fence *, fence)
  			     __field(int, seqno)
  			     ),
  
  		    TP_fast_assign(
-			   __entry->fence = (u64)fence;
+			   __entry->fence = fence;
  			   __entry->seqno = fence->seqno;
  			   ),
  
-		    TP_printk("fence=0x%016llx, seqno=%d",
+		    TP_printk("fence=%p, seqno=%d",
  			      __entry->fence, __entry->seqno)
  );
  
@@ -83,16 +83,16 @@ DECLARE_EVENT_CLASS(xe_bo,
  		    TP_STRUCT__entry(
  			     __field(size_t, size)
  			     __field(u32, flags)
-			     __field(u64, vm)
+			     __field(struct xe_vm *, vm)
  			     ),
  
  		    TP_fast_assign(
  			   __entry->size = bo->size;
  			   __entry->flags = bo->flags;
-			   __entry->vm = (unsigned long)bo->vm;
+			   __entry->vm = bo->vm;
  			   ),
  
-		    TP_printk("size=%zu, flags=0x%02x, vm=0x%016llx",
+		    TP_printk("size=%zu, flags=0x%02x, vm=%p",
  			      __entry->size, __entry->flags, __entry->vm)
  );
  
@@ -346,16 +346,16 @@ DECLARE_EVENT_CLASS(xe_hw_fence,
  		    TP_STRUCT__entry(
  			     __field(u64, ctx)
  			     __field(u32, seqno)
-			     __field(u64, fence)
+			     __field(struct xe_hw_fence *, fence)
  			     ),
  
  		    TP_fast_assign(
  			   __entry->ctx = fence->dma.context;
  			   __entry->seqno = fence->dma.seqno;
-			   __entry->fence = (unsigned long)fence;
+			   __entry->fence = fence;
  			   ),
  
-		    TP_printk("ctx=0x%016llx, fence=0x%016llx, seqno=%u",
+		    TP_printk("ctx=0x%016llx, fence=%p, seqno=%u",
  			      __entry->ctx, __entry->fence, __entry->seqno)
  );
  
@@ -384,7 +384,7 @@ DECLARE_EVENT_CLASS(xe_vma,
  		    TP_ARGS(vma),
  
  		    TP_STRUCT__entry(
-			     __field(u64, vma)
+			     __field(struct xe_vma *, vma)
  			     __field(u32, asid)
  			     __field(u64, start)
  			     __field(u64, end)
@@ -392,14 +392,14 @@ DECLARE_EVENT_CLASS(xe_vma,
  			     ),
  
  		    TP_fast_assign(
-			   __entry->vma = (unsigned long)vma;
+			   __entry->vma = vma;
  			   __entry->asid = xe_vma_vm(vma)->usm.asid;
  			   __entry->start = xe_vma_start(vma);
  			   __entry->end = xe_vma_end(vma) - 1;
  			   __entry->ptr = xe_vma_userptr(vma);
  			   ),
  
-		    TP_printk("vma=0x%016llx, asid=0x%05x, start=0x%012llx, end=0x%012llx, ptr=0x%012llx,",
+		    TP_printk("vma=%p, asid=0x%05x, start=0x%012llx, end=0x%012llx, userptr=0x%012llx,",
  			      __entry->vma, __entry->asid, __entry->start,
  			      __entry->end, __entry->ptr)
  )
@@ -484,16 +484,16 @@ DECLARE_EVENT_CLASS(xe_vm,
  		    TP_ARGS(vm),
  
  		    TP_STRUCT__entry(
-			     __field(u64, vm)
+			     __field(struct xe_vm *, vm)
  			     __field(u32, asid)
  			     ),
  
  		    TP_fast_assign(
-			   __entry->vm = (unsigned long)vm;
+			   __entry->vm = vm;
  			   __entry->asid = vm->usm.asid;
  			   ),
  
-		    TP_printk("vm=0x%016llx, asid=0x%05x",  __entry->vm,
+		    TP_printk("vm=%p, asid=0x%05x",  __entry->vm,
  			      __entry->asid)
  );
  
-------------8<-----------------


thanks
Lucas De Marchi


More information about the Intel-xe mailing list