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

[Nirmoy]

On 8/19/20 2:58 PM, Christian König wrote:
> Am 19.08.20 um 14:51 schrieb Shashank Sharma:
>> On 19/08/20 6:15 pm, Nirmoy wrote:
>>> 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.koenig at amd.com>
>>>>>>> Cc: Alex Deucher <alexander.deucher at amd.com>
>>>>>>> Signed-off-by: Christian König <christian.koenig at amd.com>
>>>>>>> Signed-off-by: Shashank Sharma <shashank.sharma at 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 ?
>> My understanding is that this is due to a server-client arrangement, 
>> where a client process can create a memory map and fill it for the 
>> submission, but it doesnt have privilege to  do that, and that should 
>> be done by the server process (like X/wayland etc)
>
> That's close but not quite correct.
>
> One use case is what Shashank describes here where the server 
> allocates something on behalves of the client and because of delayed 
> updates we end up updating the page tables in a different process then 
> where something is really used.
>
> But the more command use case is eviction where process A is kicking 
> out BOs of process B and because of this the page table of process B 
> are updated in the context of A.
>

Understood, thanks Christain and Shashank for the explanation!


Nirmoy.


> Regards,
> Christian.
>
>
>>
>> - Shashank
>>
>