[PATCH 1/2] drm/amdgpu: Add support for logging process info in amdgpu_vm.

[PATCH 1/2] drm/amdgpu: Add support for logging process info in amdgpu_vm.

[Andrey Grodzovsky]

Add process and thread names and pids and a function to extract
this info from relevant amdgpu_vm.

Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 14 ++++++++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 14 ++++++++++++++
 2 files changed, 28 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 8370660..8ec459e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -2942,3 +2942,17 @@ int amdgpu_vm_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
 
 	return 0;
 }
+
+void amdgpu_vm_task_info(struct amdgpu_device *adev,
+		  unsigned int pasid, struct amdgpu_task_info *task_info)
+{
+	struct amdgpu_vm *vm;
+
+	spin_lock(&adev->vm_manager.pasid_lock);
+
+	vm = idr_find(&adev->vm_manager.pasid_idr, pasid);
+	if (vm)
+		*task_info = vm->task_info;
+
+	spin_unlock(&adev->vm_manager.pasid_lock);
+}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
index 061b99a..88a1d18 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
@@ -164,6 +164,14 @@ struct amdgpu_vm_pt {
 #define AMDGPU_VM_FAULT_PASID(fault) ((u64)(fault) >> 48)
 #define AMDGPU_VM_FAULT_ADDR(fault)  ((u64)(fault) & 0xfffffffff000ULL)
 
+
+struct amdgpu_task_info {
+	char	process_name[TASK_COMM_LEN];
+	char	task_name[TASK_COMM_LEN];
+	pid_t	pid;
+	pid_t	tgid;
+};
+
 struct amdgpu_vm {
 	/* tree of virtual addresses mapped */
 	struct rb_root_cached	va;
@@ -215,6 +223,9 @@ struct amdgpu_vm {
 
 	/* Valid while the PD is reserved or fenced */
 	uint64_t		pd_phys_addr;
+
+	/* Some basic info about the task */
+	struct amdgpu_task_info task_info;
 };
 
 struct amdgpu_vm_manager {
@@ -317,4 +328,7 @@ bool amdgpu_vm_need_pipeline_sync(struct amdgpu_ring *ring,
 				  struct amdgpu_job *job);
 void amdgpu_vm_check_compute_bug(struct amdgpu_device *adev);
 
+void amdgpu_vm_task_info(struct amdgpu_device *adev,
+		  unsigned int pasid, struct amdgpu_task_info *task_info);
+
 #endif
-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 2/2] drm/admgpu: Present amdgpu_task_info in VM_FAULTS.

[Andrey Grodzovsky]

Extract and present the reposnsible process and thread when
VM_FAULT happens.

Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 12 ++++++++++++
 drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c  | 10 ++++++++--
 drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c  |  9 +++++++--
 3 files changed, 27 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index 7a625f3..1c483ea 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -187,6 +187,18 @@ static int amdgpu_cs_parser_init(struct amdgpu_cs_parser *p, void *data)
 	if (p->uf_entry.robj)
 		p->job->uf_addr = uf_offset;
 	kfree(chunk_array);
+
+	/* Use this opportunity to fill in task info for the vm */
+	if (!vm->task_info.pid) {
+		vm->task_info.pid = current->pid;
+		get_task_comm(vm->task_info.task_name, current);
+
+		if (current->group_leader->mm == current->mm) {
+			vm->task_info.tgid = current->group_leader->pid;
+			get_task_comm(vm->task_info.process_name, current->group_leader);
+		}
+	}
+
 	return 0;
 
 free_all_kdata:
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
index 08753e7..7ad19f9 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
@@ -46,6 +46,7 @@
 
 #include "ivsrcid/ivsrcid_vislands30.h"
 
+#include "amdgpu_vm.h"
 
 static void gmc_v8_0_set_gmc_funcs(struct amdgpu_device *adev);
 static void gmc_v8_0_set_irq_funcs(struct amdgpu_device *adev);
@@ -1449,8 +1450,13 @@ static int gmc_v8_0_process_interrupt(struct amdgpu_device *adev,
 		gmc_v8_0_set_fault_enable_default(adev, false);
 
 	if (printk_ratelimit()) {
-		dev_err(adev->dev, "GPU fault detected: %d 0x%08x\n",
-			entry->src_id, entry->src_data[0]);
+		struct amdgpu_task_info task_info = { 0 };
+
+		amdgpu_vm_task_info(adev, entry->pasid, &task_info);
+
+		dev_err(adev->dev, "GPU fault detected: %d 0x%08x for process %s pid %d thread %s pid %d\n",
+			entry->src_id, entry->src_data[0], task_info.process_name,
+			task_info.tgid, task_info.task_name, task_info.pid);
 		dev_err(adev->dev, "  VM_CONTEXT1_PROTECTION_FAULT_ADDR   0x%08X\n",
 			addr);
 		dev_err(adev->dev, "  VM_CONTEXT1_PROTECTION_FAULT_STATUS 0x%08X\n",
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
index 691a659..384a89c 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
@@ -259,11 +259,16 @@ static int gmc_v9_0_process_interrupt(struct amdgpu_device *adev,
 	}
 
 	if (printk_ratelimit()) {
+		struct amdgpu_task_info task_info = { 0 };
+
+		amdgpu_vm_task_info(adev, entry->pasid, &task_info);
+
 		dev_err(adev->dev,
-			"[%s] VMC page fault (src_id:%u ring:%u vmid:%u pasid:%u)\n",
+			"[%s] VMC page fault (src_id:%u ring:%u vmid:%u pasid:%u, for process %s pid %d thread %s pid %d\n)\n",
 			entry->vmid_src ? "mmhub" : "gfxhub",
 			entry->src_id, entry->ring_id, entry->vmid,
-			entry->pasid);
+			entry->pasid, task_info.process_name, task_info.tgid,
+			task_info.task_name, task_info.pid);
 		dev_err(adev->dev, "  at page 0x%016llx from %d\n",
 			addr, entry->client_id);
 		if (!amdgpu_sriov_vf(adev))
-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/2] drm/amdgpu: Add support for logging process info in amdgpu_vm.

[Christian König]

Am 04.07.2018 um 16:10 schrieb Andrey Grodzovsky:
> Add process and thread names and pids and a function to extract
> this info from relevant amdgpu_vm.
>
> Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 14 ++++++++++++++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 14 ++++++++++++++
>   2 files changed, 28 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index 8370660..8ec459e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -2942,3 +2942,17 @@ int amdgpu_vm_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
>   
>   	return 0;
>   }
> +

Please add some sphinx documentation here.

> +void amdgpu_vm_task_info(struct amdgpu_device *adev,
> +		  unsigned int pasid, struct amdgpu_task_info *task_info)

What editor/settings do you use?

When I trow those lines into vim's auto-formater it comes up with the 
following:

void amdgpu_vm_task_info(struct amdgpu_device *adev, unsigned int pasid,
                          struct amdgpu_task_info *task_info)

Not an issue at all, but I would like to know where that comes from 
cause it is a repeating pattern from multiple people.

Apart from that whole set looks like a nice addition to me,
Christian.

> +{
> +	struct amdgpu_vm *vm;
> +
> +	spin_lock(&adev->vm_manager.pasid_lock);
> +
> +	vm = idr_find(&adev->vm_manager.pasid_idr, pasid);
> +	if (vm)
> +		*task_info = vm->task_info;
> +
> +	spin_unlock(&adev->vm_manager.pasid_lock);
> +}
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> index 061b99a..88a1d18 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> @@ -164,6 +164,14 @@ struct amdgpu_vm_pt {
>   #define AMDGPU_VM_FAULT_PASID(fault) ((u64)(fault) >> 48)
>   #define AMDGPU_VM_FAULT_ADDR(fault)  ((u64)(fault) & 0xfffffffff000ULL)
>   
> +
> +struct amdgpu_task_info {
> +	char	process_name[TASK_COMM_LEN];
> +	char	task_name[TASK_COMM_LEN];
> +	pid_t	pid;
> +	pid_t	tgid;
> +};
> +
>   struct amdgpu_vm {
>   	/* tree of virtual addresses mapped */
>   	struct rb_root_cached	va;
> @@ -215,6 +223,9 @@ struct amdgpu_vm {
>   
>   	/* Valid while the PD is reserved or fenced */
>   	uint64_t		pd_phys_addr;
> +
> +	/* Some basic info about the task */
> +	struct amdgpu_task_info task_info;
>   };
>   
>   struct amdgpu_vm_manager {
> @@ -317,4 +328,7 @@ bool amdgpu_vm_need_pipeline_sync(struct amdgpu_ring *ring,
>   				  struct amdgpu_job *job);
>   void amdgpu_vm_check_compute_bug(struct amdgpu_device *adev);
>   
> +void amdgpu_vm_task_info(struct amdgpu_device *adev,
> +		  unsigned int pasid, struct amdgpu_task_info *task_info);
> +
>   #endif

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/2] drm/amdgpu: Add support for logging process info in amdgpu_vm.

[Andrey Grodzovsky]

On 07/04/2018 10:17 AM, Christian König wrote:
> Am 04.07.2018 um 16:10 schrieb Andrey Grodzovsky:
>> Add process and thread names and pids and a function to extract
>> this info from relevant amdgpu_vm.
>>
>> Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 14 ++++++++++++++
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 14 ++++++++++++++
>>   2 files changed, 28 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> index 8370660..8ec459e 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> @@ -2942,3 +2942,17 @@ int amdgpu_vm_ioctl(struct drm_device *dev, 
>> void *data, struct drm_file *filp)
>>         return 0;
>>   }
>> +
>
> Please add some sphinx documentation here.
>
>> +void amdgpu_vm_task_info(struct amdgpu_device *adev,
>> +          unsigned int pasid, struct amdgpu_task_info *task_info)
>
> What editor/settings do you use?
>
> When I trow those lines into vim's auto-formater it comes up with the 
> following:
>
> void amdgpu_vm_task_info(struct amdgpu_device *adev, unsigned int pasid,
>                          struct amdgpu_task_info *task_info)

I am using eclipse with CDT, I am not very into configuring the editor 
settings so not sure.
How can I use vim to vim autoformat to fix these indentation issues ? Do 
i need to install
some extra plug-in and configure it for kernel style indentation ?

Andrey

>
> Not an issue at all, but I would like to know where that comes from 
> cause it is a repeating pattern from multiple people.
>
> Apart from that whole set looks like a nice addition to me,
> Christian.
>
>> +{
>> +    struct amdgpu_vm *vm;
>> +
>> +    spin_lock(&adev->vm_manager.pasid_lock);
>> +
>> +    vm = idr_find(&adev->vm_manager.pasid_idr, pasid);
>> +    if (vm)
>> +        *task_info = vm->task_info;
>> +
>> +    spin_unlock(&adev->vm_manager.pasid_lock);
>> +}
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
>> index 061b99a..88a1d18 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
>> @@ -164,6 +164,14 @@ struct amdgpu_vm_pt {
>>   #define AMDGPU_VM_FAULT_PASID(fault) ((u64)(fault) >> 48)
>>   #define AMDGPU_VM_FAULT_ADDR(fault)  ((u64)(fault) & 
>> 0xfffffffff000ULL)
>>   +
>> +struct amdgpu_task_info {
>> +    char    process_name[TASK_COMM_LEN];
>> +    char    task_name[TASK_COMM_LEN];
>> +    pid_t    pid;
>> +    pid_t    tgid;
>> +};
>> +
>>   struct amdgpu_vm {
>>       /* tree of virtual addresses mapped */
>>       struct rb_root_cached    va;
>> @@ -215,6 +223,9 @@ struct amdgpu_vm {
>>         /* Valid while the PD is reserved or fenced */
>>       uint64_t        pd_phys_addr;
>> +
>> +    /* Some basic info about the task */
>> +    struct amdgpu_task_info task_info;
>>   };
>>     struct amdgpu_vm_manager {
>> @@ -317,4 +328,7 @@ bool amdgpu_vm_need_pipeline_sync(struct 
>> amdgpu_ring *ring,
>>                     struct amdgpu_job *job);
>>   void amdgpu_vm_check_compute_bug(struct amdgpu_device *adev);
>>   +void amdgpu_vm_task_info(struct amdgpu_device *adev,
>> +          unsigned int pasid, struct amdgpu_task_info *task_info);
>> +
>>   #endif
>

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/2] drm/amdgpu: Add support for logging process info in amdgpu_vm.

[Christian König]

Am 04.07.2018 um 16:22 schrieb Andrey Grodzovsky:
>
>
> On 07/04/2018 10:17 AM, Christian König wrote:
>> Am 04.07.2018 um 16:10 schrieb Andrey Grodzovsky:
>>> Add process and thread names and pids and a function to extract
>>> this info from relevant amdgpu_vm.
>>>
>>> Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
>>> ---
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 14 ++++++++++++++
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 14 ++++++++++++++
>>>   2 files changed, 28 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> index 8370660..8ec459e 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> @@ -2942,3 +2942,17 @@ int amdgpu_vm_ioctl(struct drm_device *dev, 
>>> void *data, struct drm_file *filp)
>>>         return 0;
>>>   }
>>> +
>>
>> Please add some sphinx documentation here.
>>
>>> +void amdgpu_vm_task_info(struct amdgpu_device *adev,
>>> +          unsigned int pasid, struct amdgpu_task_info *task_info)
>>
>> What editor/settings do you use?
>>
>> When I trow those lines into vim's auto-formater it comes up with the 
>> following:
>>
>> void amdgpu_vm_task_info(struct amdgpu_device *adev, unsigned int pasid,
>>                          struct amdgpu_task_info *task_info)
>
> I am using eclipse with CDT, I am not very into configuring the editor 
> settings so not sure.
> How can I use vim to vim autoformat to fix these indentation issues ? 
> Do i need to install
> some extra plug-in and configure it for kernel style indentation ?

I'm using vim with linuxsty.vim. It's not much of an issue anyway.

I just want to avoid that one editor automatically changes code into one 
style while another one changes it automatically into another style.

Well, you probably know what I mean :)

Christian.

>
> Andrey
>
>>
>> Not an issue at all, but I would like to know where that comes from 
>> cause it is a repeating pattern from multiple people.
>>
>> Apart from that whole set looks like a nice addition to me,
>> Christian.
>>
>>> +{
>>> +    struct amdgpu_vm *vm;
>>> +
>>> +    spin_lock(&adev->vm_manager.pasid_lock);
>>> +
>>> +    vm = idr_find(&adev->vm_manager.pasid_idr, pasid);
>>> +    if (vm)
>>> +        *task_info = vm->task_info;
>>> +
>>> +    spin_unlock(&adev->vm_manager.pasid_lock);
>>> +}
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h 
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
>>> index 061b99a..88a1d18 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
>>> @@ -164,6 +164,14 @@ struct amdgpu_vm_pt {
>>>   #define AMDGPU_VM_FAULT_PASID(fault) ((u64)(fault) >> 48)
>>>   #define AMDGPU_VM_FAULT_ADDR(fault)  ((u64)(fault) & 
>>> 0xfffffffff000ULL)
>>>   +
>>> +struct amdgpu_task_info {
>>> +    char    process_name[TASK_COMM_LEN];
>>> +    char    task_name[TASK_COMM_LEN];
>>> +    pid_t    pid;
>>> +    pid_t    tgid;
>>> +};
>>> +
>>>   struct amdgpu_vm {
>>>       /* tree of virtual addresses mapped */
>>>       struct rb_root_cached    va;
>>> @@ -215,6 +223,9 @@ struct amdgpu_vm {
>>>         /* Valid while the PD is reserved or fenced */
>>>       uint64_t        pd_phys_addr;
>>> +
>>> +    /* Some basic info about the task */
>>> +    struct amdgpu_task_info task_info;
>>>   };
>>>     struct amdgpu_vm_manager {
>>> @@ -317,4 +328,7 @@ bool amdgpu_vm_need_pipeline_sync(struct 
>>> amdgpu_ring *ring,
>>>                     struct amdgpu_job *job);
>>>   void amdgpu_vm_check_compute_bug(struct amdgpu_device *adev);
>>>   +void amdgpu_vm_task_info(struct amdgpu_device *adev,
>>> +          unsigned int pasid, struct amdgpu_task_info *task_info);
>>> +
>>>   #endif
>>
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx