Hi Christian,

On 8/19/20 2:08 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.koe...@amd.com>
Cc: Alex Deucher <alexander.deuc...@amd.com>
Signed-off-by: Christian König <christian.koe...@amd.com>
Signed-off-by: Shashank Sharma <shashank.sha...@amd.com>
---
   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.


Noob question:

Why these two pids are different ? Is it like that, the cpid-2114 process created a page/memory-area and now pid-2128 using that page/memory-area to submit a command ?



Regards,

Nirmoy




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.

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@lists.freedesktop.org
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&amp;data=02%7C01%7Cnirmoy.das%40amd.com%7Ccf56b7d79fe847f9e0ae08d84438a89e%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637334357456981915&amp;sdata=pix8Q%2FIHwTA4CGoYRVdKyDEzQHmx4ASKgAbcgwUKWBQ%3D&amp;reserved=0
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Reply via email to