* [Intel-gfx] [PATCH i-g-t 1/6] tests/api_intel_allocator: Fix build warning
@ 2021-11-19 12:59 ` Tvrtko Ursulin
0 siblings, 0 replies; 31+ messages in thread
From: Tvrtko Ursulin @ 2021-11-19 12:59 UTC (permalink / raw)
To: igt-dev; +Cc: Intel-gfx
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
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [igt-dev] [PATCH i-g-t 1/6] tests/api_intel_allocator: Fix build warning
@ 2021-11-19 12:59 ` Tvrtko Ursulin
0 siblings, 0 replies; 31+ messages in thread
From: Tvrtko Ursulin @ 2021-11-19 12:59 UTC (permalink / raw)
To: igt-dev; +Cc: Intel-gfx, 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
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Intel-gfx] [PATCH i-g-t 2/6] igt/drm_read: Fix build warning
2021-11-19 12:59 ` [igt-dev] " Tvrtko Ursulin
(?)
@ 2021-11-19 12:59 ` Tvrtko Ursulin
-1 siblings, 0 replies; 31+ messages in thread
From: Tvrtko Ursulin @ 2021-11-19 12:59 UTC (permalink / raw)
To: igt-dev; +Cc: Intel-gfx
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
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Intel-gfx] [PATCH i-g-t 3/6] igt/core: Fix build warning
2021-11-19 12:59 ` [igt-dev] " Tvrtko Ursulin
@ 2021-11-19 12:59 ` Tvrtko Ursulin
-1 siblings, 0 replies; 31+ messages in thread
From: Tvrtko Ursulin @ 2021-11-19 12:59 UTC (permalink / raw)
To: igt-dev; +Cc: Intel-gfx
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
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [igt-dev] [PATCH i-g-t 3/6] igt/core: Fix build warning
@ 2021-11-19 12:59 ` Tvrtko Ursulin
0 siblings, 0 replies; 31+ messages in thread
From: Tvrtko Ursulin @ 2021-11-19 12:59 UTC (permalink / raw)
To: igt-dev; +Cc: Intel-gfx, Petri Latvala, 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
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Intel-gfx] [PATCH i-g-t 4/6] igt/drm_read: Fix build warning
2021-11-19 12:59 ` [igt-dev] " Tvrtko Ursulin
@ 2021-11-19 12:59 ` Tvrtko Ursulin
-1 siblings, 0 replies; 31+ messages in thread
From: Tvrtko Ursulin @ 2021-11-19 12:59 UTC (permalink / raw)
To: igt-dev; +Cc: Intel-gfx
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
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [igt-dev] [PATCH i-g-t 4/6] igt/drm_read: Fix build warning
@ 2021-11-19 12:59 ` Tvrtko Ursulin
0 siblings, 0 replies; 31+ messages in thread
From: Tvrtko Ursulin @ 2021-11-19 12:59 UTC (permalink / raw)
To: igt-dev; +Cc: Intel-gfx, Petri Latvala, 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
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Intel-gfx] [PATCH i-g-t 5/6] intel_gpu_top: Remove clients support
2021-11-19 12:59 ` [igt-dev] " Tvrtko Ursulin
@ 2021-11-19 12:59 ` Tvrtko Ursulin
-1 siblings, 0 replies; 31+ messages in thread
From: Tvrtko Ursulin @ 2021-11-19 12:59 UTC (permalink / raw)
To: igt-dev; +Cc: Daniel Vetter, Intel-gfx
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
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [igt-dev] [PATCH i-g-t 5/6] intel_gpu_top: Remove clients support
@ 2021-11-19 12:59 ` Tvrtko Ursulin
0 siblings, 0 replies; 31+ messages in thread
From: Tvrtko Ursulin @ 2021-11-19 12:59 UTC (permalink / raw)
To: igt-dev; +Cc: Intel-gfx, 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
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Intel-gfx] [PATCH i-g-t 6/6] intel_gpu_top: Add a sanity check discovered busy metric is per engine
2021-11-19 12:59 ` [igt-dev] " Tvrtko Ursulin
@ 2021-11-19 12:59 ` Tvrtko Ursulin
-1 siblings, 0 replies; 31+ messages in thread
From: Tvrtko Ursulin @ 2021-11-19 12:59 UTC (permalink / raw)
To: igt-dev; +Cc: Intel-gfx
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
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [igt-dev] [PATCH i-g-t 6/6] intel_gpu_top: Add a sanity check discovered busy metric is per engine
@ 2021-11-19 12:59 ` Tvrtko Ursulin
0 siblings, 0 replies; 31+ messages in thread
From: Tvrtko Ursulin @ 2021-11-19 12:59 UTC (permalink / raw)
To: igt-dev; +Cc: Intel-gfx, Dmitry Rogozhkin, 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
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [igt-dev] ✗ Fi.CI.BUILD: failure for series starting with [i-g-t,1/6] tests/api_intel_allocator: Fix build warning
2021-11-19 12:59 ` [igt-dev] " Tvrtko Ursulin
` (5 preceding siblings ...)
(?)
@ 2021-11-19 13:38 ` Patchwork
-1 siblings, 0 replies; 31+ messages in thread
From: Patchwork @ 2021-11-19 13:38 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: igt-dev
== 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.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Intel-gfx] [PATCH i-g-t 3/6] igt/core: Fix build warning
2021-11-19 12:59 ` [igt-dev] " Tvrtko Ursulin
(?)
@ 2021-11-19 13:53 ` Petri Latvala
2021-11-19 15:34 ` [igt-dev] " Tvrtko Ursulin
-1 siblings, 1 reply; 31+ messages in thread
From: Petri Latvala @ 2021-11-19 13:53 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: igt-dev, Intel-gfx
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
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* [igt-dev] ✗ GitLab.Pipeline: warning for series starting with [i-g-t,1/6] tests/api_intel_allocator: Fix build warning
2021-11-19 12:59 ` [igt-dev] " Tvrtko Ursulin
` (6 preceding siblings ...)
(?)
@ 2021-11-19 13:56 ` Patchwork
-1 siblings, 0 replies; 31+ messages in thread
From: Patchwork @ 2021-11-19 13:56 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: igt-dev
== 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
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Intel-gfx] [PATCH i-g-t 3/6] igt/core: Fix build warning
2021-11-19 13:53 ` [Intel-gfx] " Petri Latvala
@ 2021-11-19 15:34 ` Tvrtko Ursulin
0 siblings, 0 replies; 31+ messages in thread
From: Tvrtko Ursulin @ 2021-11-19 15:34 UTC (permalink / raw)
To: Petri Latvala; +Cc: igt-dev, Intel-gfx
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
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [igt-dev] [PATCH i-g-t 3/6] igt/core: Fix build warning
@ 2021-11-19 15:34 ` Tvrtko Ursulin
0 siblings, 0 replies; 31+ messages in thread
From: Tvrtko Ursulin @ 2021-11-19 15:34 UTC (permalink / raw)
To: Petri Latvala; +Cc: igt-dev, Intel-gfx, 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
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Intel-gfx] [PATCH i-g-t 3/6] igt/core: Fix build warning
2021-11-19 15:34 ` [igt-dev] " Tvrtko Ursulin
(?)
@ 2021-11-19 15:41 ` Petri Latvala
2021-11-19 15:54 ` Petri Latvala
-1 siblings, 1 reply; 31+ messages in thread
From: Petri Latvala @ 2021-11-19 15:41 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: igt-dev, Intel-gfx
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
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 3/6] igt/core: Fix build warning
2021-11-19 15:41 ` [Intel-gfx] " Petri Latvala
@ 2021-11-19 15:54 ` Petri Latvala
0 siblings, 0 replies; 31+ messages in thread
From: Petri Latvala @ 2021-11-19 15:54 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: igt-dev, Intel-gfx
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);
}
^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [igt-dev] [PATCH i-g-t 3/6] igt/core: Fix build warning
@ 2021-11-19 15:54 ` Petri Latvala
0 siblings, 0 replies; 31+ messages in thread
From: Petri Latvala @ 2021-11-19 15:54 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: igt-dev, Intel-gfx, Tvrtko Ursulin
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);
}
^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [Intel-gfx] [PATCH i-g-t 1/6] tests/api_intel_allocator: Fix build warning
2021-11-19 12:59 ` [igt-dev] " Tvrtko Ursulin
@ 2021-11-22 5:58 ` Zbigniew Kempczyński
-1 siblings, 0 replies; 31+ messages in thread
From: Zbigniew Kempczyński @ 2021-11-22 5:58 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: igt-dev, Intel-gfx
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
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [igt-dev] [PATCH i-g-t 1/6] tests/api_intel_allocator: Fix build warning
@ 2021-11-22 5:58 ` Zbigniew Kempczyński
0 siblings, 0 replies; 31+ messages in thread
From: Zbigniew Kempczyński @ 2021-11-22 5:58 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: igt-dev, Intel-gfx, Tvrtko Ursulin
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
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 3/6] igt/core: Fix build warning
2021-11-19 15:54 ` Petri Latvala
@ 2021-11-22 9:03 ` Tvrtko Ursulin
-1 siblings, 0 replies; 31+ messages in thread
From: Tvrtko Ursulin @ 2021-11-22 9:03 UTC (permalink / raw)
To: Petri Latvala; +Cc: igt-dev, Intel-gfx
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
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [igt-dev] [PATCH i-g-t 3/6] igt/core: Fix build warning
@ 2021-11-22 9:03 ` Tvrtko Ursulin
0 siblings, 0 replies; 31+ messages in thread
From: Tvrtko Ursulin @ 2021-11-22 9:03 UTC (permalink / raw)
To: Petri Latvala; +Cc: igt-dev, Intel-gfx, 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
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 5/6] intel_gpu_top: Remove clients support
2021-11-19 12:59 ` [igt-dev] " Tvrtko Ursulin
(?)
@ 2021-11-26 13:07 ` Tvrtko Ursulin
2021-11-26 14:01 ` Petri Latvala
-1 siblings, 1 reply; 31+ messages in thread
From: Tvrtko Ursulin @ 2021-11-26 13:07 UTC (permalink / raw)
To: igt-dev; +Cc: Intel-gfx
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
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 5/6] intel_gpu_top: Remove clients support
2021-11-26 13:07 ` [Intel-gfx] " Tvrtko Ursulin
@ 2021-11-26 14:01 ` Petri Latvala
0 siblings, 0 replies; 31+ messages in thread
From: Petri Latvala @ 2021-11-26 14:01 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: igt-dev, Intel-gfx
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
> >
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [igt-dev] [PATCH i-g-t 5/6] intel_gpu_top: Remove clients support
@ 2021-11-26 14:01 ` Petri Latvala
0 siblings, 0 replies; 31+ messages in thread
From: Petri Latvala @ 2021-11-26 14:01 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: igt-dev, Intel-gfx, Daniel Vetter, Tvrtko Ursulin
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
> >
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 5/6] intel_gpu_top: Remove clients support
2021-11-26 14:01 ` Petri Latvala
@ 2021-11-26 14:16 ` Tvrtko Ursulin
-1 siblings, 0 replies; 31+ messages in thread
From: Tvrtko Ursulin @ 2021-11-26 14:16 UTC (permalink / raw)
To: Petri Latvala; +Cc: igt-dev, Intel-gfx
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
>>>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [igt-dev] [PATCH i-g-t 5/6] intel_gpu_top: Remove clients support
@ 2021-11-26 14:16 ` Tvrtko Ursulin
0 siblings, 0 replies; 31+ messages in thread
From: Tvrtko Ursulin @ 2021-11-26 14:16 UTC (permalink / raw)
To: Petri Latvala; +Cc: igt-dev, Intel-gfx, Daniel Vetter, 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
>>>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Intel-gfx] [PATCH i-g-t 6/6] intel_gpu_top: Add a sanity check discovered busy metric is per engine
2021-11-19 12:59 ` [igt-dev] " Tvrtko Ursulin
@ 2021-12-01 2:20 ` Rogozhkin, Dmitry V
-1 siblings, 0 replies; 31+ messages in thread
From: Rogozhkin, Dmitry V @ 2021-12-01 2:20 UTC (permalink / raw)
To: igt-dev, tvrtko.ursulin; +Cc: Intel-gfx
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.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [igt-dev] [PATCH i-g-t 6/6] intel_gpu_top: Add a sanity check discovered busy metric is per engine
@ 2021-12-01 2:20 ` Rogozhkin, Dmitry V
0 siblings, 0 replies; 31+ messages in thread
From: Rogozhkin, Dmitry V @ 2021-12-01 2:20 UTC (permalink / raw)
To: igt-dev, tvrtko.ursulin; +Cc: Intel-gfx, Ursulin, Tvrtko
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.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Intel-gfx] [PATCH i-g-t 6/6] intel_gpu_top: Add a sanity check discovered busy metric is per engine
2021-12-01 2:20 ` [igt-dev] " Rogozhkin, Dmitry V
(?)
@ 2021-12-01 10:06 ` Tvrtko Ursulin
-1 siblings, 0 replies; 31+ messages in thread
From: Tvrtko Ursulin @ 2021-12-01 10:06 UTC (permalink / raw)
To: Rogozhkin, Dmitry V, igt-dev; +Cc: Intel-gfx
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
^ permalink raw reply [flat|nested] 31+ messages in thread
end of thread, other threads:[~2021-12-01 10:06 UTC | newest]
Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-19 12:59 [Intel-gfx] [PATCH i-g-t 1/6] tests/api_intel_allocator: Fix build warning Tvrtko Ursulin
2021-11-19 12:59 ` [igt-dev] " Tvrtko Ursulin
2021-11-19 12:59 ` [Intel-gfx] [PATCH i-g-t 2/6] igt/drm_read: " Tvrtko Ursulin
2021-11-19 12:59 ` [Intel-gfx] [PATCH i-g-t 3/6] igt/core: " Tvrtko Ursulin
2021-11-19 12:59 ` [igt-dev] " Tvrtko Ursulin
2021-11-19 13:53 ` [Intel-gfx] " Petri Latvala
2021-11-19 15:34 ` Tvrtko Ursulin
2021-11-19 15:34 ` [igt-dev] " Tvrtko Ursulin
2021-11-19 15:41 ` [Intel-gfx] " Petri Latvala
2021-11-19 15:54 ` [Intel-gfx] [igt-dev] " Petri Latvala
2021-11-19 15:54 ` Petri Latvala
2021-11-22 9:03 ` [Intel-gfx] " Tvrtko Ursulin
2021-11-22 9:03 ` Tvrtko Ursulin
2021-11-19 12:59 ` [Intel-gfx] [PATCH i-g-t 4/6] igt/drm_read: " Tvrtko Ursulin
2021-11-19 12:59 ` [igt-dev] " Tvrtko Ursulin
2021-11-19 12:59 ` [Intel-gfx] [PATCH i-g-t 5/6] intel_gpu_top: Remove clients support Tvrtko Ursulin
2021-11-19 12:59 ` [igt-dev] " Tvrtko Ursulin
2021-11-26 13:07 ` [Intel-gfx] " Tvrtko Ursulin
2021-11-26 14:01 ` Petri Latvala
2021-11-26 14:01 ` Petri Latvala
2021-11-26 14:16 ` [Intel-gfx] " Tvrtko Ursulin
2021-11-26 14:16 ` Tvrtko Ursulin
2021-11-19 12:59 ` [Intel-gfx] [PATCH i-g-t 6/6] intel_gpu_top: Add a sanity check discovered busy metric is per engine Tvrtko Ursulin
2021-11-19 12:59 ` [igt-dev] " Tvrtko Ursulin
2021-12-01 2:20 ` [Intel-gfx] " Rogozhkin, Dmitry V
2021-12-01 2:20 ` [igt-dev] " Rogozhkin, Dmitry V
2021-12-01 10:06 ` [Intel-gfx] " Tvrtko Ursulin
2021-11-19 13:38 ` [igt-dev] ✗ Fi.CI.BUILD: failure for series starting with [i-g-t,1/6] tests/api_intel_allocator: Fix build warning Patchwork
2021-11-19 13:56 ` [igt-dev] ✗ GitLab.Pipeline: warning " Patchwork
2021-11-22 5:58 ` [Intel-gfx] [PATCH i-g-t 1/6] " Zbigniew Kempczyński
2021-11-22 5:58 ` [igt-dev] " Zbigniew Kempczyński
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.