[Intel-gfx] [PATCH i-g-t] intel_gpu_top: Always sort the clients array after update

* [Intel-gfx] [PATCH i-g-t] intel_gpu_top: Always sort the clients array after update
@ 2021-02-03 10:26 ` Tvrtko Ursulin
  0 siblings, 0 replies; 5+ messages in thread
From: Tvrtko Ursulin @ 2021-02-03 10:26 UTC (permalink / raw)
  To: igt-dev; +Cc: Intel-gfx

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

Walking the client "list" makes assumptions about the order of active and
free slots which means we need to sort the array after every update.

Patch is mostly just code movement with the only functional difference of
eliminating two subsequent scans with no sort in between This closes a
very short window there list iteration could get confused if sysfs clients
would change rapidly and unfavourably during tool startup.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 tools/intel_gpu_top.c | 132 +++++++++++++++++++++---------------------
 1 file changed, 66 insertions(+), 66 deletions(-)

diff --git a/tools/intel_gpu_top.c b/tools/intel_gpu_top.c
index dffc6ebecc57..1ffdf5ee64b4 100644
--- a/tools/intel_gpu_top.c
+++ b/tools/intel_gpu_top.c
@@ -899,71 +899,6 @@ read_client_sysfs(char *buf, int bufsize, const char *sysfs_root,
 	return __read_client_field(*client_root, field, buf, bufsize);
 }
 
-static void scan_clients(struct clients *clients)
-{
-	struct dirent *dent;
-	struct client *c;
-	unsigned int id;
-	int tmp;
-	DIR *d;
-
-	if (!clients)
-		return;
-
-	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;
-
-	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;
-	}
-}
-
 static int client_last_cmp(const void *_a, const void *_b)
 {
 	const struct client *a = _a;
@@ -1060,6 +995,72 @@ static void sort_clients(struct clients *clients)
 	}
 }
 
+static void scan_clients(struct clients *clients)
+{
+	struct dirent *dent;
+	struct client *c;
+	unsigned int id;
+	int tmp;
+	DIR *d;
+
+	if (!clients)
+		return;
+
+	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;
+
+	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;
+	}
+
+	sort_clients(clients);
+}
 
 static const char *bars[] = { " ", "▏", "▎", "▍", "▌", "▋", "▊", "▉", "█" };
 
@@ -2305,7 +2306,6 @@ int main(int argc, char **argv)
 		t = (double)(engines->ts.cur - engines->ts.prev) / 1e9;
 
 		scan_clients(clients);
-		sort_clients(clients);
 
 		if (stop_top)
 			break;
-- 
2.27.0

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

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