[PATCH] lib/igt_drm_clients: lazy stat process

[Lucas De Marchi]

On Thu, May 02, 2024 at 11:20:48AM GMT, Tvrtko Ursulin wrote:
>
>On 01/05/2024 18:38, Lucas De Marchi wrote:
>>Only try to get task data like pid and name when that is required.
>>Typically most of the processes in a system don't have a drm fd open.
>>So only open and use /proc/<pid>/stat when it's needed.
>>
>>For the data below, system is running with just a single drm client:
>>
>>Before:
>>  Performance counter stats for './build/tools/gputop -d 0.1 -n 10':
>>
>>             507.19 msec task-clock                       #    0.334 CPUs utilized
>>                 29      context-switches                 #   57.178 /sec
>>                  4      cpu-migrations                   #    7.887 /sec
>>                 75      page-faults                      #  147.874 /sec
>>        (...)
>>        1.518253637 seconds time elapsed
>>        0.007989000 seconds user
>>        0.503351000 seconds sys
>>
>>After:
>>  Performance counter stats for './build/tools/gputop -d 0.1 -n 10':
>>
>>             384.01 msec task-clock                       #    0.275 CPUs utilized
>>                 17      context-switches                 #   44.270 /sec
>>                  1      cpu-migrations                   #    2.604 /sec
>>                 74      page-faults                      #  192.705 /sec
>>        (...)
>>        1.397640800 seconds time elapsed
>>        0.008106000 seconds user
>>        0.385048000 seconds sys
>>
>>Looking only at the execution time of igt_drm_clients_scan() over 10
>>runs.
>>Before: 47138890 nsec
>>After:  33978388 nsec
>>
>>So in both methods it shows a ~25% speedup. It is expected that in a
>>system with more clients this won't be as high.
>>
>>Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>
>>---
>>
>>Extracted as is from https://patchwork.freedesktop.org/series/132059/ as
>>not really related.
>>
>>  lib/igt_drm_clients.c | 50 +++++++++++++++++++++----------------------
>>  1 file changed, 25 insertions(+), 25 deletions(-)
>>
>>diff --git a/lib/igt_drm_clients.c b/lib/igt_drm_clients.c
>>index 40d494441..ab0c2cec2 100644
>>--- a/lib/igt_drm_clients.c
>>+++ b/lib/igt_drm_clients.c
>>@@ -368,26 +368,33 @@ static size_t readat2buf(int at, const char *name, char *buf, const size_t sz)
>>  	}
>>  }
>>-static bool get_task_name(const char *buffer, char *out, unsigned long sz)
>>+static void get_task_data(int pid_dir, unsigned int *pid, char *task, size_t tasksz)
>>  {
>>-	char *s = index(buffer, '(');
>>-	char *e = rindex(buffer, ')');
>>-	unsigned int len;
>>+	char buf[4096];
>>+	char *s, *e;
>>+	size_t len;
>>+	if (!readat2buf(pid_dir, "stat", buf, sizeof(buf)))
>>+		return;
>>+
>>+	s = strchr(buf, '(');
>>+	e = strchr(s, ')');
>>  	if (!s || !e)
>>-		return false;
>>-	assert(e >= s);
>>+		return;
>>  	len = e - ++s;
>>-	if(!len || (len + 1) >= sz)
>>-		return false;
>>+	if (!len)
>>+		return;
>>-	strncpy(out, s, len);
>>-	out[len] = 0;
>>+	if (len + 1 > tasksz)
>>+		len = tasksz - 1;
>>-	return true;
>>+	strncpy(task, s, len);
>>+	task[len] = 0;
>>+	*pid = atoi(buf);
>>  }
>>+
>>  static bool is_drm_fd(int fd_dir, const char *name, unsigned int *minor)
>>  {
>>  	struct stat stat;
>>@@ -481,13 +488,11 @@ igt_drm_clients_scan(struct igt_drm_clients *clients,
>>  		return clients;
>>  	while ((proc_dent = readdir(proc_dir)) != NULL) {
>>-		unsigned int client_pid, minor = 0;
>>+		unsigned int client_pid = 0, minor = 0;
>>  		int pid_dir = -1, fd_dir = -1;
>>  		struct dirent *fdinfo_dent;
>>  		char client_name[64] = { };
>>  		DIR *fdinfo_dir = NULL;
>>-		char buf[4096];
>>-		size_t count;
>>  		if (proc_dent->d_type != DT_DIR)
>>  			continue;
>>@@ -499,17 +504,6 @@ igt_drm_clients_scan(struct igt_drm_clients *clients,
>>  		if (pid_dir < 0)
>>  			continue;
>>-		count = readat2buf(pid_dir, "stat", buf, sizeof(buf));
>>-		if (!count)
>>-			goto next;
>>-
>>-		client_pid = atoi(buf);
>>-		if (!client_pid)
>>-			goto next;
>>-
>>-		if (!get_task_name(buf, client_name, sizeof(client_name)))
>>-			goto next;
>>-
>>  		fd_dir = openat(pid_dir, "fd", O_DIRECTORY | O_RDONLY);
>>  		if (fd_dir < 0)
>>  			goto next;
>>@@ -542,6 +536,12 @@ igt_drm_clients_scan(struct igt_drm_clients *clients,
>>  						 minor, info.id))
>>  				continue; /* Skip duplicate fds. */
>>+			if (!client_pid) {
>>+				get_task_data(pid_dir, &client_pid, client_name,
>>+					      sizeof(client_name));
>>+				assert(client_pid > 0);
>>+			}
>>+
>>  			c = igt_drm_clients_find(clients, IGT_DRM_CLIENT_PROBE,
>>  						 minor, info.id);
>>  			if (!c)
>
>Nice improvement and well spotted!
>
>Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin at igalia.com>

applied, thanks

Lucas De Marchi