[PATCH 1/2] amdgpu: add the amdgpu_vm ptr in the vm_bo_map/unmap events
Pierre-Eric Pelloux-Prayer
pierre-eric at damsy.net
Mon Jun 3 11:52:21 UTC 2024
Hi Christia,
Le 03/06/2024 à 11:58, Christian König a écrit :
> Am 03.06.24 um 10:46 schrieb Pierre-Eric Pelloux-Prayer:
>> These 2 traces events are tied to a specific VM so in order for them
>> to be useful for a tool we need to trace the amdgpu_vm as well.
>
> The bo_va already contains the VM pointer the map/unmap operation
> belongs to.
>
Indeed, I've missed that. I'll fix that in v2.
>>
>> Signed-off-by: Pierre-Eric Pelloux-Prayer
>> <pierre-eric.pelloux-prayer at amd.com>
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h | 20 ++++++++++++--------
>> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 8 ++++----
>> 2 files changed, 16 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
>> index f539b1d00234..c84050d318d6 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
>> @@ -243,10 +243,11 @@ TRACE_EVENT(amdgpu_vm_grab_id,
>> );
>> TRACE_EVENT(amdgpu_vm_bo_map,
>> - TP_PROTO(struct amdgpu_bo_va *bo_va,
>> + TP_PROTO(struct amdgpu_vm *vm, struct amdgpu_bo_va *bo_va,
>> struct amdgpu_bo_va_mapping *mapping),
>> - TP_ARGS(bo_va, mapping),
>> + TP_ARGS(vm, bo_va, mapping),
>> TP_STRUCT__entry(
>> + __field(struct amdgpu_vm *, vm)
>> __field(struct amdgpu_bo *, bo)
>> __field(long, start)
>> __field(long, last)
>> @@ -255,22 +256,24 @@ TRACE_EVENT(amdgpu_vm_bo_map,
>> ),
>> TP_fast_assign(
>> + __entry->vm = vm;
>> __entry->bo = bo_va ? bo_va->base.bo : NULL;
>> __entry->start = mapping->start;
>> __entry->last = mapping->last;
>> __entry->offset = mapping->offset;
>> __entry->flags = mapping->flags;
>> ),
>> - TP_printk("bo=%p, start=%lx, last=%lx, offset=%010llx,
>> flags=%llx",
>> - __entry->bo, __entry->start, __entry->last,
>> + TP_printk("vm=%p bo=%p, start=%lx, last=%lx, offset=%010llx,
>> flags=%llx",
>> + __entry->vm, __entry->bo, __entry->start, __entry->last,
>> __entry->offset, __entry->flags)
>> );
>> TRACE_EVENT(amdgpu_vm_bo_unmap,
>> - TP_PROTO(struct amdgpu_bo_va *bo_va,
>> + TP_PROTO(struct amdgpu_vm *vm, struct amdgpu_bo_va *bo_va,
>> struct amdgpu_bo_va_mapping *mapping),
>> - TP_ARGS(bo_va, mapping),
>> + TP_ARGS(vm, bo_va, mapping),
>> TP_STRUCT__entry(
>> + __field(struct amdgpu_vm *, vm)
>> __field(struct amdgpu_bo *, bo)
>> __field(long, start)
>> __field(long, last)
>> @@ -279,14 +282,15 @@ TRACE_EVENT(amdgpu_vm_bo_unmap,
>> ),
>> TP_fast_assign(
>> + __entry->vm = vm;
>> __entry->bo = bo_va ? bo_va->base.bo : NULL;
>> __entry->start = mapping->start;
>> __entry->last = mapping->last;
>> __entry->offset = mapping->offset;
>> __entry->flags = mapping->flags;
>> ),
>> - TP_printk("bo=%p, start=%lx, last=%lx, offset=%010llx,
>> flags=%llx",
>> - __entry->bo, __entry->start, __entry->last,
>> + TP_printk("vm=%p bo=%p, start=%lx, last=%lx, offset=%010llx,
>> flags=%llx",
>> + __entry->vm, __entry->bo, __entry->start, __entry->last,
>> __entry->offset, __entry->flags)
>> );
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> index 3abfa66d72a2..e04928d2e26a 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> @@ -1642,7 +1642,7 @@ static void amdgpu_vm_bo_insert_map(struct
>> amdgpu_device *adev,
>> if (amdgpu_vm_is_bo_always_valid(vm, bo) && !bo_va->base.moved)
>> amdgpu_vm_bo_moved(&bo_va->base);
>> - trace_amdgpu_vm_bo_map(bo_va, mapping);
>> + trace_amdgpu_vm_bo_map(vm, bo_va, mapping);
>> }
>> /* Validate operation parameters to prevent potential abuse */
>> @@ -1834,7 +1834,7 @@ int amdgpu_vm_bo_unmap(struct amdgpu_device *adev,
>> list_del(&mapping->list);
>> amdgpu_vm_it_remove(mapping, &vm->va);
>> mapping->bo_va = NULL;
>> - trace_amdgpu_vm_bo_unmap(bo_va, mapping);
>> + trace_amdgpu_vm_bo_unmap(vm, bo_va, mapping);
>> if (valid)
>> list_add(&mapping->list, &vm->freed);
>> @@ -1929,7 +1929,7 @@ int amdgpu_vm_bo_clear_mappings(struct
>> amdgpu_device *adev,
>> tmp->bo_va = NULL;
>> list_add(&tmp->list, &vm->freed);
>> - trace_amdgpu_vm_bo_unmap(NULL, tmp);
>> + trace_amdgpu_vm_bo_unmap(vm, NULL, tmp);
>
> That bo_va is NULL here is probably a bug and should be fixed.
Would something like this work?
trace_amdgpu_vm_bo_unmap(tmp->bo_va, tmp);
tmp->bo_va = NULL;
list_add(&tmp->list, &vm->freed);
Thanks,
Pierre-Eric
>
> Regards,
> Christian.
>
>> }
>> /* Insert partial mapping before the range */
>> @@ -2056,7 +2056,7 @@ void amdgpu_vm_bo_del(struct amdgpu_device *adev,
>> list_del(&mapping->list);
>> amdgpu_vm_it_remove(mapping, &vm->va);
>> mapping->bo_va = NULL;
>> - trace_amdgpu_vm_bo_unmap(bo_va, mapping);
>> + trace_amdgpu_vm_bo_unmap(vm, bo_va, mapping);
>> list_add(&mapping->list, &vm->freed);
>> }
>> list_for_each_entry_safe(mapping, next, &bo_va->invalids, list) {
More information about the amd-gfx
mailing list