Re: [PATCH v4] drm/amdgpu: add new trace event for page table update v3

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Am 19.08.20 um 14:19 schrieb Shashank Sharma:
On 19/08/20 5:38 pm, Christian König wrote:
Am 19.08.20 um 13:52 schrieb Shashank Sharma:
On 13/08/20 1:28 pm, Christian König wrote:
Am 13.08.20 um 05:04 schrieb Shashank Sharma:
This patch adds a new trace event to track the PTE update
events. This specific event will provide information like:
- start and end of virtual memory mapping
- HW engine flags for the map
- physical address for mapping

This will be particularly useful for memory profiling tools
(like RMV) which are monitoring the page table update events.

V2: Added physical address lookup logic in trace point
V3: switch to use __dynamic_array
       added nptes int the TPprint arguments list
       added page size in the arg list
V4: Addressed Christian's review comments
       add start/end instead of seg
       use incr instead of page_sz to be accurate

Cc: Christian König <christian.koenig@xxxxxxx>
Cc: Alex Deucher <alexander.deucher@xxxxxxx>
Signed-off-by: Christian König <christian.koenig@xxxxxxx>
Signed-off-by: Shashank Sharma <shashank.sharma@xxxxxxx>
---
    drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h | 37 +++++++++++++++++++++++
    drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c    |  9 ++++--
    2 files changed, 44 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
index 63e734a125fb..df12cf8466c2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
@@ -321,6 +321,43 @@ DEFINE_EVENT(amdgpu_vm_mapping, amdgpu_vm_bo_cs,
    	    TP_ARGS(mapping)
    );
+TRACE_EVENT(amdgpu_vm_update_ptes,
+	    TP_PROTO(struct amdgpu_vm_update_params *p,
+		     uint64_t start, uint64_t end,
+		     unsigned int nptes, uint64_t dst,
+		     uint64_t incr, uint64_t flags),
+	TP_ARGS(p, start, end, nptes, dst, incr, flags),
+	TP_STRUCT__entry(
+			 __field(u64, start)
+			 __field(u64, end)
+			 __field(u64, flags)
+			 __field(unsigned int, nptes)
+			 __field(u64, incr)
+			 __dynamic_array(u64, dst, nptes)
As discussed with the trace subsystem maintainer we need to add the pid
and probably the VM context ID we use here to identify the updated VM.

Christian.
I printed both vm->task_info.pid Vs current->pid for testing, and I can see different values coming out of .

gnome-shell-2114  [011]    41.812894: amdgpu_vm_update_ptes: start:0x0800102e80 end:0x0800102e82, flags:0x80, incr:4096, pid=2128 vmid=0 cpid=2114

pid is vm->task_info.pid=2128 whereas cpid=2114 is current.pid.

Which is the one we want to send with the event ?
That is vm->task_info.pid, since this is the PID which is using the VM
for command submission.
got it.
Trace event by default seems to be adding the process name and id at the header of the event (gnome-shell-2114), which is same as current.pid


Also, is it ok to extract vmid from job->vmid ?
Only in trace_amdgpu_vm_grab_id(), in all other cases it's probably not
assigned yet.
Ok, let me check how can we get vmid from this context we are sending the event from. Or maybe I we should  keep V5 with pid only, and later send a separate patch to add vmid ?

I'm not sure how you want to get a VMID in the first place. We dynamically assign VMIDs during command submission.

See the amdgpu_vm_grab_id trace point.

Christian.


- Shashank

Christian.

Regards

Shashank

+	),
+
+	TP_fast_assign(
+			unsigned int i;
+
+			__entry->start = start;
+			__entry->end = end;
+			__entry->flags = flags;
+			__entry->incr = incr;
+			__entry->nptes = nptes;
+			for (i = 0; i < nptes; ++i) {
+				u64 addr = p->pages_addr ? amdgpu_vm_map_gart(
+					p->pages_addr, dst) : dst;
+
+				((u64 *)__get_dynamic_array(dst))[i] = addr;
+				dst += incr;
+			}
+	),
+	TP_printk("start:0x%010llx end:0x%010llx, flags:0x%llx, incr:%llu,"
+		  " dst:\n%s", __entry->start, __entry->end, __entry->flags,
+		  __entry->incr, __print_array(
+		  __get_dynamic_array(dst), __entry->nptes, 8))
+);
+
    TRACE_EVENT(amdgpu_vm_set_ptes,
    	    TP_PROTO(uint64_t pe, uint64_t addr, unsigned count,
    		     uint32_t incr, uint64_t flags, bool direct),
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 71e005cf2952..b5dbb5e8bc61 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -1513,17 +1513,22 @@ static int amdgpu_vm_update_ptes(struct amdgpu_vm_update_params *params,
    		do {
    			uint64_t upd_end = min(entry_end, frag_end);
    			unsigned nptes = (upd_end - frag_start) >> shift;
+			uint64_t upd_flags = flags | AMDGPU_PTE_FRAG(frag);
/* This can happen when we set higher level PDs to
    			 * silent to stop fault floods.
    			 */
    			nptes = max(nptes, 1u);
+
+			trace_amdgpu_vm_update_ptes(params, frag_start, upd_end,
+						    nptes, dst, incr,
+						    upd_flags);
    			amdgpu_vm_update_flags(params, pt, cursor.level,
    					       pe_start, dst, nptes, incr,
-					       flags | AMDGPU_PTE_FRAG(frag));
+					       upd_flags);
pe_start += nptes * 8;
-			dst += (uint64_t)nptes * AMDGPU_GPU_PAGE_SIZE << shift;
+			dst += nptes * incr;
frag_start = upd_end;
    			if (frag_start >= frag_end) {
_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux