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.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.
>> 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@lists.freedesktop.org
>> 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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
