On 19/08/20 7:00 pm, Christian König wrote: > Am 19.08.20 um 15:08 schrieb Shashank Sharma: >> On 19/08/20 6:34 pm, Christian König wrote: >>> Am 19.08.20 um 14:37 schrieb Shashank Sharma: >>>> 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 ? >>> Ah! No that's something completely different :) >>> >>> The VMID is a 4bit hardware identifier used for TLB optimization. >>> >>> The VM context ID is an unique 64bit number, we usually use >>> vm->immediate.fence_context for this. >> Damn, why is it never what you hope it to be :). Thanks for this information, I will check this out first. > Multiple reasons :) > > One is that I'm not a native speaker of English and only had very > limited formal education in the language :) > > Another one is that the hardware and driver is rather complicated. Nah, I think you were pretty clear in the communication, I will be a good software engineer, and go ahead and blame it on the hardware :D. Thanks again, I will check to add the vm context here. - Shashank > > Regards, > Christian. _______________________________________________ amd-gfx mailing list amd-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/amd-gfx
