[Intel-gfx] [PATCH i-g-t 1/6] tests/api_intel_allocator: Fix build warning

[Intel-gfx] [PATCH i-g-t 1/6] tests/api_intel_allocator: Fix build warning

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

.../tests/i915/api_intel_allocator.c: In function ‘basic_alloc’:
.../tests/i915/api_intel_allocator.c:158:25: warning: this ‘if’ clause does not guard... [-Wmisleading-indentation]
  158 |                         if (j == i)
      |                         ^~

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
---
 tests/i915/api_intel_allocator.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/i915/api_intel_allocator.c b/tests/i915/api_intel_allocator.c
index 4b74317ed3d0..6d7764ca3f51 100644
--- a/tests/i915/api_intel_allocator.c
+++ b/tests/i915/api_intel_allocator.c
@@ -157,7 +157,7 @@ static void basic_alloc(int fd, int cnt, uint8_t type)
 		for (j = 0; j < cnt; j++) {
 			if (j == i)
 				continue;
-				igt_assert(!overlaps(&obj[i], &obj[j]));
+			igt_assert(!overlaps(&obj[i], &obj[j]));
 		}
 	}
 
-- 
2.32.0

[igt-dev] [PATCH i-g-t 1/6] tests/api_intel_allocator: Fix build warning

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

.../tests/i915/api_intel_allocator.c: In function ‘basic_alloc’:
.../tests/i915/api_intel_allocator.c:158:25: warning: this ‘if’ clause does not guard... [-Wmisleading-indentation]
  158 |                         if (j == i)
      |                         ^~

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
---
 tests/i915/api_intel_allocator.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/i915/api_intel_allocator.c b/tests/i915/api_intel_allocator.c
index 4b74317ed3d0..6d7764ca3f51 100644
--- a/tests/i915/api_intel_allocator.c
+++ b/tests/i915/api_intel_allocator.c
@@ -157,7 +157,7 @@ static void basic_alloc(int fd, int cnt, uint8_t type)
 		for (j = 0; j < cnt; j++) {
 			if (j == i)
 				continue;
-				igt_assert(!overlaps(&obj[i], &obj[j]));
+			igt_assert(!overlaps(&obj[i], &obj[j]));
 		}
 	}
 
-- 
2.32.0

[Intel-gfx] [PATCH i-g-t 2/6] igt/drm_read: Fix build warning

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

.../tests/drm_read.c: In function ‘test_short_buffer_wakeup’:
.../tests/drm_read.c:224:17: warning: ‘pthread_yield’ is deprecated: pthread_yield is deprecated, use sched_yield instead [-Wdeprecated-declarations]
  224 |                 pthread_yield();
      |                 ^~~~~~~~~~~~~

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Petri Latvala <petri.latvala@intel.com>
---
 tests/drm_read.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/drm_read.c b/tests/drm_read.c
index 3186e6097187..4a966a23b550 100644
--- a/tests/drm_read.c
+++ b/tests/drm_read.c
@@ -221,7 +221,7 @@ static void test_short_buffer_wakeup(int in, enum pipe pipe)
 		pthread_mutex_unlock(&w.mutex);
 
 		/* Give each thread a chance to sleep in drm_read() */
-		pthread_yield();
+		sched_yield();
 
 		/* One event should wake all threads as none consume */
 		generate_event(w.fd, pipe);
-- 
2.32.0

[Intel-gfx] [PATCH i-g-t 3/6] igt/core: Fix build warning

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

.../lib/igt_thread.c: In function ‘__igt_unique____igt_constructor_l66’:
.../lib/igt_thread.c:68:9: warning: ‘pthread_setspecific’ expecting 1 byte in a region of size 0 [-Wstringop-overread]
   68 |         pthread_setspecific(__igt_is_main_thread, (void*) 0x1);
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Petri Latvala <petri.latvala@intel.com>
---
 lib/igt_core.c   | 6 ++++--
 lib/igt_thread.c | 4 +++-
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/lib/igt_core.c b/lib/igt_core.c
index ec05535cd56e..acb9743c4a24 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -2752,6 +2752,8 @@ void igt_vlog(const char *domain, enum igt_log_level level, const char *format,
 	char *line, *formatted_line;
 	char *thread_id;
 	const char *program_name;
+	const uintptr_t false_key = 0;
+	const uintptr_t true_key = 1;
 	const char *igt_log_level_str[] = {
 		"DEBUG",
 		"INFO",
@@ -2796,9 +2798,9 @@ void igt_vlog(const char *domain, enum igt_log_level level, const char *format,
 	}
 
 	if (line[strlen(line) - 1] == '\n')
-		pthread_setspecific(__vlog_line_continuation, (void*) false);
+		pthread_setspecific(__vlog_line_continuation, &false_key);
 	else
-		pthread_setspecific(__vlog_line_continuation, (void*) true);
+		pthread_setspecific(__vlog_line_continuation, &true_key);
 
 	/* append log buffer */
 	_igt_log_buffer_append(formatted_line);
diff --git a/lib/igt_thread.c b/lib/igt_thread.c
index 5bdda4102def..f5de2d57eaaa 100644
--- a/lib/igt_thread.c
+++ b/lib/igt_thread.c
@@ -64,6 +64,8 @@ bool igt_thread_is_main(void)
 }
 
 igt_constructor {
+	const uintptr_t key = 1;
+
 	pthread_key_create(&__igt_is_main_thread, NULL);
-	pthread_setspecific(__igt_is_main_thread, (void*) 0x1);
+	pthread_setspecific(__igt_is_main_thread, &key);
 }
-- 
2.32.0

[igt-dev] [PATCH i-g-t 3/6] igt/core: Fix build warning

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

.../lib/igt_thread.c: In function ‘__igt_unique____igt_constructor_l66’:
.../lib/igt_thread.c:68:9: warning: ‘pthread_setspecific’ expecting 1 byte in a region of size 0 [-Wstringop-overread]
   68 |         pthread_setspecific(__igt_is_main_thread, (void*) 0x1);
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Petri Latvala <petri.latvala@intel.com>
---
 lib/igt_core.c   | 6 ++++--
 lib/igt_thread.c | 4 +++-
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/lib/igt_core.c b/lib/igt_core.c
index ec05535cd56e..acb9743c4a24 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -2752,6 +2752,8 @@ void igt_vlog(const char *domain, enum igt_log_level level, const char *format,
 	char *line, *formatted_line;
 	char *thread_id;
 	const char *program_name;
+	const uintptr_t false_key = 0;
+	const uintptr_t true_key = 1;
 	const char *igt_log_level_str[] = {
 		"DEBUG",
 		"INFO",
@@ -2796,9 +2798,9 @@ void igt_vlog(const char *domain, enum igt_log_level level, const char *format,
 	}
 
 	if (line[strlen(line) - 1] == '\n')
-		pthread_setspecific(__vlog_line_continuation, (void*) false);
+		pthread_setspecific(__vlog_line_continuation, &false_key);
 	else
-		pthread_setspecific(__vlog_line_continuation, (void*) true);
+		pthread_setspecific(__vlog_line_continuation, &true_key);
 
 	/* append log buffer */
 	_igt_log_buffer_append(formatted_line);
diff --git a/lib/igt_thread.c b/lib/igt_thread.c
index 5bdda4102def..f5de2d57eaaa 100644
--- a/lib/igt_thread.c
+++ b/lib/igt_thread.c
@@ -64,6 +64,8 @@ bool igt_thread_is_main(void)
 }
 
 igt_constructor {
+	const uintptr_t key = 1;
+
 	pthread_key_create(&__igt_is_main_thread, NULL);
-	pthread_setspecific(__igt_is_main_thread, (void*) 0x1);
+	pthread_setspecific(__igt_is_main_thread, &key);
 }
-- 
2.32.0

Re: [Intel-gfx] [PATCH i-g-t 3/6] igt/core: Fix build warning

[Petri Latvala]

On Fri, Nov 19, 2021 at 12:59:42PM +0000, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> .../lib/igt_thread.c: In function ‘__igt_unique____igt_constructor_l66’:
> .../lib/igt_thread.c:68:9: warning: ‘pthread_setspecific’ expecting 1 byte in a region of size 0 [-Wstringop-overread]
>    68 |         pthread_setspecific(__igt_is_main_thread, (void*) 0x1);
>       |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Petri Latvala <petri.latvala@intel.com>
> ---
>  lib/igt_core.c   | 6 ++++--
>  lib/igt_thread.c | 4 +++-
>  2 files changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/igt_core.c b/lib/igt_core.c
> index ec05535cd56e..acb9743c4a24 100644
> --- a/lib/igt_core.c
> +++ b/lib/igt_core.c
> @@ -2752,6 +2752,8 @@ void igt_vlog(const char *domain, enum igt_log_level level, const char *format,
>  	char *line, *formatted_line;
>  	char *thread_id;
>  	const char *program_name;
> +	const uintptr_t false_key = 0;
> +	const uintptr_t true_key = 1;
>  	const char *igt_log_level_str[] = {
>  		"DEBUG",
>  		"INFO",
> @@ -2796,9 +2798,9 @@ void igt_vlog(const char *domain, enum igt_log_level level, const char *format,
>  	}
>  
>  	if (line[strlen(line) - 1] == '\n')
> -		pthread_setspecific(__vlog_line_continuation, (void*) false);
> +		pthread_setspecific(__vlog_line_continuation, &false_key);
>  	else
> -		pthread_setspecific(__vlog_line_continuation, (void*) true);
> +		pthread_setspecific(__vlog_line_continuation, &true_key);

Quoting the usage of this:
        if (pthread_getspecific(__vlog_line_continuation)) {

That's going to give a non-null pointer in both cases.



>  
>  	/* append log buffer */
>  	_igt_log_buffer_append(formatted_line);
> diff --git a/lib/igt_thread.c b/lib/igt_thread.c
> index 5bdda4102def..f5de2d57eaaa 100644
> --- a/lib/igt_thread.c
> +++ b/lib/igt_thread.c
> @@ -64,6 +64,8 @@ bool igt_thread_is_main(void)
>  }
>  
>  igt_constructor {
> +	const uintptr_t key = 1;
> +
>  	pthread_key_create(&__igt_is_main_thread, NULL);
> -	pthread_setspecific(__igt_is_main_thread, (void*) 0x1);
> +	pthread_setspecific(__igt_is_main_thread, &key);

This is fine, __igt_is_main_thread key will have non-null only on the
main thread. But still a bit sus slapping the address of a
function-local variable to setspecific, we might just be trading a
compiler warning for a new future one.


-- 
Petri Latvala


>  }
> -- 
> 2.32.0
> 

Re: [Intel-gfx] [PATCH i-g-t 3/6] igt/core: Fix build warning

[Tvrtko Ursulin]

On 19/11/2021 13:53, Petri Latvala wrote:
> On Fri, Nov 19, 2021 at 12:59:42PM +0000, Tvrtko Ursulin wrote:
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> .../lib/igt_thread.c: In function ‘__igt_unique____igt_constructor_l66’:
>> .../lib/igt_thread.c:68:9: warning: ‘pthread_setspecific’ expecting 1 byte in a region of size 0 [-Wstringop-overread]
>>     68 |         pthread_setspecific(__igt_is_main_thread, (void*) 0x1);
>>        |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> Cc: Petri Latvala <petri.latvala@intel.com>
>> ---
>>   lib/igt_core.c   | 6 ++++--
>>   lib/igt_thread.c | 4 +++-
>>   2 files changed, 7 insertions(+), 3 deletions(-)
>>
>> diff --git a/lib/igt_core.c b/lib/igt_core.c
>> index ec05535cd56e..acb9743c4a24 100644
>> --- a/lib/igt_core.c
>> +++ b/lib/igt_core.c
>> @@ -2752,6 +2752,8 @@ void igt_vlog(const char *domain, enum igt_log_level level, const char *format,
>>   	char *line, *formatted_line;
>>   	char *thread_id;
>>   	const char *program_name;
>> +	const uintptr_t false_key = 0;
>> +	const uintptr_t true_key = 1;
>>   	const char *igt_log_level_str[] = {
>>   		"DEBUG",
>>   		"INFO",
>> @@ -2796,9 +2798,9 @@ void igt_vlog(const char *domain, enum igt_log_level level, const char *format,
>>   	}
>>   
>>   	if (line[strlen(line) - 1] == '\n')
>> -		pthread_setspecific(__vlog_line_continuation, (void*) false);
>> +		pthread_setspecific(__vlog_line_continuation, &false_key);
>>   	else
>> -		pthread_setspecific(__vlog_line_continuation, (void*) true);
>> +		pthread_setspecific(__vlog_line_continuation, &true_key);
> 
> Quoting the usage of this:
>          if (pthread_getspecific(__vlog_line_continuation)) {
> 
> That's going to give a non-null pointer in both cases.

Doh..

> 
> 
> 
>>   
>>   	/* append log buffer */
>>   	_igt_log_buffer_append(formatted_line);
>> diff --git a/lib/igt_thread.c b/lib/igt_thread.c
>> index 5bdda4102def..f5de2d57eaaa 100644
>> --- a/lib/igt_thread.c
>> +++ b/lib/igt_thread.c
>> @@ -64,6 +64,8 @@ bool igt_thread_is_main(void)
>>   }
>>   
>>   igt_constructor {
>> +	const uintptr_t key = 1;
>> +
>>   	pthread_key_create(&__igt_is_main_thread, NULL);
>> -	pthread_setspecific(__igt_is_main_thread, (void*) 0x1);
>> +	pthread_setspecific(__igt_is_main_thread, &key);
> 
> This is fine, __igt_is_main_thread key will have non-null only on the
> main thread. But still a bit sus slapping the address of a
> function-local variable to setspecific, we might just be trading a
> compiler warning for a new future one.

And this is then the same - why do you say this one is okay? :)

Okay I wasn't sufficiently focused while trying to fix this. No idea 
then apart for playing with pragmas.

Regards,

Tvrtko

Re: [igt-dev] [PATCH i-g-t 3/6] igt/core: Fix build warning

[Tvrtko Ursulin]

On 19/11/2021 13:53, Petri Latvala wrote:
> On Fri, Nov 19, 2021 at 12:59:42PM +0000, Tvrtko Ursulin wrote:
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> .../lib/igt_thread.c: In function ‘__igt_unique____igt_constructor_l66’:
>> .../lib/igt_thread.c:68:9: warning: ‘pthread_setspecific’ expecting 1 byte in a region of size 0 [-Wstringop-overread]
>>     68 |         pthread_setspecific(__igt_is_main_thread, (void*) 0x1);
>>        |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> Cc: Petri Latvala <petri.latvala@intel.com>
>> ---
>>   lib/igt_core.c   | 6 ++++--
>>   lib/igt_thread.c | 4 +++-
>>   2 files changed, 7 insertions(+), 3 deletions(-)
>>
>> diff --git a/lib/igt_core.c b/lib/igt_core.c
>> index ec05535cd56e..acb9743c4a24 100644
>> --- a/lib/igt_core.c
>> +++ b/lib/igt_core.c
>> @@ -2752,6 +2752,8 @@ void igt_vlog(const char *domain, enum igt_log_level level, const char *format,
>>   	char *line, *formatted_line;
>>   	char *thread_id;
>>   	const char *program_name;
>> +	const uintptr_t false_key = 0;
>> +	const uintptr_t true_key = 1;
>>   	const char *igt_log_level_str[] = {
>>   		"DEBUG",
>>   		"INFO",
>> @@ -2796,9 +2798,9 @@ void igt_vlog(const char *domain, enum igt_log_level level, const char *format,
>>   	}
>>   
>>   	if (line[strlen(line) - 1] == '\n')
>> -		pthread_setspecific(__vlog_line_continuation, (void*) false);
>> +		pthread_setspecific(__vlog_line_continuation, &false_key);
>>   	else
>> -		pthread_setspecific(__vlog_line_continuation, (void*) true);
>> +		pthread_setspecific(__vlog_line_continuation, &true_key);
> 
> Quoting the usage of this:
>          if (pthread_getspecific(__vlog_line_continuation)) {
> 
> That's going to give a non-null pointer in both cases.

Doh..

> 
> 
> 
>>   
>>   	/* append log buffer */
>>   	_igt_log_buffer_append(formatted_line);
>> diff --git a/lib/igt_thread.c b/lib/igt_thread.c
>> index 5bdda4102def..f5de2d57eaaa 100644
>> --- a/lib/igt_thread.c
>> +++ b/lib/igt_thread.c
>> @@ -64,6 +64,8 @@ bool igt_thread_is_main(void)
>>   }
>>   
>>   igt_constructor {
>> +	const uintptr_t key = 1;
>> +
>>   	pthread_key_create(&__igt_is_main_thread, NULL);
>> -	pthread_setspecific(__igt_is_main_thread, (void*) 0x1);
>> +	pthread_setspecific(__igt_is_main_thread, &key);
> 
> This is fine, __igt_is_main_thread key will have non-null only on the
> main thread. But still a bit sus slapping the address of a
> function-local variable to setspecific, we might just be trading a
> compiler warning for a new future one.

And this is then the same - why do you say this one is okay? :)

Okay I wasn't sufficiently focused while trying to fix this. No idea 
then apart for playing with pragmas.

Regards,

Tvrtko

Re: [Intel-gfx] [PATCH i-g-t 3/6] igt/core: Fix build warning

[Petri Latvala]

On Fri, Nov 19, 2021 at 03:34:54PM +0000, Tvrtko Ursulin wrote:
> 
> On 19/11/2021 13:53, Petri Latvala wrote:
> > On Fri, Nov 19, 2021 at 12:59:42PM +0000, Tvrtko Ursulin wrote:
> > > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > > 
> > > .../lib/igt_thread.c: In function ‘__igt_unique____igt_constructor_l66’:
> > > .../lib/igt_thread.c:68:9: warning: ‘pthread_setspecific’ expecting 1 byte in a region of size 0 [-Wstringop-overread]
> > >     68 |         pthread_setspecific(__igt_is_main_thread, (void*) 0x1);
> > >        |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > 
> > > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > > Cc: Petri Latvala <petri.latvala@intel.com>
> > > ---
> > >   lib/igt_core.c   | 6 ++++--
> > >   lib/igt_thread.c | 4 +++-
> > >   2 files changed, 7 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/lib/igt_core.c b/lib/igt_core.c
> > > index ec05535cd56e..acb9743c4a24 100644
> > > --- a/lib/igt_core.c
> > > +++ b/lib/igt_core.c
> > > @@ -2752,6 +2752,8 @@ void igt_vlog(const char *domain, enum igt_log_level level, const char *format,
> > >   	char *line, *formatted_line;
> > >   	char *thread_id;
> > >   	const char *program_name;
> > > +	const uintptr_t false_key = 0;
> > > +	const uintptr_t true_key = 1;
> > >   	const char *igt_log_level_str[] = {
> > >   		"DEBUG",
> > >   		"INFO",
> > > @@ -2796,9 +2798,9 @@ void igt_vlog(const char *domain, enum igt_log_level level, const char *format,
> > >   	}
> > >   	if (line[strlen(line) - 1] == '\n')
> > > -		pthread_setspecific(__vlog_line_continuation, (void*) false);
> > > +		pthread_setspecific(__vlog_line_continuation, &false_key);
> > >   	else
> > > -		pthread_setspecific(__vlog_line_continuation, (void*) true);
> > > +		pthread_setspecific(__vlog_line_continuation, &true_key);
> > 
> > Quoting the usage of this:
> >          if (pthread_getspecific(__vlog_line_continuation)) {
> > 
> > That's going to give a non-null pointer in both cases.
> 
> Doh..
> 
> > 
> > 
> > 
> > >   	/* append log buffer */
> > >   	_igt_log_buffer_append(formatted_line);
> > > diff --git a/lib/igt_thread.c b/lib/igt_thread.c
> > > index 5bdda4102def..f5de2d57eaaa 100644
> > > --- a/lib/igt_thread.c
> > > +++ b/lib/igt_thread.c
> > > @@ -64,6 +64,8 @@ bool igt_thread_is_main(void)
> > >   }
> > >   igt_constructor {
> > > +	const uintptr_t key = 1;
> > > +
> > >   	pthread_key_create(&__igt_is_main_thread, NULL);
> > > -	pthread_setspecific(__igt_is_main_thread, (void*) 0x1);
> > > +	pthread_setspecific(__igt_is_main_thread, &key);
> > 
> > This is fine, __igt_is_main_thread key will have non-null only on the
> > main thread. But still a bit sus slapping the address of a
> > function-local variable to setspecific, we might just be trading a
> > compiler warning for a new future one.
> 
> And this is then the same - why do you say this one is okay? :)

Because setspecific for this key is called from only one thread so it
kinda works.


-- 
Petri Latvala

> 
> Okay I wasn't sufficiently focused while trying to fix this. No idea then
> apart for playing with pragmas.
> 
> Regards,
> 
> Tvrtko

Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 3/6] igt/core: Fix build warning

[Petri Latvala]

On Fri, Nov 19, 2021 at 05:41:08PM +0200, Petri Latvala wrote:
> On Fri, Nov 19, 2021 at 03:34:54PM +0000, Tvrtko Ursulin wrote:
> > On 19/11/2021 13:53, Petri Latvala wrote:
> > > On Fri, Nov 19, 2021 at 12:59:42PM +0000, Tvrtko Ursulin wrote:
> > Okay I wasn't sufficiently focused while trying to fix this. No idea then
> > apart for playing with pragmas.


How's this:

diff --git a/lib/igt_core.c b/lib/igt_core.c
index ec05535c..6a4d0270 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -2727,6 +2727,8 @@ void igt_log(const char *domain, enum igt_log_level level, const char *format, .
 }
 
 static pthread_key_t __vlog_line_continuation;
+static const bool __dummy_true = true;
+static const bool __dummy_false = false;
 
 igt_constructor {
 	pthread_key_create(&__vlog_line_continuation, NULL);
@@ -2751,6 +2753,7 @@ void igt_vlog(const char *domain, enum igt_log_level level, const char *format,
 	FILE *file;
 	char *line, *formatted_line;
 	char *thread_id;
+	void *line_continuation;
 	const char *program_name;
 	const char *igt_log_level_str[] = {
 		"DEBUG",
@@ -2785,7 +2788,8 @@ void igt_vlog(const char *domain, enum igt_log_level level, const char *format,
 	if (vasprintf(&line, format, args) == -1)
 		return;
 
-	if (pthread_getspecific(__vlog_line_continuation)) {
+	line_continuation = pthread_getspecific(__vlog_line_continuation);
+	if (line_continuation != NULL && *(bool *)line_continuation) {
 		formatted_line = strdup(line);
 		if (!formatted_line)
 			goto out;
@@ -2796,9 +2800,9 @@ void igt_vlog(const char *domain, enum igt_log_level level, const char *format,
 	}
 
 	if (line[strlen(line) - 1] == '\n')
-		pthread_setspecific(__vlog_line_continuation, (void*) false);
+		pthread_setspecific(__vlog_line_continuation, &__dummy_false);
 	else
-		pthread_setspecific(__vlog_line_continuation, (void*) true);
+		pthread_setspecific(__vlog_line_continuation, &__dummy_true);
 
 	/* append log buffer */
 	_igt_log_buffer_append(formatted_line);
diff --git a/lib/igt_thread.c b/lib/igt_thread.c
index 5bdda410..0d7bce80 100644
--- a/lib/igt_thread.c
+++ b/lib/igt_thread.c
@@ -29,6 +29,7 @@
 #include "igt_thread.h"
 
 static pthread_key_t __igt_is_main_thread;
+static const bool __dummy_true = true;
 
 static _Atomic(bool) __thread_failed = false;
 
@@ -65,5 +66,5 @@ bool igt_thread_is_main(void)
 
 igt_constructor {
 	pthread_key_create(&__igt_is_main_thread, NULL);
-	pthread_setspecific(__igt_is_main_thread, (void*) 0x1);
+	pthread_setspecific(__igt_is_main_thread, &__dummy_true);
 }

Re: [igt-dev] [PATCH i-g-t 3/6] igt/core: Fix build warning

[Petri Latvala]

On Fri, Nov 19, 2021 at 05:41:08PM +0200, Petri Latvala wrote:
> On Fri, Nov 19, 2021 at 03:34:54PM +0000, Tvrtko Ursulin wrote:
> > On 19/11/2021 13:53, Petri Latvala wrote:
> > > On Fri, Nov 19, 2021 at 12:59:42PM +0000, Tvrtko Ursulin wrote:
> > Okay I wasn't sufficiently focused while trying to fix this. No idea then
> > apart for playing with pragmas.


How's this:

diff --git a/lib/igt_core.c b/lib/igt_core.c
index ec05535c..6a4d0270 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -2727,6 +2727,8 @@ void igt_log(const char *domain, enum igt_log_level level, const char *format, .
 }
 
 static pthread_key_t __vlog_line_continuation;
+static const bool __dummy_true = true;
+static const bool __dummy_false = false;
 
 igt_constructor {
 	pthread_key_create(&__vlog_line_continuation, NULL);
@@ -2751,6 +2753,7 @@ void igt_vlog(const char *domain, enum igt_log_level level, const char *format,
 	FILE *file;
 	char *line, *formatted_line;
 	char *thread_id;
+	void *line_continuation;
 	const char *program_name;
 	const char *igt_log_level_str[] = {
 		"DEBUG",
@@ -2785,7 +2788,8 @@ void igt_vlog(const char *domain, enum igt_log_level level, const char *format,
 	if (vasprintf(&line, format, args) == -1)
 		return;
 
-	if (pthread_getspecific(__vlog_line_continuation)) {
+	line_continuation = pthread_getspecific(__vlog_line_continuation);
+	if (line_continuation != NULL && *(bool *)line_continuation) {
 		formatted_line = strdup(line);
 		if (!formatted_line)
 			goto out;
@@ -2796,9 +2800,9 @@ void igt_vlog(const char *domain, enum igt_log_level level, const char *format,
 	}
 
 	if (line[strlen(line) - 1] == '\n')
-		pthread_setspecific(__vlog_line_continuation, (void*) false);
+		pthread_setspecific(__vlog_line_continuation, &__dummy_false);
 	else
-		pthread_setspecific(__vlog_line_continuation, (void*) true);
+		pthread_setspecific(__vlog_line_continuation, &__dummy_true);
 
 	/* append log buffer */
 	_igt_log_buffer_append(formatted_line);
diff --git a/lib/igt_thread.c b/lib/igt_thread.c
index 5bdda410..0d7bce80 100644
--- a/lib/igt_thread.c
+++ b/lib/igt_thread.c
@@ -29,6 +29,7 @@
 #include "igt_thread.h"
 
 static pthread_key_t __igt_is_main_thread;
+static const bool __dummy_true = true;
 
 static _Atomic(bool) __thread_failed = false;
 
@@ -65,5 +66,5 @@ bool igt_thread_is_main(void)
 
 igt_constructor {
 	pthread_key_create(&__igt_is_main_thread, NULL);
-	pthread_setspecific(__igt_is_main_thread, (void*) 0x1);
+	pthread_setspecific(__igt_is_main_thread, &__dummy_true);
 }

Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 3/6] igt/core: Fix build warning

[Tvrtko Ursulin]

On 19/11/2021 15:54, Petri Latvala wrote:
> On Fri, Nov 19, 2021 at 05:41:08PM +0200, Petri Latvala wrote:
>> On Fri, Nov 19, 2021 at 03:34:54PM +0000, Tvrtko Ursulin wrote:
>>> On 19/11/2021 13:53, Petri Latvala wrote:
>>>> On Fri, Nov 19, 2021 at 12:59:42PM +0000, Tvrtko Ursulin wrote:
>>> Okay I wasn't sufficiently focused while trying to fix this. No idea then
>>> apart for playing with pragmas.
> 
> 
> How's this:
> 
> diff --git a/lib/igt_core.c b/lib/igt_core.c
> index ec05535c..6a4d0270 100644
> --- a/lib/igt_core.c
> +++ b/lib/igt_core.c
> @@ -2727,6 +2727,8 @@ void igt_log(const char *domain, enum igt_log_level level, const char *format, .
>   }
>   
>   static pthread_key_t __vlog_line_continuation;
> +static const bool __dummy_true = true;
> +static const bool __dummy_false = false;
>   
>   igt_constructor {
>   	pthread_key_create(&__vlog_line_continuation, NULL);
> @@ -2751,6 +2753,7 @@ void igt_vlog(const char *domain, enum igt_log_level level, const char *format,
>   	FILE *file;
>   	char *line, *formatted_line;
>   	char *thread_id;
> +	void *line_continuation;
>   	const char *program_name;
>   	const char *igt_log_level_str[] = {
>   		"DEBUG",
> @@ -2785,7 +2788,8 @@ void igt_vlog(const char *domain, enum igt_log_level level, const char *format,
>   	if (vasprintf(&line, format, args) == -1)
>   		return;
>   
> -	if (pthread_getspecific(__vlog_line_continuation)) {
> +	line_continuation = pthread_getspecific(__vlog_line_continuation);
> +	if (line_continuation != NULL && *(bool *)line_continuation) {
>   		formatted_line = strdup(line);
>   		if (!formatted_line)
>   			goto out;
> @@ -2796,9 +2800,9 @@ void igt_vlog(const char *domain, enum igt_log_level level, const char *format,
>   	}
>   
>   	if (line[strlen(line) - 1] == '\n')
> -		pthread_setspecific(__vlog_line_continuation, (void*) false);
> +		pthread_setspecific(__vlog_line_continuation, &__dummy_false);
>   	else
> -		pthread_setspecific(__vlog_line_continuation, (void*) true);
> +		pthread_setspecific(__vlog_line_continuation, &__dummy_true);
>   
>   	/* append log buffer */
>   	_igt_log_buffer_append(formatted_line);
> diff --git a/lib/igt_thread.c b/lib/igt_thread.c
> index 5bdda410..0d7bce80 100644
> --- a/lib/igt_thread.c
> +++ b/lib/igt_thread.c
> @@ -29,6 +29,7 @@
>   #include "igt_thread.h"
>   
>   static pthread_key_t __igt_is_main_thread;
> +static const bool __dummy_true = true;
>   
>   static _Atomic(bool) __thread_failed = false;
>   
> @@ -65,5 +66,5 @@ bool igt_thread_is_main(void)
>   
>   igt_constructor {
>   	pthread_key_create(&__igt_is_main_thread, NULL);
> -	pthread_setspecific(__igt_is_main_thread, (void*) 0x1);
> +	pthread_setspecific(__igt_is_main_thread, &__dummy_true);

LGTM.

Regards,

Tvrtko

Re: [igt-dev] [PATCH i-g-t 3/6] igt/core: Fix build warning

[Tvrtko Ursulin]

On 19/11/2021 15:54, Petri Latvala wrote:
> On Fri, Nov 19, 2021 at 05:41:08PM +0200, Petri Latvala wrote:
>> On Fri, Nov 19, 2021 at 03:34:54PM +0000, Tvrtko Ursulin wrote:
>>> On 19/11/2021 13:53, Petri Latvala wrote:
>>>> On Fri, Nov 19, 2021 at 12:59:42PM +0000, Tvrtko Ursulin wrote:
>>> Okay I wasn't sufficiently focused while trying to fix this. No idea then
>>> apart for playing with pragmas.
> 
> 
> How's this:
> 
> diff --git a/lib/igt_core.c b/lib/igt_core.c
> index ec05535c..6a4d0270 100644
> --- a/lib/igt_core.c
> +++ b/lib/igt_core.c
> @@ -2727,6 +2727,8 @@ void igt_log(const char *domain, enum igt_log_level level, const char *format, .
>   }
>   
>   static pthread_key_t __vlog_line_continuation;
> +static const bool __dummy_true = true;
> +static const bool __dummy_false = false;
>   
>   igt_constructor {
>   	pthread_key_create(&__vlog_line_continuation, NULL);
> @@ -2751,6 +2753,7 @@ void igt_vlog(const char *domain, enum igt_log_level level, const char *format,
>   	FILE *file;
>   	char *line, *formatted_line;
>   	char *thread_id;
> +	void *line_continuation;
>   	const char *program_name;
>   	const char *igt_log_level_str[] = {
>   		"DEBUG",
> @@ -2785,7 +2788,8 @@ void igt_vlog(const char *domain, enum igt_log_level level, const char *format,
>   	if (vasprintf(&line, format, args) == -1)
>   		return;
>   
> -	if (pthread_getspecific(__vlog_line_continuation)) {
> +	line_continuation = pthread_getspecific(__vlog_line_continuation);
> +	if (line_continuation != NULL && *(bool *)line_continuation) {
>   		formatted_line = strdup(line);
>   		if (!formatted_line)
>   			goto out;
> @@ -2796,9 +2800,9 @@ void igt_vlog(const char *domain, enum igt_log_level level, const char *format,
>   	}
>   
>   	if (line[strlen(line) - 1] == '\n')
> -		pthread_setspecific(__vlog_line_continuation, (void*) false);
> +		pthread_setspecific(__vlog_line_continuation, &__dummy_false);
>   	else
> -		pthread_setspecific(__vlog_line_continuation, (void*) true);
> +		pthread_setspecific(__vlog_line_continuation, &__dummy_true);
>   
>   	/* append log buffer */
>   	_igt_log_buffer_append(formatted_line);
> diff --git a/lib/igt_thread.c b/lib/igt_thread.c
> index 5bdda410..0d7bce80 100644
> --- a/lib/igt_thread.c
> +++ b/lib/igt_thread.c
> @@ -29,6 +29,7 @@
>   #include "igt_thread.h"
>   
>   static pthread_key_t __igt_is_main_thread;
> +static const bool __dummy_true = true;
>   
>   static _Atomic(bool) __thread_failed = false;
>   
> @@ -65,5 +66,5 @@ bool igt_thread_is_main(void)
>   
>   igt_constructor {
>   	pthread_key_create(&__igt_is_main_thread, NULL);
> -	pthread_setspecific(__igt_is_main_thread, (void*) 0x1);
> +	pthread_setspecific(__igt_is_main_thread, &__dummy_true);

LGTM.

Regards,

Tvrtko

[Intel-gfx] [PATCH i-g-t 4/6] igt/drm_read: Fix build warning

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

In function ‘read’,
    inlined from ‘test_invalid_buffer’ at ../../home/tursulin/wc/intel-gpu-tools/tests/drm_read.c:113:2,
    inlined from ‘__igt_unique____real_main258’ at ../../home/tursulin/wc/intel-gpu-tools/tests/drm_read.c:297:3:
/usr/include/x86_64-linux-gnu/bits/unistd.h:47:10: warning: ‘__read_alias’ writing 4096 bytes into a region of size 0 overflows the destination [-Wstringop-overflow=]
   47 |   return __read_alias (__fd, __buf, __nbytes);
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Petri Latvala <petri.latvala@intel.com>
---
 tests/drm_read.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/tests/drm_read.c b/tests/drm_read.c
index 4a966a23b550..d422220d3c4a 100644
--- a/tests/drm_read.c
+++ b/tests/drm_read.c
@@ -289,8 +289,16 @@ igt_main
 		igt_require(kmstest_get_vblank(fd, pipe, 0));
 	}
 
+/*
+ * Has to be here and not in (or around) test_invalid_buffer() to work around
+ * a gcc bug https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98512.
+ */
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wstringop-overflow"
+#pragma GCC diagnostic ignored "-Wstringop-overflow="
 	igt_subtest("invalid-buffer")
 		test_invalid_buffer(fd);
+#pragma GCC diagnostic pop
 
 	igt_subtest("fault-buffer")
 		test_fault_buffer(fd, pipe);
-- 
2.32.0

[igt-dev] [PATCH i-g-t 4/6] igt/drm_read: Fix build warning

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

In function ‘read’,
    inlined from ‘test_invalid_buffer’ at ../../home/tursulin/wc/intel-gpu-tools/tests/drm_read.c:113:2,
    inlined from ‘__igt_unique____real_main258’ at ../../home/tursulin/wc/intel-gpu-tools/tests/drm_read.c:297:3:
/usr/include/x86_64-linux-gnu/bits/unistd.h:47:10: warning: ‘__read_alias’ writing 4096 bytes into a region of size 0 overflows the destination [-Wstringop-overflow=]
   47 |   return __read_alias (__fd, __buf, __nbytes);
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Petri Latvala <petri.latvala@intel.com>
---
 tests/drm_read.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/tests/drm_read.c b/tests/drm_read.c
index 4a966a23b550..d422220d3c4a 100644
--- a/tests/drm_read.c
+++ b/tests/drm_read.c
@@ -289,8 +289,16 @@ igt_main
 		igt_require(kmstest_get_vblank(fd, pipe, 0));
 	}
 
+/*
+ * Has to be here and not in (or around) test_invalid_buffer() to work around
+ * a gcc bug https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98512.
+ */
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wstringop-overflow"
+#pragma GCC diagnostic ignored "-Wstringop-overflow="
 	igt_subtest("invalid-buffer")
 		test_invalid_buffer(fd);
+#pragma GCC diagnostic pop
 
 	igt_subtest("fault-buffer")
 		test_fault_buffer(fd, pipe);
-- 
2.32.0

[Intel-gfx] [PATCH i-g-t 5/6] intel_gpu_top: Remove clients support

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

When kernel feature was removed the intel_gpu_top part was forgotten.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 man/intel_gpu_top.rst |   4 -
 tools/intel_gpu_top.c | 810 +-----------------------------------------
 2 files changed, 1 insertion(+), 813 deletions(-)

diff --git a/man/intel_gpu_top.rst b/man/intel_gpu_top.rst
index f4dbfc5b44d9..b3b765b05feb 100644
--- a/man/intel_gpu_top.rst
+++ b/man/intel_gpu_top.rst
@@ -56,10 +56,6 @@ Supported keys:
     'q'    Exit from the tool.
     'h'    Show interactive help.
     '1'    Toggle between aggregated engine class and physical engine mode.
-    'n'    Toggle display of numeric client busyness overlay.
-    's'    Toggle between sort modes (runtime, total runtime, pid, client id).
-    'i'    Toggle display of clients which used no GPU time.
-    'H'    Toggle between per PID aggregation and individual clients.
 
 DEVICE SELECTION
 ================
diff --git a/tools/intel_gpu_top.c b/tools/intel_gpu_top.c
index 7311038a39f4..41c59a72c09d 100644
--- a/tools/intel_gpu_top.c
+++ b/tools/intel_gpu_top.c
@@ -627,562 +627,6 @@ static void pmu_sample(struct engines *engines)
 	}
 }
 
-enum client_status {
-	FREE = 0, /* mbz */
-	ALIVE,
-	PROBE
-};
-
-struct clients;
-
-struct client {
-	struct clients *clients;
-
-	enum client_status status;
-	int sysfs_root;
-	int busy_root;
-	unsigned int id;
-	unsigned int pid;
-	char name[24];
-	char print_name[24];
-	unsigned int samples;
-	unsigned long total_runtime;
-	unsigned long last_runtime;
-	struct engines *engines;
-	unsigned long *val;
-	uint64_t *last;
-};
-
-struct clients {
-	unsigned int num_clients;
-	unsigned int active_clients;
-
-	unsigned int num_classes;
-	struct engine_class *class;
-
-	char sysfs_root[128];
-
-	struct client *client;
-};
-
-#define for_each_client(clients, c, tmp) \
-	for ((tmp) = (clients)->num_clients, c = (clients)->client; \
-	     (tmp > 0); (tmp)--, (c)++)
-
-static struct clients *init_clients(const char *drm_card)
-{
-	struct clients *clients;
-	const char *slash;
-	ssize_t ret;
-	int dir;
-
-	clients = malloc(sizeof(*clients));
-	if (!clients)
-		return NULL;
-
-	memset(clients, 0, sizeof(*clients));
-
-	if (drm_card) {
-		slash = rindex(drm_card, '/');
-		assert(slash);
-	} else {
-		slash = "card0";
-	}
-
-	ret = snprintf(clients->sysfs_root, sizeof(clients->sysfs_root),
-		       "/sys/class/drm/%s/clients/", slash);
-	assert(ret > 0 && ret < sizeof(clients->sysfs_root));
-
-	dir = open(clients->sysfs_root, O_DIRECTORY | O_RDONLY);
-	if (dir < 0) {
-		free(clients);
-		clients = NULL;
-	} else {
-		close(dir);
-	}
-
-	return clients;
-}
-
-static int __read_to_buf(int fd, char *buf, unsigned int bufsize)
-{
-	ssize_t ret;
-	int err;
-
-	ret = read(fd, buf, bufsize - 1);
-	err = errno;
-	if (ret < 1) {
-		errno = ret < 0 ? err : ENOMSG;
-
-		return -1;
-	}
-
-	if (ret > 1 && buf[ret - 1] == '\n')
-		buf[ret - 1] = '\0';
-	else
-		buf[ret] = '\0';
-
-	return 0;
-}
-
-static int
-__read_client_field(int root, const char *field, char *buf, unsigned int bufsize)
-{
-	int fd, ret;
-
-	fd = openat(root, field, O_RDONLY);
-	if (fd < 0)
-		return -1;
-
-	ret = __read_to_buf(fd, buf, bufsize);
-
-	close(fd);
-
-	return ret;
-}
-
-static uint64_t
-read_client_busy(struct client *client, unsigned int class)
-{
-	const char *class_str[] = { "0", "1", "2", "3", "4", "5", "6", "7" };
-	char buf[256], *b;
-	int ret;
-
-	assert(class < ARRAY_SIZE(class_str));
-	if (class >= ARRAY_SIZE(class_str))
-		return 0;
-
-	assert(client->sysfs_root >= 0);
-	if (client->sysfs_root < 0)
-		return 0;
-
-	if (client->busy_root < 0)
-		client->busy_root = openat(client->sysfs_root, "busy",
-					   O_RDONLY | O_DIRECTORY);
-
-	assert(client->busy_root);
-	if (client->busy_root < 0)
-		return 0;
-
-	ret = __read_client_field(client->busy_root, class_str[class], buf,
-				  sizeof(buf));
-	if (ret) {
-		close(client->busy_root);
-		client->busy_root = -1;
-		return 0;
-	}
-
-	/*
-	 * Handle both single integer and key=value formats by skipping
-	 * leading non-digits.
-	 */
-	b = buf;
-	while (*b && !isdigit(*b))
-		b++;
-
-	return strtoull(b, NULL, 10);
-}
-
-static struct client *
-find_client(struct clients *clients, enum client_status status, unsigned int id)
-{
-	unsigned int start, num;
-	struct client *c;
-
-	start = status == FREE ? clients->active_clients : 0; /* Free block at the end. */
-	num = clients->num_clients - start;
-
-	for (c = &clients->client[start]; num; c++, num--) {
-		if (status != c->status)
-			continue;
-
-		if (status == FREE || c->id == id)
-			return c;
-	}
-
-	return NULL;
-}
-
-static void update_client(struct client *c, unsigned int pid, char *name)
-{
-	uint64_t val[c->clients->num_classes];
-	unsigned int i;
-
-	if (c->pid != pid)
-		c->pid = pid;
-
-	if (strcmp(c->name, name)) {
-		char *p;
-
-		strncpy(c->name, name, sizeof(c->name) - 1);
-		strncpy(c->print_name, name, sizeof(c->print_name) - 1);
-
-		p = c->print_name;
-		while (*p) {
-			if (!isprint(*p))
-				*p = '*';
-			p++;
-		}
-	}
-
-	for (i = 0; i < c->clients->num_classes; i++)
-		val[i] = read_client_busy(c, c->clients->class[i].class);
-
-	c->last_runtime = 0;
-	c->total_runtime = 0;
-
-	for (i = 0; i < c->clients->num_classes; i++) {
-		if (val[i] < c->last[i])
-			continue; /* It will catch up soon. */
-
-		c->total_runtime += val[i];
-		c->val[i] = val[i] - c->last[i];
-		c->last_runtime += c->val[i];
-		c->last[i] = val[i];
-	}
-
-	c->samples++;
-	c->status = ALIVE;
-}
-
-static void
-add_client(struct clients *clients, unsigned int id, unsigned int pid,
-	   char *name, int sysfs_root)
-{
-	struct client *c;
-
-	assert(!find_client(clients, ALIVE, id));
-
-	c = find_client(clients, FREE, 0);
-	if (!c) {
-		unsigned int idx = clients->num_clients;
-
-		clients->num_clients += (clients->num_clients + 2) / 2;
-		clients->client = realloc(clients->client,
-					  clients->num_clients * sizeof(*c));
-		assert(clients->client);
-
-		c = &clients->client[idx];
-		memset(c, 0, (clients->num_clients - idx) * sizeof(*c));
-	}
-
-	c->sysfs_root = sysfs_root;
-	c->busy_root = -1;
-	c->id = id;
-	c->clients = clients;
-	c->val = calloc(clients->num_classes, sizeof(c->val));
-	c->last = calloc(clients->num_classes, sizeof(c->last));
-	assert(c->val && c->last);
-
-	update_client(c, pid, name);
-}
-
-static void free_client(struct client *c)
-{
-	if (c->sysfs_root >= 0)
-		close(c->sysfs_root);
-	if (c->busy_root >= 0)
-		close(c->busy_root);
-	free(c->val);
-	free(c->last);
-	memset(c, 0, sizeof(*c));
-}
-
-static int
-read_client_sysfs(char *buf, int bufsize, const char *sysfs_root,
-		  unsigned int id, const char *field, int *client_root)
-{
-	ssize_t ret;
-
-	if (*client_root < 0) {
-		char namebuf[256];
-
-		ret = snprintf(namebuf, sizeof(namebuf), "%s/%u",
-			       sysfs_root, id);
-		assert(ret > 0 && ret < sizeof(namebuf));
-		if (ret <= 0 || ret == sizeof(namebuf))
-			return -1;
-
-		*client_root = open(namebuf, O_RDONLY | O_DIRECTORY);
-	}
-
-	if (*client_root < 0)
-		return -1;
-
-	return __read_client_field(*client_root, field, buf, bufsize);
-}
-
-static int client_last_cmp(const void *_a, const void *_b)
-{
-	const struct client *a = _a;
-	const struct client *b = _b;
-	long tot_a, tot_b;
-
-	/*
-	 * Sort clients in descending order of runtime in the previous sampling
-	 * period for active ones, followed by inactive. Tie-breaker is client
-	 * id.
-	 */
-
-	tot_a = a->status == ALIVE ? a->last_runtime : -1;
-	tot_b = b->status == ALIVE ? b->last_runtime : -1;
-
-	tot_b -= tot_a;
-	if (tot_b > 0)
-		return 1;
-	if (tot_b < 0)
-		return -1;
-
-	return (int)b->id - a->id;
-}
-
-static int client_total_cmp(const void *_a, const void *_b)
-{
-	const struct client *a = _a;
-	const struct client *b = _b;
-	long tot_a, tot_b;
-
-	tot_a = a->status == ALIVE ? a->total_runtime : -1;
-	tot_b = b->status == ALIVE ? b->total_runtime : -1;
-
-	tot_b -= tot_a;
-	if (tot_b > 0)
-		return 1;
-	if (tot_b < 0)
-		return -1;
-
-	return (int)b->id - a->id;
-}
-
-static int client_id_cmp(const void *_a, const void *_b)
-{
-	const struct client *a = _a;
-	const struct client *b = _b;
-	int id_a, id_b;
-
-	id_a = a->status == ALIVE ? a->id : -1;
-	id_b = b->status == ALIVE ? b->id : -1;
-
-	id_b -= id_a;
-	if (id_b > 0)
-		return 1;
-	if (id_b < 0)
-		return -1;
-
-	return (int)b->id - a->id;
-}
-
-static int client_pid_cmp(const void *_a, const void *_b)
-{
-	const struct client *a = _a;
-	const struct client *b = _b;
-	int pid_a, pid_b;
-
-	pid_a = a->status == ALIVE ? a->pid : INT_MAX;
-	pid_b = b->status == ALIVE ? b->pid : INT_MAX;
-
-	pid_b -= pid_a;
-	if (pid_b > 0)
-		return -1;
-	if (pid_b < 0)
-		return 1;
-
-	return (int)a->id - b->id;
-}
-
-static int (*client_cmp)(const void *, const void *) = client_last_cmp;
-
-static struct clients *sort_clients(struct clients *clients,
-				    int (*cmp)(const void *, const void *))
-{
-	unsigned int active, free;
-	struct client *c;
-	int tmp;
-
-	if (!clients)
-		return clients;
-
-	qsort(clients->client, clients->num_clients, sizeof(*clients->client),
-	      cmp);
-
-	/* Trim excessive array space. */
-	active = 0;
-	for_each_client(clients, c, tmp) {
-		if (c->status != ALIVE)
-			break; /* Active clients are first in the array. */
-		active++;
-	}
-
-	clients->active_clients = active;
-
-	free = clients->num_clients - active;
-	if (free > clients->num_clients / 2) {
-		active = clients->num_clients - free / 2;
-		if (active != clients->num_clients) {
-			clients->num_clients = active;
-			clients->client = realloc(clients->client,
-						  clients->num_clients *
-						  sizeof(*c));
-		}
-	}
-
-	return clients;
-}
-
-static bool aggregate_pids = true;
-
-static struct clients *display_clients(struct clients *clients)
-{
-	struct client *ac, *c, *cp = NULL;
-	struct clients *aggregated;
-	int tmp, num = 0;
-
-	if (!aggregate_pids)
-		goto out;
-
-	/* Sort by pid first to make it easy to aggregate while walking. */
-	sort_clients(clients, client_pid_cmp);
-
-	aggregated = calloc(1, sizeof(*clients));
-	assert(aggregated);
-
-	ac = calloc(clients->num_clients, sizeof(*c));
-	assert(ac);
-
-	aggregated->num_classes = clients->num_classes;
-	aggregated->class = clients->class;
-	aggregated->client = ac;
-
-	for_each_client(clients, c, tmp) {
-		unsigned int i;
-
-		if (c->status == FREE)
-			break;
-
-		assert(c->status == ALIVE);
-
-		if ((cp && c->pid != cp->pid) || !cp) {
-			ac = &aggregated->client[num++];
-
-			/* New pid. */
-			ac->clients = aggregated;
-			ac->status = ALIVE;
-			ac->id = -c->pid;
-			ac->pid = c->pid;
-			ac->busy_root = -1;
-			ac->sysfs_root = -1;
-			strcpy(ac->name, c->name);
-			strcpy(ac->print_name, c->print_name);
-			ac->engines = c->engines;
-			ac->val = calloc(clients->num_classes,
-					 sizeof(ac->val[0]));
-			assert(ac->val);
-			ac->samples = 1;
-		}
-
-		cp = c;
-
-		if (c->samples < 2)
-			continue;
-
-		ac->samples = 2; /* All what matters for display. */
-		ac->total_runtime += c->total_runtime;
-		ac->last_runtime += c->last_runtime;
-
-		for (i = 0; i < clients->num_classes; i++)
-			ac->val[i] += c->val[i];
-	}
-
-	aggregated->num_clients = num;
-	aggregated->active_clients = num;
-
-	clients = aggregated;
-
-out:
-	return sort_clients(clients, client_cmp);
-}
-
-static void free_clients(struct clients *clients)
-{
-	struct client *c;
-	unsigned int tmp;
-
-	for_each_client(clients, c, tmp) {
-		free(c->val);
-		free(c->last);
-	}
-
-	free(clients->client);
-	free(clients);
-}
-
-static struct clients *scan_clients(struct clients *clients)
-{
-	struct dirent *dent;
-	struct client *c;
-	unsigned int id;
-	int tmp;
-	DIR *d;
-
-	if (!clients)
-		return clients;
-
-	for_each_client(clients, c, tmp) {
-		assert(c->status != PROBE);
-		if (c->status == ALIVE)
-			c->status = PROBE;
-		else
-			break; /* Free block at the end of array. */
-	}
-
-	d = opendir(clients->sysfs_root);
-	if (!d)
-		return clients;
-
-	while ((dent = readdir(d)) != NULL) {
-		char name[24], pid[24];
-		int ret, root = -1, *pr;
-
-		if (dent->d_type != DT_DIR)
-			continue;
-		if (!isdigit(dent->d_name[0]))
-			continue;
-
-		id = atoi(dent->d_name);
-
-		c = find_client(clients, PROBE, id);
-
-		if (c)
-			pr = &c->sysfs_root;
-		else
-			pr = &root;
-
-		ret = read_client_sysfs(name, sizeof(name), clients->sysfs_root,
-					id, "name", pr);
-		ret |= read_client_sysfs(pid, sizeof(pid), clients->sysfs_root,
-					id, "pid", pr);
-		if (!ret) {
-			if (!c)
-				add_client(clients, id, atoi(pid), name, root);
-			else
-				update_client(c, atoi(pid), name);
-		} else if (c) {
-			c->status = PROBE; /* Will be deleted below. */
-		}
-	}
-
-	closedir(d);
-
-	for_each_client(clients, c, tmp) {
-		if (c->status == PROBE)
-			free_client(c);
-		else if (c->status == FREE)
-			break;
-	}
-
-	return display_clients(clients);
-}
-
 static const char *bars[] = { " ", "▏", "▎", "▍", "▌", "▋", "▊", "▉", "█" };
 
 static void n_spaces(const unsigned int n)
@@ -1324,18 +768,6 @@ json_close_struct(void)
 		fflush(stdout);
 }
 
-static void
-__json_add_member(const char *key, const char *val)
-{
-	assert(json_indent_level < ARRAY_SIZE(json_indent));
-
-	fprintf(out, "%s%s\"%s\": \"%s\"",
-		json_struct_members ? ",\n" : "",
-		json_indent[json_indent_level], key, val);
-
-	json_struct_members++;
-}
-
 static unsigned int
 json_add_member(const struct cnt_group *parent, struct cnt_item *item,
 		unsigned int headers)
@@ -2061,157 +1493,6 @@ print_engines(struct engines *engines, double t, int lines, int w, int h)
 	return lines;
 }
 
-static int
-print_clients_header(struct clients *clients, int lines,
-		     int con_w, int con_h, int *class_w)
-{
-	if (output_mode == INTERACTIVE) {
-		const char *pidname = "   PID              NAME ";
-		unsigned int num_active = 0;
-		int len = strlen(pidname);
-
-		if (lines++ >= con_h)
-			return lines;
-
-		printf("\033[7m");
-		printf("%s", pidname);
-
-		if (lines++ >= con_h || len >= con_w)
-			return lines;
-
-		if (clients->num_classes) {
-			unsigned int i;
-			int width;
-
-			for (i = 0; i < clients->num_classes; i++) {
-				if (clients->class[i].num_engines)
-					num_active++;
-			}
-
-			*class_w = width = (con_w - len) / num_active;
-
-			for (i = 0; i < clients->num_classes; i++) {
-				const char *name = clients->class[i].name;
-				int name_len = strlen(name);
-				int pad = (width - name_len) / 2;
-				int spaces = width - pad - name_len;
-
-				if (!clients->class[i].num_engines)
-					continue; /* Assert in the ideal world. */
-
-				if (pad < 0 || spaces < 0)
-					continue;
-
-				n_spaces(pad);
-				printf("%s", name);
-				n_spaces(spaces);
-				len += pad + name_len + spaces;
-			}
-		}
-
-		n_spaces(con_w - len);
-		printf("\033[0m\n");
-	} else {
-		if (clients->num_classes)
-			pops->open_struct("clients");
-	}
-
-	return lines;
-}
-
-static bool numeric_clients;
-static bool filter_idle;
-
-static int
-print_client(struct client *c, struct engines *engines, double t, int lines,
-	     int con_w, int con_h, unsigned int period_us, int *class_w)
-{
-	struct clients *clients = c->clients;
-	unsigned int i;
-
-	if (output_mode == INTERACTIVE) {
-		if (filter_idle && (!c->total_runtime || c->samples < 2))
-			return lines;
-
-		lines++;
-
-		printf("%6u %17s ", c->pid, c->print_name);
-
-		for (i = 0; c->samples > 1 && i < clients->num_classes; i++) {
-			double pct;
-
-			if (!clients->class[i].num_engines)
-				continue; /* Assert in the ideal world. */
-
-			pct = (double)c->val[i] / period_us / 1e3 * 100 /
-			      clients->class[i].num_engines;
-
-			/*
-			 * Guard against possible time-drift between sampling
-			 * client data and time we obtained our time-delta from
-			 * PMU.
-			 */
-			if (pct > 100.0)
-				pct = 100.0;
-
-			print_percentage_bar(pct, *class_w, numeric_clients);
-		}
-
-		putchar('\n');
-	} else if (output_mode == JSON) {
-		char buf[64];
-
-		snprintf(buf, sizeof(buf), "%u", c->id);
-		pops->open_struct(buf);
-
-		__json_add_member("name", c->print_name);
-
-		snprintf(buf, sizeof(buf), "%u", c->pid);
-		__json_add_member("pid", buf);
-
-		if (c->samples > 1) {
-			pops->open_struct("engine-classes");
-
-			for (i = 0; i < clients->num_classes; i++) {
-				double pct;
-
-				snprintf(buf, sizeof(buf), "%s",
-					clients->class[i].name);
-				pops->open_struct(buf);
-
-				pct = (double)c->val[i] / period_us / 1e3 * 100;
-				snprintf(buf, sizeof(buf), "%f", pct);
-				__json_add_member("busy", buf);
-
-				__json_add_member("unit", "%");
-
-				pops->close_struct();
-			}
-
-			pops->close_struct();
-		}
-
-		pops->close_struct();
-	}
-
-	return lines;
-}
-
-static int
-print_clients_footer(struct clients *clients, double t,
-		     int lines, int con_w, int con_h)
-{
-	if (output_mode == INTERACTIVE) {
-		if (lines++ < con_h)
-			printf("\n");
-	} else {
-		if (clients->num_classes)
-			pops->close_struct();
-	}
-
-	return lines;
-}
-
 static bool stop_top;
 
 static void sigint_handler(int  sig)
@@ -2267,31 +1548,6 @@ static void interactive_stdin(void)
 	assert(ret == 0);
 }
 
-static void select_client_sort(void)
-{
-	struct {
-		int (*cmp)(const void *, const void *);
-		const char *msg;
-	} cmp[] = {
-		{ client_last_cmp, "Sorting clients by current GPU usage." },
-		{ client_total_cmp, "Sorting clients by accummulated GPU usage." },
-		{ client_pid_cmp, "Sorting clients by pid." },
-		{ client_id_cmp, "Sorting clients by sysfs id." },
-	};
-	static unsigned int client_sort;
-
-bump:
-	if (++client_sort >= ARRAY_SIZE(cmp))
-		client_sort = 0;
-
-	client_cmp = cmp[client_sort].cmp;
-	header_msg = cmp[client_sort].msg;
-
-	/* Sort by client id makes no sense with pid aggregation. */
-	if (aggregate_pids && client_cmp == client_id_cmp)
-		goto bump;
-}
-
 static bool in_help;
 
 static void process_help_stdin(void)
@@ -2334,29 +1590,9 @@ static void process_normal_stdin(void)
 			else
 				header_msg = "Showing physical engines.";
 			break;
-		case 'i':
-			filter_idle ^= true;
-			if (filter_idle)
-				header_msg = "Hiding inactive clients.";
-			else
-				header_msg = "Showing inactive clients.";
-			break;
-		case 'n':
-			numeric_clients ^= true;
-			break;
-		case 's':
-			select_client_sort();
-			break;
 		case 'h':
 			in_help = true;
 			break;
-		case 'H':
-			aggregate_pids ^= true;
-			if (aggregate_pids)
-				header_msg = "Aggregating clients.";
-			else
-				header_msg = "Showing individual clients.";
-			break;
 		};
 	}
 }
@@ -2384,10 +1620,6 @@ static void show_help_screen(void)
 	printf(
 "Help for interactive commands:\n\n"
 "    '1'    Toggle between aggregated engine class and physical engine mode.\n"
-"    'n'    Toggle display of numeric client busyness overlay.\n"
-"    's'    Toggle between sort modes (runtime, total runtime, pid, client id).\n"
-"    'i'    Toggle display of clients which used no GPU time.\n"
-"    'H'    Toggle between per PID aggregation and individual clients.\n"
 "\n"
 "    'h' or 'q'    Exit interactive help.\n"
 "\n");
@@ -2396,7 +1628,6 @@ static void show_help_screen(void)
 int main(int argc, char **argv)
 {
 	unsigned int period_us = DEFAULT_PERIOD_MS * 1000;
-	struct clients *clients = NULL;
 	int con_w = -1, con_h = -1;
 	char *output_path = NULL;
 	struct engines *engines;
@@ -2530,23 +1761,15 @@ int main(int argc, char **argv)
 
 	ret = EXIT_SUCCESS;
 
-	clients = init_clients(card.pci_slot_name[0] ? card.card : NULL);
 	init_engine_classes(engines);
-	if (clients) {
-		clients->num_classes = engines->num_classes;
-		clients->class = engines->class;
-	}
 
 	pmu_sample(engines);
-	scan_clients(clients);
 	codename = igt_device_get_pretty_name(&card, false);
 
 	while (!stop_top) {
-		struct clients *disp_clients;
 		bool consumed = false;
-		int j, lines = 0;
 		struct winsize ws;
-		struct client *c;
+		int lines = 0;
 		double t;
 
 		/* Update terminal size. */
@@ -2565,8 +1788,6 @@ int main(int argc, char **argv)
 		pmu_sample(engines);
 		t = (double)(engines->ts.cur - engines->ts.prev) / 1e9;
 
-		disp_clients = scan_clients(clients);
-
 		if (stop_top)
 			break;
 
@@ -2586,41 +1807,12 @@ int main(int argc, char **argv)
 
 			lines = print_engines(engines, t, lines, con_w, con_h);
 
-			if (disp_clients) {
-				int class_w;
-
-				lines = print_clients_header(disp_clients, lines,
-							     con_w, con_h,
-							     &class_w);
-
-				for_each_client(disp_clients, c, j) {
-					assert(c->status != PROBE);
-					if (c->status != ALIVE)
-						break; /* Active clients are first in the array. */
-
-					if (lines >= con_h)
-						break;
-
-					lines = print_client(c, engines, t,
-							     lines, con_w,
-							     con_h, period_us,
-							     &class_w);
-				}
-
-				lines = print_clients_footer(disp_clients, t,
-							     lines, con_w,
-							     con_h);
-			}
-
 			pops->close_struct();
 		}
 
 		if (stop_top)
 			break;
 
-		if (disp_clients != clients)
-			free_clients(disp_clients);
-
 		if (output_mode == INTERACTIVE)
 			process_stdin(period_us);
 		else
-- 
2.32.0

[igt-dev] [PATCH i-g-t 5/6] intel_gpu_top: Remove clients support

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

When kernel feature was removed the intel_gpu_top part was forgotten.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 man/intel_gpu_top.rst |   4 -
 tools/intel_gpu_top.c | 810 +-----------------------------------------
 2 files changed, 1 insertion(+), 813 deletions(-)

diff --git a/man/intel_gpu_top.rst b/man/intel_gpu_top.rst
index f4dbfc5b44d9..b3b765b05feb 100644
--- a/man/intel_gpu_top.rst
+++ b/man/intel_gpu_top.rst
@@ -56,10 +56,6 @@ Supported keys:
     'q'    Exit from the tool.
     'h'    Show interactive help.
     '1'    Toggle between aggregated engine class and physical engine mode.
-    'n'    Toggle display of numeric client busyness overlay.
-    's'    Toggle between sort modes (runtime, total runtime, pid, client id).
-    'i'    Toggle display of clients which used no GPU time.
-    'H'    Toggle between per PID aggregation and individual clients.
 
 DEVICE SELECTION
 ================
diff --git a/tools/intel_gpu_top.c b/tools/intel_gpu_top.c
index 7311038a39f4..41c59a72c09d 100644
--- a/tools/intel_gpu_top.c
+++ b/tools/intel_gpu_top.c
@@ -627,562 +627,6 @@ static void pmu_sample(struct engines *engines)
 	}
 }
 
-enum client_status {
-	FREE = 0, /* mbz */
-	ALIVE,
-	PROBE
-};
-
-struct clients;
-
-struct client {
-	struct clients *clients;
-
-	enum client_status status;
-	int sysfs_root;
-	int busy_root;
-	unsigned int id;
-	unsigned int pid;
-	char name[24];
-	char print_name[24];
-	unsigned int samples;
-	unsigned long total_runtime;
-	unsigned long last_runtime;
-	struct engines *engines;
-	unsigned long *val;
-	uint64_t *last;
-};
-
-struct clients {
-	unsigned int num_clients;
-	unsigned int active_clients;
-
-	unsigned int num_classes;
-	struct engine_class *class;
-
-	char sysfs_root[128];
-
-	struct client *client;
-};
-
-#define for_each_client(clients, c, tmp) \
-	for ((tmp) = (clients)->num_clients, c = (clients)->client; \
-	     (tmp > 0); (tmp)--, (c)++)
-
-static struct clients *init_clients(const char *drm_card)
-{
-	struct clients *clients;
-	const char *slash;
-	ssize_t ret;
-	int dir;
-
-	clients = malloc(sizeof(*clients));
-	if (!clients)
-		return NULL;
-
-	memset(clients, 0, sizeof(*clients));
-
-	if (drm_card) {
-		slash = rindex(drm_card, '/');
-		assert(slash);
-	} else {
-		slash = "card0";
-	}
-
-	ret = snprintf(clients->sysfs_root, sizeof(clients->sysfs_root),
-		       "/sys/class/drm/%s/clients/", slash);
-	assert(ret > 0 && ret < sizeof(clients->sysfs_root));
-
-	dir = open(clients->sysfs_root, O_DIRECTORY | O_RDONLY);
-	if (dir < 0) {
-		free(clients);
-		clients = NULL;
-	} else {
-		close(dir);
-	}
-
-	return clients;
-}
-
-static int __read_to_buf(int fd, char *buf, unsigned int bufsize)
-{
-	ssize_t ret;
-	int err;
-
-	ret = read(fd, buf, bufsize - 1);
-	err = errno;
-	if (ret < 1) {
-		errno = ret < 0 ? err : ENOMSG;
-
-		return -1;
-	}
-
-	if (ret > 1 && buf[ret - 1] == '\n')
-		buf[ret - 1] = '\0';
-	else
-		buf[ret] = '\0';
-
-	return 0;
-}
-
-static int
-__read_client_field(int root, const char *field, char *buf, unsigned int bufsize)
-{
-	int fd, ret;
-
-	fd = openat(root, field, O_RDONLY);
-	if (fd < 0)
-		return -1;
-
-	ret = __read_to_buf(fd, buf, bufsize);
-
-	close(fd);
-
-	return ret;
-}
-
-static uint64_t
-read_client_busy(struct client *client, unsigned int class)
-{
-	const char *class_str[] = { "0", "1", "2", "3", "4", "5", "6", "7" };
-	char buf[256], *b;
-	int ret;
-
-	assert(class < ARRAY_SIZE(class_str));
-	if (class >= ARRAY_SIZE(class_str))
-		return 0;
-
-	assert(client->sysfs_root >= 0);
-	if (client->sysfs_root < 0)
-		return 0;
-
-	if (client->busy_root < 0)
-		client->busy_root = openat(client->sysfs_root, "busy",
-					   O_RDONLY | O_DIRECTORY);
-
-	assert(client->busy_root);
-	if (client->busy_root < 0)
-		return 0;
-
-	ret = __read_client_field(client->busy_root, class_str[class], buf,
-				  sizeof(buf));
-	if (ret) {
-		close(client->busy_root);
-		client->busy_root = -1;
-		return 0;
-	}
-
-	/*
-	 * Handle both single integer and key=value formats by skipping
-	 * leading non-digits.
-	 */
-	b = buf;
-	while (*b && !isdigit(*b))
-		b++;
-
-	return strtoull(b, NULL, 10);
-}
-
-static struct client *
-find_client(struct clients *clients, enum client_status status, unsigned int id)
-{
-	unsigned int start, num;
-	struct client *c;
-
-	start = status == FREE ? clients->active_clients : 0; /* Free block at the end. */
-	num = clients->num_clients - start;
-
-	for (c = &clients->client[start]; num; c++, num--) {
-		if (status != c->status)
-			continue;
-
-		if (status == FREE || c->id == id)
-			return c;
-	}
-
-	return NULL;
-}
-
-static void update_client(struct client *c, unsigned int pid, char *name)
-{
-	uint64_t val[c->clients->num_classes];
-	unsigned int i;
-
-	if (c->pid != pid)
-		c->pid = pid;
-
-	if (strcmp(c->name, name)) {
-		char *p;
-
-		strncpy(c->name, name, sizeof(c->name) - 1);
-		strncpy(c->print_name, name, sizeof(c->print_name) - 1);
-
-		p = c->print_name;
-		while (*p) {
-			if (!isprint(*p))
-				*p = '*';
-			p++;
-		}
-	}
-
-	for (i = 0; i < c->clients->num_classes; i++)
-		val[i] = read_client_busy(c, c->clients->class[i].class);
-
-	c->last_runtime = 0;
-	c->total_runtime = 0;
-
-	for (i = 0; i < c->clients->num_classes; i++) {
-		if (val[i] < c->last[i])
-			continue; /* It will catch up soon. */
-
-		c->total_runtime += val[i];
-		c->val[i] = val[i] - c->last[i];
-		c->last_runtime += c->val[i];
-		c->last[i] = val[i];
-	}
-
-	c->samples++;
-	c->status = ALIVE;
-}
-
-static void
-add_client(struct clients *clients, unsigned int id, unsigned int pid,
-	   char *name, int sysfs_root)
-{
-	struct client *c;
-
-	assert(!find_client(clients, ALIVE, id));
-
-	c = find_client(clients, FREE, 0);
-	if (!c) {
-		unsigned int idx = clients->num_clients;
-
-		clients->num_clients += (clients->num_clients + 2) / 2;
-		clients->client = realloc(clients->client,
-					  clients->num_clients * sizeof(*c));
-		assert(clients->client);
-
-		c = &clients->client[idx];
-		memset(c, 0, (clients->num_clients - idx) * sizeof(*c));
-	}
-
-	c->sysfs_root = sysfs_root;
-	c->busy_root = -1;
-	c->id = id;
-	c->clients = clients;
-	c->val = calloc(clients->num_classes, sizeof(c->val));
-	c->last = calloc(clients->num_classes, sizeof(c->last));
-	assert(c->val && c->last);
-
-	update_client(c, pid, name);
-}
-
-static void free_client(struct client *c)
-{
-	if (c->sysfs_root >= 0)
-		close(c->sysfs_root);
-	if (c->busy_root >= 0)
-		close(c->busy_root);
-	free(c->val);
-	free(c->last);
-	memset(c, 0, sizeof(*c));
-}
-
-static int
-read_client_sysfs(char *buf, int bufsize, const char *sysfs_root,
-		  unsigned int id, const char *field, int *client_root)
-{
-	ssize_t ret;
-
-	if (*client_root < 0) {
-		char namebuf[256];
-
-		ret = snprintf(namebuf, sizeof(namebuf), "%s/%u",
-			       sysfs_root, id);
-		assert(ret > 0 && ret < sizeof(namebuf));
-		if (ret <= 0 || ret == sizeof(namebuf))
-			return -1;
-
-		*client_root = open(namebuf, O_RDONLY | O_DIRECTORY);
-	}
-
-	if (*client_root < 0)
-		return -1;
-
-	return __read_client_field(*client_root, field, buf, bufsize);
-}
-
-static int client_last_cmp(const void *_a, const void *_b)
-{
-	const struct client *a = _a;
-	const struct client *b = _b;
-	long tot_a, tot_b;
-
-	/*
-	 * Sort clients in descending order of runtime in the previous sampling
-	 * period for active ones, followed by inactive. Tie-breaker is client
-	 * id.
-	 */
-
-	tot_a = a->status == ALIVE ? a->last_runtime : -1;
-	tot_b = b->status == ALIVE ? b->last_runtime : -1;
-
-	tot_b -= tot_a;
-	if (tot_b > 0)
-		return 1;
-	if (tot_b < 0)
-		return -1;
-
-	return (int)b->id - a->id;
-}
-
-static int client_total_cmp(const void *_a, const void *_b)
-{
-	const struct client *a = _a;
-	const struct client *b = _b;
-	long tot_a, tot_b;
-
-	tot_a = a->status == ALIVE ? a->total_runtime : -1;
-	tot_b = b->status == ALIVE ? b->total_runtime : -1;
-
-	tot_b -= tot_a;
-	if (tot_b > 0)
-		return 1;
-	if (tot_b < 0)
-		return -1;
-
-	return (int)b->id - a->id;
-}
-
-static int client_id_cmp(const void *_a, const void *_b)
-{
-	const struct client *a = _a;
-	const struct client *b = _b;
-	int id_a, id_b;
-
-	id_a = a->status == ALIVE ? a->id : -1;
-	id_b = b->status == ALIVE ? b->id : -1;
-
-	id_b -= id_a;
-	if (id_b > 0)
-		return 1;
-	if (id_b < 0)
-		return -1;
-
-	return (int)b->id - a->id;
-}
-
-static int client_pid_cmp(const void *_a, const void *_b)
-{
-	const struct client *a = _a;
-	const struct client *b = _b;
-	int pid_a, pid_b;
-
-	pid_a = a->status == ALIVE ? a->pid : INT_MAX;
-	pid_b = b->status == ALIVE ? b->pid : INT_MAX;
-
-	pid_b -= pid_a;
-	if (pid_b > 0)
-		return -1;
-	if (pid_b < 0)
-		return 1;
-
-	return (int)a->id - b->id;
-}
-
-static int (*client_cmp)(const void *, const void *) = client_last_cmp;
-
-static struct clients *sort_clients(struct clients *clients,
-				    int (*cmp)(const void *, const void *))
-{
-	unsigned int active, free;
-	struct client *c;
-	int tmp;
-
-	if (!clients)
-		return clients;
-
-	qsort(clients->client, clients->num_clients, sizeof(*clients->client),
-	      cmp);
-
-	/* Trim excessive array space. */
-	active = 0;
-	for_each_client(clients, c, tmp) {
-		if (c->status != ALIVE)
-			break; /* Active clients are first in the array. */
-		active++;
-	}
-
-	clients->active_clients = active;
-
-	free = clients->num_clients - active;
-	if (free > clients->num_clients / 2) {
-		active = clients->num_clients - free / 2;
-		if (active != clients->num_clients) {
-			clients->num_clients = active;
-			clients->client = realloc(clients->client,
-						  clients->num_clients *
-						  sizeof(*c));
-		}
-	}
-
-	return clients;
-}
-
-static bool aggregate_pids = true;
-
-static struct clients *display_clients(struct clients *clients)
-{
-	struct client *ac, *c, *cp = NULL;
-	struct clients *aggregated;
-	int tmp, num = 0;
-
-	if (!aggregate_pids)
-		goto out;
-
-	/* Sort by pid first to make it easy to aggregate while walking. */
-	sort_clients(clients, client_pid_cmp);
-
-	aggregated = calloc(1, sizeof(*clients));
-	assert(aggregated);
-
-	ac = calloc(clients->num_clients, sizeof(*c));
-	assert(ac);
-
-	aggregated->num_classes = clients->num_classes;
-	aggregated->class = clients->class;
-	aggregated->client = ac;
-
-	for_each_client(clients, c, tmp) {
-		unsigned int i;
-
-		if (c->status == FREE)
-			break;
-
-		assert(c->status == ALIVE);
-
-		if ((cp && c->pid != cp->pid) || !cp) {
-			ac = &aggregated->client[num++];
-
-			/* New pid. */
-			ac->clients = aggregated;
-			ac->status = ALIVE;
-			ac->id = -c->pid;
-			ac->pid = c->pid;
-			ac->busy_root = -1;
-			ac->sysfs_root = -1;
-			strcpy(ac->name, c->name);
-			strcpy(ac->print_name, c->print_name);
-			ac->engines = c->engines;
-			ac->val = calloc(clients->num_classes,
-					 sizeof(ac->val[0]));
-			assert(ac->val);
-			ac->samples = 1;
-		}
-
-		cp = c;
-
-		if (c->samples < 2)
-			continue;
-
-		ac->samples = 2; /* All what matters for display. */
-		ac->total_runtime += c->total_runtime;
-		ac->last_runtime += c->last_runtime;
-
-		for (i = 0; i < clients->num_classes; i++)
-			ac->val[i] += c->val[i];
-	}
-
-	aggregated->num_clients = num;
-	aggregated->active_clients = num;
-
-	clients = aggregated;
-
-out:
-	return sort_clients(clients, client_cmp);
-}
-
-static void free_clients(struct clients *clients)
-{
-	struct client *c;
-	unsigned int tmp;
-
-	for_each_client(clients, c, tmp) {
-		free(c->val);
-		free(c->last);
-	}
-
-	free(clients->client);
-	free(clients);
-}
-
-static struct clients *scan_clients(struct clients *clients)
-{
-	struct dirent *dent;
-	struct client *c;
-	unsigned int id;
-	int tmp;
-	DIR *d;
-
-	if (!clients)
-		return clients;
-
-	for_each_client(clients, c, tmp) {
-		assert(c->status != PROBE);
-		if (c->status == ALIVE)
-			c->status = PROBE;
-		else
-			break; /* Free block at the end of array. */
-	}
-
-	d = opendir(clients->sysfs_root);
-	if (!d)
-		return clients;
-
-	while ((dent = readdir(d)) != NULL) {
-		char name[24], pid[24];
-		int ret, root = -1, *pr;
-
-		if (dent->d_type != DT_DIR)
-			continue;
-		if (!isdigit(dent->d_name[0]))
-			continue;
-
-		id = atoi(dent->d_name);
-
-		c = find_client(clients, PROBE, id);
-
-		if (c)
-			pr = &c->sysfs_root;
-		else
-			pr = &root;
-
-		ret = read_client_sysfs(name, sizeof(name), clients->sysfs_root,
-					id, "name", pr);
-		ret |= read_client_sysfs(pid, sizeof(pid), clients->sysfs_root,
-					id, "pid", pr);
-		if (!ret) {
-			if (!c)
-				add_client(clients, id, atoi(pid), name, root);
-			else
-				update_client(c, atoi(pid), name);
-		} else if (c) {
-			c->status = PROBE; /* Will be deleted below. */
-		}
-	}
-
-	closedir(d);
-
-	for_each_client(clients, c, tmp) {
-		if (c->status == PROBE)
-			free_client(c);
-		else if (c->status == FREE)
-			break;
-	}
-
-	return display_clients(clients);
-}
-
 static const char *bars[] = { " ", "▏", "▎", "▍", "▌", "▋", "▊", "▉", "█" };
 
 static void n_spaces(const unsigned int n)
@@ -1324,18 +768,6 @@ json_close_struct(void)
 		fflush(stdout);
 }
 
-static void
-__json_add_member(const char *key, const char *val)
-{
-	assert(json_indent_level < ARRAY_SIZE(json_indent));
-
-	fprintf(out, "%s%s\"%s\": \"%s\"",
-		json_struct_members ? ",\n" : "",
-		json_indent[json_indent_level], key, val);
-
-	json_struct_members++;
-}
-
 static unsigned int
 json_add_member(const struct cnt_group *parent, struct cnt_item *item,
 		unsigned int headers)
@@ -2061,157 +1493,6 @@ print_engines(struct engines *engines, double t, int lines, int w, int h)
 	return lines;
 }
 
-static int
-print_clients_header(struct clients *clients, int lines,
-		     int con_w, int con_h, int *class_w)
-{
-	if (output_mode == INTERACTIVE) {
-		const char *pidname = "   PID              NAME ";
-		unsigned int num_active = 0;
-		int len = strlen(pidname);
-
-		if (lines++ >= con_h)
-			return lines;
-
-		printf("\033[7m");
-		printf("%s", pidname);
-
-		if (lines++ >= con_h || len >= con_w)
-			return lines;
-
-		if (clients->num_classes) {
-			unsigned int i;
-			int width;
-
-			for (i = 0; i < clients->num_classes; i++) {
-				if (clients->class[i].num_engines)
-					num_active++;
-			}
-
-			*class_w = width = (con_w - len) / num_active;
-
-			for (i = 0; i < clients->num_classes; i++) {
-				const char *name = clients->class[i].name;
-				int name_len = strlen(name);
-				int pad = (width - name_len) / 2;
-				int spaces = width - pad - name_len;
-
-				if (!clients->class[i].num_engines)
-					continue; /* Assert in the ideal world. */
-
-				if (pad < 0 || spaces < 0)
-					continue;
-
-				n_spaces(pad);
-				printf("%s", name);
-				n_spaces(spaces);
-				len += pad + name_len + spaces;
-			}
-		}
-
-		n_spaces(con_w - len);
-		printf("\033[0m\n");
-	} else {
-		if (clients->num_classes)
-			pops->open_struct("clients");
-	}
-
-	return lines;
-}
-
-static bool numeric_clients;
-static bool filter_idle;
-
-static int
-print_client(struct client *c, struct engines *engines, double t, int lines,
-	     int con_w, int con_h, unsigned int period_us, int *class_w)
-{
-	struct clients *clients = c->clients;
-	unsigned int i;
-
-	if (output_mode == INTERACTIVE) {
-		if (filter_idle && (!c->total_runtime || c->samples < 2))
-			return lines;
-
-		lines++;
-
-		printf("%6u %17s ", c->pid, c->print_name);
-
-		for (i = 0; c->samples > 1 && i < clients->num_classes; i++) {
-			double pct;
-
-			if (!clients->class[i].num_engines)
-				continue; /* Assert in the ideal world. */
-
-			pct = (double)c->val[i] / period_us / 1e3 * 100 /
-			      clients->class[i].num_engines;
-
-			/*
-			 * Guard against possible time-drift between sampling
-			 * client data and time we obtained our time-delta from
-			 * PMU.
-			 */
-			if (pct > 100.0)
-				pct = 100.0;
-
-			print_percentage_bar(pct, *class_w, numeric_clients);
-		}
-
-		putchar('\n');
-	} else if (output_mode == JSON) {
-		char buf[64];
-
-		snprintf(buf, sizeof(buf), "%u", c->id);
-		pops->open_struct(buf);
-
-		__json_add_member("name", c->print_name);
-
-		snprintf(buf, sizeof(buf), "%u", c->pid);
-		__json_add_member("pid", buf);
-
-		if (c->samples > 1) {
-			pops->open_struct("engine-classes");
-
-			for (i = 0; i < clients->num_classes; i++) {
-				double pct;
-
-				snprintf(buf, sizeof(buf), "%s",
-					clients->class[i].name);
-				pops->open_struct(buf);
-
-				pct = (double)c->val[i] / period_us / 1e3 * 100;
-				snprintf(buf, sizeof(buf), "%f", pct);
-				__json_add_member("busy", buf);
-
-				__json_add_member("unit", "%");
-
-				pops->close_struct();
-			}
-
-			pops->close_struct();
-		}
-
-		pops->close_struct();
-	}
-
-	return lines;
-}
-
-static int
-print_clients_footer(struct clients *clients, double t,
-		     int lines, int con_w, int con_h)
-{
-	if (output_mode == INTERACTIVE) {
-		if (lines++ < con_h)
-			printf("\n");
-	} else {
-		if (clients->num_classes)
-			pops->close_struct();
-	}
-
-	return lines;
-}
-
 static bool stop_top;
 
 static void sigint_handler(int  sig)
@@ -2267,31 +1548,6 @@ static void interactive_stdin(void)
 	assert(ret == 0);
 }
 
-static void select_client_sort(void)
-{
-	struct {
-		int (*cmp)(const void *, const void *);
-		const char *msg;
-	} cmp[] = {
-		{ client_last_cmp, "Sorting clients by current GPU usage." },
-		{ client_total_cmp, "Sorting clients by accummulated GPU usage." },
-		{ client_pid_cmp, "Sorting clients by pid." },
-		{ client_id_cmp, "Sorting clients by sysfs id." },
-	};
-	static unsigned int client_sort;
-
-bump:
-	if (++client_sort >= ARRAY_SIZE(cmp))
-		client_sort = 0;
-
-	client_cmp = cmp[client_sort].cmp;
-	header_msg = cmp[client_sort].msg;
-
-	/* Sort by client id makes no sense with pid aggregation. */
-	if (aggregate_pids && client_cmp == client_id_cmp)
-		goto bump;
-}
-
 static bool in_help;
 
 static void process_help_stdin(void)
@@ -2334,29 +1590,9 @@ static void process_normal_stdin(void)
 			else
 				header_msg = "Showing physical engines.";
 			break;
-		case 'i':
-			filter_idle ^= true;
-			if (filter_idle)
-				header_msg = "Hiding inactive clients.";
-			else
-				header_msg = "Showing inactive clients.";
-			break;
-		case 'n':
-			numeric_clients ^= true;
-			break;
-		case 's':
-			select_client_sort();
-			break;
 		case 'h':
 			in_help = true;
 			break;
-		case 'H':
-			aggregate_pids ^= true;
-			if (aggregate_pids)
-				header_msg = "Aggregating clients.";
-			else
-				header_msg = "Showing individual clients.";
-			break;
 		};
 	}
 }
@@ -2384,10 +1620,6 @@ static void show_help_screen(void)
 	printf(
 "Help for interactive commands:\n\n"
 "    '1'    Toggle between aggregated engine class and physical engine mode.\n"
-"    'n'    Toggle display of numeric client busyness overlay.\n"
-"    's'    Toggle between sort modes (runtime, total runtime, pid, client id).\n"
-"    'i'    Toggle display of clients which used no GPU time.\n"
-"    'H'    Toggle between per PID aggregation and individual clients.\n"
 "\n"
 "    'h' or 'q'    Exit interactive help.\n"
 "\n");
@@ -2396,7 +1628,6 @@ static void show_help_screen(void)
 int main(int argc, char **argv)
 {
 	unsigned int period_us = DEFAULT_PERIOD_MS * 1000;
-	struct clients *clients = NULL;
 	int con_w = -1, con_h = -1;
 	char *output_path = NULL;
 	struct engines *engines;
@@ -2530,23 +1761,15 @@ int main(int argc, char **argv)
 
 	ret = EXIT_SUCCESS;
 
-	clients = init_clients(card.pci_slot_name[0] ? card.card : NULL);
 	init_engine_classes(engines);
-	if (clients) {
-		clients->num_classes = engines->num_classes;
-		clients->class = engines->class;
-	}
 
 	pmu_sample(engines);
-	scan_clients(clients);
 	codename = igt_device_get_pretty_name(&card, false);
 
 	while (!stop_top) {
-		struct clients *disp_clients;
 		bool consumed = false;
-		int j, lines = 0;
 		struct winsize ws;
-		struct client *c;
+		int lines = 0;
 		double t;
 
 		/* Update terminal size. */
@@ -2565,8 +1788,6 @@ int main(int argc, char **argv)
 		pmu_sample(engines);
 		t = (double)(engines->ts.cur - engines->ts.prev) / 1e9;
 
-		disp_clients = scan_clients(clients);
-
 		if (stop_top)
 			break;
 
@@ -2586,41 +1807,12 @@ int main(int argc, char **argv)
 
 			lines = print_engines(engines, t, lines, con_w, con_h);
 
-			if (disp_clients) {
-				int class_w;
-
-				lines = print_clients_header(disp_clients, lines,
-							     con_w, con_h,
-							     &class_w);
-
-				for_each_client(disp_clients, c, j) {
-					assert(c->status != PROBE);
-					if (c->status != ALIVE)
-						break; /* Active clients are first in the array. */
-
-					if (lines >= con_h)
-						break;
-
-					lines = print_client(c, engines, t,
-							     lines, con_w,
-							     con_h, period_us,
-							     &class_w);
-				}
-
-				lines = print_clients_footer(disp_clients, t,
-							     lines, con_w,
-							     con_h);
-			}
-
 			pops->close_struct();
 		}
 
 		if (stop_top)
 			break;
 
-		if (disp_clients != clients)
-			free_clients(disp_clients);
-
 		if (output_mode == INTERACTIVE)
 			process_stdin(period_us);
 		else
-- 
2.32.0

Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 5/6] intel_gpu_top: Remove clients support

[Tvrtko Ursulin]

On 19/11/2021 12:59, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> When kernel feature was removed the intel_gpu_top part was forgotten.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>

Will someone ack this or we carry this code until it ships, if it hasn't 
already?

Regards,

Tvrtko

> ---
>   man/intel_gpu_top.rst |   4 -
>   tools/intel_gpu_top.c | 810 +-----------------------------------------
>   2 files changed, 1 insertion(+), 813 deletions(-)
> 
> diff --git a/man/intel_gpu_top.rst b/man/intel_gpu_top.rst
> index f4dbfc5b44d9..b3b765b05feb 100644
> --- a/man/intel_gpu_top.rst
> +++ b/man/intel_gpu_top.rst
> @@ -56,10 +56,6 @@ Supported keys:
>       'q'    Exit from the tool.
>       'h'    Show interactive help.
>       '1'    Toggle between aggregated engine class and physical engine mode.
> -    'n'    Toggle display of numeric client busyness overlay.
> -    's'    Toggle between sort modes (runtime, total runtime, pid, client id).
> -    'i'    Toggle display of clients which used no GPU time.
> -    'H'    Toggle between per PID aggregation and individual clients.
>   
>   DEVICE SELECTION
>   ================
> diff --git a/tools/intel_gpu_top.c b/tools/intel_gpu_top.c
> index 7311038a39f4..41c59a72c09d 100644
> --- a/tools/intel_gpu_top.c
> +++ b/tools/intel_gpu_top.c
> @@ -627,562 +627,6 @@ static void pmu_sample(struct engines *engines)
>   	}
>   }
>   
> -enum client_status {
> -	FREE = 0, /* mbz */
> -	ALIVE,
> -	PROBE
> -};
> -
> -struct clients;
> -
> -struct client {
> -	struct clients *clients;
> -
> -	enum client_status status;
> -	int sysfs_root;
> -	int busy_root;
> -	unsigned int id;
> -	unsigned int pid;
> -	char name[24];
> -	char print_name[24];
> -	unsigned int samples;
> -	unsigned long total_runtime;
> -	unsigned long last_runtime;
> -	struct engines *engines;
> -	unsigned long *val;
> -	uint64_t *last;
> -};
> -
> -struct clients {
> -	unsigned int num_clients;
> -	unsigned int active_clients;
> -
> -	unsigned int num_classes;
> -	struct engine_class *class;
> -
> -	char sysfs_root[128];
> -
> -	struct client *client;
> -};
> -
> -#define for_each_client(clients, c, tmp) \
> -	for ((tmp) = (clients)->num_clients, c = (clients)->client; \
> -	     (tmp > 0); (tmp)--, (c)++)
> -
> -static struct clients *init_clients(const char *drm_card)
> -{
> -	struct clients *clients;
> -	const char *slash;
> -	ssize_t ret;
> -	int dir;
> -
> -	clients = malloc(sizeof(*clients));
> -	if (!clients)
> -		return NULL;
> -
> -	memset(clients, 0, sizeof(*clients));
> -
> -	if (drm_card) {
> -		slash = rindex(drm_card, '/');
> -		assert(slash);
> -	} else {
> -		slash = "card0";
> -	}
> -
> -	ret = snprintf(clients->sysfs_root, sizeof(clients->sysfs_root),
> -		       "/sys/class/drm/%s/clients/", slash);
> -	assert(ret > 0 && ret < sizeof(clients->sysfs_root));
> -
> -	dir = open(clients->sysfs_root, O_DIRECTORY | O_RDONLY);
> -	if (dir < 0) {
> -		free(clients);
> -		clients = NULL;
> -	} else {
> -		close(dir);
> -	}
> -
> -	return clients;
> -}
> -
> -static int __read_to_buf(int fd, char *buf, unsigned int bufsize)
> -{
> -	ssize_t ret;
> -	int err;
> -
> -	ret = read(fd, buf, bufsize - 1);
> -	err = errno;
> -	if (ret < 1) {
> -		errno = ret < 0 ? err : ENOMSG;
> -
> -		return -1;
> -	}
> -
> -	if (ret > 1 && buf[ret - 1] == '\n')
> -		buf[ret - 1] = '\0';
> -	else
> -		buf[ret] = '\0';
> -
> -	return 0;
> -}
> -
> -static int
> -__read_client_field(int root, const char *field, char *buf, unsigned int bufsize)
> -{
> -	int fd, ret;
> -
> -	fd = openat(root, field, O_RDONLY);
> -	if (fd < 0)
> -		return -1;
> -
> -	ret = __read_to_buf(fd, buf, bufsize);
> -
> -	close(fd);
> -
> -	return ret;
> -}
> -
> -static uint64_t
> -read_client_busy(struct client *client, unsigned int class)
> -{
> -	const char *class_str[] = { "0", "1", "2", "3", "4", "5", "6", "7" };
> -	char buf[256], *b;
> -	int ret;
> -
> -	assert(class < ARRAY_SIZE(class_str));
> -	if (class >= ARRAY_SIZE(class_str))
> -		return 0;
> -
> -	assert(client->sysfs_root >= 0);
> -	if (client->sysfs_root < 0)
> -		return 0;
> -
> -	if (client->busy_root < 0)
> -		client->busy_root = openat(client->sysfs_root, "busy",
> -					   O_RDONLY | O_DIRECTORY);
> -
> -	assert(client->busy_root);
> -	if (client->busy_root < 0)
> -		return 0;
> -
> -	ret = __read_client_field(client->busy_root, class_str[class], buf,
> -				  sizeof(buf));
> -	if (ret) {
> -		close(client->busy_root);
> -		client->busy_root = -1;
> -		return 0;
> -	}
> -
> -	/*
> -	 * Handle both single integer and key=value formats by skipping
> -	 * leading non-digits.
> -	 */
> -	b = buf;
> -	while (*b && !isdigit(*b))
> -		b++;
> -
> -	return strtoull(b, NULL, 10);
> -}
> -
> -static struct client *
> -find_client(struct clients *clients, enum client_status status, unsigned int id)
> -{
> -	unsigned int start, num;
> -	struct client *c;
> -
> -	start = status == FREE ? clients->active_clients : 0; /* Free block at the end. */
> -	num = clients->num_clients - start;
> -
> -	for (c = &clients->client[start]; num; c++, num--) {
> -		if (status != c->status)
> -			continue;
> -
> -		if (status == FREE || c->id == id)
> -			return c;
> -	}
> -
> -	return NULL;
> -}
> -
> -static void update_client(struct client *c, unsigned int pid, char *name)
> -{
> -	uint64_t val[c->clients->num_classes];
> -	unsigned int i;
> -
> -	if (c->pid != pid)
> -		c->pid = pid;
> -
> -	if (strcmp(c->name, name)) {
> -		char *p;
> -
> -		strncpy(c->name, name, sizeof(c->name) - 1);
> -		strncpy(c->print_name, name, sizeof(c->print_name) - 1);
> -
> -		p = c->print_name;
> -		while (*p) {
> -			if (!isprint(*p))
> -				*p = '*';
> -			p++;
> -		}
> -	}
> -
> -	for (i = 0; i < c->clients->num_classes; i++)
> -		val[i] = read_client_busy(c, c->clients->class[i].class);
> -
> -	c->last_runtime = 0;
> -	c->total_runtime = 0;
> -
> -	for (i = 0; i < c->clients->num_classes; i++) {
> -		if (val[i] < c->last[i])
> -			continue; /* It will catch up soon. */
> -
> -		c->total_runtime += val[i];
> -		c->val[i] = val[i] - c->last[i];
> -		c->last_runtime += c->val[i];
> -		c->last[i] = val[i];
> -	}
> -
> -	c->samples++;
> -	c->status = ALIVE;
> -}
> -
> -static void
> -add_client(struct clients *clients, unsigned int id, unsigned int pid,
> -	   char *name, int sysfs_root)
> -{
> -	struct client *c;
> -
> -	assert(!find_client(clients, ALIVE, id));
> -
> -	c = find_client(clients, FREE, 0);
> -	if (!c) {
> -		unsigned int idx = clients->num_clients;
> -
> -		clients->num_clients += (clients->num_clients + 2) / 2;
> -		clients->client = realloc(clients->client,
> -					  clients->num_clients * sizeof(*c));
> -		assert(clients->client);
> -
> -		c = &clients->client[idx];
> -		memset(c, 0, (clients->num_clients - idx) * sizeof(*c));
> -	}
> -
> -	c->sysfs_root = sysfs_root;
> -	c->busy_root = -1;
> -	c->id = id;
> -	c->clients = clients;
> -	c->val = calloc(clients->num_classes, sizeof(c->val));
> -	c->last = calloc(clients->num_classes, sizeof(c->last));
> -	assert(c->val && c->last);
> -
> -	update_client(c, pid, name);
> -}
> -
> -static void free_client(struct client *c)
> -{
> -	if (c->sysfs_root >= 0)
> -		close(c->sysfs_root);
> -	if (c->busy_root >= 0)
> -		close(c->busy_root);
> -	free(c->val);
> -	free(c->last);
> -	memset(c, 0, sizeof(*c));
> -}
> -
> -static int
> -read_client_sysfs(char *buf, int bufsize, const char *sysfs_root,
> -		  unsigned int id, const char *field, int *client_root)
> -{
> -	ssize_t ret;
> -
> -	if (*client_root < 0) {
> -		char namebuf[256];
> -
> -		ret = snprintf(namebuf, sizeof(namebuf), "%s/%u",
> -			       sysfs_root, id);
> -		assert(ret > 0 && ret < sizeof(namebuf));
> -		if (ret <= 0 || ret == sizeof(namebuf))
> -			return -1;
> -
> -		*client_root = open(namebuf, O_RDONLY | O_DIRECTORY);
> -	}
> -
> -	if (*client_root < 0)
> -		return -1;
> -
> -	return __read_client_field(*client_root, field, buf, bufsize);
> -}
> -
> -static int client_last_cmp(const void *_a, const void *_b)
> -{
> -	const struct client *a = _a;
> -	const struct client *b = _b;
> -	long tot_a, tot_b;
> -
> -	/*
> -	 * Sort clients in descending order of runtime in the previous sampling
> -	 * period for active ones, followed by inactive. Tie-breaker is client
> -	 * id.
> -	 */
> -
> -	tot_a = a->status == ALIVE ? a->last_runtime : -1;
> -	tot_b = b->status == ALIVE ? b->last_runtime : -1;
> -
> -	tot_b -= tot_a;
> -	if (tot_b > 0)
> -		return 1;
> -	if (tot_b < 0)
> -		return -1;
> -
> -	return (int)b->id - a->id;
> -}
> -
> -static int client_total_cmp(const void *_a, const void *_b)
> -{
> -	const struct client *a = _a;
> -	const struct client *b = _b;
> -	long tot_a, tot_b;
> -
> -	tot_a = a->status == ALIVE ? a->total_runtime : -1;
> -	tot_b = b->status == ALIVE ? b->total_runtime : -1;
> -
> -	tot_b -= tot_a;
> -	if (tot_b > 0)
> -		return 1;
> -	if (tot_b < 0)
> -		return -1;
> -
> -	return (int)b->id - a->id;
> -}
> -
> -static int client_id_cmp(const void *_a, const void *_b)
> -{
> -	const struct client *a = _a;
> -	const struct client *b = _b;
> -	int id_a, id_b;
> -
> -	id_a = a->status == ALIVE ? a->id : -1;
> -	id_b = b->status == ALIVE ? b->id : -1;
> -
> -	id_b -= id_a;
> -	if (id_b > 0)
> -		return 1;
> -	if (id_b < 0)
> -		return -1;
> -
> -	return (int)b->id - a->id;
> -}
> -
> -static int client_pid_cmp(const void *_a, const void *_b)
> -{
> -	const struct client *a = _a;
> -	const struct client *b = _b;
> -	int pid_a, pid_b;
> -
> -	pid_a = a->status == ALIVE ? a->pid : INT_MAX;
> -	pid_b = b->status == ALIVE ? b->pid : INT_MAX;
> -
> -	pid_b -= pid_a;
> -	if (pid_b > 0)
> -		return -1;
> -	if (pid_b < 0)
> -		return 1;
> -
> -	return (int)a->id - b->id;
> -}
> -
> -static int (*client_cmp)(const void *, const void *) = client_last_cmp;
> -
> -static struct clients *sort_clients(struct clients *clients,
> -				    int (*cmp)(const void *, const void *))
> -{
> -	unsigned int active, free;
> -	struct client *c;
> -	int tmp;
> -
> -	if (!clients)
> -		return clients;
> -
> -	qsort(clients->client, clients->num_clients, sizeof(*clients->client),
> -	      cmp);
> -
> -	/* Trim excessive array space. */
> -	active = 0;
> -	for_each_client(clients, c, tmp) {
> -		if (c->status != ALIVE)
> -			break; /* Active clients are first in the array. */
> -		active++;
> -	}
> -
> -	clients->active_clients = active;
> -
> -	free = clients->num_clients - active;
> -	if (free > clients->num_clients / 2) {
> -		active = clients->num_clients - free / 2;
> -		if (active != clients->num_clients) {
> -			clients->num_clients = active;
> -			clients->client = realloc(clients->client,
> -						  clients->num_clients *
> -						  sizeof(*c));
> -		}
> -	}
> -
> -	return clients;
> -}
> -
> -static bool aggregate_pids = true;
> -
> -static struct clients *display_clients(struct clients *clients)
> -{
> -	struct client *ac, *c, *cp = NULL;
> -	struct clients *aggregated;
> -	int tmp, num = 0;
> -
> -	if (!aggregate_pids)
> -		goto out;
> -
> -	/* Sort by pid first to make it easy to aggregate while walking. */
> -	sort_clients(clients, client_pid_cmp);
> -
> -	aggregated = calloc(1, sizeof(*clients));
> -	assert(aggregated);
> -
> -	ac = calloc(clients->num_clients, sizeof(*c));
> -	assert(ac);
> -
> -	aggregated->num_classes = clients->num_classes;
> -	aggregated->class = clients->class;
> -	aggregated->client = ac;
> -
> -	for_each_client(clients, c, tmp) {
> -		unsigned int i;
> -
> -		if (c->status == FREE)
> -			break;
> -
> -		assert(c->status == ALIVE);
> -
> -		if ((cp && c->pid != cp->pid) || !cp) {
> -			ac = &aggregated->client[num++];
> -
> -			/* New pid. */
> -			ac->clients = aggregated;
> -			ac->status = ALIVE;
> -			ac->id = -c->pid;
> -			ac->pid = c->pid;
> -			ac->busy_root = -1;
> -			ac->sysfs_root = -1;
> -			strcpy(ac->name, c->name);
> -			strcpy(ac->print_name, c->print_name);
> -			ac->engines = c->engines;
> -			ac->val = calloc(clients->num_classes,
> -					 sizeof(ac->val[0]));
> -			assert(ac->val);
> -			ac->samples = 1;
> -		}
> -
> -		cp = c;
> -
> -		if (c->samples < 2)
> -			continue;
> -
> -		ac->samples = 2; /* All what matters for display. */
> -		ac->total_runtime += c->total_runtime;
> -		ac->last_runtime += c->last_runtime;
> -
> -		for (i = 0; i < clients->num_classes; i++)
> -			ac->val[i] += c->val[i];
> -	}
> -
> -	aggregated->num_clients = num;
> -	aggregated->active_clients = num;
> -
> -	clients = aggregated;
> -
> -out:
> -	return sort_clients(clients, client_cmp);
> -}
> -
> -static void free_clients(struct clients *clients)
> -{
> -	struct client *c;
> -	unsigned int tmp;
> -
> -	for_each_client(clients, c, tmp) {
> -		free(c->val);
> -		free(c->last);
> -	}
> -
> -	free(clients->client);
> -	free(clients);
> -}
> -
> -static struct clients *scan_clients(struct clients *clients)
> -{
> -	struct dirent *dent;
> -	struct client *c;
> -	unsigned int id;
> -	int tmp;
> -	DIR *d;
> -
> -	if (!clients)
> -		return clients;
> -
> -	for_each_client(clients, c, tmp) {
> -		assert(c->status != PROBE);
> -		if (c->status == ALIVE)
> -			c->status = PROBE;
> -		else
> -			break; /* Free block at the end of array. */
> -	}
> -
> -	d = opendir(clients->sysfs_root);
> -	if (!d)
> -		return clients;
> -
> -	while ((dent = readdir(d)) != NULL) {
> -		char name[24], pid[24];
> -		int ret, root = -1, *pr;
> -
> -		if (dent->d_type != DT_DIR)
> -			continue;
> -		if (!isdigit(dent->d_name[0]))
> -			continue;
> -
> -		id = atoi(dent->d_name);
> -
> -		c = find_client(clients, PROBE, id);
> -
> -		if (c)
> -			pr = &c->sysfs_root;
> -		else
> -			pr = &root;
> -
> -		ret = read_client_sysfs(name, sizeof(name), clients->sysfs_root,
> -					id, "name", pr);
> -		ret |= read_client_sysfs(pid, sizeof(pid), clients->sysfs_root,
> -					id, "pid", pr);
> -		if (!ret) {
> -			if (!c)
> -				add_client(clients, id, atoi(pid), name, root);
> -			else
> -				update_client(c, atoi(pid), name);
> -		} else if (c) {
> -			c->status = PROBE; /* Will be deleted below. */
> -		}
> -	}
> -
> -	closedir(d);
> -
> -	for_each_client(clients, c, tmp) {
> -		if (c->status == PROBE)
> -			free_client(c);
> -		else if (c->status == FREE)
> -			break;
> -	}
> -
> -	return display_clients(clients);
> -}
> -
>   static const char *bars[] = { " ", "▏", "▎", "▍", "▌", "▋", "▊", "▉", "█" };
>   
>   static void n_spaces(const unsigned int n)
> @@ -1324,18 +768,6 @@ json_close_struct(void)
>   		fflush(stdout);
>   }
>   
> -static void
> -__json_add_member(const char *key, const char *val)
> -{
> -	assert(json_indent_level < ARRAY_SIZE(json_indent));
> -
> -	fprintf(out, "%s%s\"%s\": \"%s\"",
> -		json_struct_members ? ",\n" : "",
> -		json_indent[json_indent_level], key, val);
> -
> -	json_struct_members++;
> -}
> -
>   static unsigned int
>   json_add_member(const struct cnt_group *parent, struct cnt_item *item,
>   		unsigned int headers)
> @@ -2061,157 +1493,6 @@ print_engines(struct engines *engines, double t, int lines, int w, int h)
>   	return lines;
>   }
>   
> -static int
> -print_clients_header(struct clients *clients, int lines,
> -		     int con_w, int con_h, int *class_w)
> -{
> -	if (output_mode == INTERACTIVE) {
> -		const char *pidname = "   PID              NAME ";
> -		unsigned int num_active = 0;
> -		int len = strlen(pidname);
> -
> -		if (lines++ >= con_h)
> -			return lines;
> -
> -		printf("\033[7m");
> -		printf("%s", pidname);
> -
> -		if (lines++ >= con_h || len >= con_w)
> -			return lines;
> -
> -		if (clients->num_classes) {
> -			unsigned int i;
> -			int width;
> -
> -			for (i = 0; i < clients->num_classes; i++) {
> -				if (clients->class[i].num_engines)
> -					num_active++;
> -			}
> -
> -			*class_w = width = (con_w - len) / num_active;
> -
> -			for (i = 0; i < clients->num_classes; i++) {
> -				const char *name = clients->class[i].name;
> -				int name_len = strlen(name);
> -				int pad = (width - name_len) / 2;
> -				int spaces = width - pad - name_len;
> -
> -				if (!clients->class[i].num_engines)
> -					continue; /* Assert in the ideal world. */
> -
> -				if (pad < 0 || spaces < 0)
> -					continue;
> -
> -				n_spaces(pad);
> -				printf("%s", name);
> -				n_spaces(spaces);
> -				len += pad + name_len + spaces;
> -			}
> -		}
> -
> -		n_spaces(con_w - len);
> -		printf("\033[0m\n");
> -	} else {
> -		if (clients->num_classes)
> -			pops->open_struct("clients");
> -	}
> -
> -	return lines;
> -}
> -
> -static bool numeric_clients;
> -static bool filter_idle;
> -
> -static int
> -print_client(struct client *c, struct engines *engines, double t, int lines,
> -	     int con_w, int con_h, unsigned int period_us, int *class_w)
> -{
> -	struct clients *clients = c->clients;
> -	unsigned int i;
> -
> -	if (output_mode == INTERACTIVE) {
> -		if (filter_idle && (!c->total_runtime || c->samples < 2))
> -			return lines;
> -
> -		lines++;
> -
> -		printf("%6u %17s ", c->pid, c->print_name);
> -
> -		for (i = 0; c->samples > 1 && i < clients->num_classes; i++) {
> -			double pct;
> -
> -			if (!clients->class[i].num_engines)
> -				continue; /* Assert in the ideal world. */
> -
> -			pct = (double)c->val[i] / period_us / 1e3 * 100 /
> -			      clients->class[i].num_engines;
> -
> -			/*
> -			 * Guard against possible time-drift between sampling
> -			 * client data and time we obtained our time-delta from
> -			 * PMU.
> -			 */
> -			if (pct > 100.0)
> -				pct = 100.0;
> -
> -			print_percentage_bar(pct, *class_w, numeric_clients);
> -		}
> -
> -		putchar('\n');
> -	} else if (output_mode == JSON) {
> -		char buf[64];
> -
> -		snprintf(buf, sizeof(buf), "%u", c->id);
> -		pops->open_struct(buf);
> -
> -		__json_add_member("name", c->print_name);
> -
> -		snprintf(buf, sizeof(buf), "%u", c->pid);
> -		__json_add_member("pid", buf);
> -
> -		if (c->samples > 1) {
> -			pops->open_struct("engine-classes");
> -
> -			for (i = 0; i < clients->num_classes; i++) {
> -				double pct;
> -
> -				snprintf(buf, sizeof(buf), "%s",
> -					clients->class[i].name);
> -				pops->open_struct(buf);
> -
> -				pct = (double)c->val[i] / period_us / 1e3 * 100;
> -				snprintf(buf, sizeof(buf), "%f", pct);
> -				__json_add_member("busy", buf);
> -
> -				__json_add_member("unit", "%");
> -
> -				pops->close_struct();
> -			}
> -
> -			pops->close_struct();
> -		}
> -
> -		pops->close_struct();
> -	}
> -
> -	return lines;
> -}
> -
> -static int
> -print_clients_footer(struct clients *clients, double t,
> -		     int lines, int con_w, int con_h)
> -{
> -	if (output_mode == INTERACTIVE) {
> -		if (lines++ < con_h)
> -			printf("\n");
> -	} else {
> -		if (clients->num_classes)
> -			pops->close_struct();
> -	}
> -
> -	return lines;
> -}
> -
>   static bool stop_top;
>   
>   static void sigint_handler(int  sig)
> @@ -2267,31 +1548,6 @@ static void interactive_stdin(void)
>   	assert(ret == 0);
>   }
>   
> -static void select_client_sort(void)
> -{
> -	struct {
> -		int (*cmp)(const void *, const void *);
> -		const char *msg;
> -	} cmp[] = {
> -		{ client_last_cmp, "Sorting clients by current GPU usage." },
> -		{ client_total_cmp, "Sorting clients by accummulated GPU usage." },
> -		{ client_pid_cmp, "Sorting clients by pid." },
> -		{ client_id_cmp, "Sorting clients by sysfs id." },
> -	};
> -	static unsigned int client_sort;
> -
> -bump:
> -	if (++client_sort >= ARRAY_SIZE(cmp))
> -		client_sort = 0;
> -
> -	client_cmp = cmp[client_sort].cmp;
> -	header_msg = cmp[client_sort].msg;
> -
> -	/* Sort by client id makes no sense with pid aggregation. */
> -	if (aggregate_pids && client_cmp == client_id_cmp)
> -		goto bump;
> -}
> -
>   static bool in_help;
>   
>   static void process_help_stdin(void)
> @@ -2334,29 +1590,9 @@ static void process_normal_stdin(void)
>   			else
>   				header_msg = "Showing physical engines.";
>   			break;
> -		case 'i':
> -			filter_idle ^= true;
> -			if (filter_idle)
> -				header_msg = "Hiding inactive clients.";
> -			else
> -				header_msg = "Showing inactive clients.";
> -			break;
> -		case 'n':
> -			numeric_clients ^= true;
> -			break;
> -		case 's':
> -			select_client_sort();
> -			break;
>   		case 'h':
>   			in_help = true;
>   			break;
> -		case 'H':
> -			aggregate_pids ^= true;
> -			if (aggregate_pids)
> -				header_msg = "Aggregating clients.";
> -			else
> -				header_msg = "Showing individual clients.";
> -			break;
>   		};
>   	}
>   }
> @@ -2384,10 +1620,6 @@ static void show_help_screen(void)
>   	printf(
>   "Help for interactive commands:\n\n"
>   "    '1'    Toggle between aggregated engine class and physical engine mode.\n"
> -"    'n'    Toggle display of numeric client busyness overlay.\n"
> -"    's'    Toggle between sort modes (runtime, total runtime, pid, client id).\n"
> -"    'i'    Toggle display of clients which used no GPU time.\n"
> -"    'H'    Toggle between per PID aggregation and individual clients.\n"
>   "\n"
>   "    'h' or 'q'    Exit interactive help.\n"
>   "\n");
> @@ -2396,7 +1628,6 @@ static void show_help_screen(void)
>   int main(int argc, char **argv)
>   {
>   	unsigned int period_us = DEFAULT_PERIOD_MS * 1000;
> -	struct clients *clients = NULL;
>   	int con_w = -1, con_h = -1;
>   	char *output_path = NULL;
>   	struct engines *engines;
> @@ -2530,23 +1761,15 @@ int main(int argc, char **argv)
>   
>   	ret = EXIT_SUCCESS;
>   
> -	clients = init_clients(card.pci_slot_name[0] ? card.card : NULL);
>   	init_engine_classes(engines);
> -	if (clients) {
> -		clients->num_classes = engines->num_classes;
> -		clients->class = engines->class;
> -	}
>   
>   	pmu_sample(engines);
> -	scan_clients(clients);
>   	codename = igt_device_get_pretty_name(&card, false);
>   
>   	while (!stop_top) {
> -		struct clients *disp_clients;
>   		bool consumed = false;
> -		int j, lines = 0;
>   		struct winsize ws;
> -		struct client *c;
> +		int lines = 0;
>   		double t;
>   
>   		/* Update terminal size. */
> @@ -2565,8 +1788,6 @@ int main(int argc, char **argv)
>   		pmu_sample(engines);
>   		t = (double)(engines->ts.cur - engines->ts.prev) / 1e9;
>   
> -		disp_clients = scan_clients(clients);
> -
>   		if (stop_top)
>   			break;
>   
> @@ -2586,41 +1807,12 @@ int main(int argc, char **argv)
>   
>   			lines = print_engines(engines, t, lines, con_w, con_h);
>   
> -			if (disp_clients) {
> -				int class_w;
> -
> -				lines = print_clients_header(disp_clients, lines,
> -							     con_w, con_h,
> -							     &class_w);
> -
> -				for_each_client(disp_clients, c, j) {
> -					assert(c->status != PROBE);
> -					if (c->status != ALIVE)
> -						break; /* Active clients are first in the array. */
> -
> -					if (lines >= con_h)
> -						break;
> -
> -					lines = print_client(c, engines, t,
> -							     lines, con_w,
> -							     con_h, period_us,
> -							     &class_w);
> -				}
> -
> -				lines = print_clients_footer(disp_clients, t,
> -							     lines, con_w,
> -							     con_h);
> -			}
> -
>   			pops->close_struct();
>   		}
>   
>   		if (stop_top)
>   			break;
>   
> -		if (disp_clients != clients)
> -			free_clients(disp_clients);
> -
>   		if (output_mode == INTERACTIVE)
>   			process_stdin(period_us);
>   		else
> 

Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 5/6] intel_gpu_top: Remove clients support

[Petri Latvala]

On Fri, Nov 26, 2021 at 01:07:24PM +0000, Tvrtko Ursulin wrote:
> 
> On 19/11/2021 12:59, Tvrtko Ursulin wrote:
> > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > 
> > When kernel feature was removed the intel_gpu_top part was forgotten.
> > 
> > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> Will someone ack this or we carry this code until it ships, if it hasn't
> already?

Does that question mean client busyness will some day return?

Either way,
Acked-by: Petri Latvala <petri.latvala@intel.com>


> 
> Regards,
> 
> Tvrtko
> 
> > ---
> >   man/intel_gpu_top.rst |   4 -
> >   tools/intel_gpu_top.c | 810 +-----------------------------------------
> >   2 files changed, 1 insertion(+), 813 deletions(-)
> > 
> > diff --git a/man/intel_gpu_top.rst b/man/intel_gpu_top.rst
> > index f4dbfc5b44d9..b3b765b05feb 100644
> > --- a/man/intel_gpu_top.rst
> > +++ b/man/intel_gpu_top.rst
> > @@ -56,10 +56,6 @@ Supported keys:
> >       'q'    Exit from the tool.
> >       'h'    Show interactive help.
> >       '1'    Toggle between aggregated engine class and physical engine mode.
> > -    'n'    Toggle display of numeric client busyness overlay.
> > -    's'    Toggle between sort modes (runtime, total runtime, pid, client id).
> > -    'i'    Toggle display of clients which used no GPU time.
> > -    'H'    Toggle between per PID aggregation and individual clients.
> >   DEVICE SELECTION
> >   ================
> > diff --git a/tools/intel_gpu_top.c b/tools/intel_gpu_top.c
> > index 7311038a39f4..41c59a72c09d 100644
> > --- a/tools/intel_gpu_top.c
> > +++ b/tools/intel_gpu_top.c
> > @@ -627,562 +627,6 @@ static void pmu_sample(struct engines *engines)
> >   	}
> >   }
> > -enum client_status {
> > -	FREE = 0, /* mbz */
> > -	ALIVE,
> > -	PROBE
> > -};
> > -
> > -struct clients;
> > -
> > -struct client {
> > -	struct clients *clients;
> > -
> > -	enum client_status status;
> > -	int sysfs_root;
> > -	int busy_root;
> > -	unsigned int id;
> > -	unsigned int pid;
> > -	char name[24];
> > -	char print_name[24];
> > -	unsigned int samples;
> > -	unsigned long total_runtime;
> > -	unsigned long last_runtime;
> > -	struct engines *engines;
> > -	unsigned long *val;
> > -	uint64_t *last;
> > -};
> > -
> > -struct clients {
> > -	unsigned int num_clients;
> > -	unsigned int active_clients;
> > -
> > -	unsigned int num_classes;
> > -	struct engine_class *class;
> > -
> > -	char sysfs_root[128];
> > -
> > -	struct client *client;
> > -};
> > -
> > -#define for_each_client(clients, c, tmp) \
> > -	for ((tmp) = (clients)->num_clients, c = (clients)->client; \
> > -	     (tmp > 0); (tmp)--, (c)++)
> > -
> > -static struct clients *init_clients(const char *drm_card)
> > -{
> > -	struct clients *clients;
> > -	const char *slash;
> > -	ssize_t ret;
> > -	int dir;
> > -
> > -	clients = malloc(sizeof(*clients));
> > -	if (!clients)
> > -		return NULL;
> > -
> > -	memset(clients, 0, sizeof(*clients));
> > -
> > -	if (drm_card) {
> > -		slash = rindex(drm_card, '/');
> > -		assert(slash);
> > -	} else {
> > -		slash = "card0";
> > -	}
> > -
> > -	ret = snprintf(clients->sysfs_root, sizeof(clients->sysfs_root),
> > -		       "/sys/class/drm/%s/clients/", slash);
> > -	assert(ret > 0 && ret < sizeof(clients->sysfs_root));
> > -
> > -	dir = open(clients->sysfs_root, O_DIRECTORY | O_RDONLY);
> > -	if (dir < 0) {
> > -		free(clients);
> > -		clients = NULL;
> > -	} else {
> > -		close(dir);
> > -	}
> > -
> > -	return clients;
> > -}
> > -
> > -static int __read_to_buf(int fd, char *buf, unsigned int bufsize)
> > -{
> > -	ssize_t ret;
> > -	int err;
> > -
> > -	ret = read(fd, buf, bufsize - 1);
> > -	err = errno;
> > -	if (ret < 1) {
> > -		errno = ret < 0 ? err : ENOMSG;
> > -
> > -		return -1;
> > -	}
> > -
> > -	if (ret > 1 && buf[ret - 1] == '\n')
> > -		buf[ret - 1] = '\0';
> > -	else
> > -		buf[ret] = '\0';
> > -
> > -	return 0;
> > -}
> > -
> > -static int
> > -__read_client_field(int root, const char *field, char *buf, unsigned int bufsize)
> > -{
> > -	int fd, ret;
> > -
> > -	fd = openat(root, field, O_RDONLY);
> > -	if (fd < 0)
> > -		return -1;
> > -
> > -	ret = __read_to_buf(fd, buf, bufsize);
> > -
> > -	close(fd);
> > -
> > -	return ret;
> > -}
> > -
> > -static uint64_t
> > -read_client_busy(struct client *client, unsigned int class)
> > -{
> > -	const char *class_str[] = { "0", "1", "2", "3", "4", "5", "6", "7" };
> > -	char buf[256], *b;
> > -	int ret;
> > -
> > -	assert(class < ARRAY_SIZE(class_str));
> > -	if (class >= ARRAY_SIZE(class_str))
> > -		return 0;
> > -
> > -	assert(client->sysfs_root >= 0);
> > -	if (client->sysfs_root < 0)
> > -		return 0;
> > -
> > -	if (client->busy_root < 0)
> > -		client->busy_root = openat(client->sysfs_root, "busy",
> > -					   O_RDONLY | O_DIRECTORY);
> > -
> > -	assert(client->busy_root);
> > -	if (client->busy_root < 0)
> > -		return 0;
> > -
> > -	ret = __read_client_field(client->busy_root, class_str[class], buf,
> > -				  sizeof(buf));
> > -	if (ret) {
> > -		close(client->busy_root);
> > -		client->busy_root = -1;
> > -		return 0;
> > -	}
> > -
> > -	/*
> > -	 * Handle both single integer and key=value formats by skipping
> > -	 * leading non-digits.
> > -	 */
> > -	b = buf;
> > -	while (*b && !isdigit(*b))
> > -		b++;
> > -
> > -	return strtoull(b, NULL, 10);
> > -}
> > -
> > -static struct client *
> > -find_client(struct clients *clients, enum client_status status, unsigned int id)
> > -{
> > -	unsigned int start, num;
> > -	struct client *c;
> > -
> > -	start = status == FREE ? clients->active_clients : 0; /* Free block at the end. */
> > -	num = clients->num_clients - start;
> > -
> > -	for (c = &clients->client[start]; num; c++, num--) {
> > -		if (status != c->status)
> > -			continue;
> > -
> > -		if (status == FREE || c->id == id)
> > -			return c;
> > -	}
> > -
> > -	return NULL;
> > -}
> > -
> > -static void update_client(struct client *c, unsigned int pid, char *name)
> > -{
> > -	uint64_t val[c->clients->num_classes];
> > -	unsigned int i;
> > -
> > -	if (c->pid != pid)
> > -		c->pid = pid;
> > -
> > -	if (strcmp(c->name, name)) {
> > -		char *p;
> > -
> > -		strncpy(c->name, name, sizeof(c->name) - 1);
> > -		strncpy(c->print_name, name, sizeof(c->print_name) - 1);
> > -
> > -		p = c->print_name;
> > -		while (*p) {
> > -			if (!isprint(*p))
> > -				*p = '*';
> > -			p++;
> > -		}
> > -	}
> > -
> > -	for (i = 0; i < c->clients->num_classes; i++)
> > -		val[i] = read_client_busy(c, c->clients->class[i].class);
> > -
> > -	c->last_runtime = 0;
> > -	c->total_runtime = 0;
> > -
> > -	for (i = 0; i < c->clients->num_classes; i++) {
> > -		if (val[i] < c->last[i])
> > -			continue; /* It will catch up soon. */
> > -
> > -		c->total_runtime += val[i];
> > -		c->val[i] = val[i] - c->last[i];
> > -		c->last_runtime += c->val[i];
> > -		c->last[i] = val[i];
> > -	}
> > -
> > -	c->samples++;
> > -	c->status = ALIVE;
> > -}
> > -
> > -static void
> > -add_client(struct clients *clients, unsigned int id, unsigned int pid,
> > -	   char *name, int sysfs_root)
> > -{
> > -	struct client *c;
> > -
> > -	assert(!find_client(clients, ALIVE, id));
> > -
> > -	c = find_client(clients, FREE, 0);
> > -	if (!c) {
> > -		unsigned int idx = clients->num_clients;
> > -
> > -		clients->num_clients += (clients->num_clients + 2) / 2;
> > -		clients->client = realloc(clients->client,
> > -					  clients->num_clients * sizeof(*c));
> > -		assert(clients->client);
> > -
> > -		c = &clients->client[idx];
> > -		memset(c, 0, (clients->num_clients - idx) * sizeof(*c));
> > -	}
> > -
> > -	c->sysfs_root = sysfs_root;
> > -	c->busy_root = -1;
> > -	c->id = id;
> > -	c->clients = clients;
> > -	c->val = calloc(clients->num_classes, sizeof(c->val));
> > -	c->last = calloc(clients->num_classes, sizeof(c->last));
> > -	assert(c->val && c->last);
> > -
> > -	update_client(c, pid, name);
> > -}
> > -
> > -static void free_client(struct client *c)
> > -{
> > -	if (c->sysfs_root >= 0)
> > -		close(c->sysfs_root);
> > -	if (c->busy_root >= 0)
> > -		close(c->busy_root);
> > -	free(c->val);
> > -	free(c->last);
> > -	memset(c, 0, sizeof(*c));
> > -}
> > -
> > -static int
> > -read_client_sysfs(char *buf, int bufsize, const char *sysfs_root,
> > -		  unsigned int id, const char *field, int *client_root)
> > -{
> > -	ssize_t ret;
> > -
> > -	if (*client_root < 0) {
> > -		char namebuf[256];
> > -
> > -		ret = snprintf(namebuf, sizeof(namebuf), "%s/%u",
> > -			       sysfs_root, id);
> > -		assert(ret > 0 && ret < sizeof(namebuf));
> > -		if (ret <= 0 || ret == sizeof(namebuf))
> > -			return -1;
> > -
> > -		*client_root = open(namebuf, O_RDONLY | O_DIRECTORY);
> > -	}
> > -
> > -	if (*client_root < 0)
> > -		return -1;
> > -
> > -	return __read_client_field(*client_root, field, buf, bufsize);
> > -}
> > -
> > -static int client_last_cmp(const void *_a, const void *_b)
> > -{
> > -	const struct client *a = _a;
> > -	const struct client *b = _b;
> > -	long tot_a, tot_b;
> > -
> > -	/*
> > -	 * Sort clients in descending order of runtime in the previous sampling
> > -	 * period for active ones, followed by inactive. Tie-breaker is client
> > -	 * id.
> > -	 */
> > -
> > -	tot_a = a->status == ALIVE ? a->last_runtime : -1;
> > -	tot_b = b->status == ALIVE ? b->last_runtime : -1;
> > -
> > -	tot_b -= tot_a;
> > -	if (tot_b > 0)
> > -		return 1;
> > -	if (tot_b < 0)
> > -		return -1;
> > -
> > -	return (int)b->id - a->id;
> > -}
> > -
> > -static int client_total_cmp(const void *_a, const void *_b)
> > -{
> > -	const struct client *a = _a;
> > -	const struct client *b = _b;
> > -	long tot_a, tot_b;
> > -
> > -	tot_a = a->status == ALIVE ? a->total_runtime : -1;
> > -	tot_b = b->status == ALIVE ? b->total_runtime : -1;
> > -
> > -	tot_b -= tot_a;
> > -	if (tot_b > 0)
> > -		return 1;
> > -	if (tot_b < 0)
> > -		return -1;
> > -
> > -	return (int)b->id - a->id;
> > -}
> > -
> > -static int client_id_cmp(const void *_a, const void *_b)
> > -{
> > -	const struct client *a = _a;
> > -	const struct client *b = _b;
> > -	int id_a, id_b;
> > -
> > -	id_a = a->status == ALIVE ? a->id : -1;
> > -	id_b = b->status == ALIVE ? b->id : -1;
> > -
> > -	id_b -= id_a;
> > -	if (id_b > 0)
> > -		return 1;
> > -	if (id_b < 0)
> > -		return -1;
> > -
> > -	return (int)b->id - a->id;
> > -}
> > -
> > -static int client_pid_cmp(const void *_a, const void *_b)
> > -{
> > -	const struct client *a = _a;
> > -	const struct client *b = _b;
> > -	int pid_a, pid_b;
> > -
> > -	pid_a = a->status == ALIVE ? a->pid : INT_MAX;
> > -	pid_b = b->status == ALIVE ? b->pid : INT_MAX;
> > -
> > -	pid_b -= pid_a;
> > -	if (pid_b > 0)
> > -		return -1;
> > -	if (pid_b < 0)
> > -		return 1;
> > -
> > -	return (int)a->id - b->id;
> > -}
> > -
> > -static int (*client_cmp)(const void *, const void *) = client_last_cmp;
> > -
> > -static struct clients *sort_clients(struct clients *clients,
> > -				    int (*cmp)(const void *, const void *))
> > -{
> > -	unsigned int active, free;
> > -	struct client *c;
> > -	int tmp;
> > -
> > -	if (!clients)
> > -		return clients;
> > -
> > -	qsort(clients->client, clients->num_clients, sizeof(*clients->client),
> > -	      cmp);
> > -
> > -	/* Trim excessive array space. */
> > -	active = 0;
> > -	for_each_client(clients, c, tmp) {
> > -		if (c->status != ALIVE)
> > -			break; /* Active clients are first in the array. */
> > -		active++;
> > -	}
> > -
> > -	clients->active_clients = active;
> > -
> > -	free = clients->num_clients - active;
> > -	if (free > clients->num_clients / 2) {
> > -		active = clients->num_clients - free / 2;
> > -		if (active != clients->num_clients) {
> > -			clients->num_clients = active;
> > -			clients->client = realloc(clients->client,
> > -						  clients->num_clients *
> > -						  sizeof(*c));
> > -		}
> > -	}
> > -
> > -	return clients;
> > -}
> > -
> > -static bool aggregate_pids = true;
> > -
> > -static struct clients *display_clients(struct clients *clients)
> > -{
> > -	struct client *ac, *c, *cp = NULL;
> > -	struct clients *aggregated;
> > -	int tmp, num = 0;
> > -
> > -	if (!aggregate_pids)
> > -		goto out;
> > -
> > -	/* Sort by pid first to make it easy to aggregate while walking. */
> > -	sort_clients(clients, client_pid_cmp);
> > -
> > -	aggregated = calloc(1, sizeof(*clients));
> > -	assert(aggregated);
> > -
> > -	ac = calloc(clients->num_clients, sizeof(*c));
> > -	assert(ac);
> > -
> > -	aggregated->num_classes = clients->num_classes;
> > -	aggregated->class = clients->class;
> > -	aggregated->client = ac;
> > -
> > -	for_each_client(clients, c, tmp) {
> > -		unsigned int i;
> > -
> > -		if (c->status == FREE)
> > -			break;
> > -
> > -		assert(c->status == ALIVE);
> > -
> > -		if ((cp && c->pid != cp->pid) || !cp) {
> > -			ac = &aggregated->client[num++];
> > -
> > -			/* New pid. */
> > -			ac->clients = aggregated;
> > -			ac->status = ALIVE;
> > -			ac->id = -c->pid;
> > -			ac->pid = c->pid;
> > -			ac->busy_root = -1;
> > -			ac->sysfs_root = -1;
> > -			strcpy(ac->name, c->name);
> > -			strcpy(ac->print_name, c->print_name);
> > -			ac->engines = c->engines;
> > -			ac->val = calloc(clients->num_classes,
> > -					 sizeof(ac->val[0]));
> > -			assert(ac->val);
> > -			ac->samples = 1;
> > -		}
> > -
> > -		cp = c;
> > -
> > -		if (c->samples < 2)
> > -			continue;
> > -
> > -		ac->samples = 2; /* All what matters for display. */
> > -		ac->total_runtime += c->total_runtime;
> > -		ac->last_runtime += c->last_runtime;
> > -
> > -		for (i = 0; i < clients->num_classes; i++)
> > -			ac->val[i] += c->val[i];
> > -	}
> > -
> > -	aggregated->num_clients = num;
> > -	aggregated->active_clients = num;
> > -
> > -	clients = aggregated;
> > -
> > -out:
> > -	return sort_clients(clients, client_cmp);
> > -}
> > -
> > -static void free_clients(struct clients *clients)
> > -{
> > -	struct client *c;
> > -	unsigned int tmp;
> > -
> > -	for_each_client(clients, c, tmp) {
> > -		free(c->val);
> > -		free(c->last);
> > -	}
> > -
> > -	free(clients->client);
> > -	free(clients);
> > -}
> > -
> > -static struct clients *scan_clients(struct clients *clients)
> > -{
> > -	struct dirent *dent;
> > -	struct client *c;
> > -	unsigned int id;
> > -	int tmp;
> > -	DIR *d;
> > -
> > -	if (!clients)
> > -		return clients;
> > -
> > -	for_each_client(clients, c, tmp) {
> > -		assert(c->status != PROBE);
> > -		if (c->status == ALIVE)
> > -			c->status = PROBE;
> > -		else
> > -			break; /* Free block at the end of array. */
> > -	}
> > -
> > -	d = opendir(clients->sysfs_root);
> > -	if (!d)
> > -		return clients;
> > -
> > -	while ((dent = readdir(d)) != NULL) {
> > -		char name[24], pid[24];
> > -		int ret, root = -1, *pr;
> > -
> > -		if (dent->d_type != DT_DIR)
> > -			continue;
> > -		if (!isdigit(dent->d_name[0]))
> > -			continue;
> > -
> > -		id = atoi(dent->d_name);
> > -
> > -		c = find_client(clients, PROBE, id);
> > -
> > -		if (c)
> > -			pr = &c->sysfs_root;
> > -		else
> > -			pr = &root;
> > -
> > -		ret = read_client_sysfs(name, sizeof(name), clients->sysfs_root,
> > -					id, "name", pr);
> > -		ret |= read_client_sysfs(pid, sizeof(pid), clients->sysfs_root,
> > -					id, "pid", pr);
> > -		if (!ret) {
> > -			if (!c)
> > -				add_client(clients, id, atoi(pid), name, root);
> > -			else
> > -				update_client(c, atoi(pid), name);
> > -		} else if (c) {
> > -			c->status = PROBE; /* Will be deleted below. */
> > -		}
> > -	}
> > -
> > -	closedir(d);
> > -
> > -	for_each_client(clients, c, tmp) {
> > -		if (c->status == PROBE)
> > -			free_client(c);
> > -		else if (c->status == FREE)
> > -			break;
> > -	}
> > -
> > -	return display_clients(clients);
> > -}
> > -
> >   static const char *bars[] = { " ", "▏", "▎", "▍", "▌", "▋", "▊", "▉", "█" };
> >   static void n_spaces(const unsigned int n)
> > @@ -1324,18 +768,6 @@ json_close_struct(void)
> >   		fflush(stdout);
> >   }
> > -static void
> > -__json_add_member(const char *key, const char *val)
> > -{
> > -	assert(json_indent_level < ARRAY_SIZE(json_indent));
> > -
> > -	fprintf(out, "%s%s\"%s\": \"%s\"",
> > -		json_struct_members ? ",\n" : "",
> > -		json_indent[json_indent_level], key, val);
> > -
> > -	json_struct_members++;
> > -}
> > -
> >   static unsigned int
> >   json_add_member(const struct cnt_group *parent, struct cnt_item *item,
> >   		unsigned int headers)
> > @@ -2061,157 +1493,6 @@ print_engines(struct engines *engines, double t, int lines, int w, int h)
> >   	return lines;
> >   }
> > -static int
> > -print_clients_header(struct clients *clients, int lines,
> > -		     int con_w, int con_h, int *class_w)
> > -{
> > -	if (output_mode == INTERACTIVE) {
> > -		const char *pidname = "   PID              NAME ";
> > -		unsigned int num_active = 0;
> > -		int len = strlen(pidname);
> > -
> > -		if (lines++ >= con_h)
> > -			return lines;
> > -
> > -		printf("\033[7m");
> > -		printf("%s", pidname);
> > -
> > -		if (lines++ >= con_h || len >= con_w)
> > -			return lines;
> > -
> > -		if (clients->num_classes) {
> > -			unsigned int i;
> > -			int width;
> > -
> > -			for (i = 0; i < clients->num_classes; i++) {
> > -				if (clients->class[i].num_engines)
> > -					num_active++;
> > -			}
> > -
> > -			*class_w = width = (con_w - len) / num_active;
> > -
> > -			for (i = 0; i < clients->num_classes; i++) {
> > -				const char *name = clients->class[i].name;
> > -				int name_len = strlen(name);
> > -				int pad = (width - name_len) / 2;
> > -				int spaces = width - pad - name_len;
> > -
> > -				if (!clients->class[i].num_engines)
> > -					continue; /* Assert in the ideal world. */
> > -
> > -				if (pad < 0 || spaces < 0)
> > -					continue;
> > -
> > -				n_spaces(pad);
> > -				printf("%s", name);
> > -				n_spaces(spaces);
> > -				len += pad + name_len + spaces;
> > -			}
> > -		}
> > -
> > -		n_spaces(con_w - len);
> > -		printf("\033[0m\n");
> > -	} else {
> > -		if (clients->num_classes)
> > -			pops->open_struct("clients");
> > -	}
> > -
> > -	return lines;
> > -}
> > -
> > -static bool numeric_clients;
> > -static bool filter_idle;
> > -
> > -static int
> > -print_client(struct client *c, struct engines *engines, double t, int lines,
> > -	     int con_w, int con_h, unsigned int period_us, int *class_w)
> > -{
> > -	struct clients *clients = c->clients;
> > -	unsigned int i;
> > -
> > -	if (output_mode == INTERACTIVE) {
> > -		if (filter_idle && (!c->total_runtime || c->samples < 2))
> > -			return lines;
> > -
> > -		lines++;
> > -
> > -		printf("%6u %17s ", c->pid, c->print_name);
> > -
> > -		for (i = 0; c->samples > 1 && i < clients->num_classes; i++) {
> > -			double pct;
> > -
> > -			if (!clients->class[i].num_engines)
> > -				continue; /* Assert in the ideal world. */
> > -
> > -			pct = (double)c->val[i] / period_us / 1e3 * 100 /
> > -			      clients->class[i].num_engines;
> > -
> > -			/*
> > -			 * Guard against possible time-drift between sampling
> > -			 * client data and time we obtained our time-delta from
> > -			 * PMU.
> > -			 */
> > -			if (pct > 100.0)
> > -				pct = 100.0;
> > -
> > -			print_percentage_bar(pct, *class_w, numeric_clients);
> > -		}
> > -
> > -		putchar('\n');
> > -	} else if (output_mode == JSON) {
> > -		char buf[64];
> > -
> > -		snprintf(buf, sizeof(buf), "%u", c->id);
> > -		pops->open_struct(buf);
> > -
> > -		__json_add_member("name", c->print_name);
> > -
> > -		snprintf(buf, sizeof(buf), "%u", c->pid);
> > -		__json_add_member("pid", buf);
> > -
> > -		if (c->samples > 1) {
> > -			pops->open_struct("engine-classes");
> > -
> > -			for (i = 0; i < clients->num_classes; i++) {
> > -				double pct;
> > -
> > -				snprintf(buf, sizeof(buf), "%s",
> > -					clients->class[i].name);
> > -				pops->open_struct(buf);
> > -
> > -				pct = (double)c->val[i] / period_us / 1e3 * 100;
> > -				snprintf(buf, sizeof(buf), "%f", pct);
> > -				__json_add_member("busy", buf);
> > -
> > -				__json_add_member("unit", "%");
> > -
> > -				pops->close_struct();
> > -			}
> > -
> > -			pops->close_struct();
> > -		}
> > -
> > -		pops->close_struct();
> > -	}
> > -
> > -	return lines;
> > -}
> > -
> > -static int
> > -print_clients_footer(struct clients *clients, double t,
> > -		     int lines, int con_w, int con_h)
> > -{
> > -	if (output_mode == INTERACTIVE) {
> > -		if (lines++ < con_h)
> > -			printf("\n");
> > -	} else {
> > -		if (clients->num_classes)
> > -			pops->close_struct();
> > -	}
> > -
> > -	return lines;
> > -}
> > -
> >   static bool stop_top;
> >   static void sigint_handler(int  sig)
> > @@ -2267,31 +1548,6 @@ static void interactive_stdin(void)
> >   	assert(ret == 0);
> >   }
> > -static void select_client_sort(void)
> > -{
> > -	struct {
> > -		int (*cmp)(const void *, const void *);
> > -		const char *msg;
> > -	} cmp[] = {
> > -		{ client_last_cmp, "Sorting clients by current GPU usage." },
> > -		{ client_total_cmp, "Sorting clients by accummulated GPU usage." },
> > -		{ client_pid_cmp, "Sorting clients by pid." },
> > -		{ client_id_cmp, "Sorting clients by sysfs id." },
> > -	};
> > -	static unsigned int client_sort;
> > -
> > -bump:
> > -	if (++client_sort >= ARRAY_SIZE(cmp))
> > -		client_sort = 0;
> > -
> > -	client_cmp = cmp[client_sort].cmp;
> > -	header_msg = cmp[client_sort].msg;
> > -
> > -	/* Sort by client id makes no sense with pid aggregation. */
> > -	if (aggregate_pids && client_cmp == client_id_cmp)
> > -		goto bump;
> > -}
> > -
> >   static bool in_help;
> >   static void process_help_stdin(void)
> > @@ -2334,29 +1590,9 @@ static void process_normal_stdin(void)
> >   			else
> >   				header_msg = "Showing physical engines.";
> >   			break;
> > -		case 'i':
> > -			filter_idle ^= true;
> > -			if (filter_idle)
> > -				header_msg = "Hiding inactive clients.";
> > -			else
> > -				header_msg = "Showing inactive clients.";
> > -			break;
> > -		case 'n':
> > -			numeric_clients ^= true;
> > -			break;
> > -		case 's':
> > -			select_client_sort();
> > -			break;
> >   		case 'h':
> >   			in_help = true;
> >   			break;
> > -		case 'H':
> > -			aggregate_pids ^= true;
> > -			if (aggregate_pids)
> > -				header_msg = "Aggregating clients.";
> > -			else
> > -				header_msg = "Showing individual clients.";
> > -			break;
> >   		};
> >   	}
> >   }
> > @@ -2384,10 +1620,6 @@ static void show_help_screen(void)
> >   	printf(
> >   "Help for interactive commands:\n\n"
> >   "    '1'    Toggle between aggregated engine class and physical engine mode.\n"
> > -"    'n'    Toggle display of numeric client busyness overlay.\n"
> > -"    's'    Toggle between sort modes (runtime, total runtime, pid, client id).\n"
> > -"    'i'    Toggle display of clients which used no GPU time.\n"
> > -"    'H'    Toggle between per PID aggregation and individual clients.\n"
> >   "\n"
> >   "    'h' or 'q'    Exit interactive help.\n"
> >   "\n");
> > @@ -2396,7 +1628,6 @@ static void show_help_screen(void)
> >   int main(int argc, char **argv)
> >   {
> >   	unsigned int period_us = DEFAULT_PERIOD_MS * 1000;
> > -	struct clients *clients = NULL;
> >   	int con_w = -1, con_h = -1;
> >   	char *output_path = NULL;
> >   	struct engines *engines;
> > @@ -2530,23 +1761,15 @@ int main(int argc, char **argv)
> >   	ret = EXIT_SUCCESS;
> > -	clients = init_clients(card.pci_slot_name[0] ? card.card : NULL);
> >   	init_engine_classes(engines);
> > -	if (clients) {
> > -		clients->num_classes = engines->num_classes;
> > -		clients->class = engines->class;
> > -	}
> >   	pmu_sample(engines);
> > -	scan_clients(clients);
> >   	codename = igt_device_get_pretty_name(&card, false);
> >   	while (!stop_top) {
> > -		struct clients *disp_clients;
> >   		bool consumed = false;
> > -		int j, lines = 0;
> >   		struct winsize ws;
> > -		struct client *c;
> > +		int lines = 0;
> >   		double t;
> >   		/* Update terminal size. */
> > @@ -2565,8 +1788,6 @@ int main(int argc, char **argv)
> >   		pmu_sample(engines);
> >   		t = (double)(engines->ts.cur - engines->ts.prev) / 1e9;
> > -		disp_clients = scan_clients(clients);
> > -
> >   		if (stop_top)
> >   			break;
> > @@ -2586,41 +1807,12 @@ int main(int argc, char **argv)
> >   			lines = print_engines(engines, t, lines, con_w, con_h);
> > -			if (disp_clients) {
> > -				int class_w;
> > -
> > -				lines = print_clients_header(disp_clients, lines,
> > -							     con_w, con_h,
> > -							     &class_w);
> > -
> > -				for_each_client(disp_clients, c, j) {
> > -					assert(c->status != PROBE);
> > -					if (c->status != ALIVE)
> > -						break; /* Active clients are first in the array. */
> > -
> > -					if (lines >= con_h)
> > -						break;
> > -
> > -					lines = print_client(c, engines, t,
> > -							     lines, con_w,
> > -							     con_h, period_us,
> > -							     &class_w);
> > -				}
> > -
> > -				lines = print_clients_footer(disp_clients, t,
> > -							     lines, con_w,
> > -							     con_h);
> > -			}
> > -
> >   			pops->close_struct();
> >   		}
> >   		if (stop_top)
> >   			break;
> > -		if (disp_clients != clients)
> > -			free_clients(disp_clients);
> > -
> >   		if (output_mode == INTERACTIVE)
> >   			process_stdin(period_us);
> >   		else
> > 

Re: [igt-dev] [PATCH i-g-t 5/6] intel_gpu_top: Remove clients support

[Petri Latvala]

On Fri, Nov 26, 2021 at 01:07:24PM +0000, Tvrtko Ursulin wrote:
> 
> On 19/11/2021 12:59, Tvrtko Ursulin wrote:
> > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > 
> > When kernel feature was removed the intel_gpu_top part was forgotten.
> > 
> > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> Will someone ack this or we carry this code until it ships, if it hasn't
> already?

Does that question mean client busyness will some day return?

Either way,
Acked-by: Petri Latvala <petri.latvala@intel.com>


> 
> Regards,
> 
> Tvrtko
> 
> > ---
> >   man/intel_gpu_top.rst |   4 -
> >   tools/intel_gpu_top.c | 810 +-----------------------------------------
> >   2 files changed, 1 insertion(+), 813 deletions(-)
> > 
> > diff --git a/man/intel_gpu_top.rst b/man/intel_gpu_top.rst
> > index f4dbfc5b44d9..b3b765b05feb 100644
> > --- a/man/intel_gpu_top.rst
> > +++ b/man/intel_gpu_top.rst
> > @@ -56,10 +56,6 @@ Supported keys:
> >       'q'    Exit from the tool.
> >       'h'    Show interactive help.
> >       '1'    Toggle between aggregated engine class and physical engine mode.
> > -    'n'    Toggle display of numeric client busyness overlay.
> > -    's'    Toggle between sort modes (runtime, total runtime, pid, client id).
> > -    'i'    Toggle display of clients which used no GPU time.
> > -    'H'    Toggle between per PID aggregation and individual clients.
> >   DEVICE SELECTION
> >   ================
> > diff --git a/tools/intel_gpu_top.c b/tools/intel_gpu_top.c
> > index 7311038a39f4..41c59a72c09d 100644
> > --- a/tools/intel_gpu_top.c
> > +++ b/tools/intel_gpu_top.c
> > @@ -627,562 +627,6 @@ static void pmu_sample(struct engines *engines)
> >   	}
> >   }
> > -enum client_status {
> > -	FREE = 0, /* mbz */
> > -	ALIVE,
> > -	PROBE
> > -};
> > -
> > -struct clients;
> > -
> > -struct client {
> > -	struct clients *clients;
> > -
> > -	enum client_status status;
> > -	int sysfs_root;
> > -	int busy_root;
> > -	unsigned int id;
> > -	unsigned int pid;
> > -	char name[24];
> > -	char print_name[24];
> > -	unsigned int samples;
> > -	unsigned long total_runtime;
> > -	unsigned long last_runtime;
> > -	struct engines *engines;
> > -	unsigned long *val;
> > -	uint64_t *last;
> > -};
> > -
> > -struct clients {
> > -	unsigned int num_clients;
> > -	unsigned int active_clients;
> > -
> > -	unsigned int num_classes;
> > -	struct engine_class *class;
> > -
> > -	char sysfs_root[128];
> > -
> > -	struct client *client;
> > -};
> > -
> > -#define for_each_client(clients, c, tmp) \
> > -	for ((tmp) = (clients)->num_clients, c = (clients)->client; \
> > -	     (tmp > 0); (tmp)--, (c)++)
> > -
> > -static struct clients *init_clients(const char *drm_card)
> > -{
> > -	struct clients *clients;
> > -	const char *slash;
> > -	ssize_t ret;
> > -	int dir;
> > -
> > -	clients = malloc(sizeof(*clients));
> > -	if (!clients)
> > -		return NULL;
> > -
> > -	memset(clients, 0, sizeof(*clients));
> > -
> > -	if (drm_card) {
> > -		slash = rindex(drm_card, '/');
> > -		assert(slash);
> > -	} else {
> > -		slash = "card0";
> > -	}
> > -
> > -	ret = snprintf(clients->sysfs_root, sizeof(clients->sysfs_root),
> > -		       "/sys/class/drm/%s/clients/", slash);
> > -	assert(ret > 0 && ret < sizeof(clients->sysfs_root));
> > -
> > -	dir = open(clients->sysfs_root, O_DIRECTORY | O_RDONLY);
> > -	if (dir < 0) {
> > -		free(clients);
> > -		clients = NULL;
> > -	} else {
> > -		close(dir);
> > -	}
> > -
> > -	return clients;
> > -}
> > -
> > -static int __read_to_buf(int fd, char *buf, unsigned int bufsize)
> > -{
> > -	ssize_t ret;
> > -	int err;
> > -
> > -	ret = read(fd, buf, bufsize - 1);
> > -	err = errno;
> > -	if (ret < 1) {
> > -		errno = ret < 0 ? err : ENOMSG;
> > -
> > -		return -1;
> > -	}
> > -
> > -	if (ret > 1 && buf[ret - 1] == '\n')
> > -		buf[ret - 1] = '\0';
> > -	else
> > -		buf[ret] = '\0';
> > -
> > -	return 0;
> > -}
> > -
> > -static int
> > -__read_client_field(int root, const char *field, char *buf, unsigned int bufsize)
> > -{
> > -	int fd, ret;
> > -
> > -	fd = openat(root, field, O_RDONLY);
> > -	if (fd < 0)
> > -		return -1;
> > -
> > -	ret = __read_to_buf(fd, buf, bufsize);
> > -
> > -	close(fd);
> > -
> > -	return ret;
> > -}
> > -
> > -static uint64_t
> > -read_client_busy(struct client *client, unsigned int class)
> > -{
> > -	const char *class_str[] = { "0", "1", "2", "3", "4", "5", "6", "7" };
> > -	char buf[256], *b;
> > -	int ret;
> > -
> > -	assert(class < ARRAY_SIZE(class_str));
> > -	if (class >= ARRAY_SIZE(class_str))
> > -		return 0;
> > -
> > -	assert(client->sysfs_root >= 0);
> > -	if (client->sysfs_root < 0)
> > -		return 0;
> > -
> > -	if (client->busy_root < 0)
> > -		client->busy_root = openat(client->sysfs_root, "busy",
> > -					   O_RDONLY | O_DIRECTORY);
> > -
> > -	assert(client->busy_root);
> > -	if (client->busy_root < 0)
> > -		return 0;
> > -
> > -	ret = __read_client_field(client->busy_root, class_str[class], buf,
> > -				  sizeof(buf));
> > -	if (ret) {
> > -		close(client->busy_root);
> > -		client->busy_root = -1;
> > -		return 0;
> > -	}
> > -
> > -	/*
> > -	 * Handle both single integer and key=value formats by skipping
> > -	 * leading non-digits.
> > -	 */
> > -	b = buf;
> > -	while (*b && !isdigit(*b))
> > -		b++;
> > -
> > -	return strtoull(b, NULL, 10);
> > -}
> > -
> > -static struct client *
> > -find_client(struct clients *clients, enum client_status status, unsigned int id)
> > -{
> > -	unsigned int start, num;
> > -	struct client *c;
> > -
> > -	start = status == FREE ? clients->active_clients : 0; /* Free block at the end. */
> > -	num = clients->num_clients - start;
> > -
> > -	for (c = &clients->client[start]; num; c++, num--) {
> > -		if (status != c->status)
> > -			continue;
> > -
> > -		if (status == FREE || c->id == id)
> > -			return c;
> > -	}
> > -
> > -	return NULL;
> > -}
> > -
> > -static void update_client(struct client *c, unsigned int pid, char *name)
> > -{
> > -	uint64_t val[c->clients->num_classes];
> > -	unsigned int i;
> > -
> > -	if (c->pid != pid)
> > -		c->pid = pid;
> > -
> > -	if (strcmp(c->name, name)) {
> > -		char *p;
> > -
> > -		strncpy(c->name, name, sizeof(c->name) - 1);
> > -		strncpy(c->print_name, name, sizeof(c->print_name) - 1);
> > -
> > -		p = c->print_name;
> > -		while (*p) {
> > -			if (!isprint(*p))
> > -				*p = '*';
> > -			p++;
> > -		}
> > -	}
> > -
> > -	for (i = 0; i < c->clients->num_classes; i++)
> > -		val[i] = read_client_busy(c, c->clients->class[i].class);
> > -
> > -	c->last_runtime = 0;
> > -	c->total_runtime = 0;
> > -
> > -	for (i = 0; i < c->clients->num_classes; i++) {
> > -		if (val[i] < c->last[i])
> > -			continue; /* It will catch up soon. */
> > -
> > -		c->total_runtime += val[i];
> > -		c->val[i] = val[i] - c->last[i];
> > -		c->last_runtime += c->val[i];
> > -		c->last[i] = val[i];
> > -	}
> > -
> > -	c->samples++;
> > -	c->status = ALIVE;
> > -}
> > -
> > -static void
> > -add_client(struct clients *clients, unsigned int id, unsigned int pid,
> > -	   char *name, int sysfs_root)
> > -{
> > -	struct client *c;
> > -
> > -	assert(!find_client(clients, ALIVE, id));
> > -
> > -	c = find_client(clients, FREE, 0);
> > -	if (!c) {
> > -		unsigned int idx = clients->num_clients;
> > -
> > -		clients->num_clients += (clients->num_clients + 2) / 2;
> > -		clients->client = realloc(clients->client,
> > -					  clients->num_clients * sizeof(*c));
> > -		assert(clients->client);
> > -
> > -		c = &clients->client[idx];
> > -		memset(c, 0, (clients->num_clients - idx) * sizeof(*c));
> > -	}
> > -
> > -	c->sysfs_root = sysfs_root;
> > -	c->busy_root = -1;
> > -	c->id = id;
> > -	c->clients = clients;
> > -	c->val = calloc(clients->num_classes, sizeof(c->val));
> > -	c->last = calloc(clients->num_classes, sizeof(c->last));
> > -	assert(c->val && c->last);
> > -
> > -	update_client(c, pid, name);
> > -}
> > -
> > -static void free_client(struct client *c)
> > -{
> > -	if (c->sysfs_root >= 0)
> > -		close(c->sysfs_root);
> > -	if (c->busy_root >= 0)
> > -		close(c->busy_root);
> > -	free(c->val);
> > -	free(c->last);
> > -	memset(c, 0, sizeof(*c));
> > -}
> > -
> > -static int
> > -read_client_sysfs(char *buf, int bufsize, const char *sysfs_root,
> > -		  unsigned int id, const char *field, int *client_root)
> > -{
> > -	ssize_t ret;
> > -
> > -	if (*client_root < 0) {
> > -		char namebuf[256];
> > -
> > -		ret = snprintf(namebuf, sizeof(namebuf), "%s/%u",
> > -			       sysfs_root, id);
> > -		assert(ret > 0 && ret < sizeof(namebuf));
> > -		if (ret <= 0 || ret == sizeof(namebuf))
> > -			return -1;
> > -
> > -		*client_root = open(namebuf, O_RDONLY | O_DIRECTORY);
> > -	}
> > -
> > -	if (*client_root < 0)
> > -		return -1;
> > -
> > -	return __read_client_field(*client_root, field, buf, bufsize);
> > -}
> > -
> > -static int client_last_cmp(const void *_a, const void *_b)
> > -{
> > -	const struct client *a = _a;
> > -	const struct client *b = _b;
> > -	long tot_a, tot_b;
> > -
> > -	/*
> > -	 * Sort clients in descending order of runtime in the previous sampling
> > -	 * period for active ones, followed by inactive. Tie-breaker is client
> > -	 * id.
> > -	 */
> > -
> > -	tot_a = a->status == ALIVE ? a->last_runtime : -1;
> > -	tot_b = b->status == ALIVE ? b->last_runtime : -1;
> > -
> > -	tot_b -= tot_a;
> > -	if (tot_b > 0)
> > -		return 1;
> > -	if (tot_b < 0)
> > -		return -1;
> > -
> > -	return (int)b->id - a->id;
> > -}
> > -
> > -static int client_total_cmp(const void *_a, const void *_b)
> > -{
> > -	const struct client *a = _a;
> > -	const struct client *b = _b;
> > -	long tot_a, tot_b;
> > -
> > -	tot_a = a->status == ALIVE ? a->total_runtime : -1;
> > -	tot_b = b->status == ALIVE ? b->total_runtime : -1;
> > -
> > -	tot_b -= tot_a;
> > -	if (tot_b > 0)
> > -		return 1;
> > -	if (tot_b < 0)
> > -		return -1;
> > -
> > -	return (int)b->id - a->id;
> > -}
> > -
> > -static int client_id_cmp(const void *_a, const void *_b)
> > -{
> > -	const struct client *a = _a;
> > -	const struct client *b = _b;
> > -	int id_a, id_b;
> > -
> > -	id_a = a->status == ALIVE ? a->id : -1;
> > -	id_b = b->status == ALIVE ? b->id : -1;
> > -
> > -	id_b -= id_a;
> > -	if (id_b > 0)
> > -		return 1;
> > -	if (id_b < 0)
> > -		return -1;
> > -
> > -	return (int)b->id - a->id;
> > -}
> > -
> > -static int client_pid_cmp(const void *_a, const void *_b)
> > -{
> > -	const struct client *a = _a;
> > -	const struct client *b = _b;
> > -	int pid_a, pid_b;
> > -
> > -	pid_a = a->status == ALIVE ? a->pid : INT_MAX;
> > -	pid_b = b->status == ALIVE ? b->pid : INT_MAX;
> > -
> > -	pid_b -= pid_a;
> > -	if (pid_b > 0)
> > -		return -1;
> > -	if (pid_b < 0)
> > -		return 1;
> > -
> > -	return (int)a->id - b->id;
> > -}
> > -
> > -static int (*client_cmp)(const void *, const void *) = client_last_cmp;
> > -
> > -static struct clients *sort_clients(struct clients *clients,
> > -				    int (*cmp)(const void *, const void *))
> > -{
> > -	unsigned int active, free;
> > -	struct client *c;
> > -	int tmp;
> > -
> > -	if (!clients)
> > -		return clients;
> > -
> > -	qsort(clients->client, clients->num_clients, sizeof(*clients->client),
> > -	      cmp);
> > -
> > -	/* Trim excessive array space. */
> > -	active = 0;
> > -	for_each_client(clients, c, tmp) {
> > -		if (c->status != ALIVE)
> > -			break; /* Active clients are first in the array. */
> > -		active++;
> > -	}
> > -
> > -	clients->active_clients = active;
> > -
> > -	free = clients->num_clients - active;
> > -	if (free > clients->num_clients / 2) {
> > -		active = clients->num_clients - free / 2;
> > -		if (active != clients->num_clients) {
> > -			clients->num_clients = active;
> > -			clients->client = realloc(clients->client,
> > -						  clients->num_clients *
> > -						  sizeof(*c));
> > -		}
> > -	}
> > -
> > -	return clients;
> > -}
> > -
> > -static bool aggregate_pids = true;
> > -
> > -static struct clients *display_clients(struct clients *clients)
> > -{
> > -	struct client *ac, *c, *cp = NULL;
> > -	struct clients *aggregated;
> > -	int tmp, num = 0;
> > -
> > -	if (!aggregate_pids)
> > -		goto out;
> > -
> > -	/* Sort by pid first to make it easy to aggregate while walking. */
> > -	sort_clients(clients, client_pid_cmp);
> > -
> > -	aggregated = calloc(1, sizeof(*clients));
> > -	assert(aggregated);
> > -
> > -	ac = calloc(clients->num_clients, sizeof(*c));
> > -	assert(ac);
> > -
> > -	aggregated->num_classes = clients->num_classes;
> > -	aggregated->class = clients->class;
> > -	aggregated->client = ac;
> > -
> > -	for_each_client(clients, c, tmp) {
> > -		unsigned int i;
> > -
> > -		if (c->status == FREE)
> > -			break;
> > -
> > -		assert(c->status == ALIVE);
> > -
> > -		if ((cp && c->pid != cp->pid) || !cp) {
> > -			ac = &aggregated->client[num++];
> > -
> > -			/* New pid. */
> > -			ac->clients = aggregated;
> > -			ac->status = ALIVE;
> > -			ac->id = -c->pid;
> > -			ac->pid = c->pid;
> > -			ac->busy_root = -1;
> > -			ac->sysfs_root = -1;
> > -			strcpy(ac->name, c->name);
> > -			strcpy(ac->print_name, c->print_name);
> > -			ac->engines = c->engines;
> > -			ac->val = calloc(clients->num_classes,
> > -					 sizeof(ac->val[0]));
> > -			assert(ac->val);
> > -			ac->samples = 1;
> > -		}
> > -
> > -		cp = c;
> > -
> > -		if (c->samples < 2)
> > -			continue;
> > -
> > -		ac->samples = 2; /* All what matters for display. */
> > -		ac->total_runtime += c->total_runtime;
> > -		ac->last_runtime += c->last_runtime;
> > -
> > -		for (i = 0; i < clients->num_classes; i++)
> > -			ac->val[i] += c->val[i];
> > -	}
> > -
> > -	aggregated->num_clients = num;
> > -	aggregated->active_clients = num;
> > -
> > -	clients = aggregated;
> > -
> > -out:
> > -	return sort_clients(clients, client_cmp);
> > -}
> > -
> > -static void free_clients(struct clients *clients)
> > -{
> > -	struct client *c;
> > -	unsigned int tmp;
> > -
> > -	for_each_client(clients, c, tmp) {
> > -		free(c->val);
> > -		free(c->last);
> > -	}
> > -
> > -	free(clients->client);
> > -	free(clients);
> > -}
> > -
> > -static struct clients *scan_clients(struct clients *clients)
> > -{
> > -	struct dirent *dent;
> > -	struct client *c;
> > -	unsigned int id;
> > -	int tmp;
> > -	DIR *d;
> > -
> > -	if (!clients)
> > -		return clients;
> > -
> > -	for_each_client(clients, c, tmp) {
> > -		assert(c->status != PROBE);
> > -		if (c->status == ALIVE)
> > -			c->status = PROBE;
> > -		else
> > -			break; /* Free block at the end of array. */
> > -	}
> > -
> > -	d = opendir(clients->sysfs_root);
> > -	if (!d)
> > -		return clients;
> > -
> > -	while ((dent = readdir(d)) != NULL) {
> > -		char name[24], pid[24];
> > -		int ret, root = -1, *pr;
> > -
> > -		if (dent->d_type != DT_DIR)
> > -			continue;
> > -		if (!isdigit(dent->d_name[0]))
> > -			continue;
> > -
> > -		id = atoi(dent->d_name);
> > -
> > -		c = find_client(clients, PROBE, id);
> > -
> > -		if (c)
> > -			pr = &c->sysfs_root;
> > -		else
> > -			pr = &root;
> > -
> > -		ret = read_client_sysfs(name, sizeof(name), clients->sysfs_root,
> > -					id, "name", pr);
> > -		ret |= read_client_sysfs(pid, sizeof(pid), clients->sysfs_root,
> > -					id, "pid", pr);
> > -		if (!ret) {
> > -			if (!c)
> > -				add_client(clients, id, atoi(pid), name, root);
> > -			else
> > -				update_client(c, atoi(pid), name);
> > -		} else if (c) {
> > -			c->status = PROBE; /* Will be deleted below. */
> > -		}
> > -	}
> > -
> > -	closedir(d);
> > -
> > -	for_each_client(clients, c, tmp) {
> > -		if (c->status == PROBE)
> > -			free_client(c);
> > -		else if (c->status == FREE)
> > -			break;
> > -	}
> > -
> > -	return display_clients(clients);
> > -}
> > -
> >   static const char *bars[] = { " ", "▏", "▎", "▍", "▌", "▋", "▊", "▉", "█" };
> >   static void n_spaces(const unsigned int n)
> > @@ -1324,18 +768,6 @@ json_close_struct(void)
> >   		fflush(stdout);
> >   }
> > -static void
> > -__json_add_member(const char *key, const char *val)
> > -{
> > -	assert(json_indent_level < ARRAY_SIZE(json_indent));
> > -
> > -	fprintf(out, "%s%s\"%s\": \"%s\"",
> > -		json_struct_members ? ",\n" : "",
> > -		json_indent[json_indent_level], key, val);
> > -
> > -	json_struct_members++;
> > -}
> > -
> >   static unsigned int
> >   json_add_member(const struct cnt_group *parent, struct cnt_item *item,
> >   		unsigned int headers)
> > @@ -2061,157 +1493,6 @@ print_engines(struct engines *engines, double t, int lines, int w, int h)
> >   	return lines;
> >   }
> > -static int
> > -print_clients_header(struct clients *clients, int lines,
> > -		     int con_w, int con_h, int *class_w)
> > -{
> > -	if (output_mode == INTERACTIVE) {
> > -		const char *pidname = "   PID              NAME ";
> > -		unsigned int num_active = 0;
> > -		int len = strlen(pidname);
> > -
> > -		if (lines++ >= con_h)
> > -			return lines;
> > -
> > -		printf("\033[7m");
> > -		printf("%s", pidname);
> > -
> > -		if (lines++ >= con_h || len >= con_w)
> > -			return lines;
> > -
> > -		if (clients->num_classes) {
> > -			unsigned int i;
> > -			int width;
> > -
> > -			for (i = 0; i < clients->num_classes; i++) {
> > -				if (clients->class[i].num_engines)
> > -					num_active++;
> > -			}
> > -
> > -			*class_w = width = (con_w - len) / num_active;
> > -
> > -			for (i = 0; i < clients->num_classes; i++) {
> > -				const char *name = clients->class[i].name;
> > -				int name_len = strlen(name);
> > -				int pad = (width - name_len) / 2;
> > -				int spaces = width - pad - name_len;
> > -
> > -				if (!clients->class[i].num_engines)
> > -					continue; /* Assert in the ideal world. */
> > -
> > -				if (pad < 0 || spaces < 0)
> > -					continue;
> > -
> > -				n_spaces(pad);
> > -				printf("%s", name);
> > -				n_spaces(spaces);
> > -				len += pad + name_len + spaces;
> > -			}
> > -		}
> > -
> > -		n_spaces(con_w - len);
> > -		printf("\033[0m\n");
> > -	} else {
> > -		if (clients->num_classes)
> > -			pops->open_struct("clients");
> > -	}
> > -
> > -	return lines;
> > -}
> > -
> > -static bool numeric_clients;
> > -static bool filter_idle;
> > -
> > -static int
> > -print_client(struct client *c, struct engines *engines, double t, int lines,
> > -	     int con_w, int con_h, unsigned int period_us, int *class_w)
> > -{
> > -	struct clients *clients = c->clients;
> > -	unsigned int i;
> > -
> > -	if (output_mode == INTERACTIVE) {
> > -		if (filter_idle && (!c->total_runtime || c->samples < 2))
> > -			return lines;
> > -
> > -		lines++;
> > -
> > -		printf("%6u %17s ", c->pid, c->print_name);
> > -
> > -		for (i = 0; c->samples > 1 && i < clients->num_classes; i++) {
> > -			double pct;
> > -
> > -			if (!clients->class[i].num_engines)
> > -				continue; /* Assert in the ideal world. */
> > -
> > -			pct = (double)c->val[i] / period_us / 1e3 * 100 /
> > -			      clients->class[i].num_engines;
> > -
> > -			/*
> > -			 * Guard against possible time-drift between sampling
> > -			 * client data and time we obtained our time-delta from
> > -			 * PMU.
> > -			 */
> > -			if (pct > 100.0)
> > -				pct = 100.0;
> > -
> > -			print_percentage_bar(pct, *class_w, numeric_clients);
> > -		}
> > -
> > -		putchar('\n');
> > -	} else if (output_mode == JSON) {
> > -		char buf[64];
> > -
> > -		snprintf(buf, sizeof(buf), "%u", c->id);
> > -		pops->open_struct(buf);
> > -
> > -		__json_add_member("name", c->print_name);
> > -
> > -		snprintf(buf, sizeof(buf), "%u", c->pid);
> > -		__json_add_member("pid", buf);
> > -
> > -		if (c->samples > 1) {
> > -			pops->open_struct("engine-classes");
> > -
> > -			for (i = 0; i < clients->num_classes; i++) {
> > -				double pct;
> > -
> > -				snprintf(buf, sizeof(buf), "%s",
> > -					clients->class[i].name);
> > -				pops->open_struct(buf);
> > -
> > -				pct = (double)c->val[i] / period_us / 1e3 * 100;
> > -				snprintf(buf, sizeof(buf), "%f", pct);
> > -				__json_add_member("busy", buf);
> > -
> > -				__json_add_member("unit", "%");
> > -
> > -				pops->close_struct();
> > -			}
> > -
> > -			pops->close_struct();
> > -		}
> > -
> > -		pops->close_struct();
> > -	}
> > -
> > -	return lines;
> > -}
> > -
> > -static int
> > -print_clients_footer(struct clients *clients, double t,
> > -		     int lines, int con_w, int con_h)
> > -{
> > -	if (output_mode == INTERACTIVE) {
> > -		if (lines++ < con_h)
> > -			printf("\n");
> > -	} else {
> > -		if (clients->num_classes)
> > -			pops->close_struct();
> > -	}
> > -
> > -	return lines;
> > -}
> > -
> >   static bool stop_top;
> >   static void sigint_handler(int  sig)
> > @@ -2267,31 +1548,6 @@ static void interactive_stdin(void)
> >   	assert(ret == 0);
> >   }
> > -static void select_client_sort(void)
> > -{
> > -	struct {
> > -		int (*cmp)(const void *, const void *);
> > -		const char *msg;
> > -	} cmp[] = {
> > -		{ client_last_cmp, "Sorting clients by current GPU usage." },
> > -		{ client_total_cmp, "Sorting clients by accummulated GPU usage." },
> > -		{ client_pid_cmp, "Sorting clients by pid." },
> > -		{ client_id_cmp, "Sorting clients by sysfs id." },
> > -	};
> > -	static unsigned int client_sort;
> > -
> > -bump:
> > -	if (++client_sort >= ARRAY_SIZE(cmp))
> > -		client_sort = 0;
> > -
> > -	client_cmp = cmp[client_sort].cmp;
> > -	header_msg = cmp[client_sort].msg;
> > -
> > -	/* Sort by client id makes no sense with pid aggregation. */
> > -	if (aggregate_pids && client_cmp == client_id_cmp)
> > -		goto bump;
> > -}
> > -
> >   static bool in_help;
> >   static void process_help_stdin(void)
> > @@ -2334,29 +1590,9 @@ static void process_normal_stdin(void)
> >   			else
> >   				header_msg = "Showing physical engines.";
> >   			break;
> > -		case 'i':
> > -			filter_idle ^= true;
> > -			if (filter_idle)
> > -				header_msg = "Hiding inactive clients.";
> > -			else
> > -				header_msg = "Showing inactive clients.";
> > -			break;
> > -		case 'n':
> > -			numeric_clients ^= true;
> > -			break;
> > -		case 's':
> > -			select_client_sort();
> > -			break;
> >   		case 'h':
> >   			in_help = true;
> >   			break;
> > -		case 'H':
> > -			aggregate_pids ^= true;
> > -			if (aggregate_pids)
> > -				header_msg = "Aggregating clients.";
> > -			else
> > -				header_msg = "Showing individual clients.";
> > -			break;
> >   		};
> >   	}
> >   }
> > @@ -2384,10 +1620,6 @@ static void show_help_screen(void)
> >   	printf(
> >   "Help for interactive commands:\n\n"
> >   "    '1'    Toggle between aggregated engine class and physical engine mode.\n"
> > -"    'n'    Toggle display of numeric client busyness overlay.\n"
> > -"    's'    Toggle between sort modes (runtime, total runtime, pid, client id).\n"
> > -"    'i'    Toggle display of clients which used no GPU time.\n"
> > -"    'H'    Toggle between per PID aggregation and individual clients.\n"
> >   "\n"
> >   "    'h' or 'q'    Exit interactive help.\n"
> >   "\n");
> > @@ -2396,7 +1628,6 @@ static void show_help_screen(void)
> >   int main(int argc, char **argv)
> >   {
> >   	unsigned int period_us = DEFAULT_PERIOD_MS * 1000;
> > -	struct clients *clients = NULL;
> >   	int con_w = -1, con_h = -1;
> >   	char *output_path = NULL;
> >   	struct engines *engines;
> > @@ -2530,23 +1761,15 @@ int main(int argc, char **argv)
> >   	ret = EXIT_SUCCESS;
> > -	clients = init_clients(card.pci_slot_name[0] ? card.card : NULL);
> >   	init_engine_classes(engines);
> > -	if (clients) {
> > -		clients->num_classes = engines->num_classes;
> > -		clients->class = engines->class;
> > -	}
> >   	pmu_sample(engines);
> > -	scan_clients(clients);
> >   	codename = igt_device_get_pretty_name(&card, false);
> >   	while (!stop_top) {
> > -		struct clients *disp_clients;
> >   		bool consumed = false;
> > -		int j, lines = 0;
> >   		struct winsize ws;
> > -		struct client *c;
> > +		int lines = 0;
> >   		double t;
> >   		/* Update terminal size. */
> > @@ -2565,8 +1788,6 @@ int main(int argc, char **argv)
> >   		pmu_sample(engines);
> >   		t = (double)(engines->ts.cur - engines->ts.prev) / 1e9;
> > -		disp_clients = scan_clients(clients);
> > -
> >   		if (stop_top)
> >   			break;
> > @@ -2586,41 +1807,12 @@ int main(int argc, char **argv)
> >   			lines = print_engines(engines, t, lines, con_w, con_h);
> > -			if (disp_clients) {
> > -				int class_w;
> > -
> > -				lines = print_clients_header(disp_clients, lines,
> > -							     con_w, con_h,
> > -							     &class_w);
> > -
> > -				for_each_client(disp_clients, c, j) {
> > -					assert(c->status != PROBE);
> > -					if (c->status != ALIVE)
> > -						break; /* Active clients are first in the array. */
> > -
> > -					if (lines >= con_h)
> > -						break;
> > -
> > -					lines = print_client(c, engines, t,
> > -							     lines, con_w,
> > -							     con_h, period_us,
> > -							     &class_w);
> > -				}
> > -
> > -				lines = print_clients_footer(disp_clients, t,
> > -							     lines, con_w,
> > -							     con_h);
> > -			}
> > -
> >   			pops->close_struct();
> >   		}
> >   		if (stop_top)
> >   			break;
> > -		if (disp_clients != clients)
> > -			free_clients(disp_clients);
> > -
> >   		if (output_mode == INTERACTIVE)
> >   			process_stdin(period_us);
> >   		else
> > 

Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 5/6] intel_gpu_top: Remove clients support

[Tvrtko Ursulin]

On 26/11/2021 14:01, Petri Latvala wrote:
> On Fri, Nov 26, 2021 at 01:07:24PM +0000, Tvrtko Ursulin wrote:
>>
>> On 19/11/2021 12:59, Tvrtko Ursulin wrote:
>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>
>>> When kernel feature was removed the intel_gpu_top part was forgotten.
>>>
>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>>
>> Will someone ack this or we carry this code until it ships, if it hasn't
>> already?
> 
> Does that question mean client busyness will some day return?

No, question meant I was pinging to ack the removal ever since the i915 
code was removed months ago. "Until it ships" was referring to IGT 
codebase containing defunct code potentially shipping to distros.

Regarding the return of per client busyness - I certainly hope so. i915 
and intel_gpu_top code exposing it via /proc fdinfo is available and 
given positive noises from the community we could probably merge it with 
some reviews. A couple of userspace clients are interested as well, like 
Chromium and KDE, although I think intel_gpu_top is enough really given 
AMD already has this exposed via fdinfo and they are supportive of 
documenting the shared spec.

> Either way,
> Acked-by: Petri Latvala <petri.latvala@intel.com>

Thanks!

Regards,

Tvrtko

> 
> 
>>
>> Regards,
>>
>> Tvrtko
>>
>>> ---
>>>    man/intel_gpu_top.rst |   4 -
>>>    tools/intel_gpu_top.c | 810 +-----------------------------------------
>>>    2 files changed, 1 insertion(+), 813 deletions(-)
>>>
>>> diff --git a/man/intel_gpu_top.rst b/man/intel_gpu_top.rst
>>> index f4dbfc5b44d9..b3b765b05feb 100644
>>> --- a/man/intel_gpu_top.rst
>>> +++ b/man/intel_gpu_top.rst
>>> @@ -56,10 +56,6 @@ Supported keys:
>>>        'q'    Exit from the tool.
>>>        'h'    Show interactive help.
>>>        '1'    Toggle between aggregated engine class and physical engine mode.
>>> -    'n'    Toggle display of numeric client busyness overlay.
>>> -    's'    Toggle between sort modes (runtime, total runtime, pid, client id).
>>> -    'i'    Toggle display of clients which used no GPU time.
>>> -    'H'    Toggle between per PID aggregation and individual clients.
>>>    DEVICE SELECTION
>>>    ================
>>> diff --git a/tools/intel_gpu_top.c b/tools/intel_gpu_top.c
>>> index 7311038a39f4..41c59a72c09d 100644
>>> --- a/tools/intel_gpu_top.c
>>> +++ b/tools/intel_gpu_top.c
>>> @@ -627,562 +627,6 @@ static void pmu_sample(struct engines *engines)
>>>    	}
>>>    }
>>> -enum client_status {
>>> -	FREE = 0, /* mbz */
>>> -	ALIVE,
>>> -	PROBE
>>> -};
>>> -
>>> -struct clients;
>>> -
>>> -struct client {
>>> -	struct clients *clients;
>>> -
>>> -	enum client_status status;
>>> -	int sysfs_root;
>>> -	int busy_root;
>>> -	unsigned int id;
>>> -	unsigned int pid;
>>> -	char name[24];
>>> -	char print_name[24];
>>> -	unsigned int samples;
>>> -	unsigned long total_runtime;
>>> -	unsigned long last_runtime;
>>> -	struct engines *engines;
>>> -	unsigned long *val;
>>> -	uint64_t *last;
>>> -};
>>> -
>>> -struct clients {
>>> -	unsigned int num_clients;
>>> -	unsigned int active_clients;
>>> -
>>> -	unsigned int num_classes;
>>> -	struct engine_class *class;
>>> -
>>> -	char sysfs_root[128];
>>> -
>>> -	struct client *client;
>>> -};
>>> -
>>> -#define for_each_client(clients, c, tmp) \
>>> -	for ((tmp) = (clients)->num_clients, c = (clients)->client; \
>>> -	     (tmp > 0); (tmp)--, (c)++)
>>> -
>>> -static struct clients *init_clients(const char *drm_card)
>>> -{
>>> -	struct clients *clients;
>>> -	const char *slash;
>>> -	ssize_t ret;
>>> -	int dir;
>>> -
>>> -	clients = malloc(sizeof(*clients));
>>> -	if (!clients)
>>> -		return NULL;
>>> -
>>> -	memset(clients, 0, sizeof(*clients));
>>> -
>>> -	if (drm_card) {
>>> -		slash = rindex(drm_card, '/');
>>> -		assert(slash);
>>> -	} else {
>>> -		slash = "card0";
>>> -	}
>>> -
>>> -	ret = snprintf(clients->sysfs_root, sizeof(clients->sysfs_root),
>>> -		       "/sys/class/drm/%s/clients/", slash);
>>> -	assert(ret > 0 && ret < sizeof(clients->sysfs_root));
>>> -
>>> -	dir = open(clients->sysfs_root, O_DIRECTORY | O_RDONLY);
>>> -	if (dir < 0) {
>>> -		free(clients);
>>> -		clients = NULL;
>>> -	} else {
>>> -		close(dir);
>>> -	}
>>> -
>>> -	return clients;
>>> -}
>>> -
>>> -static int __read_to_buf(int fd, char *buf, unsigned int bufsize)
>>> -{
>>> -	ssize_t ret;
>>> -	int err;
>>> -
>>> -	ret = read(fd, buf, bufsize - 1);
>>> -	err = errno;
>>> -	if (ret < 1) {
>>> -		errno = ret < 0 ? err : ENOMSG;
>>> -
>>> -		return -1;
>>> -	}
>>> -
>>> -	if (ret > 1 && buf[ret - 1] == '\n')
>>> -		buf[ret - 1] = '\0';
>>> -	else
>>> -		buf[ret] = '\0';
>>> -
>>> -	return 0;
>>> -}
>>> -
>>> -static int
>>> -__read_client_field(int root, const char *field, char *buf, unsigned int bufsize)
>>> -{
>>> -	int fd, ret;
>>> -
>>> -	fd = openat(root, field, O_RDONLY);
>>> -	if (fd < 0)
>>> -		return -1;
>>> -
>>> -	ret = __read_to_buf(fd, buf, bufsize);
>>> -
>>> -	close(fd);
>>> -
>>> -	return ret;
>>> -}
>>> -
>>> -static uint64_t
>>> -read_client_busy(struct client *client, unsigned int class)
>>> -{
>>> -	const char *class_str[] = { "0", "1", "2", "3", "4", "5", "6", "7" };
>>> -	char buf[256], *b;
>>> -	int ret;
>>> -
>>> -	assert(class < ARRAY_SIZE(class_str));
>>> -	if (class >= ARRAY_SIZE(class_str))
>>> -		return 0;
>>> -
>>> -	assert(client->sysfs_root >= 0);
>>> -	if (client->sysfs_root < 0)
>>> -		return 0;
>>> -
>>> -	if (client->busy_root < 0)
>>> -		client->busy_root = openat(client->sysfs_root, "busy",
>>> -					   O_RDONLY | O_DIRECTORY);
>>> -
>>> -	assert(client->busy_root);
>>> -	if (client->busy_root < 0)
>>> -		return 0;
>>> -
>>> -	ret = __read_client_field(client->busy_root, class_str[class], buf,
>>> -				  sizeof(buf));
>>> -	if (ret) {
>>> -		close(client->busy_root);
>>> -		client->busy_root = -1;
>>> -		return 0;
>>> -	}
>>> -
>>> -	/*
>>> -	 * Handle both single integer and key=value formats by skipping
>>> -	 * leading non-digits.
>>> -	 */
>>> -	b = buf;
>>> -	while (*b && !isdigit(*b))
>>> -		b++;
>>> -
>>> -	return strtoull(b, NULL, 10);
>>> -}
>>> -
>>> -static struct client *
>>> -find_client(struct clients *clients, enum client_status status, unsigned int id)
>>> -{
>>> -	unsigned int start, num;
>>> -	struct client *c;
>>> -
>>> -	start = status == FREE ? clients->active_clients : 0; /* Free block at the end. */
>>> -	num = clients->num_clients - start;
>>> -
>>> -	for (c = &clients->client[start]; num; c++, num--) {
>>> -		if (status != c->status)
>>> -			continue;
>>> -
>>> -		if (status == FREE || c->id == id)
>>> -			return c;
>>> -	}
>>> -
>>> -	return NULL;
>>> -}
>>> -
>>> -static void update_client(struct client *c, unsigned int pid, char *name)
>>> -{
>>> -	uint64_t val[c->clients->num_classes];
>>> -	unsigned int i;
>>> -
>>> -	if (c->pid != pid)
>>> -		c->pid = pid;
>>> -
>>> -	if (strcmp(c->name, name)) {
>>> -		char *p;
>>> -
>>> -		strncpy(c->name, name, sizeof(c->name) - 1);
>>> -		strncpy(c->print_name, name, sizeof(c->print_name) - 1);
>>> -
>>> -		p = c->print_name;
>>> -		while (*p) {
>>> -			if (!isprint(*p))
>>> -				*p = '*';
>>> -			p++;
>>> -		}
>>> -	}
>>> -
>>> -	for (i = 0; i < c->clients->num_classes; i++)
>>> -		val[i] = read_client_busy(c, c->clients->class[i].class);
>>> -
>>> -	c->last_runtime = 0;
>>> -	c->total_runtime = 0;
>>> -
>>> -	for (i = 0; i < c->clients->num_classes; i++) {
>>> -		if (val[i] < c->last[i])
>>> -			continue; /* It will catch up soon. */
>>> -
>>> -		c->total_runtime += val[i];
>>> -		c->val[i] = val[i] - c->last[i];
>>> -		c->last_runtime += c->val[i];
>>> -		c->last[i] = val[i];
>>> -	}
>>> -
>>> -	c->samples++;
>>> -	c->status = ALIVE;
>>> -}
>>> -
>>> -static void
>>> -add_client(struct clients *clients, unsigned int id, unsigned int pid,
>>> -	   char *name, int sysfs_root)
>>> -{
>>> -	struct client *c;
>>> -
>>> -	assert(!find_client(clients, ALIVE, id));
>>> -
>>> -	c = find_client(clients, FREE, 0);
>>> -	if (!c) {
>>> -		unsigned int idx = clients->num_clients;
>>> -
>>> -		clients->num_clients += (clients->num_clients + 2) / 2;
>>> -		clients->client = realloc(clients->client,
>>> -					  clients->num_clients * sizeof(*c));
>>> -		assert(clients->client);
>>> -
>>> -		c = &clients->client[idx];
>>> -		memset(c, 0, (clients->num_clients - idx) * sizeof(*c));
>>> -	}
>>> -
>>> -	c->sysfs_root = sysfs_root;
>>> -	c->busy_root = -1;
>>> -	c->id = id;
>>> -	c->clients = clients;
>>> -	c->val = calloc(clients->num_classes, sizeof(c->val));
>>> -	c->last = calloc(clients->num_classes, sizeof(c->last));
>>> -	assert(c->val && c->last);
>>> -
>>> -	update_client(c, pid, name);
>>> -}
>>> -
>>> -static void free_client(struct client *c)
>>> -{
>>> -	if (c->sysfs_root >= 0)
>>> -		close(c->sysfs_root);
>>> -	if (c->busy_root >= 0)
>>> -		close(c->busy_root);
>>> -	free(c->val);
>>> -	free(c->last);
>>> -	memset(c, 0, sizeof(*c));
>>> -}
>>> -
>>> -static int
>>> -read_client_sysfs(char *buf, int bufsize, const char *sysfs_root,
>>> -		  unsigned int id, const char *field, int *client_root)
>>> -{
>>> -	ssize_t ret;
>>> -
>>> -	if (*client_root < 0) {
>>> -		char namebuf[256];
>>> -
>>> -		ret = snprintf(namebuf, sizeof(namebuf), "%s/%u",
>>> -			       sysfs_root, id);
>>> -		assert(ret > 0 && ret < sizeof(namebuf));
>>> -		if (ret <= 0 || ret == sizeof(namebuf))
>>> -			return -1;
>>> -
>>> -		*client_root = open(namebuf, O_RDONLY | O_DIRECTORY);
>>> -	}
>>> -
>>> -	if (*client_root < 0)
>>> -		return -1;
>>> -
>>> -	return __read_client_field(*client_root, field, buf, bufsize);
>>> -}
>>> -
>>> -static int client_last_cmp(const void *_a, const void *_b)
>>> -{
>>> -	const struct client *a = _a;
>>> -	const struct client *b = _b;
>>> -	long tot_a, tot_b;
>>> -
>>> -	/*
>>> -	 * Sort clients in descending order of runtime in the previous sampling
>>> -	 * period for active ones, followed by inactive. Tie-breaker is client
>>> -	 * id.
>>> -	 */
>>> -
>>> -	tot_a = a->status == ALIVE ? a->last_runtime : -1;
>>> -	tot_b = b->status == ALIVE ? b->last_runtime : -1;
>>> -
>>> -	tot_b -= tot_a;
>>> -	if (tot_b > 0)
>>> -		return 1;
>>> -	if (tot_b < 0)
>>> -		return -1;
>>> -
>>> -	return (int)b->id - a->id;
>>> -}
>>> -
>>> -static int client_total_cmp(const void *_a, const void *_b)
>>> -{
>>> -	const struct client *a = _a;
>>> -	const struct client *b = _b;
>>> -	long tot_a, tot_b;
>>> -
>>> -	tot_a = a->status == ALIVE ? a->total_runtime : -1;
>>> -	tot_b = b->status == ALIVE ? b->total_runtime : -1;
>>> -
>>> -	tot_b -= tot_a;
>>> -	if (tot_b > 0)
>>> -		return 1;
>>> -	if (tot_b < 0)
>>> -		return -1;
>>> -
>>> -	return (int)b->id - a->id;
>>> -}
>>> -
>>> -static int client_id_cmp(const void *_a, const void *_b)
>>> -{
>>> -	const struct client *a = _a;
>>> -	const struct client *b = _b;
>>> -	int id_a, id_b;
>>> -
>>> -	id_a = a->status == ALIVE ? a->id : -1;
>>> -	id_b = b->status == ALIVE ? b->id : -1;
>>> -
>>> -	id_b -= id_a;
>>> -	if (id_b > 0)
>>> -		return 1;
>>> -	if (id_b < 0)
>>> -		return -1;
>>> -
>>> -	return (int)b->id - a->id;
>>> -}
>>> -
>>> -static int client_pid_cmp(const void *_a, const void *_b)
>>> -{
>>> -	const struct client *a = _a;
>>> -	const struct client *b = _b;
>>> -	int pid_a, pid_b;
>>> -
>>> -	pid_a = a->status == ALIVE ? a->pid : INT_MAX;
>>> -	pid_b = b->status == ALIVE ? b->pid : INT_MAX;
>>> -
>>> -	pid_b -= pid_a;
>>> -	if (pid_b > 0)
>>> -		return -1;
>>> -	if (pid_b < 0)
>>> -		return 1;
>>> -
>>> -	return (int)a->id - b->id;
>>> -}
>>> -
>>> -static int (*client_cmp)(const void *, const void *) = client_last_cmp;
>>> -
>>> -static struct clients *sort_clients(struct clients *clients,
>>> -				    int (*cmp)(const void *, const void *))
>>> -{
>>> -	unsigned int active, free;
>>> -	struct client *c;
>>> -	int tmp;
>>> -
>>> -	if (!clients)
>>> -		return clients;
>>> -
>>> -	qsort(clients->client, clients->num_clients, sizeof(*clients->client),
>>> -	      cmp);
>>> -
>>> -	/* Trim excessive array space. */
>>> -	active = 0;
>>> -	for_each_client(clients, c, tmp) {
>>> -		if (c->status != ALIVE)
>>> -			break; /* Active clients are first in the array. */
>>> -		active++;
>>> -	}
>>> -
>>> -	clients->active_clients = active;
>>> -
>>> -	free = clients->num_clients - active;
>>> -	if (free > clients->num_clients / 2) {
>>> -		active = clients->num_clients - free / 2;
>>> -		if (active != clients->num_clients) {
>>> -			clients->num_clients = active;
>>> -			clients->client = realloc(clients->client,
>>> -						  clients->num_clients *
>>> -						  sizeof(*c));
>>> -		}
>>> -	}
>>> -
>>> -	return clients;
>>> -}
>>> -
>>> -static bool aggregate_pids = true;
>>> -
>>> -static struct clients *display_clients(struct clients *clients)
>>> -{
>>> -	struct client *ac, *c, *cp = NULL;
>>> -	struct clients *aggregated;
>>> -	int tmp, num = 0;
>>> -
>>> -	if (!aggregate_pids)
>>> -		goto out;
>>> -
>>> -	/* Sort by pid first to make it easy to aggregate while walking. */
>>> -	sort_clients(clients, client_pid_cmp);
>>> -
>>> -	aggregated = calloc(1, sizeof(*clients));
>>> -	assert(aggregated);
>>> -
>>> -	ac = calloc(clients->num_clients, sizeof(*c));
>>> -	assert(ac);
>>> -
>>> -	aggregated->num_classes = clients->num_classes;
>>> -	aggregated->class = clients->class;
>>> -	aggregated->client = ac;
>>> -
>>> -	for_each_client(clients, c, tmp) {
>>> -		unsigned int i;
>>> -
>>> -		if (c->status == FREE)
>>> -			break;
>>> -
>>> -		assert(c->status == ALIVE);
>>> -
>>> -		if ((cp && c->pid != cp->pid) || !cp) {
>>> -			ac = &aggregated->client[num++];
>>> -
>>> -			/* New pid. */
>>> -			ac->clients = aggregated;
>>> -			ac->status = ALIVE;
>>> -			ac->id = -c->pid;
>>> -			ac->pid = c->pid;
>>> -			ac->busy_root = -1;
>>> -			ac->sysfs_root = -1;
>>> -			strcpy(ac->name, c->name);
>>> -			strcpy(ac->print_name, c->print_name);
>>> -			ac->engines = c->engines;
>>> -			ac->val = calloc(clients->num_classes,
>>> -					 sizeof(ac->val[0]));
>>> -			assert(ac->val);
>>> -			ac->samples = 1;
>>> -		}
>>> -
>>> -		cp = c;
>>> -
>>> -		if (c->samples < 2)
>>> -			continue;
>>> -
>>> -		ac->samples = 2; /* All what matters for display. */
>>> -		ac->total_runtime += c->total_runtime;
>>> -		ac->last_runtime += c->last_runtime;
>>> -
>>> -		for (i = 0; i < clients->num_classes; i++)
>>> -			ac->val[i] += c->val[i];
>>> -	}
>>> -
>>> -	aggregated->num_clients = num;
>>> -	aggregated->active_clients = num;
>>> -
>>> -	clients = aggregated;
>>> -
>>> -out:
>>> -	return sort_clients(clients, client_cmp);
>>> -}
>>> -
>>> -static void free_clients(struct clients *clients)
>>> -{
>>> -	struct client *c;
>>> -	unsigned int tmp;
>>> -
>>> -	for_each_client(clients, c, tmp) {
>>> -		free(c->val);
>>> -		free(c->last);
>>> -	}
>>> -
>>> -	free(clients->client);
>>> -	free(clients);
>>> -}
>>> -
>>> -static struct clients *scan_clients(struct clients *clients)
>>> -{
>>> -	struct dirent *dent;
>>> -	struct client *c;
>>> -	unsigned int id;
>>> -	int tmp;
>>> -	DIR *d;
>>> -
>>> -	if (!clients)
>>> -		return clients;
>>> -
>>> -	for_each_client(clients, c, tmp) {
>>> -		assert(c->status != PROBE);
>>> -		if (c->status == ALIVE)
>>> -			c->status = PROBE;
>>> -		else
>>> -			break; /* Free block at the end of array. */
>>> -	}
>>> -
>>> -	d = opendir(clients->sysfs_root);
>>> -	if (!d)
>>> -		return clients;
>>> -
>>> -	while ((dent = readdir(d)) != NULL) {
>>> -		char name[24], pid[24];
>>> -		int ret, root = -1, *pr;
>>> -
>>> -		if (dent->d_type != DT_DIR)
>>> -			continue;
>>> -		if (!isdigit(dent->d_name[0]))
>>> -			continue;
>>> -
>>> -		id = atoi(dent->d_name);
>>> -
>>> -		c = find_client(clients, PROBE, id);
>>> -
>>> -		if (c)
>>> -			pr = &c->sysfs_root;
>>> -		else
>>> -			pr = &root;
>>> -
>>> -		ret = read_client_sysfs(name, sizeof(name), clients->sysfs_root,
>>> -					id, "name", pr);
>>> -		ret |= read_client_sysfs(pid, sizeof(pid), clients->sysfs_root,
>>> -					id, "pid", pr);
>>> -		if (!ret) {
>>> -			if (!c)
>>> -				add_client(clients, id, atoi(pid), name, root);
>>> -			else
>>> -				update_client(c, atoi(pid), name);
>>> -		} else if (c) {
>>> -			c->status = PROBE; /* Will be deleted below. */
>>> -		}
>>> -	}
>>> -
>>> -	closedir(d);
>>> -
>>> -	for_each_client(clients, c, tmp) {
>>> -		if (c->status == PROBE)
>>> -			free_client(c);
>>> -		else if (c->status == FREE)
>>> -			break;
>>> -	}
>>> -
>>> -	return display_clients(clients);
>>> -}
>>> -
>>>    static const char *bars[] = { " ", "▏", "▎", "▍", "▌", "▋", "▊", "▉", "█" };
>>>    static void n_spaces(const unsigned int n)
>>> @@ -1324,18 +768,6 @@ json_close_struct(void)
>>>    		fflush(stdout);
>>>    }
>>> -static void
>>> -__json_add_member(const char *key, const char *val)
>>> -{
>>> -	assert(json_indent_level < ARRAY_SIZE(json_indent));
>>> -
>>> -	fprintf(out, "%s%s\"%s\": \"%s\"",
>>> -		json_struct_members ? ",\n" : "",
>>> -		json_indent[json_indent_level], key, val);
>>> -
>>> -	json_struct_members++;
>>> -}
>>> -
>>>    static unsigned int
>>>    json_add_member(const struct cnt_group *parent, struct cnt_item *item,
>>>    		unsigned int headers)
>>> @@ -2061,157 +1493,6 @@ print_engines(struct engines *engines, double t, int lines, int w, int h)
>>>    	return lines;
>>>    }
>>> -static int
>>> -print_clients_header(struct clients *clients, int lines,
>>> -		     int con_w, int con_h, int *class_w)
>>> -{
>>> -	if (output_mode == INTERACTIVE) {
>>> -		const char *pidname = "   PID              NAME ";
>>> -		unsigned int num_active = 0;
>>> -		int len = strlen(pidname);
>>> -
>>> -		if (lines++ >= con_h)
>>> -			return lines;
>>> -
>>> -		printf("\033[7m");
>>> -		printf("%s", pidname);
>>> -
>>> -		if (lines++ >= con_h || len >= con_w)
>>> -			return lines;
>>> -
>>> -		if (clients->num_classes) {
>>> -			unsigned int i;
>>> -			int width;
>>> -
>>> -			for (i = 0; i < clients->num_classes; i++) {
>>> -				if (clients->class[i].num_engines)
>>> -					num_active++;
>>> -			}
>>> -
>>> -			*class_w = width = (con_w - len) / num_active;
>>> -
>>> -			for (i = 0; i < clients->num_classes; i++) {
>>> -				const char *name = clients->class[i].name;
>>> -				int name_len = strlen(name);
>>> -				int pad = (width - name_len) / 2;
>>> -				int spaces = width - pad - name_len;
>>> -
>>> -				if (!clients->class[i].num_engines)
>>> -					continue; /* Assert in the ideal world. */
>>> -
>>> -				if (pad < 0 || spaces < 0)
>>> -					continue;
>>> -
>>> -				n_spaces(pad);
>>> -				printf("%s", name);
>>> -				n_spaces(spaces);
>>> -				len += pad + name_len + spaces;
>>> -			}
>>> -		}
>>> -
>>> -		n_spaces(con_w - len);
>>> -		printf("\033[0m\n");
>>> -	} else {
>>> -		if (clients->num_classes)
>>> -			pops->open_struct("clients");
>>> -	}
>>> -
>>> -	return lines;
>>> -}
>>> -
>>> -static bool numeric_clients;
>>> -static bool filter_idle;
>>> -
>>> -static int
>>> -print_client(struct client *c, struct engines *engines, double t, int lines,
>>> -	     int con_w, int con_h, unsigned int period_us, int *class_w)
>>> -{
>>> -	struct clients *clients = c->clients;
>>> -	unsigned int i;
>>> -
>>> -	if (output_mode == INTERACTIVE) {
>>> -		if (filter_idle && (!c->total_runtime || c->samples < 2))
>>> -			return lines;
>>> -
>>> -		lines++;
>>> -
>>> -		printf("%6u %17s ", c->pid, c->print_name);
>>> -
>>> -		for (i = 0; c->samples > 1 && i < clients->num_classes; i++) {
>>> -			double pct;
>>> -
>>> -			if (!clients->class[i].num_engines)
>>> -				continue; /* Assert in the ideal world. */
>>> -
>>> -			pct = (double)c->val[i] / period_us / 1e3 * 100 /
>>> -			      clients->class[i].num_engines;
>>> -
>>> -			/*
>>> -			 * Guard against possible time-drift between sampling
>>> -			 * client data and time we obtained our time-delta from
>>> -			 * PMU.
>>> -			 */
>>> -			if (pct > 100.0)
>>> -				pct = 100.0;
>>> -
>>> -			print_percentage_bar(pct, *class_w, numeric_clients);
>>> -		}
>>> -
>>> -		putchar('\n');
>>> -	} else if (output_mode == JSON) {
>>> -		char buf[64];
>>> -
>>> -		snprintf(buf, sizeof(buf), "%u", c->id);
>>> -		pops->open_struct(buf);
>>> -
>>> -		__json_add_member("name", c->print_name);
>>> -
>>> -		snprintf(buf, sizeof(buf), "%u", c->pid);
>>> -		__json_add_member("pid", buf);
>>> -
>>> -		if (c->samples > 1) {
>>> -			pops->open_struct("engine-classes");
>>> -
>>> -			for (i = 0; i < clients->num_classes; i++) {
>>> -				double pct;
>>> -
>>> -				snprintf(buf, sizeof(buf), "%s",
>>> -					clients->class[i].name);
>>> -				pops->open_struct(buf);
>>> -
>>> -				pct = (double)c->val[i] / period_us / 1e3 * 100;
>>> -				snprintf(buf, sizeof(buf), "%f", pct);
>>> -				__json_add_member("busy", buf);
>>> -
>>> -				__json_add_member("unit", "%");
>>> -
>>> -				pops->close_struct();
>>> -			}
>>> -
>>> -			pops->close_struct();
>>> -		}
>>> -
>>> -		pops->close_struct();
>>> -	}
>>> -
>>> -	return lines;
>>> -}
>>> -
>>> -static int
>>> -print_clients_footer(struct clients *clients, double t,
>>> -		     int lines, int con_w, int con_h)
>>> -{
>>> -	if (output_mode == INTERACTIVE) {
>>> -		if (lines++ < con_h)
>>> -			printf("\n");
>>> -	} else {
>>> -		if (clients->num_classes)
>>> -			pops->close_struct();
>>> -	}
>>> -
>>> -	return lines;
>>> -}
>>> -
>>>    static bool stop_top;
>>>    static void sigint_handler(int  sig)
>>> @@ -2267,31 +1548,6 @@ static void interactive_stdin(void)
>>>    	assert(ret == 0);
>>>    }
>>> -static void select_client_sort(void)
>>> -{
>>> -	struct {
>>> -		int (*cmp)(const void *, const void *);
>>> -		const char *msg;
>>> -	} cmp[] = {
>>> -		{ client_last_cmp, "Sorting clients by current GPU usage." },
>>> -		{ client_total_cmp, "Sorting clients by accummulated GPU usage." },
>>> -		{ client_pid_cmp, "Sorting clients by pid." },
>>> -		{ client_id_cmp, "Sorting clients by sysfs id." },
>>> -	};
>>> -	static unsigned int client_sort;
>>> -
>>> -bump:
>>> -	if (++client_sort >= ARRAY_SIZE(cmp))
>>> -		client_sort = 0;
>>> -
>>> -	client_cmp = cmp[client_sort].cmp;
>>> -	header_msg = cmp[client_sort].msg;
>>> -
>>> -	/* Sort by client id makes no sense with pid aggregation. */
>>> -	if (aggregate_pids && client_cmp == client_id_cmp)
>>> -		goto bump;
>>> -}
>>> -
>>>    static bool in_help;
>>>    static void process_help_stdin(void)
>>> @@ -2334,29 +1590,9 @@ static void process_normal_stdin(void)
>>>    			else
>>>    				header_msg = "Showing physical engines.";
>>>    			break;
>>> -		case 'i':
>>> -			filter_idle ^= true;
>>> -			if (filter_idle)
>>> -				header_msg = "Hiding inactive clients.";
>>> -			else
>>> -				header_msg = "Showing inactive clients.";
>>> -			break;
>>> -		case 'n':
>>> -			numeric_clients ^= true;
>>> -			break;
>>> -		case 's':
>>> -			select_client_sort();
>>> -			break;
>>>    		case 'h':
>>>    			in_help = true;
>>>    			break;
>>> -		case 'H':
>>> -			aggregate_pids ^= true;
>>> -			if (aggregate_pids)
>>> -				header_msg = "Aggregating clients.";
>>> -			else
>>> -				header_msg = "Showing individual clients.";
>>> -			break;
>>>    		};
>>>    	}
>>>    }
>>> @@ -2384,10 +1620,6 @@ static void show_help_screen(void)
>>>    	printf(
>>>    "Help for interactive commands:\n\n"
>>>    "    '1'    Toggle between aggregated engine class and physical engine mode.\n"
>>> -"    'n'    Toggle display of numeric client busyness overlay.\n"
>>> -"    's'    Toggle between sort modes (runtime, total runtime, pid, client id).\n"
>>> -"    'i'    Toggle display of clients which used no GPU time.\n"
>>> -"    'H'    Toggle between per PID aggregation and individual clients.\n"
>>>    "\n"
>>>    "    'h' or 'q'    Exit interactive help.\n"
>>>    "\n");
>>> @@ -2396,7 +1628,6 @@ static void show_help_screen(void)
>>>    int main(int argc, char **argv)
>>>    {
>>>    	unsigned int period_us = DEFAULT_PERIOD_MS * 1000;
>>> -	struct clients *clients = NULL;
>>>    	int con_w = -1, con_h = -1;
>>>    	char *output_path = NULL;
>>>    	struct engines *engines;
>>> @@ -2530,23 +1761,15 @@ int main(int argc, char **argv)
>>>    	ret = EXIT_SUCCESS;
>>> -	clients = init_clients(card.pci_slot_name[0] ? card.card : NULL);
>>>    	init_engine_classes(engines);
>>> -	if (clients) {
>>> -		clients->num_classes = engines->num_classes;
>>> -		clients->class = engines->class;
>>> -	}
>>>    	pmu_sample(engines);
>>> -	scan_clients(clients);
>>>    	codename = igt_device_get_pretty_name(&card, false);
>>>    	while (!stop_top) {
>>> -		struct clients *disp_clients;
>>>    		bool consumed = false;
>>> -		int j, lines = 0;
>>>    		struct winsize ws;
>>> -		struct client *c;
>>> +		int lines = 0;
>>>    		double t;
>>>    		/* Update terminal size. */
>>> @@ -2565,8 +1788,6 @@ int main(int argc, char **argv)
>>>    		pmu_sample(engines);
>>>    		t = (double)(engines->ts.cur - engines->ts.prev) / 1e9;
>>> -		disp_clients = scan_clients(clients);
>>> -
>>>    		if (stop_top)
>>>    			break;
>>> @@ -2586,41 +1807,12 @@ int main(int argc, char **argv)
>>>    			lines = print_engines(engines, t, lines, con_w, con_h);
>>> -			if (disp_clients) {
>>> -				int class_w;
>>> -
>>> -				lines = print_clients_header(disp_clients, lines,
>>> -							     con_w, con_h,
>>> -							     &class_w);
>>> -
>>> -				for_each_client(disp_clients, c, j) {
>>> -					assert(c->status != PROBE);
>>> -					if (c->status != ALIVE)
>>> -						break; /* Active clients are first in the array. */
>>> -
>>> -					if (lines >= con_h)
>>> -						break;
>>> -
>>> -					lines = print_client(c, engines, t,
>>> -							     lines, con_w,
>>> -							     con_h, period_us,
>>> -							     &class_w);
>>> -				}
>>> -
>>> -				lines = print_clients_footer(disp_clients, t,
>>> -							     lines, con_w,
>>> -							     con_h);
>>> -			}
>>> -
>>>    			pops->close_struct();
>>>    		}
>>>    		if (stop_top)
>>>    			break;
>>> -		if (disp_clients != clients)
>>> -			free_clients(disp_clients);
>>> -
>>>    		if (output_mode == INTERACTIVE)
>>>    			process_stdin(period_us);
>>>    		else
>>>

Re: [igt-dev] [PATCH i-g-t 5/6] intel_gpu_top: Remove clients support

[Tvrtko Ursulin]

On 26/11/2021 14:01, Petri Latvala wrote:
> On Fri, Nov 26, 2021 at 01:07:24PM +0000, Tvrtko Ursulin wrote:
>>
>> On 19/11/2021 12:59, Tvrtko Ursulin wrote:
>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>
>>> When kernel feature was removed the intel_gpu_top part was forgotten.
>>>
>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>>
>> Will someone ack this or we carry this code until it ships, if it hasn't
>> already?
> 
> Does that question mean client busyness will some day return?

No, question meant I was pinging to ack the removal ever since the i915 
code was removed months ago. "Until it ships" was referring to IGT 
codebase containing defunct code potentially shipping to distros.

Regarding the return of per client busyness - I certainly hope so. i915 
and intel_gpu_top code exposing it via /proc fdinfo is available and 
given positive noises from the community we could probably merge it with 
some reviews. A couple of userspace clients are interested as well, like 
Chromium and KDE, although I think intel_gpu_top is enough really given 
AMD already has this exposed via fdinfo and they are supportive of 
documenting the shared spec.

> Either way,
> Acked-by: Petri Latvala <petri.latvala@intel.com>

Thanks!

Regards,

Tvrtko

> 
> 
>>
>> Regards,
>>
>> Tvrtko
>>
>>> ---
>>>    man/intel_gpu_top.rst |   4 -
>>>    tools/intel_gpu_top.c | 810 +-----------------------------------------
>>>    2 files changed, 1 insertion(+), 813 deletions(-)
>>>
>>> diff --git a/man/intel_gpu_top.rst b/man/intel_gpu_top.rst
>>> index f4dbfc5b44d9..b3b765b05feb 100644
>>> --- a/man/intel_gpu_top.rst
>>> +++ b/man/intel_gpu_top.rst
>>> @@ -56,10 +56,6 @@ Supported keys:
>>>        'q'    Exit from the tool.
>>>        'h'    Show interactive help.
>>>        '1'    Toggle between aggregated engine class and physical engine mode.
>>> -    'n'    Toggle display of numeric client busyness overlay.
>>> -    's'    Toggle between sort modes (runtime, total runtime, pid, client id).
>>> -    'i'    Toggle display of clients which used no GPU time.
>>> -    'H'    Toggle between per PID aggregation and individual clients.
>>>    DEVICE SELECTION
>>>    ================
>>> diff --git a/tools/intel_gpu_top.c b/tools/intel_gpu_top.c
>>> index 7311038a39f4..41c59a72c09d 100644
>>> --- a/tools/intel_gpu_top.c
>>> +++ b/tools/intel_gpu_top.c
>>> @@ -627,562 +627,6 @@ static void pmu_sample(struct engines *engines)
>>>    	}
>>>    }
>>> -enum client_status {
>>> -	FREE = 0, /* mbz */
>>> -	ALIVE,
>>> -	PROBE
>>> -};
>>> -
>>> -struct clients;
>>> -
>>> -struct client {
>>> -	struct clients *clients;
>>> -
>>> -	enum client_status status;
>>> -	int sysfs_root;
>>> -	int busy_root;
>>> -	unsigned int id;
>>> -	unsigned int pid;
>>> -	char name[24];
>>> -	char print_name[24];
>>> -	unsigned int samples;
>>> -	unsigned long total_runtime;
>>> -	unsigned long last_runtime;
>>> -	struct engines *engines;
>>> -	unsigned long *val;
>>> -	uint64_t *last;
>>> -};
>>> -
>>> -struct clients {
>>> -	unsigned int num_clients;
>>> -	unsigned int active_clients;
>>> -
>>> -	unsigned int num_classes;
>>> -	struct engine_class *class;
>>> -
>>> -	char sysfs_root[128];
>>> -
>>> -	struct client *client;
>>> -};
>>> -
>>> -#define for_each_client(clients, c, tmp) \
>>> -	for ((tmp) = (clients)->num_clients, c = (clients)->client; \
>>> -	     (tmp > 0); (tmp)--, (c)++)
>>> -
>>> -static struct clients *init_clients(const char *drm_card)
>>> -{
>>> -	struct clients *clients;
>>> -	const char *slash;
>>> -	ssize_t ret;
>>> -	int dir;
>>> -
>>> -	clients = malloc(sizeof(*clients));
>>> -	if (!clients)
>>> -		return NULL;
>>> -
>>> -	memset(clients, 0, sizeof(*clients));
>>> -
>>> -	if (drm_card) {
>>> -		slash = rindex(drm_card, '/');
>>> -		assert(slash);
>>> -	} else {
>>> -		slash = "card0";
>>> -	}
>>> -
>>> -	ret = snprintf(clients->sysfs_root, sizeof(clients->sysfs_root),
>>> -		       "/sys/class/drm/%s/clients/", slash);
>>> -	assert(ret > 0 && ret < sizeof(clients->sysfs_root));
>>> -
>>> -	dir = open(clients->sysfs_root, O_DIRECTORY | O_RDONLY);
>>> -	if (dir < 0) {
>>> -		free(clients);
>>> -		clients = NULL;
>>> -	} else {
>>> -		close(dir);
>>> -	}
>>> -
>>> -	return clients;
>>> -}
>>> -
>>> -static int __read_to_buf(int fd, char *buf, unsigned int bufsize)
>>> -{
>>> -	ssize_t ret;
>>> -	int err;
>>> -
>>> -	ret = read(fd, buf, bufsize - 1);
>>> -	err = errno;
>>> -	if (ret < 1) {
>>> -		errno = ret < 0 ? err : ENOMSG;
>>> -
>>> -		return -1;
>>> -	}
>>> -
>>> -	if (ret > 1 && buf[ret - 1] == '\n')
>>> -		buf[ret - 1] = '\0';
>>> -	else
>>> -		buf[ret] = '\0';
>>> -
>>> -	return 0;
>>> -}
>>> -
>>> -static int
>>> -__read_client_field(int root, const char *field, char *buf, unsigned int bufsize)
>>> -{
>>> -	int fd, ret;
>>> -
>>> -	fd = openat(root, field, O_RDONLY);
>>> -	if (fd < 0)
>>> -		return -1;
>>> -
>>> -	ret = __read_to_buf(fd, buf, bufsize);
>>> -
>>> -	close(fd);
>>> -
>>> -	return ret;
>>> -}
>>> -
>>> -static uint64_t
>>> -read_client_busy(struct client *client, unsigned int class)
>>> -{
>>> -	const char *class_str[] = { "0", "1", "2", "3", "4", "5", "6", "7" };
>>> -	char buf[256], *b;
>>> -	int ret;
>>> -
>>> -	assert(class < ARRAY_SIZE(class_str));
>>> -	if (class >= ARRAY_SIZE(class_str))
>>> -		return 0;
>>> -
>>> -	assert(client->sysfs_root >= 0);
>>> -	if (client->sysfs_root < 0)
>>> -		return 0;
>>> -
>>> -	if (client->busy_root < 0)
>>> -		client->busy_root = openat(client->sysfs_root, "busy",
>>> -					   O_RDONLY | O_DIRECTORY);
>>> -
>>> -	assert(client->busy_root);
>>> -	if (client->busy_root < 0)
>>> -		return 0;
>>> -
>>> -	ret = __read_client_field(client->busy_root, class_str[class], buf,
>>> -				  sizeof(buf));
>>> -	if (ret) {
>>> -		close(client->busy_root);
>>> -		client->busy_root = -1;
>>> -		return 0;
>>> -	}
>>> -
>>> -	/*
>>> -	 * Handle both single integer and key=value formats by skipping
>>> -	 * leading non-digits.
>>> -	 */
>>> -	b = buf;
>>> -	while (*b && !isdigit(*b))
>>> -		b++;
>>> -
>>> -	return strtoull(b, NULL, 10);
>>> -}
>>> -
>>> -static struct client *
>>> -find_client(struct clients *clients, enum client_status status, unsigned int id)
>>> -{
>>> -	unsigned int start, num;
>>> -	struct client *c;
>>> -
>>> -	start = status == FREE ? clients->active_clients : 0; /* Free block at the end. */
>>> -	num = clients->num_clients - start;
>>> -
>>> -	for (c = &clients->client[start]; num; c++, num--) {
>>> -		if (status != c->status)
>>> -			continue;
>>> -
>>> -		if (status == FREE || c->id == id)
>>> -			return c;
>>> -	}
>>> -
>>> -	return NULL;
>>> -}
>>> -
>>> -static void update_client(struct client *c, unsigned int pid, char *name)
>>> -{
>>> -	uint64_t val[c->clients->num_classes];
>>> -	unsigned int i;
>>> -
>>> -	if (c->pid != pid)
>>> -		c->pid = pid;
>>> -
>>> -	if (strcmp(c->name, name)) {
>>> -		char *p;
>>> -
>>> -		strncpy(c->name, name, sizeof(c->name) - 1);
>>> -		strncpy(c->print_name, name, sizeof(c->print_name) - 1);
>>> -
>>> -		p = c->print_name;
>>> -		while (*p) {
>>> -			if (!isprint(*p))
>>> -				*p = '*';
>>> -			p++;
>>> -		}
>>> -	}
>>> -
>>> -	for (i = 0; i < c->clients->num_classes; i++)
>>> -		val[i] = read_client_busy(c, c->clients->class[i].class);
>>> -
>>> -	c->last_runtime = 0;
>>> -	c->total_runtime = 0;
>>> -
>>> -	for (i = 0; i < c->clients->num_classes; i++) {
>>> -		if (val[i] < c->last[i])
>>> -			continue; /* It will catch up soon. */
>>> -
>>> -		c->total_runtime += val[i];
>>> -		c->val[i] = val[i] - c->last[i];
>>> -		c->last_runtime += c->val[i];
>>> -		c->last[i] = val[i];
>>> -	}
>>> -
>>> -	c->samples++;
>>> -	c->status = ALIVE;
>>> -}
>>> -
>>> -static void
>>> -add_client(struct clients *clients, unsigned int id, unsigned int pid,
>>> -	   char *name, int sysfs_root)
>>> -{
>>> -	struct client *c;
>>> -
>>> -	assert(!find_client(clients, ALIVE, id));
>>> -
>>> -	c = find_client(clients, FREE, 0);
>>> -	if (!c) {
>>> -		unsigned int idx = clients->num_clients;
>>> -
>>> -		clients->num_clients += (clients->num_clients + 2) / 2;
>>> -		clients->client = realloc(clients->client,
>>> -					  clients->num_clients * sizeof(*c));
>>> -		assert(clients->client);
>>> -
>>> -		c = &clients->client[idx];
>>> -		memset(c, 0, (clients->num_clients - idx) * sizeof(*c));
>>> -	}
>>> -
>>> -	c->sysfs_root = sysfs_root;
>>> -	c->busy_root = -1;
>>> -	c->id = id;
>>> -	c->clients = clients;
>>> -	c->val = calloc(clients->num_classes, sizeof(c->val));
>>> -	c->last = calloc(clients->num_classes, sizeof(c->last));
>>> -	assert(c->val && c->last);
>>> -
>>> -	update_client(c, pid, name);
>>> -}
>>> -
>>> -static void free_client(struct client *c)
>>> -{
>>> -	if (c->sysfs_root >= 0)
>>> -		close(c->sysfs_root);
>>> -	if (c->busy_root >= 0)
>>> -		close(c->busy_root);
>>> -	free(c->val);
>>> -	free(c->last);
>>> -	memset(c, 0, sizeof(*c));
>>> -}
>>> -
>>> -static int
>>> -read_client_sysfs(char *buf, int bufsize, const char *sysfs_root,
>>> -		  unsigned int id, const char *field, int *client_root)
>>> -{
>>> -	ssize_t ret;
>>> -
>>> -	if (*client_root < 0) {
>>> -		char namebuf[256];
>>> -
>>> -		ret = snprintf(namebuf, sizeof(namebuf), "%s/%u",
>>> -			       sysfs_root, id);
>>> -		assert(ret > 0 && ret < sizeof(namebuf));
>>> -		if (ret <= 0 || ret == sizeof(namebuf))
>>> -			return -1;
>>> -
>>> -		*client_root = open(namebuf, O_RDONLY | O_DIRECTORY);
>>> -	}
>>> -
>>> -	if (*client_root < 0)
>>> -		return -1;
>>> -
>>> -	return __read_client_field(*client_root, field, buf, bufsize);
>>> -}
>>> -
>>> -static int client_last_cmp(const void *_a, const void *_b)
>>> -{
>>> -	const struct client *a = _a;
>>> -	const struct client *b = _b;
>>> -	long tot_a, tot_b;
>>> -
>>> -	/*
>>> -	 * Sort clients in descending order of runtime in the previous sampling
>>> -	 * period for active ones, followed by inactive. Tie-breaker is client
>>> -	 * id.
>>> -	 */
>>> -
>>> -	tot_a = a->status == ALIVE ? a->last_runtime : -1;
>>> -	tot_b = b->status == ALIVE ? b->last_runtime : -1;
>>> -
>>> -	tot_b -= tot_a;
>>> -	if (tot_b > 0)
>>> -		return 1;
>>> -	if (tot_b < 0)
>>> -		return -1;
>>> -
>>> -	return (int)b->id - a->id;
>>> -}
>>> -
>>> -static int client_total_cmp(const void *_a, const void *_b)
>>> -{
>>> -	const struct client *a = _a;
>>> -	const struct client *b = _b;
>>> -	long tot_a, tot_b;
>>> -
>>> -	tot_a = a->status == ALIVE ? a->total_runtime : -1;
>>> -	tot_b = b->status == ALIVE ? b->total_runtime : -1;
>>> -
>>> -	tot_b -= tot_a;
>>> -	if (tot_b > 0)
>>> -		return 1;
>>> -	if (tot_b < 0)
>>> -		return -1;
>>> -
>>> -	return (int)b->id - a->id;
>>> -}
>>> -
>>> -static int client_id_cmp(const void *_a, const void *_b)
>>> -{
>>> -	const struct client *a = _a;
>>> -	const struct client *b = _b;
>>> -	int id_a, id_b;
>>> -
>>> -	id_a = a->status == ALIVE ? a->id : -1;
>>> -	id_b = b->status == ALIVE ? b->id : -1;
>>> -
>>> -	id_b -= id_a;
>>> -	if (id_b > 0)
>>> -		return 1;
>>> -	if (id_b < 0)
>>> -		return -1;
>>> -
>>> -	return (int)b->id - a->id;
>>> -}
>>> -
>>> -static int client_pid_cmp(const void *_a, const void *_b)
>>> -{
>>> -	const struct client *a = _a;
>>> -	const struct client *b = _b;
>>> -	int pid_a, pid_b;
>>> -
>>> -	pid_a = a->status == ALIVE ? a->pid : INT_MAX;
>>> -	pid_b = b->status == ALIVE ? b->pid : INT_MAX;
>>> -
>>> -	pid_b -= pid_a;
>>> -	if (pid_b > 0)
>>> -		return -1;
>>> -	if (pid_b < 0)
>>> -		return 1;
>>> -
>>> -	return (int)a->id - b->id;
>>> -}
>>> -
>>> -static int (*client_cmp)(const void *, const void *) = client_last_cmp;
>>> -
>>> -static struct clients *sort_clients(struct clients *clients,
>>> -				    int (*cmp)(const void *, const void *))
>>> -{
>>> -	unsigned int active, free;
>>> -	struct client *c;
>>> -	int tmp;
>>> -
>>> -	if (!clients)
>>> -		return clients;
>>> -
>>> -	qsort(clients->client, clients->num_clients, sizeof(*clients->client),
>>> -	      cmp);
>>> -
>>> -	/* Trim excessive array space. */
>>> -	active = 0;
>>> -	for_each_client(clients, c, tmp) {
>>> -		if (c->status != ALIVE)
>>> -			break; /* Active clients are first in the array. */
>>> -		active++;
>>> -	}
>>> -
>>> -	clients->active_clients = active;
>>> -
>>> -	free = clients->num_clients - active;
>>> -	if (free > clients->num_clients / 2) {
>>> -		active = clients->num_clients - free / 2;
>>> -		if (active != clients->num_clients) {
>>> -			clients->num_clients = active;
>>> -			clients->client = realloc(clients->client,
>>> -						  clients->num_clients *
>>> -						  sizeof(*c));
>>> -		}
>>> -	}
>>> -
>>> -	return clients;
>>> -}
>>> -
>>> -static bool aggregate_pids = true;
>>> -
>>> -static struct clients *display_clients(struct clients *clients)
>>> -{
>>> -	struct client *ac, *c, *cp = NULL;
>>> -	struct clients *aggregated;
>>> -	int tmp, num = 0;
>>> -
>>> -	if (!aggregate_pids)
>>> -		goto out;
>>> -
>>> -	/* Sort by pid first to make it easy to aggregate while walking. */
>>> -	sort_clients(clients, client_pid_cmp);
>>> -
>>> -	aggregated = calloc(1, sizeof(*clients));
>>> -	assert(aggregated);
>>> -
>>> -	ac = calloc(clients->num_clients, sizeof(*c));
>>> -	assert(ac);
>>> -
>>> -	aggregated->num_classes = clients->num_classes;
>>> -	aggregated->class = clients->class;
>>> -	aggregated->client = ac;
>>> -
>>> -	for_each_client(clients, c, tmp) {
>>> -		unsigned int i;
>>> -
>>> -		if (c->status == FREE)
>>> -			break;
>>> -
>>> -		assert(c->status == ALIVE);
>>> -
>>> -		if ((cp && c->pid != cp->pid) || !cp) {
>>> -			ac = &aggregated->client[num++];
>>> -
>>> -			/* New pid. */
>>> -			ac->clients = aggregated;
>>> -			ac->status = ALIVE;
>>> -			ac->id = -c->pid;
>>> -			ac->pid = c->pid;
>>> -			ac->busy_root = -1;
>>> -			ac->sysfs_root = -1;
>>> -			strcpy(ac->name, c->name);
>>> -			strcpy(ac->print_name, c->print_name);
>>> -			ac->engines = c->engines;
>>> -			ac->val = calloc(clients->num_classes,
>>> -					 sizeof(ac->val[0]));
>>> -			assert(ac->val);
>>> -			ac->samples = 1;
>>> -		}
>>> -
>>> -		cp = c;
>>> -
>>> -		if (c->samples < 2)
>>> -			continue;
>>> -
>>> -		ac->samples = 2; /* All what matters for display. */
>>> -		ac->total_runtime += c->total_runtime;
>>> -		ac->last_runtime += c->last_runtime;
>>> -
>>> -		for (i = 0; i < clients->num_classes; i++)
>>> -			ac->val[i] += c->val[i];
>>> -	}
>>> -
>>> -	aggregated->num_clients = num;
>>> -	aggregated->active_clients = num;
>>> -
>>> -	clients = aggregated;
>>> -
>>> -out:
>>> -	return sort_clients(clients, client_cmp);
>>> -}
>>> -
>>> -static void free_clients(struct clients *clients)
>>> -{
>>> -	struct client *c;
>>> -	unsigned int tmp;
>>> -
>>> -	for_each_client(clients, c, tmp) {
>>> -		free(c->val);
>>> -		free(c->last);
>>> -	}
>>> -
>>> -	free(clients->client);
>>> -	free(clients);
>>> -}
>>> -
>>> -static struct clients *scan_clients(struct clients *clients)
>>> -{
>>> -	struct dirent *dent;
>>> -	struct client *c;
>>> -	unsigned int id;
>>> -	int tmp;
>>> -	DIR *d;
>>> -
>>> -	if (!clients)
>>> -		return clients;
>>> -
>>> -	for_each_client(clients, c, tmp) {
>>> -		assert(c->status != PROBE);
>>> -		if (c->status == ALIVE)
>>> -			c->status = PROBE;
>>> -		else
>>> -			break; /* Free block at the end of array. */
>>> -	}
>>> -
>>> -	d = opendir(clients->sysfs_root);
>>> -	if (!d)
>>> -		return clients;
>>> -
>>> -	while ((dent = readdir(d)) != NULL) {
>>> -		char name[24], pid[24];
>>> -		int ret, root = -1, *pr;
>>> -
>>> -		if (dent->d_type != DT_DIR)
>>> -			continue;
>>> -		if (!isdigit(dent->d_name[0]))
>>> -			continue;
>>> -
>>> -		id = atoi(dent->d_name);
>>> -
>>> -		c = find_client(clients, PROBE, id);
>>> -
>>> -		if (c)
>>> -			pr = &c->sysfs_root;
>>> -		else
>>> -			pr = &root;
>>> -
>>> -		ret = read_client_sysfs(name, sizeof(name), clients->sysfs_root,
>>> -					id, "name", pr);
>>> -		ret |= read_client_sysfs(pid, sizeof(pid), clients->sysfs_root,
>>> -					id, "pid", pr);
>>> -		if (!ret) {
>>> -			if (!c)
>>> -				add_client(clients, id, atoi(pid), name, root);
>>> -			else
>>> -				update_client(c, atoi(pid), name);
>>> -		} else if (c) {
>>> -			c->status = PROBE; /* Will be deleted below. */
>>> -		}
>>> -	}
>>> -
>>> -	closedir(d);
>>> -
>>> -	for_each_client(clients, c, tmp) {
>>> -		if (c->status == PROBE)
>>> -			free_client(c);
>>> -		else if (c->status == FREE)
>>> -			break;
>>> -	}
>>> -
>>> -	return display_clients(clients);
>>> -}
>>> -
>>>    static const char *bars[] = { " ", "▏", "▎", "▍", "▌", "▋", "▊", "▉", "█" };
>>>    static void n_spaces(const unsigned int n)
>>> @@ -1324,18 +768,6 @@ json_close_struct(void)
>>>    		fflush(stdout);
>>>    }
>>> -static void
>>> -__json_add_member(const char *key, const char *val)
>>> -{
>>> -	assert(json_indent_level < ARRAY_SIZE(json_indent));
>>> -
>>> -	fprintf(out, "%s%s\"%s\": \"%s\"",
>>> -		json_struct_members ? ",\n" : "",
>>> -		json_indent[json_indent_level], key, val);
>>> -
>>> -	json_struct_members++;
>>> -}
>>> -
>>>    static unsigned int
>>>    json_add_member(const struct cnt_group *parent, struct cnt_item *item,
>>>    		unsigned int headers)
>>> @@ -2061,157 +1493,6 @@ print_engines(struct engines *engines, double t, int lines, int w, int h)
>>>    	return lines;
>>>    }
>>> -static int
>>> -print_clients_header(struct clients *clients, int lines,
>>> -		     int con_w, int con_h, int *class_w)
>>> -{
>>> -	if (output_mode == INTERACTIVE) {
>>> -		const char *pidname = "   PID              NAME ";
>>> -		unsigned int num_active = 0;
>>> -		int len = strlen(pidname);
>>> -
>>> -		if (lines++ >= con_h)
>>> -			return lines;
>>> -
>>> -		printf("\033[7m");
>>> -		printf("%s", pidname);
>>> -
>>> -		if (lines++ >= con_h || len >= con_w)
>>> -			return lines;
>>> -
>>> -		if (clients->num_classes) {
>>> -			unsigned int i;
>>> -			int width;
>>> -
>>> -			for (i = 0; i < clients->num_classes; i++) {
>>> -				if (clients->class[i].num_engines)
>>> -					num_active++;
>>> -			}
>>> -
>>> -			*class_w = width = (con_w - len) / num_active;
>>> -
>>> -			for (i = 0; i < clients->num_classes; i++) {
>>> -				const char *name = clients->class[i].name;
>>> -				int name_len = strlen(name);
>>> -				int pad = (width - name_len) / 2;
>>> -				int spaces = width - pad - name_len;
>>> -
>>> -				if (!clients->class[i].num_engines)
>>> -					continue; /* Assert in the ideal world. */
>>> -
>>> -				if (pad < 0 || spaces < 0)
>>> -					continue;
>>> -
>>> -				n_spaces(pad);
>>> -				printf("%s", name);
>>> -				n_spaces(spaces);
>>> -				len += pad + name_len + spaces;
>>> -			}
>>> -		}
>>> -
>>> -		n_spaces(con_w - len);
>>> -		printf("\033[0m\n");
>>> -	} else {
>>> -		if (clients->num_classes)
>>> -			pops->open_struct("clients");
>>> -	}
>>> -
>>> -	return lines;
>>> -}
>>> -
>>> -static bool numeric_clients;
>>> -static bool filter_idle;
>>> -
>>> -static int
>>> -print_client(struct client *c, struct engines *engines, double t, int lines,
>>> -	     int con_w, int con_h, unsigned int period_us, int *class_w)
>>> -{
>>> -	struct clients *clients = c->clients;
>>> -	unsigned int i;
>>> -
>>> -	if (output_mode == INTERACTIVE) {
>>> -		if (filter_idle && (!c->total_runtime || c->samples < 2))
>>> -			return lines;
>>> -
>>> -		lines++;
>>> -
>>> -		printf("%6u %17s ", c->pid, c->print_name);
>>> -
>>> -		for (i = 0; c->samples > 1 && i < clients->num_classes; i++) {
>>> -			double pct;
>>> -
>>> -			if (!clients->class[i].num_engines)
>>> -				continue; /* Assert in the ideal world. */
>>> -
>>> -			pct = (double)c->val[i] / period_us / 1e3 * 100 /
>>> -			      clients->class[i].num_engines;
>>> -
>>> -			/*
>>> -			 * Guard against possible time-drift between sampling
>>> -			 * client data and time we obtained our time-delta from
>>> -			 * PMU.
>>> -			 */
>>> -			if (pct > 100.0)
>>> -				pct = 100.0;
>>> -
>>> -			print_percentage_bar(pct, *class_w, numeric_clients);
>>> -		}
>>> -
>>> -		putchar('\n');
>>> -	} else if (output_mode == JSON) {
>>> -		char buf[64];
>>> -
>>> -		snprintf(buf, sizeof(buf), "%u", c->id);
>>> -		pops->open_struct(buf);
>>> -
>>> -		__json_add_member("name", c->print_name);
>>> -
>>> -		snprintf(buf, sizeof(buf), "%u", c->pid);
>>> -		__json_add_member("pid", buf);
>>> -
>>> -		if (c->samples > 1) {
>>> -			pops->open_struct("engine-classes");
>>> -
>>> -			for (i = 0; i < clients->num_classes; i++) {
>>> -				double pct;
>>> -
>>> -				snprintf(buf, sizeof(buf), "%s",
>>> -					clients->class[i].name);
>>> -				pops->open_struct(buf);
>>> -
>>> -				pct = (double)c->val[i] / period_us / 1e3 * 100;
>>> -				snprintf(buf, sizeof(buf), "%f", pct);
>>> -				__json_add_member("busy", buf);
>>> -
>>> -				__json_add_member("unit", "%");
>>> -
>>> -				pops->close_struct();
>>> -			}
>>> -
>>> -			pops->close_struct();
>>> -		}
>>> -
>>> -		pops->close_struct();
>>> -	}
>>> -
>>> -	return lines;
>>> -}
>>> -
>>> -static int
>>> -print_clients_footer(struct clients *clients, double t,
>>> -		     int lines, int con_w, int con_h)
>>> -{
>>> -	if (output_mode == INTERACTIVE) {
>>> -		if (lines++ < con_h)
>>> -			printf("\n");
>>> -	} else {
>>> -		if (clients->num_classes)
>>> -			pops->close_struct();
>>> -	}
>>> -
>>> -	return lines;
>>> -}
>>> -
>>>    static bool stop_top;
>>>    static void sigint_handler(int  sig)
>>> @@ -2267,31 +1548,6 @@ static void interactive_stdin(void)
>>>    	assert(ret == 0);
>>>    }
>>> -static void select_client_sort(void)
>>> -{
>>> -	struct {
>>> -		int (*cmp)(const void *, const void *);
>>> -		const char *msg;
>>> -	} cmp[] = {
>>> -		{ client_last_cmp, "Sorting clients by current GPU usage." },
>>> -		{ client_total_cmp, "Sorting clients by accummulated GPU usage." },
>>> -		{ client_pid_cmp, "Sorting clients by pid." },
>>> -		{ client_id_cmp, "Sorting clients by sysfs id." },
>>> -	};
>>> -	static unsigned int client_sort;
>>> -
>>> -bump:
>>> -	if (++client_sort >= ARRAY_SIZE(cmp))
>>> -		client_sort = 0;
>>> -
>>> -	client_cmp = cmp[client_sort].cmp;
>>> -	header_msg = cmp[client_sort].msg;
>>> -
>>> -	/* Sort by client id makes no sense with pid aggregation. */
>>> -	if (aggregate_pids && client_cmp == client_id_cmp)
>>> -		goto bump;
>>> -}
>>> -
>>>    static bool in_help;
>>>    static void process_help_stdin(void)
>>> @@ -2334,29 +1590,9 @@ static void process_normal_stdin(void)
>>>    			else
>>>    				header_msg = "Showing physical engines.";
>>>    			break;
>>> -		case 'i':
>>> -			filter_idle ^= true;
>>> -			if (filter_idle)
>>> -				header_msg = "Hiding inactive clients.";
>>> -			else
>>> -				header_msg = "Showing inactive clients.";
>>> -			break;
>>> -		case 'n':
>>> -			numeric_clients ^= true;
>>> -			break;
>>> -		case 's':
>>> -			select_client_sort();
>>> -			break;
>>>    		case 'h':
>>>    			in_help = true;
>>>    			break;
>>> -		case 'H':
>>> -			aggregate_pids ^= true;
>>> -			if (aggregate_pids)
>>> -				header_msg = "Aggregating clients.";
>>> -			else
>>> -				header_msg = "Showing individual clients.";
>>> -			break;
>>>    		};
>>>    	}
>>>    }
>>> @@ -2384,10 +1620,6 @@ static void show_help_screen(void)
>>>    	printf(
>>>    "Help for interactive commands:\n\n"
>>>    "    '1'    Toggle between aggregated engine class and physical engine mode.\n"
>>> -"    'n'    Toggle display of numeric client busyness overlay.\n"
>>> -"    's'    Toggle between sort modes (runtime, total runtime, pid, client id).\n"
>>> -"    'i'    Toggle display of clients which used no GPU time.\n"
>>> -"    'H'    Toggle between per PID aggregation and individual clients.\n"
>>>    "\n"
>>>    "    'h' or 'q'    Exit interactive help.\n"
>>>    "\n");
>>> @@ -2396,7 +1628,6 @@ static void show_help_screen(void)
>>>    int main(int argc, char **argv)
>>>    {
>>>    	unsigned int period_us = DEFAULT_PERIOD_MS * 1000;
>>> -	struct clients *clients = NULL;
>>>    	int con_w = -1, con_h = -1;
>>>    	char *output_path = NULL;
>>>    	struct engines *engines;
>>> @@ -2530,23 +1761,15 @@ int main(int argc, char **argv)
>>>    	ret = EXIT_SUCCESS;
>>> -	clients = init_clients(card.pci_slot_name[0] ? card.card : NULL);
>>>    	init_engine_classes(engines);
>>> -	if (clients) {
>>> -		clients->num_classes = engines->num_classes;
>>> -		clients->class = engines->class;
>>> -	}
>>>    	pmu_sample(engines);
>>> -	scan_clients(clients);
>>>    	codename = igt_device_get_pretty_name(&card, false);
>>>    	while (!stop_top) {
>>> -		struct clients *disp_clients;
>>>    		bool consumed = false;
>>> -		int j, lines = 0;
>>>    		struct winsize ws;
>>> -		struct client *c;
>>> +		int lines = 0;
>>>    		double t;
>>>    		/* Update terminal size. */
>>> @@ -2565,8 +1788,6 @@ int main(int argc, char **argv)
>>>    		pmu_sample(engines);
>>>    		t = (double)(engines->ts.cur - engines->ts.prev) / 1e9;
>>> -		disp_clients = scan_clients(clients);
>>> -
>>>    		if (stop_top)
>>>    			break;
>>> @@ -2586,41 +1807,12 @@ int main(int argc, char **argv)
>>>    			lines = print_engines(engines, t, lines, con_w, con_h);
>>> -			if (disp_clients) {
>>> -				int class_w;
>>> -
>>> -				lines = print_clients_header(disp_clients, lines,
>>> -							     con_w, con_h,
>>> -							     &class_w);
>>> -
>>> -				for_each_client(disp_clients, c, j) {
>>> -					assert(c->status != PROBE);
>>> -					if (c->status != ALIVE)
>>> -						break; /* Active clients are first in the array. */
>>> -
>>> -					if (lines >= con_h)
>>> -						break;
>>> -
>>> -					lines = print_client(c, engines, t,
>>> -							     lines, con_w,
>>> -							     con_h, period_us,
>>> -							     &class_w);
>>> -				}
>>> -
>>> -				lines = print_clients_footer(disp_clients, t,
>>> -							     lines, con_w,
>>> -							     con_h);
>>> -			}
>>> -
>>>    			pops->close_struct();
>>>    		}
>>>    		if (stop_top)
>>>    			break;
>>> -		if (disp_clients != clients)
>>> -			free_clients(disp_clients);
>>> -
>>>    		if (output_mode == INTERACTIVE)
>>>    			process_stdin(period_us);
>>>    		else
>>>

[Intel-gfx] [PATCH i-g-t 6/6] intel_gpu_top: Add a sanity check discovered busy metric is per engine

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Adding a cross-check with ABI config name space and not just relying on
sysfs names.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Dmitry Rogozhkin <dmitry.v.rogozhkin@intel.com>
---
 tools/intel_gpu_top.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/tools/intel_gpu_top.c b/tools/intel_gpu_top.c
index 41c59a72c09d..81c724d1fe1c 100644
--- a/tools/intel_gpu_top.c
+++ b/tools/intel_gpu_top.c
@@ -376,6 +376,12 @@ static struct engines *discover_engines(char *device)
 			break;
 		}
 
+		/* Double check config is an engine config. */
+		if (engine->busy.config >= __I915_PMU_OTHER(0)) {
+			free((void *)engine->name);
+			continue;
+		}
+
 		engine->class = (engine->busy.config &
 				 (__I915_PMU_OTHER(0) - 1)) >>
 				I915_PMU_CLASS_SHIFT;
-- 
2.32.0

[igt-dev] [PATCH i-g-t 6/6] intel_gpu_top: Add a sanity check discovered busy metric is per engine

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Adding a cross-check with ABI config name space and not just relying on
sysfs names.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Dmitry Rogozhkin <dmitry.v.rogozhkin@intel.com>
---
 tools/intel_gpu_top.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/tools/intel_gpu_top.c b/tools/intel_gpu_top.c
index 41c59a72c09d..81c724d1fe1c 100644
--- a/tools/intel_gpu_top.c
+++ b/tools/intel_gpu_top.c
@@ -376,6 +376,12 @@ static struct engines *discover_engines(char *device)
 			break;
 		}
 
+		/* Double check config is an engine config. */
+		if (engine->busy.config >= __I915_PMU_OTHER(0)) {
+			free((void *)engine->name);
+			continue;
+		}
+
 		engine->class = (engine->busy.config &
 				 (__I915_PMU_OTHER(0) - 1)) >>
 				I915_PMU_CLASS_SHIFT;
-- 
2.32.0

Re: [Intel-gfx] [PATCH i-g-t 6/6] intel_gpu_top: Add a sanity check discovered busy metric is per engine

[Rogozhkin, Dmitry V]

On Fri, 2021-11-19 at 12:59 +0000, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Adding a cross-check with ABI config name space and not just relying
> on
> sysfs names.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Dmitry Rogozhkin <dmitry.v.rogozhkin@intel.com>
> ---
>  tools/intel_gpu_top.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/tools/intel_gpu_top.c b/tools/intel_gpu_top.c
> index 41c59a72c09d..81c724d1fe1c 100644
> --- a/tools/intel_gpu_top.c
> +++ b/tools/intel_gpu_top.c
> @@ -376,6 +376,12 @@ static struct engines *discover_engines(char
> *device)
>  			break;
>  		}
>  
> +		/* Double check config is an engine config. */
> +		if (engine->busy.config >= __I915_PMU_OTHER(0)) {
> +			free((void *)engine->name);
> +			continue;
> +		}
> +
>  		engine->class = (engine->busy.config &
>  				 (__I915_PMU_OTHER(0) - 1)) >>
>  				I915_PMU_CLASS_SHIFT;

This works for me.
Acked-by: Dmitry Rogozhkin <dmitry.v.rogozhkin@intel.com>

However, looking to the existing code down below the place where you've
added a fix, it seems to me that 'free((void *)engine->name)' might be
missing on a number of other error paths and on non-error exit path as
well.

Re: [igt-dev] [PATCH i-g-t 6/6] intel_gpu_top: Add a sanity check discovered busy metric is per engine

[Rogozhkin, Dmitry V]

On Fri, 2021-11-19 at 12:59 +0000, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Adding a cross-check with ABI config name space and not just relying
> on
> sysfs names.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Dmitry Rogozhkin <dmitry.v.rogozhkin@intel.com>
> ---
>  tools/intel_gpu_top.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/tools/intel_gpu_top.c b/tools/intel_gpu_top.c
> index 41c59a72c09d..81c724d1fe1c 100644
> --- a/tools/intel_gpu_top.c
> +++ b/tools/intel_gpu_top.c
> @@ -376,6 +376,12 @@ static struct engines *discover_engines(char
> *device)
>  			break;
>  		}
>  
> +		/* Double check config is an engine config. */
> +		if (engine->busy.config >= __I915_PMU_OTHER(0)) {
> +			free((void *)engine->name);
> +			continue;
> +		}
> +
>  		engine->class = (engine->busy.config &
>  				 (__I915_PMU_OTHER(0) - 1)) >>
>  				I915_PMU_CLASS_SHIFT;

This works for me.
Acked-by: Dmitry Rogozhkin <dmitry.v.rogozhkin@intel.com>

However, looking to the existing code down below the place where you've
added a fix, it seems to me that 'free((void *)engine->name)' might be
missing on a number of other error paths and on non-error exit path as
well.

Re: [Intel-gfx] [PATCH i-g-t 6/6] intel_gpu_top: Add a sanity check discovered busy metric is per engine

[Tvrtko Ursulin]

On 01/12/2021 02:20, Rogozhkin, Dmitry V wrote:
> On Fri, 2021-11-19 at 12:59 +0000, Tvrtko Ursulin wrote:
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> Adding a cross-check with ABI config name space and not just relying
>> on
>> sysfs names.
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> Cc: Dmitry Rogozhkin <dmitry.v.rogozhkin@intel.com>
>> ---
>>   tools/intel_gpu_top.c | 6 ++++++
>>   1 file changed, 6 insertions(+)
>>
>> diff --git a/tools/intel_gpu_top.c b/tools/intel_gpu_top.c
>> index 41c59a72c09d..81c724d1fe1c 100644
>> --- a/tools/intel_gpu_top.c
>> +++ b/tools/intel_gpu_top.c
>> @@ -376,6 +376,12 @@ static struct engines *discover_engines(char
>> *device)
>>   			break;
>>   		}
>>   
>> +		/* Double check config is an engine config. */
>> +		if (engine->busy.config >= __I915_PMU_OTHER(0)) {
>> +			free((void *)engine->name);
>> +			continue;
>> +		}
>> +
>>   		engine->class = (engine->busy.config &
>>   				 (__I915_PMU_OTHER(0) - 1)) >>
>>   				I915_PMU_CLASS_SHIFT;
> 
> This works for me.
> Acked-by: Dmitry Rogozhkin <dmitry.v.rogozhkin@intel.com>

Thanks, pushed!

> However, looking to the existing code down below the place where you've
> added a fix, it seems to me that 'free((void *)engine->name)' might be
> missing on a number of other error paths and on non-error exit path as
> well.

Error paths are all fatal (tool simply exits) so it's moot. No real 
value to cleanup piecemeal. It is the same for normal exit if that is 
what you mean. Would be nicer I guess but it's a task for a rainy idle day.

Regards,

Tvrtko

[igt-dev] ✗ Fi.CI.BUILD: failure for series starting with [i-g-t,1/6] tests/api_intel_allocator: Fix build warning

[Patchwork]

== Series Details ==

Series: series starting with [i-g-t,1/6] tests/api_intel_allocator: Fix build warning
URL   : https://patchwork.freedesktop.org/series/97101/
State : failure

== Summary ==

IGT patchset build failed on latest successful build
2e0355faad5c2e81cd6705b76e529ce526c7c9bf igt_core: Fix docs for SLOW_QUICK

287/315 testcase check sw_sync                  OK       0.13 s 
288/315 testcase check amdgpu/amd_abm           OK       0.15 s 
289/315 testcase check amdgpu/amd_assr          OK       0.15 s 
290/315 testcase check amdgpu/amd_basic         OK       0.15 s 
291/315 testcase check amdgpu/amd_bypass        OK       0.14 s 
292/315 testcase check amdgpu/amd_color         OK       0.15 s 
293/315 testcase check amdgpu/amd_cs_nop        OK       0.13 s 
294/315 testcase check amdgpu/amd_hotplug       OK       0.14 s 
295/315 testcase check amdgpu/amd_info          OK       0.13 s 
296/315 testcase check amdgpu/amd_prime         OK       0.15 s 
297/315 testcase check amdgpu/amd_max_bpc       OK       0.18 s 
298/315 testcase check amdgpu/amd_module_load   OK       0.15 s 
299/315 testcase check amdgpu/amd_mem_leak      OK       0.19 s 
300/315 testcase check amdgpu/amd_link_settings  OK       0.13 s 
301/315 testcase check amdgpu/amd_vrr_range     OK       0.13 s 
302/315 testcase check gem_concurrent_all       OK       5.30 s 
303/315 runner                                  OK       4.56 s 
304/315 runner_json                             OK       0.09 s 
305/315 assembler test/mov                      OK       0.08 s 
306/315 assembler test/frc                      OK       0.09 s 
307/315 assembler test/regtype                  OK       0.09 s 
308/315 assembler test/rndd                     OK       0.08 s 
309/315 assembler test/rndu                     OK       0.09 s 
310/315 assembler test/rnde                     OK       0.05 s 
311/315 assembler test/rnde-intsrc              OK       0.08 s 
312/315 assembler test/rndz                     OK       0.05 s 
313/315 assembler test/lzd                      OK       0.06 s 
314/315 assembler test/not                      OK       0.07 s 
315/315 assembler test/immediate                OK       0.03 s 

OK:       314
FAIL:       1
SKIP:       0
TIMEOUT:    0


The output from the failed tests:

  2/315 lib igt_abort                           FAIL     0.56 s (killed by signal 6 SIGABRT)

--- command ---
/home/cidrm/igt-gpu-tools/build/lib/tests/igt_abort
--- stderr ---
igt_abort: ../lib/tests/igt_abort.c:137: main: Assertion `strstr(err, "CRITICAL: Test abort")' failed.
-------

Full log written to /home/cidrm/igt-gpu-tools/build/meson-logs/testlog.txt
FAILED: meson-test 
/usr/bin/python3 -u /usr/bin/meson test --no-rebuild --print-errorlogs
ninja: build stopped: subcommand failed.

[igt-dev] ✗ GitLab.Pipeline: warning for series starting with [i-g-t,1/6] tests/api_intel_allocator: Fix build warning

[Patchwork]

== Series Details ==

Series: series starting with [i-g-t,1/6] tests/api_intel_allocator: Fix build warning
URL   : https://patchwork.freedesktop.org/series/97101/
State : warning

== Summary ==

Pipeline status: FAILED.

see https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/-/pipelines/449803 for the overview.

test:ninja-test has failed (https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/-/jobs/16002336):
  311/315 assembler test/rnde-intsrc              OK       0.02 s 
  312/315 assembler test/rndz                     OK       0.01 s 
  313/315 assembler test/lzd                      OK       0.02 s 
  314/315 assembler test/not                      OK       0.02 s 
  315/315 assembler test/immediate                OK       0.02 s 
  
  Ok:                  311
  Expected Fail:         3
  Fail:                  1
  Unexpected Pass:       0
  Skipped:               0
  Timeout:               0
  
  Full log written to /builds/gfx-ci/igt-ci-tags/build/meson-logs/testlog.txt
  section_end:1637330018:step_script
  section_start:1637330018:cleanup_file_variables
  Cleaning up project directory and file based variables
  section_end:1637330018:cleanup_file_variables
  ERROR: Job failed: exit code 1
  

test:ninja-test-arm64 has failed (https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/-/jobs/16002335):
  Ok:                  297
  Expected Fail:         3
  Fail:                  1
  Unexpected Pass:       0
  Skipped:               0
  Timeout:               0
  
  Full log written to /builds/gfx-ci/igt-ci-tags/build/meson-logs/testlog.txt
  section_end:1637329911:step_script
  section_start:1637329911:upload_artifacts_on_failure
  Uploading artifacts for failed job
  Uploading artifacts...
  build: found 1661 matching files and directories   
  Uploading artifacts as "archive" to coordinator... ok  id=16002335 responseStatus=201 Created token=F4gSR22q
  section_end:1637329925:upload_artifacts_on_failure
  section_start:1637329925:cleanup_file_variables
  Cleaning up project directory and file based variables
  section_end:1637329927:cleanup_file_variables
  ERROR: Job failed: exit code 1
  

test:ninja-test-armhf has failed (https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/-/jobs/16002337):
  Ok:                  297
  Expected Fail:         3
  Fail:                  1
  Unexpected Pass:       0
  Skipped:               0
  Timeout:               0
  
  Full log written to /builds/gfx-ci/igt-ci-tags/build/meson-logs/testlog.txt
  section_end:1637329927:step_script
  section_start:1637329927:upload_artifacts_on_failure
  Uploading artifacts for failed job
  Uploading artifacts...
  build: found 1661 matching files and directories   
  Uploading artifacts as "archive" to coordinator... ok  id=16002337 responseStatus=201 Created token=FeWN7Cg4
  section_end:1637329939:upload_artifacts_on_failure
  section_start:1637329939:cleanup_file_variables
  Cleaning up project directory and file based variables
  section_end:1637329940:cleanup_file_variables
  ERROR: Job failed: exit code 1
  

test:ninja-test-clang has failed (https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/-/jobs/16002334):
  311/315 assembler test/rnde-intsrc              OK       0.03 s 
  312/315 assembler test/rndz                     OK       0.03 s 
  313/315 assembler test/lzd                      OK       0.05 s 
  314/315 assembler test/not                      OK       0.04 s 
  315/315 assembler test/immediate                OK       0.03 s 
  
  Ok:                  311
  Expected Fail:         3
  Fail:                  1
  Unexpected Pass:       0
  Skipped:               0
  Timeout:               0
  
  Full log written to /builds/gfx-ci/igt-ci-tags/build/meson-logs/testlog.txt
  section_end:1637329854:step_script
  section_start:1637329854:cleanup_file_variables
  Cleaning up project directory and file based variables
  section_end:1637329855:cleanup_file_variables
  ERROR: Job failed: exit code 1
  

test:ninja-test-minimal has failed (https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/-/jobs/16002333):
  20/24 lib igt_thread                          OK       0.05 s 
  21/24 lib i915_perf_data_alignment            OK       0.03 s 
  22/24 lib igt_no_subtest                      EXPECTEDFAIL 0.03 s 
  23/24 lib igt_simple_test_subtests            EXPECTEDFAIL 0.02 s 
  24/24 lib igt_timeout                         EXPECTEDFAIL 1.03 s 
  
  Ok:                   20
  Expected Fail:         3
  Fail:                  1
  Unexpected Pass:       0
  Skipped:               0
  Timeout:               0
  
  Full log written to /builds/gfx-ci/igt-ci-tags/build/meson-logs/testlog.txt
  section_end:1637329811:step_script
  section_start:1637329811:cleanup_file_variables
  Cleaning up project directory and file based variables
  section_end:1637329811:cleanup_file_variables
  ERROR: Job failed: exit code 1
  

test:ninja-test-mips has failed (https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/-/jobs/16002332):
  Ok:                  297
  Expected Fail:         3
  Fail:                  1
  Unexpected Pass:       0
  Skipped:               0
  Timeout:               0
  
  Full log written to /builds/gfx-ci/igt-ci-tags/build/meson-logs/testlog.txt
  section_end:1637329830:step_script
  section_start:1637329830:upload_artifacts_on_failure
  Uploading artifacts for failed job
  Uploading artifacts...
  build: found 1661 matching files and directories   
  Uploading artifacts as "archive" to coordinator... ok  id=16002332 responseStatus=201 Created token=xQFk6g_R
  section_end:1637329843:upload_artifacts_on_failure
  section_start:1637329843:cleanup_file_variables
  Cleaning up project directory and file based variables
  section_end:1637329845:cleanup_file_variables
  ERROR: Job failed: exit code 1

== Logs ==

For more details see: https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/-/pipelines/449803

Re: [Intel-gfx] [PATCH i-g-t 1/6] tests/api_intel_allocator: Fix build warning

[Zbigniew Kempczyński]

On Fri, Nov 19, 2021 at 12:59:40PM +0000, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> .../tests/i915/api_intel_allocator.c: In function ‘basic_alloc’:
> .../tests/i915/api_intel_allocator.c:158:25: warning: this ‘if’ clause does not guard... [-Wmisleading-indentation]
>   158 |                         if (j == i)
>       |                         ^~
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>

I've already seen this on gcc 11.2.0 (Ubuntu 21.10) and got
locally same fix, so:

Reviewed-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>

--
Zbigniew

> ---
>  tests/i915/api_intel_allocator.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/i915/api_intel_allocator.c b/tests/i915/api_intel_allocator.c
> index 4b74317ed3d0..6d7764ca3f51 100644
> --- a/tests/i915/api_intel_allocator.c
> +++ b/tests/i915/api_intel_allocator.c
> @@ -157,7 +157,7 @@ static void basic_alloc(int fd, int cnt, uint8_t type)
>  		for (j = 0; j < cnt; j++) {
>  			if (j == i)
>  				continue;
> -				igt_assert(!overlaps(&obj[i], &obj[j]));
> +			igt_assert(!overlaps(&obj[i], &obj[j]));
>  		}
>  	}
>  
> -- 
> 2.32.0
> 

Re: [igt-dev] [PATCH i-g-t 1/6] tests/api_intel_allocator: Fix build warning

[Zbigniew Kempczyński]

On Fri, Nov 19, 2021 at 12:59:40PM +0000, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> .../tests/i915/api_intel_allocator.c: In function ‘basic_alloc’:
> .../tests/i915/api_intel_allocator.c:158:25: warning: this ‘if’ clause does not guard... [-Wmisleading-indentation]
>   158 |                         if (j == i)
>       |                         ^~
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>

I've already seen this on gcc 11.2.0 (Ubuntu 21.10) and got
locally same fix, so:

Reviewed-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>

--
Zbigniew

> ---
>  tests/i915/api_intel_allocator.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/i915/api_intel_allocator.c b/tests/i915/api_intel_allocator.c
> index 4b74317ed3d0..6d7764ca3f51 100644
> --- a/tests/i915/api_intel_allocator.c
> +++ b/tests/i915/api_intel_allocator.c
> @@ -157,7 +157,7 @@ static void basic_alloc(int fd, int cnt, uint8_t type)
>  		for (j = 0; j < cnt; j++) {
>  			if (j == i)
>  				continue;
> -				igt_assert(!overlaps(&obj[i], &obj[j]));
> +			igt_assert(!overlaps(&obj[i], &obj[j]));
>  		}
>  	}
>  
> -- 
> 2.32.0
>