On 19/08/20 6:03 pm, Christian König wrote: > 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. Actually I was adding vmid to address this last comment on V4: > and probably the VM context ID we use here to identify the updated VM. I assumed you meant the vmid by this, is that not so ? Regards Shashank > > 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://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=02%7C01%7Cshashank.sharma%40amd.com%7C294c0cf0460847953e8308d8443c163c%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637334372147156674&sdata=3sWZcuLcWVTcspxhRq6gT1SM%2BvHQCrJqmJijKgREks0%3D&reserved=0 _______________________________________________ amd-gfx mailing list amd-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/amd-gfx