[PATCH] drm/amd/amdgpu: add tracing of kernel DMA mappings and enable on init (v2)

Tom St Denis tom.stdenis at amd.com
Tue Aug 15 11:33:33 UTC 2017


On 15/08/17 07:22 AM, Christian König wrote:
> Am 14.08.2017 um 13:37 schrieb Tom St Denis:
>> This patch adds trace points to the amdgpu bo_kmap/bo/kunmap functions
>> which capture internal allocations.
>>
>> (v2): Simply add a sleep to the init.  Users can use this script to
>> load with map debugging enabled:
>>
>>     #!/bin/bash
>>     modprobe amdgpu map_debug=1 &
>>     sleep 1
>>     echo 1 > 
>> /sys/kernel/debug/tracing/events/amdgpu/amdgpu_ttm_tt_unpopulate/enable
>>     echo 1 > 
>> /sys/kernel/debug/tracing/events/amdgpu/amdgpu_ttm_tt_populate/enable
> 
> I've just realized that there is a far simpler method than that or 
> enabling trace points using the module command line.
> 
> Assuming your GPU is PCI device 01:00.0 connected through bridge 00:02.1 
> (use lspci -t to figure the connection out):
> 
> # Temporary remove the PCIe device from the bus
> echo 1 > /sys/bus/pci/devices/0000\:01\:00.0/remove
> # Load amdgpu, this allows you to enable the trace points you want and 
> also sets probes etc..
> modprobe amdpgu
> # Rescan the bus, the device subsystem will automatically probe amdgpu 
> with this device
> echo 1 > /sys/bus/pci/devices/0000\:00\:02.1/rescan

That would definitely be a bunch trickier to script up.  Nobody will run 
this by hand.  So we'd need to remove all devices that are AMD vendor ID 
but GPU only (e.g. not other controllers) which means we need to parse 
the lspci output for VGA controller related text.

Definitely doable and probably "cleaner" from a race point of view.

> Apart from that I would really like to see those trace points in TTM 
> instead. I also don't mind adding a dev or pdev pointer to the 
> ttm_bo_device structure for this.

Not against this in principle.  There's a bit of confusion... our ttm 
amdgpu functions are passed

struct ttm_tt *ttm

which we cast to

struct amdgpu_ttm_tt {
	struct ttm_dma_tt	ttm;
	struct amdgpu_device	*adev;
	u64			offset;
... <snip>

Then we use the "->ttm" of that to get dma_address[] but pages[] from 
the original ttm_tt structure ... from what I recall (when I looked into 
this last week) ttm_dma_tt and ttm_tt aren't the same structure right? 
(and looking at it right now ttm_dma_tt begins with ttm_tt....).

So wherever I put the trace in ttm I need access to dma_address[] which 
doesn't seem so clear cut.  Can I just cast a ttm_tt pointer to ttm_dma_tt?

Tom

> 
> Regards,
> Christian.
> 
>>
>> Signed-off-by: Tom St Denis <tom.stdenis at amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu.h        |  1 +
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c    | 12 ++++++++++++
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 11 ++++++++++-
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    |  4 ++--
>>   4 files changed, 25 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> index d2aaad77c353..2f5781df88c5 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> @@ -121,6 +121,7 @@ extern int amdgpu_cntl_sb_buf_per_se;
>>   extern int amdgpu_param_buf_per_se;
>>   extern int amdgpu_job_hang_limit;
>>   extern int amdgpu_lbpw;
>> +extern int amdgpu_enable_map_debugging;
>>   #ifdef CONFIG_DRM_AMDGPU_SI
>>   extern int amdgpu_si_support;
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>> index 2cdf8443e7d3..0ed777cc2f63 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>> @@ -120,6 +120,7 @@ int amdgpu_cntl_sb_buf_per_se = 0;
>>   int amdgpu_param_buf_per_se = 0;
>>   int amdgpu_job_hang_limit = 0;
>>   int amdgpu_lbpw = -1;
>> +int amdgpu_enable_map_debugging = 0;
>>   MODULE_PARM_DESC(vramlimit, "Restrict VRAM for testing, in megabytes");
>>   module_param_named(vramlimit, amdgpu_vram_limit, int, 0600);
>> @@ -263,6 +264,9 @@ module_param_named(job_hang_limit, 
>> amdgpu_job_hang_limit, int ,0444);
>>   MODULE_PARM_DESC(lbpw, "Load Balancing Per Watt (LBPW) support (1 = 
>> enable, 0 = disable, -1 = auto)");
>>   module_param_named(lbpw, amdgpu_lbpw, int, 0444);
>> +MODULE_PARM_DESC(map_debug, "Enable map debugging in kernel (1 = on, 
>> 0 = off)");
>> +module_param_named(map_debug, amdgpu_enable_map_debugging, int, 0444);
>> +
>>   #ifdef CONFIG_DRM_AMDGPU_SI
>>   #if defined(CONFIG_DRM_RADEON) || defined(CONFIG_DRM_RADEON_MODULE)
>> @@ -865,11 +869,19 @@ static struct pci_driver amdgpu_kms_pci_driver = {
>>   };
>> +// enable trace events during init
>> +static void amdgpu_enable_dma_tracers(void)
>> +{
>> +    msleep(3000);
>> +}
>>   static int __init amdgpu_init(void)
>>   {
>>       int r;
>> +    if (amdgpu_enable_map_debugging)
>> +        amdgpu_enable_dma_tracers();
>> +
>>       r = amdgpu_sync_init();
>>       if (r)
>>           goto error_sync;
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>> index e7e899190bef..a292e86fbaa7 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>> @@ -37,6 +37,9 @@
>>   #include "amdgpu.h"
>>   #include "amdgpu_trace.h"
>> +void amdgpu_trace_dma_map(struct ttm_tt *ttm);
>> +void amdgpu_trace_dma_unmap(struct ttm_tt *ttm);
>> +
>>   static void amdgpu_ttm_bo_destroy(struct ttm_buffer_object *tbo)
>>   {
>>       struct amdgpu_device *adev = amdgpu_ttm_adev(tbo->bdev);
>> @@ -625,6 +628,9 @@ int amdgpu_bo_kmap(struct amdgpu_bo *bo, void **ptr)
>>       if (r)
>>           return r;
>> +    if (unlikely(trace_amdgpu_ttm_tt_populate_enabled()) && 
>> bo->tbo.mem.mem_type != TTM_PL_VRAM)
>> +        amdgpu_trace_dma_map(bo->tbo.ttm);
>> +
>>       if (ptr)
>>           *ptr = amdgpu_bo_kptr(bo);
>> @@ -640,8 +646,11 @@ void *amdgpu_bo_kptr(struct amdgpu_bo *bo)
>>   void amdgpu_bo_kunmap(struct amdgpu_bo *bo)
>>   {
>> -    if (bo->kmap.bo)
>> +    if (bo->kmap.bo) {
>> +        if (unlikely(trace_amdgpu_ttm_tt_unpopulate_enabled()) && 
>> bo->tbo.mem.mem_type != TTM_PL_VRAM)
>> +            amdgpu_trace_dma_unmap(bo->tbo.ttm);
>>           ttm_bo_kunmap(&bo->kmap);
>> +    }
>>   }
>>   struct amdgpu_bo *amdgpu_bo_ref(struct amdgpu_bo *bo)
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> index 26665b4baf36..85727be3fbb7 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> @@ -663,7 +663,7 @@ int amdgpu_ttm_tt_get_user_pages(struct ttm_tt 
>> *ttm, struct page **pages)
>>       return r;
>>   }
>> -static void amdgpu_trace_dma_map(struct ttm_tt *ttm)
>> +void amdgpu_trace_dma_map(struct ttm_tt *ttm)
>>   {
>>       struct amdgpu_device *adev = amdgpu_ttm_adev(ttm->bdev);
>>       struct amdgpu_ttm_tt *gtt = (void *)ttm;
>> @@ -679,7 +679,7 @@ static void amdgpu_trace_dma_map(struct ttm_tt *ttm)
>>       }
>>   }
>> -static void amdgpu_trace_dma_unmap(struct ttm_tt *ttm)
>> +void amdgpu_trace_dma_unmap(struct ttm_tt *ttm)
>>   {
>>       struct amdgpu_device *adev = amdgpu_ttm_adev(ttm->bdev);
>>       struct amdgpu_ttm_tt *gtt = (void *)ttm;
> 
> 



More information about the amd-gfx mailing list