On 22/03/2018 17:44, Chris Wilson wrote:
Quoting Tvrtko Ursulin (2018-03-22 17:24:16)From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx> If we stop relying on regular GPU hangs to be detected, but trigger them manually as soon as we know our batch of interest is actually executing on the GPU, we can dramatically speed up various subtests. This is enabled by the pollable spin batch added in the previous patch. v2: * Test gem_wait after reset/wedge and with reset/wedge after a few predefined intervals since gem_wait invocation. (Chris Wilson) Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx> Suggested-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> Cc: Antonio Argenziano <antonio.argenziano@xxxxxxxxx> --- lib.tar | Bin 0 -> 102400 bytes tests/gem_eio.c | 214 ++++++++++++++++++++++++++++++++++++++++++-------------- 2 files changed, 160 insertions(+), 54 deletions(-) create mode 100644 lib.tar diff --git a/tests/gem_eio.c b/tests/gem_eio.c index 4bcc5937db39..2f9e954085ec 100644 --- a/tests/gem_eio.c +++ b/tests/gem_eio.c @@ -35,6 +35,7 @@ #include <inttypes.h> #include <errno.h> #include <sys/ioctl.h> +#include <signal.h>#include <drm.h> @@ -71,26 +72,23 @@ static void trigger_reset(int fd)gem_quiescent_gpu(fd); }-static void wedge_gpu(int fd)+static void manual_hang(int drm_fd) { - /* First idle the GPU then disable GPU resets before injecting a hang */ - gem_quiescent_gpu(fd); - - igt_require(i915_reset_control(false)); + int dir = igt_debugfs_dir(drm_fd);- igt_debug("Wedging GPU by injecting hang\n");- igt_post_hang_ring(fd, igt_hang_ring(fd, I915_EXEC_DEFAULT)); + igt_sysfs_set(dir, "i915_wedged", "-1");- igt_assert(i915_reset_control(true));+ close(dir); }-static void wedgeme(int drm_fd)+static void wedge_gpu(int fd) { - int dir = igt_debugfs_dir(drm_fd); - - igt_sysfs_set(dir, "i915_wedged", "-1"); + /* First idle the GPU then disable GPU resets before injecting a hang */ + gem_quiescent_gpu(fd);- close(dir);+ igt_require(i915_reset_control(false)); + manual_hang(fd); + igt_assert(i915_reset_control(true)); }static int __gem_throttle(int fd)@@ -149,26 +147,111 @@ static int __gem_wait(int fd, uint32_t handle, int64_t timeout) return err; }-static void test_wait(int fd)+static igt_spin_t * __spin_poll(int fd, uint32_t ctx, unsigned long flags) +{ + if (gem_can_store_dword(fd, flags)) + return __igt_spin_batch_new_poll(fd, ctx, flags); + else + return __igt_spin_batch_new(fd, ctx, flags, 0); +} + +static void __spin_wait(int fd, igt_spin_t *spin) +{ + if (spin->running) { + igt_spin_busywait_until_running(spin); + } else { + igt_debug("__spin_wait - usleep mode\n"); + usleep(500e3); /* Better than nothing! */ + } +} + +static igt_spin_t * spin_sync(int fd, uint32_t ctx, unsigned long flags) +{ + igt_spin_t *spin = __spin_poll(fd, ctx, flags); + + __spin_wait(fd, spin); + + return spin; +} + +static int debugfs_dir = -1; + +static void hang_handler(int sig) +{ + igt_sysfs_set(debugfs_dir, "i915_wedged", "-1"); +} + +static void hang_after(int fd, unsigned int us) +{ + struct sigaction sa = { .sa_handler = hang_handler }; + struct itimerval itv = { }; + + debugfs_dir = igt_debugfs_dir(fd); + igt_assert_fd(debugfs_dir); + + igt_assert_eq(sigaction(SIGALRM, &sa, NULL), 0); + + itv.it_value.tv_sec = us / 1000000;USEC_PER_SEC.+ itv.it_value.tv_usec = us % 1000000; + setitimer(ITIMER_REAL, &itv, NULL);Ok, that gives a single shot signal. I would have used struct sigevent sev = { .sigev_notify = SIGEV_THREAD, .sigev_value.sigval_int = debugfs_dir .sigev_notify_function = hang_handler }; timer_create(CLOCK_MONOTONIC, &sec, &timer); timer_settime(timer, 0, &its, NULL); Then static void hang_handler(union sigval arg) { igt_sysfs_set(arg.sival_int, "i915_wedged", 1); } No signals, nor globals required :)
I wasn't familiar with this facility.It creates a new thread, so any hopes for small microsecond delays might be ruined. I can try it if you think it is still worth it?
The problem with using a signal is that it interrupts the gem_wait() and so we don't actually check that it is being woken by the hang because it is already awake. Gah.
Hm... if I am following correctly, we end up with -ERESTARTSYS and the the ioctl can get restarted for us, if I would set SA_RESTART.
At the moment it happens to work because drmIoctl restart the signal.
+static void cleanup_hang(void) +{ + struct itimerval itv = { }; + + igt_assert_eq(setitimer(ITIMER_REAL, &itv, NULL), 0);You also need a sleep here as it does not flush inflight signals. (Also timer_destroy :)
I always pass a longer timeout to gem_wait than the signal so I think it should be guaranteed that the signal had fired before gem_wait will be exiting.
Regards, Tvrtko
+ igt_assert_fd(debugfs_dir); + close(debugfs_dir); + debugfs_dir = -1; +} + +static int __check_wait(int fd, uint32_t bo, unsigned int wait) +{ + unsigned long wait_timeout = 250e6; /* Some margin for actual reset. */ + int ret; + + if (wait) { + wait_timeout += wait * 2000; /* x2 for safety. */ + wait_timeout += 250e6; /* Margin for signal delay. */; + hang_after(fd, wait); + } else { + manual_hang(fd); + } + + ret = __gem_wait(fd, bo, wait_timeout);Ok, I understand where the concerned about how long it took to recover from reset came from :)+ + if (wait) + cleanup_hang(); + + return ret; +} + +#define TEST_WEDGE (1) + +static void test_wait(int fd, unsigned int flags, unsigned int wait) { - igt_hang_t hang; + igt_spin_t *hang;igt_require_gem(fd); - /* If the request we wait on completes due to a hang (even for+ /* + * If the request we wait on completes due to a hang (even for * that request), the user expects the return value to 0 (success). */ - hang = igt_hang_ring(fd, I915_EXEC_DEFAULT); - igt_assert_eq(__gem_wait(fd, hang.handle, -1), 0); - igt_post_hang_ring(fd, hang);- /* If the GPU is wedged during the wait, again we expect the return- * value to be 0 (success). - */ - igt_require(i915_reset_control(false)); - hang = igt_hang_ring(fd, I915_EXEC_DEFAULT); - igt_assert_eq(__gem_wait(fd, hang.handle, -1), 0); - igt_post_hang_ring(fd, hang); + if (flags & TEST_WEDGE) + igt_require(i915_reset_control(false)); + else + igt_require(i915_reset_control(true)); + + hang = spin_sync(fd, 0, I915_EXEC_DEFAULT); + + igt_assert_eq(__check_wait(fd, hang->handle, wait), 0); + + igt_spin_batch_free(fd, hang); + igt_require(i915_reset_control(true));trigger_reset(fd);@@ -181,7 +264,7 @@ static void test_suspend(int fd, int state)/* Check we can suspend when the driver is already wedged */igt_require(i915_reset_control(false)); - wedgeme(fd); + manual_hang(fd);igt_system_suspend_autoresume(state, SUSPEND_TEST_DEVICES); @@ -189,7 +272,7 @@ static void test_suspend(int fd, int state)trigger_reset(fd); }-static void test_inflight(int fd)+static void test_inflight(int fd, unsigned int wait) { const uint32_t bbe = MI_BATCH_BUFFER_END; struct drm_i915_gem_exec_object2 obj[2]; @@ -209,11 +292,10 @@ static void test_inflight(int fd) int fence[64]; /* conservative estimate of ring size */gem_quiescent_gpu(fd);- igt_debug("Starting %s on engine '%s'\n", __func__, e__->name); igt_require(i915_reset_control(false));- hang = __igt_spin_batch_new(fd, 0, engine, 0);+ hang = spin_sync(fd, 0, engine); obj[0].handle = hang->handle;memset(&execbuf, 0, sizeof(execbuf));@@ -227,7 +309,8 @@ static void test_inflight(int fd) igt_assert(fence[n] != -1); }- igt_assert_eq(__gem_wait(fd, obj[1].handle, -1), 0);+ igt_assert_eq(__check_wait(fd, obj[1].handle, wait), 0); + for (unsigned int n = 0; n < ARRAY_SIZE(fence); n++) { igt_assert_eq(sync_fence_status(fence[n]), -EIO); close(fence[n]); @@ -256,7 +339,7 @@ static void test_inflight_suspend(int fd) obj[1].handle = gem_create(fd, 4096); gem_write(fd, obj[1].handle, 0, &bbe, sizeof(bbe));- hang = __igt_spin_batch_new(fd, 0, 0, 0);+ hang = spin_sync(fd, 0, 0); obj[0].handle = hang->handle;memset(&execbuf, 0, sizeof(execbuf));@@ -273,7 +356,8 @@ static void test_inflight_suspend(int fd) igt_set_autoresume_delay(30); igt_system_suspend_autoresume(SUSPEND_STATE_MEM, SUSPEND_TEST_NONE);- igt_assert_eq(__gem_wait(fd, obj[1].handle, -1), 0);+ igt_assert_eq(__check_wait(fd, obj[1].handle, 10), 0); + for (unsigned int n = 0; n < ARRAY_SIZE(fence); n++) { igt_assert_eq(sync_fence_status(fence[n]), -EIO); close(fence[n]); @@ -301,7 +385,7 @@ static uint32_t context_create_safe(int i915) return param.ctx_id; }-static void test_inflight_contexts(int fd)+static void test_inflight_contexts(int fd, unsigned int wait) { struct drm_i915_gem_exec_object2 obj[2]; const uint32_t bbe = MI_BATCH_BUFFER_END; @@ -330,7 +414,7 @@ static void test_inflight_contexts(int fd) igt_debug("Starting %s on engine '%s'\n", __func__, e__->name); igt_require(i915_reset_control(false));- hang = __igt_spin_batch_new(fd, 0, engine, 0);+ hang = spin_sync(fd, 0, engine); obj[0].handle = hang->handle;memset(&execbuf, 0, sizeof(execbuf));@@ -345,7 +429,8 @@ static void test_inflight_contexts(int fd) igt_assert(fence[n] != -1); }- igt_assert_eq(__gem_wait(fd, obj[1].handle, -1), 0);+ igt_assert_eq(__check_wait(fd, obj[1].handle, wait), 0); + for (unsigned int n = 0; n < ARRAY_SIZE(fence); n++) { igt_assert_eq(sync_fence_status(fence[n]), -EIO); close(fence[n]); @@ -375,7 +460,7 @@ static void test_inflight_external(int fd) fence = igt_cork_plug(&cork, fd);igt_require(i915_reset_control(false));- hang = __igt_spin_batch_new(fd, 0, 0, 0); + hang = __spin_poll(fd, 0, 0);memset(&obj, 0, sizeof(obj));obj.handle = gem_create(fd, 4096); @@ -393,6 +478,9 @@ static void test_inflight_external(int fd) fence = execbuf.rsvd2 >> 32; igt_assert(fence != -1);+ __spin_wait(fd, hang);+ manual_hang(fd); + gem_sync(fd, hang->handle); /* wedged, with an unready batch */ igt_assert(!gem_bo_busy(fd, hang->handle)); igt_assert(gem_bo_busy(fd, obj.handle)); @@ -407,7 +495,7 @@ static void test_inflight_external(int fd) trigger_reset(fd); }-static void test_inflight_internal(int fd)+static void test_inflight_internal(int fd, unsigned int wait) { struct drm_i915_gem_execbuffer2 execbuf; struct drm_i915_gem_exec_object2 obj[2]; @@ -420,7 +508,7 @@ static void test_inflight_internal(int fd) igt_require(gem_has_exec_fence(fd));igt_require(i915_reset_control(false));- hang = __igt_spin_batch_new(fd, 0, 0, 0); + hang = spin_sync(fd, 0, 0);memset(obj, 0, sizeof(obj));obj[0].handle = hang->handle; @@ -441,7 +529,8 @@ static void test_inflight_internal(int fd) nfence++; }- igt_assert_eq(__gem_wait(fd, obj[1].handle, -1), 0);+ igt_assert_eq(__check_wait(fd, obj[1].handle, wait), 0); + while (nfence--) { igt_assert_eq(sync_fence_status(fences[nfence]), -EIO); close(fences[nfence]); @@ -484,29 +573,46 @@ igt_main igt_subtest("execbuf") test_execbuf(fd);- igt_subtest("wait")- test_wait(fd); - igt_subtest("suspend") test_suspend(fd, SUSPEND_STATE_MEM);igt_subtest("hibernate")test_suspend(fd, SUSPEND_STATE_DISK);- igt_subtest("in-flight")- test_inflight(fd); - - igt_subtest("in-flight-contexts") - test_inflight_contexts(fd); - igt_subtest("in-flight-external") test_inflight_external(fd);- igt_subtest("in-flight-internal") {- igt_skip_on(gem_has_semaphores(fd)); - test_inflight_internal(fd); - } - igt_subtest("in-flight-suspend") test_inflight_suspend(fd); + + igt_subtest_group { + const struct { + unsigned int wait; + const char *name; + } waits[] = { + { .wait = 0, .name = "immediate" }, + { .wait = 10, .name = "10us" },i915_request_spin is set to 2us currently :| I guess that's a really hard window to hit reliably. Maybe we should spin for 200ms just to make testing easier!+ { .wait = 10000, .name = "10ms" }, + }; + unsigned int i; + + for (i = 0; i < sizeof(waits) / sizeof(waits[0]); i++) { + igt_subtest_f("wait-%s", waits[i].name) + test_wait(fd, 0, waits[i].wait); + + igt_subtest_f("wait-wedge-%s", waits[i].name) + test_wait(fd, TEST_WEDGE, waits[i].wait);Ok.+ + igt_subtest_f("in-flight-%s", waits[i].name) + test_inflight(fd, waits[i].wait); + + igt_subtest_f("in-flight-contexts-%s", waits[i].name) + test_inflight_contexts(fd, waits[i].wait); + + igt_subtest_f("in-flight-internal-%s", waits[i].name) { + igt_skip_on(gem_has_semaphores(fd)); + test_inflight_internal(fd, waits[i].wait);And ok. -Chris _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
