* [Intel-gfx] [PATCH i-g-t] i915: Exercise sysfs client properties
@ 2021-01-22 21:34 Chris Wilson
2021-01-22 21:49 ` Chris Wilson
0 siblings, 1 reply; 4+ messages in thread
From: Chris Wilson @ 2021-01-22 21:34 UTC (permalink / raw)
To: igt-dev; +Cc: intel-gfx, Chris Wilson
We store every client name, pid and runtime under sysfs. Better check
that matches with the actual client.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
lib/igt_sysfs.c | 36 ++++
lib/igt_sysfs.h | 3 +
tests/Makefile.sources | 3 +
tests/i915/sysfs_clients.c | 361 +++++++++++++++++++++++++++++++++++++
tests/meson.build | 1 +
5 files changed, 404 insertions(+)
create mode 100644 tests/i915/sysfs_clients.c
diff --git a/lib/igt_sysfs.c b/lib/igt_sysfs.c
index 6aafe5349..e734143ba 100644
--- a/lib/igt_sysfs.c
+++ b/lib/igt_sysfs.c
@@ -378,6 +378,42 @@ uint32_t igt_sysfs_get_u32(int dir, const char *attr)
return result;
}
+/**
+ * igt_sysfs_get_u64:
+ * @dir: directory for the device from igt_sysfs_open()
+ * @attr: name of the sysfs node to open
+ *
+ * Convenience wrapper to read a unsigned 64bit integer from a sysfs file.
+ *
+ * Returns:
+ * The value read.
+ */
+uint64_t igt_sysfs_get_u64(int dir, const char *attr)
+{
+ uint64_t result;
+
+ if (igt_sysfs_scanf(dir, attr, "%"PRIu64, &result) != 1)
+ return 0;
+
+ return result;
+}
+
+/**
+ * igt_sysfs_set_u64:
+ * @dir: directory for the device from igt_sysfs_open()
+ * @attr: name of the sysfs node to open
+ * @value: value to set
+ *
+ * Convenience wrapper to write a unsigned 64bit integer to a sysfs file.
+ *
+ * Returns:
+ * True if successfully written
+ */
+bool igt_sysfs_set_u64(int dir, const char *attr, uint64_t value)
+{
+ return igt_sysfs_printf(dir, attr, "%"PRIu64, value) > 0;
+}
+
/**
* igt_sysfs_set_u32:
* @dir: directory for the device from igt_sysfs_open()
diff --git a/lib/igt_sysfs.h b/lib/igt_sysfs.h
index 64935a5ca..56741a0a3 100644
--- a/lib/igt_sysfs.h
+++ b/lib/igt_sysfs.h
@@ -47,6 +47,9 @@ int igt_sysfs_printf(int dir, const char *attr, const char *fmt, ...)
uint32_t igt_sysfs_get_u32(int dir, const char *attr);
bool igt_sysfs_set_u32(int dir, const char *attr, uint32_t value);
+uint64_t igt_sysfs_get_u64(int dir, const char *attr);
+bool igt_sysfs_set_u64(int dir, const char *attr, uint64_t value);
+
bool igt_sysfs_get_boolean(int dir, const char *attr);
bool igt_sysfs_set_boolean(int dir, const char *attr, bool value);
diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index 1c227e750..3f663fe7e 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -114,6 +114,9 @@ TESTS_progs = \
TESTS_progs += api_intel_bb
api_intel_bb_SOURCES = i915/api_intel_bb.c
+TESTS_progs += sysfs_clients
+sysfs_clients_SOURCES = i915/sysfs_clients.c
+
TESTS_progs += sysfs_defaults
sysfs_defaults_SOURCES = i915/sysfs_defaults.c
diff --git a/tests/i915/sysfs_clients.c b/tests/i915/sysfs_clients.c
new file mode 100644
index 000000000..3a5c4cedf
--- /dev/null
+++ b/tests/i915/sysfs_clients.c
@@ -0,0 +1,361 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright © 2021 Intel Corporation
+ */
+
+#include <ctype.h>
+#include <dirent.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <inttypes.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+#include <unistd.h>
+
+#include "drmtest.h"
+#include "i915/gem.h"
+#include "i915/gem_context.h"
+#include "i915/gem_engine_topology.h"
+#include "igt_aux.h"
+#include "igt_dummyload.h"
+#include "igt_sysfs.h"
+#include "ioctl_wrappers.h"
+
+static void pidname(int i915, int clients)
+{
+ struct dirent *de;
+ int sv[2], rv[2];
+ char buf[280];
+ int me = -1;
+ long count;
+ pid_t pid;
+ DIR *dir;
+ int len;
+
+ dir = fdopendir(dup(clients));
+ igt_assert(dir);
+ rewinddir(dir);
+
+ count = 0;
+ while ((de = readdir(dir))) {
+ if (!isdigit(de->d_name[0]))
+ continue;
+
+ snprintf(buf, sizeof(buf), "%s/name", de->d_name);
+ len = igt_sysfs_read(clients, buf, buf, sizeof(buf));
+ igt_assert_f(len > 0, "failed to open '%s/name'\n", de->d_name);
+ buf[len - 1] = '\0';
+ igt_debug("%s: %s\n", de->d_name, buf);
+
+ /* Ignore closed clients created by drm_driver_open() */
+ if (*buf == '<')
+ continue;
+
+ close(me);
+ me = openat(clients, de->d_name, O_DIRECTORY | O_RDONLY);
+ count++;
+ }
+ closedir(dir);
+
+ /* We expect there to be only the single client (us) running */
+ igt_assert_eq(count, 1);
+ igt_assert(me >= 0);
+
+ len = igt_sysfs_read(me, "name", buf, sizeof(buf));
+ igt_assert(len > 0);
+ buf[len - 1] = '\0';
+
+ igt_info("My name: %s\n", buf);
+ igt_assert(strcmp(buf, igt_test_name()) == 0);
+
+ igt_assert(pipe(sv) == 0);
+ igt_assert(pipe(rv) == 0);
+
+ /* If give our fd to someone else, they take over ownership of client */
+ igt_fork(child, 1) {
+ read(sv[0], &pid, sizeof(pid));
+
+ gem_context_destroy(i915, gem_context_create(i915));
+
+ pid = getpid();
+ write(rv[1], &pid, sizeof(pid));
+ }
+ close(sv[0]);
+ close(rv[1]);
+
+ /* Child exists, but not yet running, we still own the client */
+ len = igt_sysfs_read(me, "pid", buf, sizeof(buf));
+ igt_assert(len > 0);
+ buf[len - 1] = '\0';
+
+ pid = getpid();
+ igt_info("My pid: %s\n", buf);
+ igt_assert_eq(atoi(buf), pid);
+
+ /* Release and wait for the child */
+ igt_assert_eq(write(sv[1], &pid, sizeof(pid)), sizeof(pid));
+ igt_assert_eq(read(rv[0], &pid, sizeof(pid)), sizeof(pid));
+
+ /* Now child owns the client and pid should be updated to match */
+ len = igt_sysfs_read(me, "pid", buf, sizeof(buf));
+ igt_assert(len > 0);
+ buf[len - 1] = '\0';
+
+ igt_info("New pid: %s\n", buf);
+ igt_assert_eq(atoi(buf), pid);
+ igt_waitchildren();
+
+ /* Child has definitely gone, but the client should remain */
+ len = igt_sysfs_read(me, "pid", buf, sizeof(buf));
+ igt_assert(len > 0);
+ buf[len - 1] = '\0';
+
+ igt_info("Old pid: %s\n", buf);
+ igt_assert_eq(atoi(buf), pid);
+
+ close(sv[1]);
+ close(rv[0]);
+ close(me);
+}
+
+static long count_clients(int clients)
+{
+ struct dirent *de;
+ long count = 0;
+ char buf[280];
+ DIR *dir;
+
+ dir = fdopendir(dup(clients));
+ igt_assert(dir);
+ rewinddir(dir);
+
+ while ((de = readdir(dir))) {
+ int len;
+
+ if (!isdigit(de->d_name[0]))
+ continue;
+
+ snprintf(buf, sizeof(buf), "%s/name", de->d_name);
+ len = igt_sysfs_read(clients, buf, buf, sizeof(buf));
+ if (len < 0)
+ continue;
+
+ count += *buf != '<';
+ }
+ closedir(dir);
+
+ return count;
+}
+
+static void create(int i915, int clients)
+{
+ int fd[16];
+
+ /* Each new open("/dev/dri/cardN") is a new client */
+ igt_assert_eq(count_clients(clients), 1);
+ for (int i = 0; i < ARRAY_SIZE(fd); i++) {
+ fd[i] = gem_reopen_driver(i915);
+ igt_assert_eq(count_clients(clients), i + 2);
+ }
+
+ for (int i = 0; i < ARRAY_SIZE(fd); i++)
+ close(fd[i]);
+
+ /* Cleanup delayed behind rcu */
+ igt_until_timeout(30) {
+ usleep(0);
+ if (count_clients(clients) == 1)
+ break;
+ usleep(10000);
+ }
+ igt_assert_eq(count_clients(clients), 1);
+}
+
+static int find_me(int clients, pid_t pid)
+{
+ struct dirent *de;
+ char buf[280];
+ int me = -1;
+ DIR *dir;
+
+ dir = fdopendir(dup(clients));
+ igt_assert(dir);
+ rewinddir(dir);
+
+ while ((de = readdir(dir))) {
+ int ret;
+
+ if (!isdigit(de->d_name[0]))
+ continue;
+
+ snprintf(buf, sizeof(buf), "%s/pid", de->d_name);
+ ret = igt_sysfs_read(clients, buf, buf, sizeof(buf));
+ igt_assert_f(ret > 0, "failed to open '%s/pid'\n", de->d_name);
+ if (atoi(buf) != pid)
+ continue;
+
+ me = openat(clients, de->d_name, O_DIRECTORY | O_RDONLY);
+ break;
+ }
+
+ closedir(dir);
+ return me;
+}
+
+#define MAX_CLASS 64
+static int read_runtime(int client, uint64_t *runtime)
+{
+ int fd = openat(client, "busy", O_DIRECTORY | O_RDONLY);
+ DIR *dir = fdopendir(fd);
+ struct dirent *de;
+ int count = 0;
+
+ memset(runtime, 0, sizeof(*runtime) * MAX_CLASS);
+ while ((de = readdir(dir))) {
+ int class;
+
+ if (!isdigit(de->d_name[0]))
+ continue;
+
+ class = atoi(de->d_name);
+ igt_assert(class < MAX_CLASS);
+ runtime[class] = igt_sysfs_get_u64(fd, de->d_name);
+
+ count += runtime[class] != 0;
+ }
+ closedir(dir);
+
+ return count;
+}
+
+static uint64_t measured_usleep(unsigned int usec)
+{
+ struct timespec tv;
+ unsigned int slept;
+
+ slept = igt_nsec_elapsed(memset(&tv, 0, sizeof(tv)));
+ igt_assert(slept == 0);
+ do {
+ usleep(usec - slept);
+ slept = igt_nsec_elapsed(&tv) / 1000;
+ } while (slept < usec);
+
+ return igt_nsec_elapsed(&tv);
+}
+
+static void busy(int i915, int clients)
+{
+ const struct intel_execution_engine2 *e;
+ uint64_t active[MAX_CLASS];
+ uint64_t idle[MAX_CLASS];
+ uint64_t old[MAX_CLASS];
+ uint64_t classes = 0;
+ int expect = 0;
+ int64_t delay;
+ igt_spin_t *spin;
+ int me;
+
+ /* Create a fresh client with 0 runtime */
+ i915 = gem_reopen_driver(i915);
+
+ me = find_me(clients, getpid());
+ igt_assert(me != -1);
+ igt_require(faccessat(me, "busy", 0, F_OK) == 0);
+
+ spin = igt_spin_new(i915,
+ gem_context_create(i915),
+ .flags = IGT_SPIN_POLL_RUN);
+ __for_each_physical_engine(i915, e) {
+ spin->execbuf.flags &= ~63;
+ spin->execbuf.flags |= e->flags;
+ gem_execbuf(i915, &spin->execbuf);
+
+ if (!(classes & (1ull << e->class)))
+ expect++;
+ classes |= 1ull << e->class;
+ }
+ igt_spin_busywait_until_started(spin);
+
+ delay = -500000; /* 500us slack */
+ memset(old, 0, sizeof(old));
+ for (int pass = 0; pass < 5; pass++) {
+ delay += measured_usleep(1000);
+ igt_debug("delay: %'"PRIu64"ns\n", delay);
+
+ /* Check that we accumulate the runtime, while active */
+ igt_assert_eq(read_runtime(me, active), expect);
+ for (int i = 0; i < ARRAY_SIZE(active); i++) {
+ if (!active[i])
+ continue;
+
+ igt_info("active[%d]: %'"PRIu64"ns\n", i, active[i]);
+ igt_assert(active[i] > old[i]); /* monotonic */
+ igt_assert(active[i] > delay); /* within reason */
+ }
+ }
+
+ gem_quiescent_gpu(i915);
+
+ /* And again now idle */
+ igt_assert_eq(read_runtime(me, idle), expect);
+ for (int i = 0; i < ARRAY_SIZE(idle); i++) {
+ if (!idle[i])
+ continue;
+
+ igt_info("idle[%d]: %'"PRIu64"ns\n", i, idle[i]);
+ igt_assert(idle[i] >= active[i]);
+ }
+
+ gem_context_destroy(i915, spin->execbuf.rsvd1);
+ igt_spin_free(i915, spin);
+
+ /* And finally after the executing context is no more */
+ igt_assert_eq(read_runtime(me, old), expect);
+ for (int i = 0; i < ARRAY_SIZE(old); i++) {
+ if (!old[i])
+ continue;
+
+ igt_info("old[%d]: %'"PRIu64"ns\n", i, old[i]);
+ igt_assert_eq_u64(old[i], idle[i]);
+ }
+
+ close(i915);
+}
+
+igt_main
+{
+ int i915 = -1, clients = -1;
+
+ igt_fixture {
+ int sys;
+
+ /* Don't allow [too many] extra clients to be opened */
+ i915 = __drm_open_driver(DRIVER_INTEL);
+ igt_require_gem(i915);
+
+ sys = igt_sysfs_open(i915);
+ igt_require(sys != -1);
+
+ clients = openat(sys, "clients", O_RDONLY);
+ igt_require(clients != -1);
+
+ close(sys);
+ }
+
+ igt_subtest("pidname")
+ pidname(i915, clients);
+
+ igt_subtest("create")
+ create(i915, clients);
+
+ igt_subtest("busy") {
+ igt_fork(child, 1)
+ busy(i915, clients);
+ igt_waitchildren();
+ }
+
+ igt_fixture {
+ close(clients);
+ close(i915);
+ }
+}
diff --git a/tests/meson.build b/tests/meson.build
index ff924ff99..825e01833 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -236,6 +236,7 @@ i915_progs = [
'i915_query',
'i915_selftest',
'i915_suspend',
+ 'sysfs_clients',
'sysfs_defaults',
'sysfs_heartbeat_interval',
'sysfs_preempt_timeout',
--
2.30.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [Intel-gfx] [PATCH i-g-t] i915: Exercise sysfs client properties
2021-01-22 21:34 [Intel-gfx] [PATCH i-g-t] i915: Exercise sysfs client properties Chris Wilson
@ 2021-01-22 21:49 ` Chris Wilson
2021-01-25 10:20 ` [Intel-gfx] [igt-dev] " Tvrtko Ursulin
0 siblings, 1 reply; 4+ messages in thread
From: Chris Wilson @ 2021-01-22 21:49 UTC (permalink / raw)
To: igt-dev; +Cc: intel-gfx, Chris Wilson
We store every client name, pid and runtime under sysfs. Better check
that matches with the actual client.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
lib/igt_sysfs.c | 36 +++
lib/igt_sysfs.h | 3 +
tests/Makefile.sources | 3 +
tests/i915/sysfs_clients.c | 450 +++++++++++++++++++++++++++++++++++++
tests/meson.build | 1 +
5 files changed, 493 insertions(+)
create mode 100644 tests/i915/sysfs_clients.c
diff --git a/lib/igt_sysfs.c b/lib/igt_sysfs.c
index 6aafe5349..e734143ba 100644
--- a/lib/igt_sysfs.c
+++ b/lib/igt_sysfs.c
@@ -378,6 +378,42 @@ uint32_t igt_sysfs_get_u32(int dir, const char *attr)
return result;
}
+/**
+ * igt_sysfs_get_u64:
+ * @dir: directory for the device from igt_sysfs_open()
+ * @attr: name of the sysfs node to open
+ *
+ * Convenience wrapper to read a unsigned 64bit integer from a sysfs file.
+ *
+ * Returns:
+ * The value read.
+ */
+uint64_t igt_sysfs_get_u64(int dir, const char *attr)
+{
+ uint64_t result;
+
+ if (igt_sysfs_scanf(dir, attr, "%"PRIu64, &result) != 1)
+ return 0;
+
+ return result;
+}
+
+/**
+ * igt_sysfs_set_u64:
+ * @dir: directory for the device from igt_sysfs_open()
+ * @attr: name of the sysfs node to open
+ * @value: value to set
+ *
+ * Convenience wrapper to write a unsigned 64bit integer to a sysfs file.
+ *
+ * Returns:
+ * True if successfully written
+ */
+bool igt_sysfs_set_u64(int dir, const char *attr, uint64_t value)
+{
+ return igt_sysfs_printf(dir, attr, "%"PRIu64, value) > 0;
+}
+
/**
* igt_sysfs_set_u32:
* @dir: directory for the device from igt_sysfs_open()
diff --git a/lib/igt_sysfs.h b/lib/igt_sysfs.h
index 64935a5ca..56741a0a3 100644
--- a/lib/igt_sysfs.h
+++ b/lib/igt_sysfs.h
@@ -47,6 +47,9 @@ int igt_sysfs_printf(int dir, const char *attr, const char *fmt, ...)
uint32_t igt_sysfs_get_u32(int dir, const char *attr);
bool igt_sysfs_set_u32(int dir, const char *attr, uint32_t value);
+uint64_t igt_sysfs_get_u64(int dir, const char *attr);
+bool igt_sysfs_set_u64(int dir, const char *attr, uint64_t value);
+
bool igt_sysfs_get_boolean(int dir, const char *attr);
bool igt_sysfs_set_boolean(int dir, const char *attr, bool value);
diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index 1c227e750..3f663fe7e 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -114,6 +114,9 @@ TESTS_progs = \
TESTS_progs += api_intel_bb
api_intel_bb_SOURCES = i915/api_intel_bb.c
+TESTS_progs += sysfs_clients
+sysfs_clients_SOURCES = i915/sysfs_clients.c
+
TESTS_progs += sysfs_defaults
sysfs_defaults_SOURCES = i915/sysfs_defaults.c
diff --git a/tests/i915/sysfs_clients.c b/tests/i915/sysfs_clients.c
new file mode 100644
index 000000000..a77adec6d
--- /dev/null
+++ b/tests/i915/sysfs_clients.c
@@ -0,0 +1,450 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright © 2021 Intel Corporation
+ */
+
+#include <ctype.h>
+#include <dirent.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <inttypes.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+#include <unistd.h>
+
+#include "drmtest.h"
+#include "i915/gem.h"
+#include "i915/gem_context.h"
+#include "i915/gem_engine_topology.h"
+#include "igt_aux.h"
+#include "igt_dummyload.h"
+#include "igt_sysfs.h"
+#include "ioctl_wrappers.h"
+
+static void pidname(int i915, int clients)
+{
+ struct dirent *de;
+ int sv[2], rv[2];
+ char buf[280];
+ int me = -1;
+ long count;
+ pid_t pid;
+ DIR *dir;
+ int len;
+
+ dir = fdopendir(dup(clients));
+ igt_assert(dir);
+ rewinddir(dir);
+
+ count = 0;
+ while ((de = readdir(dir))) {
+ if (!isdigit(de->d_name[0]))
+ continue;
+
+ snprintf(buf, sizeof(buf), "%s/name", de->d_name);
+ len = igt_sysfs_read(clients, buf, buf, sizeof(buf));
+ igt_assert_f(len > 0, "failed to open '%s/name'\n", de->d_name);
+ buf[len - 1] = '\0';
+ igt_debug("%s: %s\n", de->d_name, buf);
+
+ /* Ignore closed clients created by drm_driver_open() */
+ if (*buf == '<')
+ continue;
+
+ close(me);
+ me = openat(clients, de->d_name, O_DIRECTORY | O_RDONLY);
+ count++;
+ }
+ closedir(dir);
+
+ /* We expect there to be only the single client (us) running */
+ igt_assert_eq(count, 1);
+ igt_assert(me >= 0);
+
+ len = igt_sysfs_read(me, "name", buf, sizeof(buf));
+ igt_assert(len > 0);
+ buf[len - 1] = '\0';
+
+ igt_info("My name: %s\n", buf);
+ igt_assert(strcmp(buf, igt_test_name()) == 0);
+
+ igt_assert(pipe(sv) == 0);
+ igt_assert(pipe(rv) == 0);
+
+ /* If give our fd to someone else, they take over ownership of client */
+ igt_fork(child, 1) {
+ read(sv[0], &pid, sizeof(pid));
+
+ gem_context_destroy(i915, gem_context_create(i915));
+
+ pid = getpid();
+ write(rv[1], &pid, sizeof(pid));
+ }
+ close(sv[0]);
+ close(rv[1]);
+
+ /* Child exists, but not yet running, we still own the client */
+ len = igt_sysfs_read(me, "pid", buf, sizeof(buf));
+ igt_assert(len > 0);
+ buf[len - 1] = '\0';
+
+ pid = getpid();
+ igt_info("My pid: %s\n", buf);
+ igt_assert_eq(atoi(buf), pid);
+
+ /* Release and wait for the child */
+ igt_assert_eq(write(sv[1], &pid, sizeof(pid)), sizeof(pid));
+ igt_assert_eq(read(rv[0], &pid, sizeof(pid)), sizeof(pid));
+
+ /* Now child owns the client and pid should be updated to match */
+ len = igt_sysfs_read(me, "pid", buf, sizeof(buf));
+ igt_assert(len > 0);
+ buf[len - 1] = '\0';
+
+ igt_info("New pid: %s\n", buf);
+ igt_assert_eq(atoi(buf), pid);
+ igt_waitchildren();
+
+ /* Child has definitely gone, but the client should remain */
+ len = igt_sysfs_read(me, "pid", buf, sizeof(buf));
+ igt_assert(len > 0);
+ buf[len - 1] = '\0';
+
+ igt_info("Old pid: %s\n", buf);
+ igt_assert_eq(atoi(buf), pid);
+
+ close(sv[1]);
+ close(rv[0]);
+ close(me);
+}
+
+static long count_clients(int clients)
+{
+ struct dirent *de;
+ long count = 0;
+ char buf[280];
+ DIR *dir;
+
+ dir = fdopendir(dup(clients));
+ igt_assert(dir);
+ rewinddir(dir);
+
+ while ((de = readdir(dir))) {
+ int len;
+
+ if (!isdigit(de->d_name[0]))
+ continue;
+
+ snprintf(buf, sizeof(buf), "%s/name", de->d_name);
+ len = igt_sysfs_read(clients, buf, buf, sizeof(buf));
+ if (len < 0)
+ continue;
+
+ count += *buf != '<';
+ }
+ closedir(dir);
+
+ return count;
+}
+
+static void create(int i915, int clients)
+{
+ int fd[16];
+
+ /* Each new open("/dev/dri/cardN") is a new client */
+ igt_assert_eq(count_clients(clients), 1);
+ for (int i = 0; i < ARRAY_SIZE(fd); i++) {
+ fd[i] = gem_reopen_driver(i915);
+ igt_assert_eq(count_clients(clients), i + 2);
+ }
+
+ for (int i = 0; i < ARRAY_SIZE(fd); i++)
+ close(fd[i]);
+
+ /* Cleanup delayed behind rcu */
+ igt_until_timeout(30) {
+ usleep(0);
+ if (count_clients(clients) == 1)
+ break;
+ usleep(10000);
+ }
+ igt_assert_eq(count_clients(clients), 1);
+}
+
+static int find_me(int clients, pid_t pid)
+{
+ struct dirent *de;
+ char buf[280];
+ int me = -1;
+ DIR *dir;
+
+ dir = fdopendir(dup(clients));
+ igt_assert(dir);
+ rewinddir(dir);
+
+ while ((de = readdir(dir))) {
+ int ret;
+
+ if (!isdigit(de->d_name[0]))
+ continue;
+
+ snprintf(buf, sizeof(buf), "%s/pid", de->d_name);
+ ret = igt_sysfs_read(clients, buf, buf, sizeof(buf));
+ igt_assert_f(ret > 0, "failed to open '%s/pid'\n", de->d_name);
+ if (atoi(buf) != pid)
+ continue;
+
+ me = openat(clients, de->d_name, O_DIRECTORY | O_RDONLY);
+ break;
+ }
+
+ closedir(dir);
+ return me;
+}
+
+#define MAX_CLASS 64
+static int read_runtime(int client, uint64_t *runtime)
+{
+ int fd = openat(client, "busy", O_DIRECTORY | O_RDONLY);
+ DIR *dir = fdopendir(fd);
+ struct dirent *de;
+ int count = 0;
+
+ memset(runtime, 0, sizeof(*runtime) * MAX_CLASS);
+ while ((de = readdir(dir))) {
+ int class;
+
+ if (!isdigit(de->d_name[0]))
+ continue;
+
+ class = atoi(de->d_name);
+ igt_assert(class < MAX_CLASS);
+ runtime[class] = igt_sysfs_get_u64(fd, de->d_name);
+
+ count += runtime[class] != 0;
+ }
+ closedir(dir);
+
+ return count;
+}
+
+static uint64_t measured_usleep(unsigned int usec)
+{
+ struct timespec tv;
+ unsigned int slept;
+
+ slept = igt_nsec_elapsed(memset(&tv, 0, sizeof(tv)));
+ igt_assert(slept == 0);
+ do {
+ usleep(usec - slept);
+ slept = igt_nsec_elapsed(&tv) / 1000;
+ } while (slept < usec);
+
+ return igt_nsec_elapsed(&tv);
+}
+
+static void
+busy_one(int device, int clients, const struct intel_execution_engine2 *e)
+{
+ uint64_t active[MAX_CLASS];
+ uint64_t idle[MAX_CLASS];
+ uint64_t old[MAX_CLASS];
+ igt_spin_t *spin;
+ int64_t delay;
+ int i915;
+ int me;
+
+ /* Create a fresh client with 0 runtime */
+ i915 = gem_reopen_driver(device);
+ gem_context_copy_engines(device, 0, i915, 0);
+
+ me = find_me(clients, getpid());
+ igt_assert(me != -1);
+ igt_require(faccessat(me, "busy", 0, F_OK) == 0);
+
+ spin = igt_spin_new(i915,
+ gem_context_clone_with_engines(i915, 0),
+ .engine = e->flags,
+ .flags = IGT_SPIN_POLL_RUN);
+ igt_spin_busywait_until_started(spin);
+
+ delay = -500000; /* 500us slack */
+ memset(old, 0, sizeof(old));
+ for (int pass = 0; pass < 5; pass++) {
+ delay += measured_usleep(1000);
+ igt_debug("delay: %'"PRIu64"ns\n", delay);
+
+ /* Check that we accumulate the runtime, while active */
+ igt_assert_eq(read_runtime(me, active), 1);
+ igt_info("active1[%d]: %'"PRIu64"ns\n", pass, active[e->class]);
+ igt_assert(active[e->class] > old[e->class]); /* monotonic */
+ igt_assert(active[e->class] > delay); /* within reason */
+ }
+
+ gem_quiescent_gpu(i915);
+
+ /* And again now idle */
+ igt_assert_eq(read_runtime(me, idle), 1);
+ igt_info("idle: %'"PRIu64"ns\n", idle[e->class]);
+ igt_assert(idle[e->class] >= active[e->class]);
+
+ gem_context_destroy(i915, spin->execbuf.rsvd1);
+
+ /* And finally after the executing context is no more */
+ igt_assert_eq(read_runtime(me, old), 1);
+ igt_info("old: %'"PRIu64"ns\n", old[e->class]);
+ igt_assert_eq_u64(old[e->class], idle[e->class]);
+
+ /* Once more on the default context for good luck */
+ igt_spin_reset(spin);
+ spin->execbuf.rsvd1 = 0;
+ gem_execbuf(i915, &spin->execbuf);
+ igt_spin_busywait_until_started(spin);
+
+ for (int pass = 0; pass < 5; pass++) {
+ delay += measured_usleep(1000);
+ igt_debug("delay: %'"PRIu64"ns\n", delay);
+
+ /* Check that we accumulate the runtime, while active */
+ igt_assert_eq(read_runtime(me, active), 1);
+ igt_info("active0[%d]: %'"PRIu64"ns\n", pass, active[e->class]);
+ igt_assert(active[e->class] > old[e->class]); /* monotonic */
+ igt_assert(active[e->class] > delay); /* within reason */
+ }
+
+ gem_quiescent_gpu(i915);
+
+
+ igt_spin_free(i915, spin);
+ close(i915);
+}
+
+static void busy_all(int device, int clients)
+{
+ const struct intel_execution_engine2 *e;
+ uint64_t active[MAX_CLASS];
+ uint64_t idle[MAX_CLASS];
+ uint64_t old[MAX_CLASS];
+ uint64_t classes = 0;
+ igt_spin_t *spin;
+ int expect = 0;
+ int64_t delay;
+ int i915;
+ int me;
+
+ /* Create a fresh client with 0 runtime */
+ i915 = gem_reopen_driver(device);
+ gem_context_copy_engines(device, 0, i915, 0);
+
+ me = find_me(clients, getpid());
+ igt_assert(me != -1);
+ igt_require(faccessat(me, "busy", 0, F_OK) == 0);
+
+ spin = igt_spin_new(i915,
+ gem_context_clone_with_engines(i915, 0),
+ .flags = IGT_SPIN_POLL_RUN);
+ __for_each_physical_engine(i915, e) {
+ spin->execbuf.flags &= ~63;
+ spin->execbuf.flags |= e->flags;
+ gem_execbuf(i915, &spin->execbuf);
+
+ if (!(classes & (1ull << e->class)))
+ expect++;
+ classes |= 1ull << e->class;
+ }
+ igt_spin_busywait_until_started(spin);
+
+ delay = -500000; /* 500us slack */
+ memset(old, 0, sizeof(old));
+ for (int pass = 0; pass < 5; pass++) {
+ delay += measured_usleep(1000);
+ igt_debug("delay: %'"PRIu64"ns\n", delay);
+
+ /* Check that we accumulate the runtime, while active */
+ igt_assert_eq(read_runtime(me, active), expect);
+ for (int i = 0; i < ARRAY_SIZE(active); i++) {
+ if (!active[i])
+ continue;
+
+ igt_info("active[%d]: %'"PRIu64"ns\n", i, active[i]);
+ igt_assert(active[i] > old[i]); /* monotonic */
+ igt_assert(active[i] > delay); /* within reason */
+ }
+ }
+
+ gem_quiescent_gpu(i915);
+
+ /* And again now idle */
+ igt_assert_eq(read_runtime(me, idle), expect);
+ for (int i = 0; i < ARRAY_SIZE(idle); i++) {
+ if (!idle[i])
+ continue;
+
+ igt_info("idle[%d]: %'"PRIu64"ns\n", i, idle[i]);
+ igt_assert(idle[i] >= active[i]);
+ }
+
+ gem_context_destroy(i915, spin->execbuf.rsvd1);
+ igt_spin_free(i915, spin);
+
+ /* And finally after the executing context is no more */
+ igt_assert_eq(read_runtime(me, old), expect);
+ for (int i = 0; i < ARRAY_SIZE(old); i++) {
+ if (!old[i])
+ continue;
+
+ igt_info("old[%d]: %'"PRIu64"ns\n", i, old[i]);
+ igt_assert_eq_u64(old[i], idle[i]);
+ }
+
+ close(i915);
+}
+
+igt_main
+{
+ const struct intel_execution_engine2 *e;
+ int i915 = -1, clients = -1;
+
+ igt_fixture {
+ int sys;
+
+ /* Don't allow [too many] extra clients to be opened */
+ i915 = __drm_open_driver(DRIVER_INTEL);
+ igt_require_gem(i915);
+
+ sys = igt_sysfs_open(i915);
+ igt_require(sys != -1);
+
+ clients = openat(sys, "clients", O_RDONLY);
+ igt_require(clients != -1);
+
+ close(sys);
+ }
+
+ igt_subtest("pidname")
+ pidname(i915, clients);
+
+ igt_subtest("create")
+ create(i915, clients);
+
+ igt_subtest_with_dynamic("busy") {
+ __for_each_physical_engine(i915, e) {
+ igt_dynamic_f("%s", e->name) {
+ igt_fork(child, 1)
+ busy_one(i915, clients, e);
+ igt_waitchildren();
+ }
+ }
+ }
+
+ igt_subtest("busy-all") {
+ igt_fork(child, 1)
+ busy_all(i915, clients);
+ igt_waitchildren();
+ }
+
+ igt_fixture {
+ close(clients);
+ close(i915);
+ }
+}
diff --git a/tests/meson.build b/tests/meson.build
index ff924ff99..825e01833 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -236,6 +236,7 @@ i915_progs = [
'i915_query',
'i915_selftest',
'i915_suspend',
+ 'sysfs_clients',
'sysfs_defaults',
'sysfs_heartbeat_interval',
'sysfs_preempt_timeout',
--
2.30.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [Intel-gfx] [igt-dev] [PATCH i-g-t] i915: Exercise sysfs client properties
2021-01-22 21:49 ` Chris Wilson
@ 2021-01-25 10:20 ` Tvrtko Ursulin
2021-01-25 10:29 ` Chris Wilson
0 siblings, 1 reply; 4+ messages in thread
From: Tvrtko Ursulin @ 2021-01-25 10:20 UTC (permalink / raw)
To: Chris Wilson, igt-dev; +Cc: intel-gfx
On 22/01/2021 21:49, Chris Wilson wrote:
> We store every client name, pid and runtime under sysfs. Better check
> that matches with the actual client.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
> lib/igt_sysfs.c | 36 +++
> lib/igt_sysfs.h | 3 +
> tests/Makefile.sources | 3 +
> tests/i915/sysfs_clients.c | 450 +++++++++++++++++++++++++++++++++++++
> tests/meson.build | 1 +
> 5 files changed, 493 insertions(+)
> create mode 100644 tests/i915/sysfs_clients.c
>
> diff --git a/lib/igt_sysfs.c b/lib/igt_sysfs.c
> index 6aafe5349..e734143ba 100644
> --- a/lib/igt_sysfs.c
> +++ b/lib/igt_sysfs.c
> @@ -378,6 +378,42 @@ uint32_t igt_sysfs_get_u32(int dir, const char *attr)
> return result;
> }
>
> +/**
> + * igt_sysfs_get_u64:
> + * @dir: directory for the device from igt_sysfs_open()
> + * @attr: name of the sysfs node to open
> + *
> + * Convenience wrapper to read a unsigned 64bit integer from a sysfs file.
> + *
> + * Returns:
> + * The value read.
> + */
> +uint64_t igt_sysfs_get_u64(int dir, const char *attr)
> +{
> + uint64_t result;
> +
> + if (igt_sysfs_scanf(dir, attr, "%"PRIu64, &result) != 1)
> + return 0;
> +
> + return result;
> +}
> +
> +/**
> + * igt_sysfs_set_u64:
> + * @dir: directory for the device from igt_sysfs_open()
> + * @attr: name of the sysfs node to open
> + * @value: value to set
> + *
> + * Convenience wrapper to write a unsigned 64bit integer to a sysfs file.
> + *
> + * Returns:
> + * True if successfully written
> + */
> +bool igt_sysfs_set_u64(int dir, const char *attr, uint64_t value)
> +{
> + return igt_sysfs_printf(dir, attr, "%"PRIu64, value) > 0;
> +}
> +
> /**
> * igt_sysfs_set_u32:
> * @dir: directory for the device from igt_sysfs_open()
> diff --git a/lib/igt_sysfs.h b/lib/igt_sysfs.h
> index 64935a5ca..56741a0a3 100644
> --- a/lib/igt_sysfs.h
> +++ b/lib/igt_sysfs.h
> @@ -47,6 +47,9 @@ int igt_sysfs_printf(int dir, const char *attr, const char *fmt, ...)
> uint32_t igt_sysfs_get_u32(int dir, const char *attr);
> bool igt_sysfs_set_u32(int dir, const char *attr, uint32_t value);
>
> +uint64_t igt_sysfs_get_u64(int dir, const char *attr);
> +bool igt_sysfs_set_u64(int dir, const char *attr, uint64_t value);
> +
> bool igt_sysfs_get_boolean(int dir, const char *attr);
> bool igt_sysfs_set_boolean(int dir, const char *attr, bool value);
>
> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> index 1c227e750..3f663fe7e 100644
> --- a/tests/Makefile.sources
> +++ b/tests/Makefile.sources
> @@ -114,6 +114,9 @@ TESTS_progs = \
> TESTS_progs += api_intel_bb
> api_intel_bb_SOURCES = i915/api_intel_bb.c
>
> +TESTS_progs += sysfs_clients
> +sysfs_clients_SOURCES = i915/sysfs_clients.c
> +
> TESTS_progs += sysfs_defaults
> sysfs_defaults_SOURCES = i915/sysfs_defaults.c
>
> diff --git a/tests/i915/sysfs_clients.c b/tests/i915/sysfs_clients.c
> new file mode 100644
> index 000000000..a77adec6d
> --- /dev/null
> +++ b/tests/i915/sysfs_clients.c
> @@ -0,0 +1,450 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright © 2021 Intel Corporation
> + */
> +
> +#include <ctype.h>
> +#include <dirent.h>
> +#include <errno.h>
> +#include <fcntl.h>
> +#include <inttypes.h>
> +#include <sys/stat.h>
> +#include <sys/types.h>
> +#include <unistd.h>
> +
> +#include "drmtest.h"
> +#include "i915/gem.h"
> +#include "i915/gem_context.h"
> +#include "i915/gem_engine_topology.h"
> +#include "igt_aux.h"
> +#include "igt_dummyload.h"
> +#include "igt_sysfs.h"
> +#include "ioctl_wrappers.h"
> +
> +static void pidname(int i915, int clients)
> +{
> + struct dirent *de;
> + int sv[2], rv[2];
> + char buf[280];
> + int me = -1;
> + long count;
> + pid_t pid;
> + DIR *dir;
> + int len;
> +
> + dir = fdopendir(dup(clients));
> + igt_assert(dir);
> + rewinddir(dir);
> +
> + count = 0;
> + while ((de = readdir(dir))) {
> + if (!isdigit(de->d_name[0]))
> + continue;
> +
> + snprintf(buf, sizeof(buf), "%s/name", de->d_name);
> + len = igt_sysfs_read(clients, buf, buf, sizeof(buf));
> + igt_assert_f(len > 0, "failed to open '%s/name'\n", de->d_name);
> + buf[len - 1] = '\0';
> + igt_debug("%s: %s\n", de->d_name, buf);
> +
> + /* Ignore closed clients created by drm_driver_open() */
> + if (*buf == '<')
> + continue;
> +
> + close(me);
> + me = openat(clients, de->d_name, O_DIRECTORY | O_RDONLY);
> + count++;
> + }
> + closedir(dir);
> +
> + /* We expect there to be only the single client (us) running */
> + igt_assert_eq(count, 1);
> + igt_assert(me >= 0);
> +
> + len = igt_sysfs_read(me, "name", buf, sizeof(buf));
> + igt_assert(len > 0);
> + buf[len - 1] = '\0';
We could add a helper like igt_syfs_str or something since this repeats
a lot but not important straight away.
> +
> + igt_info("My name: %s\n", buf);
> + igt_assert(strcmp(buf, igt_test_name()) == 0);
> +
> + igt_assert(pipe(sv) == 0);
> + igt_assert(pipe(rv) == 0);
> +
> + /* If give our fd to someone else, they take over ownership of client */
> + igt_fork(child, 1) {
> + read(sv[0], &pid, sizeof(pid));
> +
> + gem_context_destroy(i915, gem_context_create(i915));
> +
> + pid = getpid();
> + write(rv[1], &pid, sizeof(pid));
> + }
> + close(sv[0]);
> + close(rv[1]);
> +
> + /* Child exists, but not yet running, we still own the client */
> + len = igt_sysfs_read(me, "pid", buf, sizeof(buf));
> + igt_assert(len > 0);
> + buf[len - 1] = '\0';
> +
> + pid = getpid();
> + igt_info("My pid: %s\n", buf);
> + igt_assert_eq(atoi(buf), pid);
> +
> + /* Release and wait for the child */
> + igt_assert_eq(write(sv[1], &pid, sizeof(pid)), sizeof(pid));
> + igt_assert_eq(read(rv[0], &pid, sizeof(pid)), sizeof(pid));
> +
> + /* Now child owns the client and pid should be updated to match */
> + len = igt_sysfs_read(me, "pid", buf, sizeof(buf));
> + igt_assert(len > 0);
> + buf[len - 1] = '\0';
> +
> + igt_info("New pid: %s\n", buf);
> + igt_assert_eq(atoi(buf), pid);
> + igt_waitchildren();
> +
> + /* Child has definitely gone, but the client should remain */
> + len = igt_sysfs_read(me, "pid", buf, sizeof(buf));
> + igt_assert(len > 0);
> + buf[len - 1] = '\0';
> +
> + igt_info("Old pid: %s\n", buf);
> + igt_assert_eq(atoi(buf), pid);
"Old pid" as in pid of the child which exited. Do we have a IGT helper
to cross check what the child sent via pipe matches what fork said? Or
what waitpid reported as exited.
Also, would it revert back to parent pid if parent did one context
create at this point? Could be worth checking if so.
> +
> + close(sv[1]);
> + close(rv[0]);
> + close(me);
> +}
> +
> +static long count_clients(int clients)
> +{
> + struct dirent *de;
> + long count = 0;
> + char buf[280];
> + DIR *dir;
> +
> + dir = fdopendir(dup(clients));
> + igt_assert(dir);
> + rewinddir(dir);
> +
> + while ((de = readdir(dir))) {
> + int len;
> +
> + if (!isdigit(de->d_name[0]))
> + continue;
> +
> + snprintf(buf, sizeof(buf), "%s/name", de->d_name);
> + len = igt_sysfs_read(clients, buf, buf, sizeof(buf));
> + if (len < 0)
> + continue;
> +
> + count += *buf != '<';
> + }
> + closedir(dir);
> +
> + return count;
> +}
> +
> +static void create(int i915, int clients)
> +{
> + int fd[16];
> +
> + /* Each new open("/dev/dri/cardN") is a new client */
> + igt_assert_eq(count_clients(clients), 1);
> + for (int i = 0; i < ARRAY_SIZE(fd); i++) {
> + fd[i] = gem_reopen_driver(i915);
> + igt_assert_eq(count_clients(clients), i + 2);
> + }
> +
> + for (int i = 0; i < ARRAY_SIZE(fd); i++)
> + close(fd[i]);
> +
> + /* Cleanup delayed behind rcu */
> + igt_until_timeout(30) {
> + usleep(0);
Intention to yield?
> + if (count_clients(clients) == 1)
> + break;
> + usleep(10000);
> + }
> + igt_assert_eq(count_clients(clients), 1);
A variant which closes a single random client and does a cross check in
every loop iteration could be useful then opens a new client checking
id's are unique. Can be added later.
> +}
> +
> +static int find_me(int clients, pid_t pid)
> +{
> + struct dirent *de;
> + char buf[280];
> + int me = -1;
> + DIR *dir;
> +
> + dir = fdopendir(dup(clients));
> + igt_assert(dir);
> + rewinddir(dir);
> +
> + while ((de = readdir(dir))) {
> + int ret;
> +
> + if (!isdigit(de->d_name[0]))
> + continue;
> +
> + snprintf(buf, sizeof(buf), "%s/pid", de->d_name);
> + ret = igt_sysfs_read(clients, buf, buf, sizeof(buf));
> + igt_assert_f(ret > 0, "failed to open '%s/pid'\n", de->d_name);
> + if (atoi(buf) != pid)
> + continue;
> +
> + me = openat(clients, de->d_name, O_DIRECTORY | O_RDONLY);
> + break;
> + }
> +
> + closedir(dir);
> + return me;
> +}
> +
> +#define MAX_CLASS 64
> +static int read_runtime(int client, uint64_t *runtime)
> +{
> + int fd = openat(client, "busy", O_DIRECTORY | O_RDONLY);
> + DIR *dir = fdopendir(fd);
> + struct dirent *de;
> + int count = 0;
> +
> + memset(runtime, 0, sizeof(*runtime) * MAX_CLASS);
> + while ((de = readdir(dir))) {
> + int class;
> +
> + if (!isdigit(de->d_name[0]))
> + continue;
> +
> + class = atoi(de->d_name);
> + igt_assert(class < MAX_CLASS);
> + runtime[class] = igt_sysfs_get_u64(fd, de->d_name);
> +
> + count += runtime[class] != 0;
> + }
> + closedir(dir);
> +
> + return count;
> +}
> +
> +static uint64_t measured_usleep(unsigned int usec)
> +{
> + struct timespec tv;
> + unsigned int slept;
> +
> + slept = igt_nsec_elapsed(memset(&tv, 0, sizeof(tv)));
> + igt_assert(slept == 0);
> + do {
> + usleep(usec - slept);
> + slept = igt_nsec_elapsed(&tv) / 1000;
> + } while (slept < usec);
> +
> + return igt_nsec_elapsed(&tv);
> +}
> +
> +static void
> +busy_one(int device, int clients, const struct intel_execution_engine2 *e)
> +{
> + uint64_t active[MAX_CLASS];
> + uint64_t idle[MAX_CLASS];
> + uint64_t old[MAX_CLASS];
> + igt_spin_t *spin;
> + int64_t delay;
> + int i915;
> + int me;
> +
> + /* Create a fresh client with 0 runtime */
> + i915 = gem_reopen_driver(device);
> + gem_context_copy_engines(device, 0, i915, 0);
> +
> + me = find_me(clients, getpid());
> + igt_assert(me != -1);
> + igt_require(faccessat(me, "busy", 0, F_OK) == 0);
> +
> + spin = igt_spin_new(i915,
> + gem_context_clone_with_engines(i915, 0),
> + .engine = e->flags,
> + .flags = IGT_SPIN_POLL_RUN);
> + igt_spin_busywait_until_started(spin);
> +
> + delay = -500000; /* 500us slack */
> + memset(old, 0, sizeof(old));
> + for (int pass = 0; pass < 5; pass++) {
> + delay += measured_usleep(1000);
> + igt_debug("delay: %'"PRIu64"ns\n", delay);
> +
> + /* Check that we accumulate the runtime, while active */
> + igt_assert_eq(read_runtime(me, active), 1);
> + igt_info("active1[%d]: %'"PRIu64"ns\n", pass, active[e->class]);
> + igt_assert(active[e->class] > old[e->class]); /* monotonic */
> + igt_assert(active[e->class] > delay); /* within reason */
Sending greetings to GuC soon. Will need to emit a high prio pulse to
preempt the spinner, maybe, no, probably won't be enough.
> + }
> +
> + gem_quiescent_gpu(i915);
> +
> + /* And again now idle */
> + igt_assert_eq(read_runtime(me, idle), 1);
> + igt_info("idle: %'"PRIu64"ns\n", idle[e->class]);
> + igt_assert(idle[e->class] >= active[e->class]);
> +
> + gem_context_destroy(i915, spin->execbuf.rsvd1);
> +
> + /* And finally after the executing context is no more */
> + igt_assert_eq(read_runtime(me, old), 1);
> + igt_info("old: %'"PRIu64"ns\n", old[e->class]);
> + igt_assert_eq_u64(old[e->class], idle[e->class]);
> +
> + /* Once more on the default context for good luck */
> + igt_spin_reset(spin);
> + spin->execbuf.rsvd1 = 0;
> + gem_execbuf(i915, &spin->execbuf);
> + igt_spin_busywait_until_started(spin);
> +
> + for (int pass = 0; pass < 5; pass++) {
> + delay += measured_usleep(1000);
> + igt_debug("delay: %'"PRIu64"ns\n", delay);
> +
> + /* Check that we accumulate the runtime, while active */
> + igt_assert_eq(read_runtime(me, active), 1);
> + igt_info("active0[%d]: %'"PRIu64"ns\n", pass, active[e->class]);
> + igt_assert(active[e->class] > old[e->class]); /* monotonic */
> + igt_assert(active[e->class] > delay); /* within reason */
> + }
> +
> + gem_quiescent_gpu(i915);
> +
> +
> + igt_spin_free(i915, spin);
> + close(i915);
> +}
> +
> +static void busy_all(int device, int clients)
> +{
> + const struct intel_execution_engine2 *e;
> + uint64_t active[MAX_CLASS];
> + uint64_t idle[MAX_CLASS];
> + uint64_t old[MAX_CLASS];
> + uint64_t classes = 0;
> + igt_spin_t *spin;
> + int expect = 0;
> + int64_t delay;
> + int i915;
> + int me;
> +
> + /* Create a fresh client with 0 runtime */
> + i915 = gem_reopen_driver(device);
> + gem_context_copy_engines(device, 0, i915, 0);
> +
> + me = find_me(clients, getpid());
> + igt_assert(me != -1);
> + igt_require(faccessat(me, "busy", 0, F_OK) == 0);
> +
> + spin = igt_spin_new(i915,
> + gem_context_clone_with_engines(i915, 0),
> + .flags = IGT_SPIN_POLL_RUN);
> + __for_each_physical_engine(i915, e) {
> + spin->execbuf.flags &= ~63;
> + spin->execbuf.flags |= e->flags;
> + gem_execbuf(i915, &spin->execbuf);
> +
> + if (!(classes & (1ull << e->class)))
> + expect++;
> + classes |= 1ull << e->class;
> + }
> + igt_spin_busywait_until_started(spin);
> +
> + delay = -500000; /* 500us slack */
> + memset(old, 0, sizeof(old));
> + for (int pass = 0; pass < 5; pass++) {
> + delay += measured_usleep(1000);
> + igt_debug("delay: %'"PRIu64"ns\n", delay);
> +
> + /* Check that we accumulate the runtime, while active */
> + igt_assert_eq(read_runtime(me, active), expect);
> + for (int i = 0; i < ARRAY_SIZE(active); i++) {
> + if (!active[i])
> + continue;
Don't you want do skip based on the bitmap in classes here? Although the
assert on expect will catch failures to account some class already so
optional I guess.
> +
> + igt_info("active[%d]: %'"PRIu64"ns\n", i, active[i]);
> + igt_assert(active[i] > old[i]); /* monotonic */
> + igt_assert(active[i] > delay); /* within reason */
> + }
> + }
> +
> + gem_quiescent_gpu(i915);
> +
> + /* And again now idle */
> + igt_assert_eq(read_runtime(me, idle), expect);
> + for (int i = 0; i < ARRAY_SIZE(idle); i++) {
> + if (!idle[i])
> + continue;
> +
> + igt_info("idle[%d]: %'"PRIu64"ns\n", i, idle[i]);
> + igt_assert(idle[i] >= active[i]);
> + }
> +
> + gem_context_destroy(i915, spin->execbuf.rsvd1);
> + igt_spin_free(i915, spin);
> +
> + /* And finally after the executing context is no more */
> + igt_assert_eq(read_runtime(me, old), expect);
> + for (int i = 0; i < ARRAY_SIZE(old); i++) {
> + if (!old[i])
> + continue;
> +
> + igt_info("old[%d]: %'"PRIu64"ns\n", i, old[i]);
> + igt_assert_eq_u64(old[i], idle[i]);
> + }
> +
> + close(i915);
> +}
> +
> +igt_main
> +{
> + const struct intel_execution_engine2 *e;
> + int i915 = -1, clients = -1;
> +
> + igt_fixture {
> + int sys;
> +
> + /* Don't allow [too many] extra clients to be opened */
> + i915 = __drm_open_driver(DRIVER_INTEL);
> + igt_require_gem(i915);
> +
> + sys = igt_sysfs_open(i915);
> + igt_require(sys != -1);
> +
> + clients = openat(sys, "clients", O_RDONLY);
> + igt_require(clients != -1);
> +
> + close(sys);
> + }
> +
> + igt_subtest("pidname")
> + pidname(i915, clients);
> +
> + igt_subtest("create")
> + create(i915, clients);
> +
> + igt_subtest_with_dynamic("busy") {
> + __for_each_physical_engine(i915, e) {
> + igt_dynamic_f("%s", e->name) {
> + igt_fork(child, 1)
> + busy_one(i915, clients, e);
> + igt_waitchildren();
> + }
> + }
> + }
> +
> + igt_subtest("busy-all") {
> + igt_fork(child, 1)
> + busy_all(i915, clients);
> + igt_waitchildren();
> + }
> +
> + igt_fixture {
> + close(clients);
> + close(i915);
> + }
> +}
> diff --git a/tests/meson.build b/tests/meson.build
> index ff924ff99..825e01833 100644
> --- a/tests/meson.build
> +++ b/tests/meson.build
> @@ -236,6 +236,7 @@ i915_progs = [
> 'i915_query',
> 'i915_selftest',
> 'i915_suspend',
> + 'sysfs_clients',
> 'sysfs_defaults',
> 'sysfs_heartbeat_interval',
> 'sysfs_preempt_timeout',
>
No issues found, just comments so:
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Intel-gfx] [igt-dev] [PATCH i-g-t] i915: Exercise sysfs client properties
2021-01-25 10:20 ` [Intel-gfx] [igt-dev] " Tvrtko Ursulin
@ 2021-01-25 10:29 ` Chris Wilson
0 siblings, 0 replies; 4+ messages in thread
From: Chris Wilson @ 2021-01-25 10:29 UTC (permalink / raw)
To: Tvrtko Ursulin, igt-dev; +Cc: intel-gfx
Quoting Tvrtko Ursulin (2021-01-25 10:20:15)
>
> On 22/01/2021 21:49, Chris Wilson wrote:
> > We store every client name, pid and runtime under sysfs. Better check
> > that matches with the actual client.
> >
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > ---
> > lib/igt_sysfs.c | 36 +++
> > lib/igt_sysfs.h | 3 +
> > tests/Makefile.sources | 3 +
> > tests/i915/sysfs_clients.c | 450 +++++++++++++++++++++++++++++++++++++
> > tests/meson.build | 1 +
> > 5 files changed, 493 insertions(+)
> > create mode 100644 tests/i915/sysfs_clients.c
> >
> > diff --git a/lib/igt_sysfs.c b/lib/igt_sysfs.c
> > index 6aafe5349..e734143ba 100644
> > --- a/lib/igt_sysfs.c
> > +++ b/lib/igt_sysfs.c
> > @@ -378,6 +378,42 @@ uint32_t igt_sysfs_get_u32(int dir, const char *attr)
> > return result;
> > }
> >
> > +/**
> > + * igt_sysfs_get_u64:
> > + * @dir: directory for the device from igt_sysfs_open()
> > + * @attr: name of the sysfs node to open
> > + *
> > + * Convenience wrapper to read a unsigned 64bit integer from a sysfs file.
> > + *
> > + * Returns:
> > + * The value read.
> > + */
> > +uint64_t igt_sysfs_get_u64(int dir, const char *attr)
> > +{
> > + uint64_t result;
> > +
> > + if (igt_sysfs_scanf(dir, attr, "%"PRIu64, &result) != 1)
> > + return 0;
> > +
> > + return result;
> > +}
> > +
> > +/**
> > + * igt_sysfs_set_u64:
> > + * @dir: directory for the device from igt_sysfs_open()
> > + * @attr: name of the sysfs node to open
> > + * @value: value to set
> > + *
> > + * Convenience wrapper to write a unsigned 64bit integer to a sysfs file.
> > + *
> > + * Returns:
> > + * True if successfully written
> > + */
> > +bool igt_sysfs_set_u64(int dir, const char *attr, uint64_t value)
> > +{
> > + return igt_sysfs_printf(dir, attr, "%"PRIu64, value) > 0;
> > +}
> > +
> > /**
> > * igt_sysfs_set_u32:
> > * @dir: directory for the device from igt_sysfs_open()
> > diff --git a/lib/igt_sysfs.h b/lib/igt_sysfs.h
> > index 64935a5ca..56741a0a3 100644
> > --- a/lib/igt_sysfs.h
> > +++ b/lib/igt_sysfs.h
> > @@ -47,6 +47,9 @@ int igt_sysfs_printf(int dir, const char *attr, const char *fmt, ...)
> > uint32_t igt_sysfs_get_u32(int dir, const char *attr);
> > bool igt_sysfs_set_u32(int dir, const char *attr, uint32_t value);
> >
> > +uint64_t igt_sysfs_get_u64(int dir, const char *attr);
> > +bool igt_sysfs_set_u64(int dir, const char *attr, uint64_t value);
> > +
> > bool igt_sysfs_get_boolean(int dir, const char *attr);
> > bool igt_sysfs_set_boolean(int dir, const char *attr, bool value);
> >
> > diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> > index 1c227e750..3f663fe7e 100644
> > --- a/tests/Makefile.sources
> > +++ b/tests/Makefile.sources
> > @@ -114,6 +114,9 @@ TESTS_progs = \
> > TESTS_progs += api_intel_bb
> > api_intel_bb_SOURCES = i915/api_intel_bb.c
> >
> > +TESTS_progs += sysfs_clients
> > +sysfs_clients_SOURCES = i915/sysfs_clients.c
> > +
> > TESTS_progs += sysfs_defaults
> > sysfs_defaults_SOURCES = i915/sysfs_defaults.c
> >
> > diff --git a/tests/i915/sysfs_clients.c b/tests/i915/sysfs_clients.c
> > new file mode 100644
> > index 000000000..a77adec6d
> > --- /dev/null
> > +++ b/tests/i915/sysfs_clients.c
> > @@ -0,0 +1,450 @@
> > +/* SPDX-License-Identifier: MIT */
> > +/*
> > + * Copyright © 2021 Intel Corporation
> > + */
> > +
> > +#include <ctype.h>
> > +#include <dirent.h>
> > +#include <errno.h>
> > +#include <fcntl.h>
> > +#include <inttypes.h>
> > +#include <sys/stat.h>
> > +#include <sys/types.h>
> > +#include <unistd.h>
> > +
> > +#include "drmtest.h"
> > +#include "i915/gem.h"
> > +#include "i915/gem_context.h"
> > +#include "i915/gem_engine_topology.h"
> > +#include "igt_aux.h"
> > +#include "igt_dummyload.h"
> > +#include "igt_sysfs.h"
> > +#include "ioctl_wrappers.h"
> > +
> > +static void pidname(int i915, int clients)
> > +{
> > + struct dirent *de;
> > + int sv[2], rv[2];
> > + char buf[280];
> > + int me = -1;
> > + long count;
> > + pid_t pid;
> > + DIR *dir;
> > + int len;
> > +
> > + dir = fdopendir(dup(clients));
> > + igt_assert(dir);
> > + rewinddir(dir);
> > +
> > + count = 0;
> > + while ((de = readdir(dir))) {
> > + if (!isdigit(de->d_name[0]))
> > + continue;
> > +
> > + snprintf(buf, sizeof(buf), "%s/name", de->d_name);
> > + len = igt_sysfs_read(clients, buf, buf, sizeof(buf));
> > + igt_assert_f(len > 0, "failed to open '%s/name'\n", de->d_name);
> > + buf[len - 1] = '\0';
> > + igt_debug("%s: %s\n", de->d_name, buf);
> > +
> > + /* Ignore closed clients created by drm_driver_open() */
> > + if (*buf == '<')
> > + continue;
> > +
> > + close(me);
> > + me = openat(clients, de->d_name, O_DIRECTORY | O_RDONLY);
> > + count++;
> > + }
> > + closedir(dir);
> > +
> > + /* We expect there to be only the single client (us) running */
> > + igt_assert_eq(count, 1);
> > + igt_assert(me >= 0);
> > +
> > + len = igt_sysfs_read(me, "name", buf, sizeof(buf));
> > + igt_assert(len > 0);
> > + buf[len - 1] = '\0';
>
> We could add a helper like igt_syfs_str or something since this repeats
> a lot but not important straight away.
-> strterm()
>
> > +
> > + igt_info("My name: %s\n", buf);
> > + igt_assert(strcmp(buf, igt_test_name()) == 0);
> > +
> > + igt_assert(pipe(sv) == 0);
> > + igt_assert(pipe(rv) == 0);
> > +
> > + /* If give our fd to someone else, they take over ownership of client */
> > + igt_fork(child, 1) {
> > + read(sv[0], &pid, sizeof(pid));
> > +
> > + gem_context_destroy(i915, gem_context_create(i915));
> > +
> > + pid = getpid();
> > + write(rv[1], &pid, sizeof(pid));
> > + }
> > + close(sv[0]);
> > + close(rv[1]);
> > +
> > + /* Child exists, but not yet running, we still own the client */
> > + len = igt_sysfs_read(me, "pid", buf, sizeof(buf));
> > + igt_assert(len > 0);
> > + buf[len - 1] = '\0';
> > +
> > + pid = getpid();
> > + igt_info("My pid: %s\n", buf);
> > + igt_assert_eq(atoi(buf), pid);
> > +
> > + /* Release and wait for the child */
> > + igt_assert_eq(write(sv[1], &pid, sizeof(pid)), sizeof(pid));
> > + igt_assert_eq(read(rv[0], &pid, sizeof(pid)), sizeof(pid));
> > +
> > + /* Now child owns the client and pid should be updated to match */
> > + len = igt_sysfs_read(me, "pid", buf, sizeof(buf));
> > + igt_assert(len > 0);
> > + buf[len - 1] = '\0';
> > +
> > + igt_info("New pid: %s\n", buf);
> > + igt_assert_eq(atoi(buf), pid);
> > + igt_waitchildren();
> > +
> > + /* Child has definitely gone, but the client should remain */
> > + len = igt_sysfs_read(me, "pid", buf, sizeof(buf));
> > + igt_assert(len > 0);
> > + buf[len - 1] = '\0';
> > +
> > + igt_info("Old pid: %s\n", buf);
> > + igt_assert_eq(atoi(buf), pid);
>
> "Old pid" as in pid of the child which exited. Do we have a IGT helper
> to cross check what the child sent via pipe matches what fork said? Or
> what waitpid reported as exited.
For the sake of the test, you can just scroll back and check ;)
> Also, would it revert back to parent pid if parent did one context
> create at this point? Could be worth checking if so.
I added such a test.
> > +
> > + close(sv[1]);
> > + close(rv[0]);
> > + close(me);
> > +}
> > +
> > +static long count_clients(int clients)
> > +{
> > + struct dirent *de;
> > + long count = 0;
> > + char buf[280];
> > + DIR *dir;
> > +
> > + dir = fdopendir(dup(clients));
> > + igt_assert(dir);
> > + rewinddir(dir);
> > +
> > + while ((de = readdir(dir))) {
> > + int len;
> > +
> > + if (!isdigit(de->d_name[0]))
> > + continue;
> > +
> > + snprintf(buf, sizeof(buf), "%s/name", de->d_name);
> > + len = igt_sysfs_read(clients, buf, buf, sizeof(buf));
> > + if (len < 0)
> > + continue;
> > +
> > + count += *buf != '<';
> > + }
> > + closedir(dir);
> > +
> > + return count;
> > +}
> > +
> > +static void create(int i915, int clients)
> > +{
> > + int fd[16];
> > +
> > + /* Each new open("/dev/dri/cardN") is a new client */
> > + igt_assert_eq(count_clients(clients), 1);
> > + for (int i = 0; i < ARRAY_SIZE(fd); i++) {
> > + fd[i] = gem_reopen_driver(i915);
> > + igt_assert_eq(count_clients(clients), i + 2);
> > + }
> > +
> > + for (int i = 0; i < ARRAY_SIZE(fd); i++)
> > + close(fd[i]);
> > +
> > + /* Cleanup delayed behind rcu */
> > + igt_until_timeout(30) {
> > + usleep(0);
>
> Intention to yield?
If you think that's more likely to work :)
>
> > + if (count_clients(clients) == 1)
> > + break;
> > + usleep(10000);
> > + }
> > + igt_assert_eq(count_clients(clients), 1);
>
> A variant which closes a single random client and does a cross check in
> every loop iteration could be useful then opens a new client checking
> id's are unique. Can be added later.
One not-recycling test coming up.
> > + delay = -500000; /* 500us slack */
> > + memset(old, 0, sizeof(old));
> > + for (int pass = 0; pass < 5; pass++) {
> > + delay += measured_usleep(1000);
> > + igt_debug("delay: %'"PRIu64"ns\n", delay);
> > +
> > + /* Check that we accumulate the runtime, while active */
> > + igt_assert_eq(read_runtime(me, active), 1);
> > + igt_info("active1[%d]: %'"PRIu64"ns\n", pass, active[e->class]);
> > + igt_assert(active[e->class] > old[e->class]); /* monotonic */
> > + igt_assert(active[e->class] > delay); /* within reason */
>
> Sending greetings to GuC soon. Will need to emit a high prio pulse to
> preempt the spinner, maybe, no, probably won't be enough.
Exactly. The GuC falls far short of what we need atm. A pulse from every
sysfs read() and wait for pulse completion before returning? What
happens for non-preemptible workloads?
> > + delay = -500000; /* 500us slack */
> > + memset(old, 0, sizeof(old));
> > + for (int pass = 0; pass < 5; pass++) {
> > + delay += measured_usleep(1000);
> > + igt_debug("delay: %'"PRIu64"ns\n", delay);
> > +
> > + /* Check that we accumulate the runtime, while active */
> > + igt_assert_eq(read_runtime(me, active), expect);
> > + for (int i = 0; i < ARRAY_SIZE(active); i++) {
> > + if (!active[i])
> > + continue;
>
> Don't you want do skip based on the bitmap in classes here? Although the
> assert on expect will catch failures to account some class already so
> optional I guess.
Bitmap came afterwards, and indeed it should have replaced the !active[i].
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-01-25 10:29 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-22 21:34 [Intel-gfx] [PATCH i-g-t] i915: Exercise sysfs client properties Chris Wilson
2021-01-22 21:49 ` Chris Wilson
2021-01-25 10:20 ` [Intel-gfx] [igt-dev] " Tvrtko Ursulin
2021-01-25 10:29 ` Chris Wilson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).