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

[Christian König]

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.

Regards,
Christian.


>
> - Shashank
>