All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/amdgpu: Add uid info to process BO list
@ 2020-09-21 19:18 Madhav Chauhan
  2020-09-21 19:24 ` Christian König
  0 siblings, 1 reply; 8+ messages in thread
From: Madhav Chauhan @ 2020-09-21 19:18 UTC (permalink / raw)
  To: amd-gfx
  Cc: kishore.surampalli, mihir.patel, shashank.sharma, Madhav Chauhan,
	alexander.deucher, athar.saleem

UID is helpful while doing analysis of BO allocated
by a process.

Signed-off-by: Madhav Chauhan <madhav.chauhan@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index f4c2e2e75b8f..c1982349ec7b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -892,6 +892,7 @@ static int amdgpu_debugfs_gem_info(struct seq_file *m, void *data)
 	struct drm_info_node *node = (struct drm_info_node *)m->private;
 	struct drm_device *dev = node->minor->dev;
 	struct drm_file *file;
+	kuid_t uid;
 	int r;
 
 	r = mutex_lock_interruptible(&dev->filelist_mutex);
@@ -909,7 +910,10 @@ static int amdgpu_debugfs_gem_info(struct seq_file *m, void *data)
 		 */
 		rcu_read_lock();
 		task = pid_task(file->pid, PIDTYPE_PID);
-		seq_printf(m, "pid %8d command %s:\n", pid_nr(file->pid),
+		uid = task ? __task_cred(task)->euid : GLOBAL_ROOT_UID;
+		seq_printf(m, "pid %8d uid %5d command %s:\n",
+			   pid_nr(file->pid),
+			   from_kuid_munged(seq_user_ns(m), uid),
 			   task ? task->comm : "<unknown>");
 		rcu_read_unlock();
 
-- 
2.17.1

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

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm/amdgpu: Add uid info to process BO list
  2020-09-21 19:18 [PATCH] drm/amdgpu: Add uid info to process BO list Madhav Chauhan
@ 2020-09-21 19:24 ` Christian König
  2020-09-21 19:55   ` Chauhan, Madhav
  0 siblings, 1 reply; 8+ messages in thread
From: Christian König @ 2020-09-21 19:24 UTC (permalink / raw)
  To: Madhav Chauhan, amd-gfx
  Cc: alexander.deucher, kishore.surampalli, mihir.patel, athar.saleem,
	shashank.sharma

Am 21.09.20 um 21:18 schrieb Madhav Chauhan:
> UID is helpful while doing analysis of BO allocated
> by a process.

Looks like a bit overkill to me, why not get the uid from the process info?

Christian.

>
> Signed-off-by: Madhav Chauhan <madhav.chauhan@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> index f4c2e2e75b8f..c1982349ec7b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> @@ -892,6 +892,7 @@ static int amdgpu_debugfs_gem_info(struct seq_file *m, void *data)
>   	struct drm_info_node *node = (struct drm_info_node *)m->private;
>   	struct drm_device *dev = node->minor->dev;
>   	struct drm_file *file;
> +	kuid_t uid;
>   	int r;
>   
>   	r = mutex_lock_interruptible(&dev->filelist_mutex);
> @@ -909,7 +910,10 @@ static int amdgpu_debugfs_gem_info(struct seq_file *m, void *data)
>   		 */
>   		rcu_read_lock();
>   		task = pid_task(file->pid, PIDTYPE_PID);
> -		seq_printf(m, "pid %8d command %s:\n", pid_nr(file->pid),
> +		uid = task ? __task_cred(task)->euid : GLOBAL_ROOT_UID;
> +		seq_printf(m, "pid %8d uid %5d command %s:\n",
> +			   pid_nr(file->pid),
> +			   from_kuid_munged(seq_user_ns(m), uid),
>   			   task ? task->comm : "<unknown>");
>   		rcu_read_unlock();
>   

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: [PATCH] drm/amdgpu: Add uid info to process BO list
  2020-09-21 19:24 ` Christian König
@ 2020-09-21 19:55   ` Chauhan, Madhav
  2020-09-22  6:44     ` Christian König
  0 siblings, 1 reply; 8+ messages in thread
From: Chauhan, Madhav @ 2020-09-21 19:55 UTC (permalink / raw)
  To: Koenig, Christian, amd-gfx
  Cc: Deucher, Alexander, Surampalli, Kishore, Patel, Mihir, Saleem,
	Athar, Sharma, Shashank

[AMD Public Use]

-----Original Message-----
From: Christian König <ckoenig.leichtzumerken@gmail.com> 
Sent: Tuesday, September 22, 2020 12:54 AM
To: Chauhan, Madhav <Madhav.Chauhan@amd.com>; amd-gfx@lists.freedesktop.org
Cc: Surampalli, Kishore <Kishore.Surampalli@amd.com>; Patel, Mihir <Mihir.Patel@amd.com>; Sharma, Shashank <Shashank.Sharma@amd.com>; Deucher, Alexander <Alexander.Deucher@amd.com>; Saleem, Athar <Athar.Saleem@amd.com>
Subject: Re: [PATCH] drm/amdgpu: Add uid info to process BO list

Am 21.09.20 um 21:18 schrieb Madhav Chauhan:
> UID is helpful while doing analysis of BO allocated by a process.

Looks like a bit overkill to me, why not get the uid from the process info?

Not sure if I got your point , but used the similar method implemented at drm level inside drm_debugfs.c. Thanks

Regards,
Madhav

Christian.

>
> Signed-off-by: Madhav Chauhan <madhav.chauhan@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> index f4c2e2e75b8f..c1982349ec7b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> @@ -892,6 +892,7 @@ static int amdgpu_debugfs_gem_info(struct seq_file *m, void *data)
>   	struct drm_info_node *node = (struct drm_info_node *)m->private;
>   	struct drm_device *dev = node->minor->dev;
>   	struct drm_file *file;
> +	kuid_t uid;
>   	int r;
>   
>   	r = mutex_lock_interruptible(&dev->filelist_mutex);
> @@ -909,7 +910,10 @@ static int amdgpu_debugfs_gem_info(struct seq_file *m, void *data)
>   		 */
>   		rcu_read_lock();
>   		task = pid_task(file->pid, PIDTYPE_PID);
> -		seq_printf(m, "pid %8d command %s:\n", pid_nr(file->pid),
> +		uid = task ? __task_cred(task)->euid : GLOBAL_ROOT_UID;
> +		seq_printf(m, "pid %8d uid %5d command %s:\n",
> +			   pid_nr(file->pid),
> +			   from_kuid_munged(seq_user_ns(m), uid),
>   			   task ? task->comm : "<unknown>");
>   		rcu_read_unlock();
>   
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm/amdgpu: Add uid info to process BO list
  2020-09-21 19:55   ` Chauhan, Madhav
@ 2020-09-22  6:44     ` Christian König
  2020-09-22 10:38       ` Chauhan, Madhav
  0 siblings, 1 reply; 8+ messages in thread
From: Christian König @ 2020-09-22  6:44 UTC (permalink / raw)
  To: Chauhan, Madhav, amd-gfx
  Cc: Deucher, Alexander, Surampalli, Kishore, Patel, Mihir, Saleem,
	Athar, Sharma, Shashank

Am 21.09.20 um 21:55 schrieb Chauhan, Madhav:
> [AMD Public Use]
>
> -----Original Message-----
> From: Christian König <ckoenig.leichtzumerken@gmail.com>
> Sent: Tuesday, September 22, 2020 12:54 AM
> To: Chauhan, Madhav <Madhav.Chauhan@amd.com>; amd-gfx@lists.freedesktop.org
> Cc: Surampalli, Kishore <Kishore.Surampalli@amd.com>; Patel, Mihir <Mihir.Patel@amd.com>; Sharma, Shashank <Shashank.Sharma@amd.com>; Deucher, Alexander <Alexander.Deucher@amd.com>; Saleem, Athar <Athar.Saleem@amd.com>
> Subject: Re: [PATCH] drm/amdgpu: Add uid info to process BO list
>
> Am 21.09.20 um 21:18 schrieb Madhav Chauhan:
>> UID is helpful while doing analysis of BO allocated by a process.
> Looks like a bit overkill to me, why not get the uid from the process info?
>
> Not sure if I got your point , but used the similar method implemented at drm level inside drm_debugfs.c. Thanks

Good argument, but I'm not sure if we should duplicate that here. What 
do you need this for?

Christian.

>
> Regards,
> Madhav
>
> Christian.
>
>> Signed-off-by: Madhav Chauhan <madhav.chauhan@amd.com>
>> ---
>>    drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 6 +++++-
>>    1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>> index f4c2e2e75b8f..c1982349ec7b 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>> @@ -892,6 +892,7 @@ static int amdgpu_debugfs_gem_info(struct seq_file *m, void *data)
>>    	struct drm_info_node *node = (struct drm_info_node *)m->private;
>>    	struct drm_device *dev = node->minor->dev;
>>    	struct drm_file *file;
>> +	kuid_t uid;
>>    	int r;
>>    
>>    	r = mutex_lock_interruptible(&dev->filelist_mutex);
>> @@ -909,7 +910,10 @@ static int amdgpu_debugfs_gem_info(struct seq_file *m, void *data)
>>    		 */
>>    		rcu_read_lock();
>>    		task = pid_task(file->pid, PIDTYPE_PID);
>> -		seq_printf(m, "pid %8d command %s:\n", pid_nr(file->pid),
>> +		uid = task ? __task_cred(task)->euid : GLOBAL_ROOT_UID;
>> +		seq_printf(m, "pid %8d uid %5d command %s:\n",
>> +			   pid_nr(file->pid),
>> +			   from_kuid_munged(seq_user_ns(m), uid),
>>    			   task ? task->comm : "<unknown>");
>>    		rcu_read_unlock();
>>    

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: [PATCH] drm/amdgpu: Add uid info to process BO list
  2020-09-22  6:44     ` Christian König
@ 2020-09-22 10:38       ` Chauhan, Madhav
  2020-09-22 12:54         ` Christian König
  0 siblings, 1 reply; 8+ messages in thread
From: Chauhan, Madhav @ 2020-09-22 10:38 UTC (permalink / raw)
  To: Koenig, Christian, amd-gfx
  Cc: Deucher, Alexander, Surampalli, Kishore, Patel, Mihir, Saleem,
	Athar, Sharma, Shashank

[AMD Public Use]

-----Original Message-----
From: Koenig, Christian <Christian.Koenig@amd.com> 
Sent: Tuesday, September 22, 2020 12:15 PM
To: Chauhan, Madhav <Madhav.Chauhan@amd.com>; amd-gfx@lists.freedesktop.org
Cc: Surampalli, Kishore <Kishore.Surampalli@amd.com>; Patel, Mihir <Mihir.Patel@amd.com>; Sharma, Shashank <Shashank.Sharma@amd.com>; Deucher, Alexander <Alexander.Deucher@amd.com>; Saleem, Athar <Athar.Saleem@amd.com>
Subject: Re: [PATCH] drm/amdgpu: Add uid info to process BO list

Am 21.09.20 um 21:55 schrieb Chauhan, Madhav:
> [AMD Public Use]
>
> -----Original Message-----
> From: Christian König <ckoenig.leichtzumerken@gmail.com>
> Sent: Tuesday, September 22, 2020 12:54 AM
> To: Chauhan, Madhav <Madhav.Chauhan@amd.com>; 
> amd-gfx@lists.freedesktop.org
> Cc: Surampalli, Kishore <Kishore.Surampalli@amd.com>; Patel, Mihir 
> <Mihir.Patel@amd.com>; Sharma, Shashank <Shashank.Sharma@amd.com>; 
> Deucher, Alexander <Alexander.Deucher@amd.com>; Saleem, Athar 
> <Athar.Saleem@amd.com>
> Subject: Re: [PATCH] drm/amdgpu: Add uid info to process BO list
>
> Am 21.09.20 um 21:18 schrieb Madhav Chauhan:
>> UID is helpful while doing analysis of BO allocated by a process.
> Looks like a bit overkill to me, why not get the uid from the process info?
>
> Not sure if I got your point , but used the similar method implemented 
> at drm level inside drm_debugfs.c. Thanks

Good argument, but I'm not sure if we should duplicate that here. What do you need this for?

Thanks, We need details of BOs allocated by a process and associated UID so that we can do memory perf analysis using some scripts
To find the top consumer of GPU memory and see if those application can be optimized. 

Clients information at DRM level doesn’t print list of BO per process and since that is handled by amdgpu driver specific
Functions.  So all the BO list information at one place is really useful and needed by our customers as various other vendors
Already provide this.

Regards,
Madhav

Christian.

>
> Regards,
> Madhav
>
> Christian.
>
>> Signed-off-by: Madhav Chauhan <madhav.chauhan@amd.com>
>> ---
>>    drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 6 +++++-
>>    1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>> index f4c2e2e75b8f..c1982349ec7b 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>> @@ -892,6 +892,7 @@ static int amdgpu_debugfs_gem_info(struct seq_file *m, void *data)
>>    	struct drm_info_node *node = (struct drm_info_node *)m->private;
>>    	struct drm_device *dev = node->minor->dev;
>>    	struct drm_file *file;
>> +	kuid_t uid;
>>    	int r;
>>    
>>    	r = mutex_lock_interruptible(&dev->filelist_mutex);
>> @@ -909,7 +910,10 @@ static int amdgpu_debugfs_gem_info(struct seq_file *m, void *data)
>>    		 */
>>    		rcu_read_lock();
>>    		task = pid_task(file->pid, PIDTYPE_PID);
>> -		seq_printf(m, "pid %8d command %s:\n", pid_nr(file->pid),
>> +		uid = task ? __task_cred(task)->euid : GLOBAL_ROOT_UID;
>> +		seq_printf(m, "pid %8d uid %5d command %s:\n",
>> +			   pid_nr(file->pid),
>> +			   from_kuid_munged(seq_user_ns(m), uid),
>>    			   task ? task->comm : "<unknown>");
>>    		rcu_read_unlock();
>>    
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm/amdgpu: Add uid info to process BO list
  2020-09-22 10:38       ` Chauhan, Madhav
@ 2020-09-22 12:54         ` Christian König
  2020-09-22 14:39           ` Chauhan, Madhav
  0 siblings, 1 reply; 8+ messages in thread
From: Christian König @ 2020-09-22 12:54 UTC (permalink / raw)
  To: Chauhan, Madhav, Koenig, Christian, amd-gfx
  Cc: Deucher, Alexander, Surampalli, Kishore, Patel, Mihir, Saleem,
	Athar, Sharma, Shashank

Am 22.09.20 um 12:38 schrieb Chauhan, Madhav:
> [AMD Public Use]
>
> -----Original Message-----
> From: Koenig, Christian <Christian.Koenig@amd.com>
> Sent: Tuesday, September 22, 2020 12:15 PM
> To: Chauhan, Madhav <Madhav.Chauhan@amd.com>; amd-gfx@lists.freedesktop.org
> Cc: Surampalli, Kishore <Kishore.Surampalli@amd.com>; Patel, Mihir <Mihir.Patel@amd.com>; Sharma, Shashank <Shashank.Sharma@amd.com>; Deucher, Alexander <Alexander.Deucher@amd.com>; Saleem, Athar <Athar.Saleem@amd.com>
> Subject: Re: [PATCH] drm/amdgpu: Add uid info to process BO list
>
> Am 21.09.20 um 21:55 schrieb Chauhan, Madhav:
>> [AMD Public Use]
>>
>> -----Original Message-----
>> From: Christian König <ckoenig.leichtzumerken@gmail.com>
>> Sent: Tuesday, September 22, 2020 12:54 AM
>> To: Chauhan, Madhav <Madhav.Chauhan@amd.com>;
>> amd-gfx@lists.freedesktop.org
>> Cc: Surampalli, Kishore <Kishore.Surampalli@amd.com>; Patel, Mihir
>> <Mihir.Patel@amd.com>; Sharma, Shashank <Shashank.Sharma@amd.com>;
>> Deucher, Alexander <Alexander.Deucher@amd.com>; Saleem, Athar
>> <Athar.Saleem@amd.com>
>> Subject: Re: [PATCH] drm/amdgpu: Add uid info to process BO list
>>
>> Am 21.09.20 um 21:18 schrieb Madhav Chauhan:
>>> UID is helpful while doing analysis of BO allocated by a process.
>> Looks like a bit overkill to me, why not get the uid from the process info?
>>
>> Not sure if I got your point , but used the similar method implemented
>> at drm level inside drm_debugfs.c. Thanks
> Good argument, but I'm not sure if we should duplicate that here. What do you need this for?
>
> Thanks, We need details of BOs allocated by a process and associated UID so that we can do memory perf analysis using some scripts
> To find the top consumer of GPU memory and see if those application can be optimized.
>
> Clients information at DRM level doesn’t print list of BO per process and since that is handled by amdgpu driver specific
> Functions.  So all the BO list information at one place is really useful and needed by our customers as various other vendors
> Already provide this.

Well that is exactly the explanation I didn't want to hear :(

See both the drm client list as well as the amdgpu GEM info are only 
debugfs files and only intended for providing some information for 
debugging and are not 100% reliable for the use case you have here.

The first problem is that on modern installations the file descriptor is 
often opened by the X server instead of the application.

So for example you end up with:
> pid     1382 command Xorg:
>     0x00000001:      2097152 byte VRAM @ 0x0000002a00
>     0x00000002:         4096 byte  GTT @ 0x00000006c7
> ....
>     0x00000090:       266240 byte VRAM @ 0x000005e800
>     0x00000091:      2097152 byte VRAM @ 0x000004e200
>     0x00000092:      2097152 byte  GTT
> pid     1382 command Xorg:
>     0x00000001:      2097152 byte VRAM @ 0x0000002800
> ...

Then next problem is that the amdgpu_gem_info is completely inaccurate 
regarding the used memory of an application, since the same BO is 
sometimes opened multiple times. That's also the reason why we don't 
provide a total of the consumed memory. It basically just informs you 
which handle is what.

Then last the debugfs files are not a stable interface and not meant to 
be consumed by scripts and/or frontend applications. You should instead 
use sysfs for this.

Regards,
Christian.

>
> Regards,
> Madhav
>
> Christian.
>
>> Regards,
>> Madhav
>>
>> Christian.
>>
>>> Signed-off-by: Madhav Chauhan <madhav.chauhan@amd.com>
>>> ---
>>>     drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 6 +++++-
>>>     1 file changed, 5 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>>> index f4c2e2e75b8f..c1982349ec7b 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>>> @@ -892,6 +892,7 @@ static int amdgpu_debugfs_gem_info(struct seq_file *m, void *data)
>>>     	struct drm_info_node *node = (struct drm_info_node *)m->private;
>>>     	struct drm_device *dev = node->minor->dev;
>>>     	struct drm_file *file;
>>> +	kuid_t uid;
>>>     	int r;
>>>     
>>>     	r = mutex_lock_interruptible(&dev->filelist_mutex);
>>> @@ -909,7 +910,10 @@ static int amdgpu_debugfs_gem_info(struct seq_file *m, void *data)
>>>     		 */
>>>     		rcu_read_lock();
>>>     		task = pid_task(file->pid, PIDTYPE_PID);
>>> -		seq_printf(m, "pid %8d command %s:\n", pid_nr(file->pid),
>>> +		uid = task ? __task_cred(task)->euid : GLOBAL_ROOT_UID;
>>> +		seq_printf(m, "pid %8d uid %5d command %s:\n",
>>> +			   pid_nr(file->pid),
>>> +			   from_kuid_munged(seq_user_ns(m), uid),
>>>     			   task ? task->comm : "<unknown>");
>>>     		rcu_read_unlock();
>>>     
> _______________________________________________
> 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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: [PATCH] drm/amdgpu: Add uid info to process BO list
  2020-09-22 12:54         ` Christian König
@ 2020-09-22 14:39           ` Chauhan, Madhav
  2020-09-22 15:02             ` Alex Deucher
  0 siblings, 1 reply; 8+ messages in thread
From: Chauhan, Madhav @ 2020-09-22 14:39 UTC (permalink / raw)
  To: Koenig, Christian, amd-gfx
  Cc: Deucher, Alexander, Surampalli, Kishore, Patel, Mihir, Saleem,
	 Athar, Sharma, Shashank

[AMD Public Use]

-----Original Message-----
From: Christian König <ckoenig.leichtzumerken@gmail.com> 
Sent: Tuesday, September 22, 2020 6:25 PM
To: Chauhan, Madhav <Madhav.Chauhan@amd.com>; Koenig, Christian <Christian.Koenig@amd.com>; amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Surampalli, Kishore <Kishore.Surampalli@amd.com>; Patel, Mihir <Mihir.Patel@amd.com>; Saleem, Athar <Athar.Saleem@amd.com>; Sharma, Shashank <Shashank.Sharma@amd.com>
Subject: Re: [PATCH] drm/amdgpu: Add uid info to process BO list

Am 22.09.20 um 12:38 schrieb Chauhan, Madhav:
> [AMD Public Use]
>
> -----Original Message-----
> From: Koenig, Christian <Christian.Koenig@amd.com>
> Sent: Tuesday, September 22, 2020 12:15 PM
> To: Chauhan, Madhav <Madhav.Chauhan@amd.com>; 
> amd-gfx@lists.freedesktop.org
> Cc: Surampalli, Kishore <Kishore.Surampalli@amd.com>; Patel, Mihir 
> <Mihir.Patel@amd.com>; Sharma, Shashank <Shashank.Sharma@amd.com>; 
> Deucher, Alexander <Alexander.Deucher@amd.com>; Saleem, Athar 
> <Athar.Saleem@amd.com>
> Subject: Re: [PATCH] drm/amdgpu: Add uid info to process BO list
>
> Am 21.09.20 um 21:55 schrieb Chauhan, Madhav:
>> [AMD Public Use]
>>
>> -----Original Message-----
>> From: Christian König <ckoenig.leichtzumerken@gmail.com>
>> Sent: Tuesday, September 22, 2020 12:54 AM
>> To: Chauhan, Madhav <Madhav.Chauhan@amd.com>; 
>> amd-gfx@lists.freedesktop.org
>> Cc: Surampalli, Kishore <Kishore.Surampalli@amd.com>; Patel, Mihir 
>> <Mihir.Patel@amd.com>; Sharma, Shashank <Shashank.Sharma@amd.com>; 
>> Deucher, Alexander <Alexander.Deucher@amd.com>; Saleem, Athar 
>> <Athar.Saleem@amd.com>
>> Subject: Re: [PATCH] drm/amdgpu: Add uid info to process BO list
>>
>> Am 21.09.20 um 21:18 schrieb Madhav Chauhan:
>>> UID is helpful while doing analysis of BO allocated by a process.
>> Looks like a bit overkill to me, why not get the uid from the process info?
>>
>> Not sure if I got your point , but used the similar method 
>> implemented at drm level inside drm_debugfs.c. Thanks
> Good argument, but I'm not sure if we should duplicate that here. What do you need this for?
>
> Thanks, We need details of BOs allocated by a process and associated 
> UID so that we can do memory perf analysis using some scripts To find the top consumer of GPU memory and see if those application can be optimized.
>
> Clients information at DRM level doesn’t print list of BO per process 
> and since that is handled by amdgpu driver specific Functions.  So all 
> the BO list information at one place is really useful and needed by our customers as various other vendors Already provide this.

Well that is exactly the explanation I didn't want to hear :(

See both the drm client list as well as the amdgpu GEM info are only debugfs files and only intended for providing some information for debugging and are not 100% reliable for the use case you have here.

The first problem is that on modern installations the file descriptor is often opened by the X server instead of the application.

So for example you end up with:
> pid     1382 command Xorg:
>     0x00000001:      2097152 byte VRAM @ 0x0000002a00
>     0x00000002:         4096 byte  GTT @ 0x00000006c7 ....
>     0x00000090:       266240 byte VRAM @ 0x000005e800
>     0x00000091:      2097152 byte VRAM @ 0x000004e200
>     0x00000092:      2097152 byte  GTT 

pid     1382 command Xorg:
>     0x00000001:      2097152 byte VRAM @ 0x0000002800 ...

Then next problem is that the amdgpu_gem_info is completely inaccurate regarding the used memory of an application, since the same BO is sometimes opened multiple times. That's also the reason why we don't provide a total of the consumed memory. It basically just informs you which handle is what.

Then last the debugfs files are not a stable interface and not meant to be consumed by scripts and/or frontend applications. You should instead use sysfs for this.

Thanks for clarifying. 
UID should remain consistent even though X Server opens device on behalf of App??
Do you mean we may have entry for same BO inside file1->object_idr and file2->object_idr ?? 

So which interface inside sysfs/ we need to use to get:
- Total Memory consumed by an application/pid
- Details of BO like size, domain  (VRAM/GTT) etc per application/pid??

Regards,
Madhav 

Regards,
Christian.

>
> Regards,
> Madhav
>
> Christian.
>
>> Regards,
>> Madhav
>>
>> Christian.
>>
>>> Signed-off-by: Madhav Chauhan <madhav.chauhan@amd.com>
>>> ---
>>>     drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 6 +++++-
>>>     1 file changed, 5 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>>> index f4c2e2e75b8f..c1982349ec7b 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>>> @@ -892,6 +892,7 @@ static int amdgpu_debugfs_gem_info(struct seq_file *m, void *data)
>>>     	struct drm_info_node *node = (struct drm_info_node *)m->private;
>>>     	struct drm_device *dev = node->minor->dev;
>>>     	struct drm_file *file;
>>> +	kuid_t uid;
>>>     	int r;
>>>     
>>>     	r = mutex_lock_interruptible(&dev->filelist_mutex);
>>> @@ -909,7 +910,10 @@ static int amdgpu_debugfs_gem_info(struct seq_file *m, void *data)
>>>     		 */
>>>     		rcu_read_lock();
>>>     		task = pid_task(file->pid, PIDTYPE_PID);
>>> -		seq_printf(m, "pid %8d command %s:\n", pid_nr(file->pid),
>>> +		uid = task ? __task_cred(task)->euid : GLOBAL_ROOT_UID;
>>> +		seq_printf(m, "pid %8d uid %5d command %s:\n",
>>> +			   pid_nr(file->pid),
>>> +			   from_kuid_munged(seq_user_ns(m), uid),
>>>     			   task ? task->comm : "<unknown>");
>>>     		rcu_read_unlock();
>>>     
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flist
> s.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&amp;data=02%7C01%7CMa
> dhav.Chauhan%40amd.com%7C13036a57ccd2478eebb108d85ef6b0e8%7C3dd8961fe4
> 884e608e11a82d994e183d%7C0%7C0%7C637363760926411977&amp;sdata=XORY%2Fh
> iAOLlde1xhc0tUd%2FTQlCyG2cqvVH2Jn%2FiPtF8%3D&amp;reserved=0
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm/amdgpu: Add uid info to process BO list
  2020-09-22 14:39           ` Chauhan, Madhav
@ 2020-09-22 15:02             ` Alex Deucher
  0 siblings, 0 replies; 8+ messages in thread
From: Alex Deucher @ 2020-09-22 15:02 UTC (permalink / raw)
  To: Chauhan, Madhav
  Cc: Surampalli, Kishore, Patel, Mihir, Sharma, Shashank, amd-gfx,
	Deucher, Alexander, Koenig, Christian, Saleem, Athar

On Tue, Sep 22, 2020 at 10:39 AM Chauhan, Madhav <Madhav.Chauhan@amd.com> wrote:
>
> [AMD Public Use]
>
> -----Original Message-----
> From: Christian König <ckoenig.leichtzumerken@gmail.com>
> Sent: Tuesday, September 22, 2020 6:25 PM
> To: Chauhan, Madhav <Madhav.Chauhan@amd.com>; Koenig, Christian <Christian.Koenig@amd.com>; amd-gfx@lists.freedesktop.org
> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Surampalli, Kishore <Kishore.Surampalli@amd.com>; Patel, Mihir <Mihir.Patel@amd.com>; Saleem, Athar <Athar.Saleem@amd.com>; Sharma, Shashank <Shashank.Sharma@amd.com>
> Subject: Re: [PATCH] drm/amdgpu: Add uid info to process BO list
>
> Am 22.09.20 um 12:38 schrieb Chauhan, Madhav:
> > [AMD Public Use]
> >
> > -----Original Message-----
> > From: Koenig, Christian <Christian.Koenig@amd.com>
> > Sent: Tuesday, September 22, 2020 12:15 PM
> > To: Chauhan, Madhav <Madhav.Chauhan@amd.com>;
> > amd-gfx@lists.freedesktop.org
> > Cc: Surampalli, Kishore <Kishore.Surampalli@amd.com>; Patel, Mihir
> > <Mihir.Patel@amd.com>; Sharma, Shashank <Shashank.Sharma@amd.com>;
> > Deucher, Alexander <Alexander.Deucher@amd.com>; Saleem, Athar
> > <Athar.Saleem@amd.com>
> > Subject: Re: [PATCH] drm/amdgpu: Add uid info to process BO list
> >
> > Am 21.09.20 um 21:55 schrieb Chauhan, Madhav:
> >> [AMD Public Use]
> >>
> >> -----Original Message-----
> >> From: Christian König <ckoenig.leichtzumerken@gmail.com>
> >> Sent: Tuesday, September 22, 2020 12:54 AM
> >> To: Chauhan, Madhav <Madhav.Chauhan@amd.com>;
> >> amd-gfx@lists.freedesktop.org
> >> Cc: Surampalli, Kishore <Kishore.Surampalli@amd.com>; Patel, Mihir
> >> <Mihir.Patel@amd.com>; Sharma, Shashank <Shashank.Sharma@amd.com>;
> >> Deucher, Alexander <Alexander.Deucher@amd.com>; Saleem, Athar
> >> <Athar.Saleem@amd.com>
> >> Subject: Re: [PATCH] drm/amdgpu: Add uid info to process BO list
> >>
> >> Am 21.09.20 um 21:18 schrieb Madhav Chauhan:
> >>> UID is helpful while doing analysis of BO allocated by a process.
> >> Looks like a bit overkill to me, why not get the uid from the process info?
> >>
> >> Not sure if I got your point , but used the similar method
> >> implemented at drm level inside drm_debugfs.c. Thanks
> > Good argument, but I'm not sure if we should duplicate that here. What do you need this for?
> >
> > Thanks, We need details of BOs allocated by a process and associated
> > UID so that we can do memory perf analysis using some scripts To find the top consumer of GPU memory and see if those application can be optimized.
> >
> > Clients information at DRM level doesn’t print list of BO per process
> > and since that is handled by amdgpu driver specific Functions.  So all
> > the BO list information at one place is really useful and needed by our customers as various other vendors Already provide this.
>
> Well that is exactly the explanation I didn't want to hear :(
>
> See both the drm client list as well as the amdgpu GEM info are only debugfs files and only intended for providing some information for debugging and are not 100% reliable for the use case you have here.
>
> The first problem is that on modern installations the file descriptor is often opened by the X server instead of the application.
>
> So for example you end up with:
> > pid     1382 command Xorg:
> >     0x00000001:      2097152 byte VRAM @ 0x0000002a00
> >     0x00000002:         4096 byte  GTT @ 0x00000006c7 ....
> >     0x00000090:       266240 byte VRAM @ 0x000005e800
> >     0x00000091:      2097152 byte VRAM @ 0x000004e200
> >     0x00000092:      2097152 byte  GTT
>
> pid     1382 command Xorg:
> >     0x00000001:      2097152 byte VRAM @ 0x0000002800 ...
>
> Then next problem is that the amdgpu_gem_info is completely inaccurate regarding the used memory of an application, since the same BO is sometimes opened multiple times. That's also the reason why we don't provide a total of the consumed memory. It basically just informs you which handle is what.
>
> Then last the debugfs files are not a stable interface and not meant to be consumed by scripts and/or frontend applications. You should instead use sysfs for this.
>
> Thanks for clarifying.
> UID should remain consistent even though X Server opens device on behalf of App??
> Do you mean we may have entry for same BO inside file1->object_idr and file2->object_idr ??
>
> So which interface inside sysfs/ we need to use to get:
> - Total Memory consumed by an application/pid
> - Details of BO like size, domain  (VRAM/GTT) etc per application/pid??

Whether you use this interface or another, it's not clear how you
should do the accounting for shared buffers.

Alex

>
> Regards,
> Madhav
>
> Regards,
> Christian.
>
> >
> > Regards,
> > Madhav
> >
> > Christian.
> >
> >> Regards,
> >> Madhav
> >>
> >> Christian.
> >>
> >>> Signed-off-by: Madhav Chauhan <madhav.chauhan@amd.com>
> >>> ---
> >>>     drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 6 +++++-
> >>>     1 file changed, 5 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> >>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> >>> index f4c2e2e75b8f..c1982349ec7b 100644
> >>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> >>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> >>> @@ -892,6 +892,7 @@ static int amdgpu_debugfs_gem_info(struct seq_file *m, void *data)
> >>>             struct drm_info_node *node = (struct drm_info_node *)m->private;
> >>>             struct drm_device *dev = node->minor->dev;
> >>>             struct drm_file *file;
> >>> +   kuid_t uid;
> >>>             int r;
> >>>
> >>>             r = mutex_lock_interruptible(&dev->filelist_mutex);
> >>> @@ -909,7 +910,10 @@ static int amdgpu_debugfs_gem_info(struct seq_file *m, void *data)
> >>>                      */
> >>>                     rcu_read_lock();
> >>>                     task = pid_task(file->pid, PIDTYPE_PID);
> >>> -           seq_printf(m, "pid %8d command %s:\n", pid_nr(file->pid),
> >>> +           uid = task ? __task_cred(task)->euid : GLOBAL_ROOT_UID;
> >>> +           seq_printf(m, "pid %8d uid %5d command %s:\n",
> >>> +                      pid_nr(file->pid),
> >>> +                      from_kuid_munged(seq_user_ns(m), uid),
> >>>                                task ? task->comm : "<unknown>");
> >>>                     rcu_read_unlock();
> >>>
> > _______________________________________________
> > amd-gfx mailing list
> > amd-gfx@lists.freedesktop.org
> > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flist
> > s.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&amp;data=02%7C01%7CMa
> > dhav.Chauhan%40amd.com%7C13036a57ccd2478eebb108d85ef6b0e8%7C3dd8961fe4
> > 884e608e11a82d994e183d%7C0%7C0%7C637363760926411977&amp;sdata=XORY%2Fh
> > iAOLlde1xhc0tUd%2FTQlCyG2cqvVH2Jn%2FiPtF8%3D&amp;reserved=0
> _______________________________________________
> 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

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2020-09-22 15:02 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-21 19:18 [PATCH] drm/amdgpu: Add uid info to process BO list Madhav Chauhan
2020-09-21 19:24 ` Christian König
2020-09-21 19:55   ` Chauhan, Madhav
2020-09-22  6:44     ` Christian König
2020-09-22 10:38       ` Chauhan, Madhav
2020-09-22 12:54         ` Christian König
2020-09-22 14:39           ` Chauhan, Madhav
2020-09-22 15:02             ` Alex Deucher

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.