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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

- Shashank

> Regards,
> Christian.
>
>>
>> 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&amp;data=02%7C01%7Cshashank.sharma%40amd.com%7C294c0cf0460847953e8308d8443c163c%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637334372147156674&amp;sdata=3sWZcuLcWVTcspxhRq6gT1SM%2BvHQCrJqmJijKgREks0%3D&amp;reserved=0
_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux