From 942d6d9940b542202c61e2c4504754feae1fe255 Mon Sep 17 00:00:00 2001 From: Jason Ekstrand Date: Wed, 7 Apr 2021 14:23:49 -0500 Subject: tests/i915/gem_ctx_persistence: Drop the engine replace subtests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We're going to start disallowing non-trivial uses of setparam for engines precisely to make races like this impossible. It'll also make these test cases invalid. Signed-off-by: Jason Ekstrand Reviewed-by: Zbigniew Kempczyński Acked-by: Maarten Lankhorst --- tests/i915/gem_ctx_persistence.c | 210 --------------------------------------- 1 file changed, 210 deletions(-) (limited to 'tests/i915/gem_ctx_persistence.c') diff --git a/tests/i915/gem_ctx_persistence.c b/tests/i915/gem_ctx_persistence.c index 91c30176..59077cc0 100644 --- a/tests/i915/gem_ctx_persistence.c +++ b/tests/i915/gem_ctx_persistence.c @@ -1085,191 +1085,6 @@ static void many_contexts(int i915) gem_quiescent_gpu(i915); } -static void replace_engines(int i915, const struct intel_execution_engine2 *e) -{ - I915_DEFINE_CONTEXT_PARAM_ENGINES(engines, 1) = { - .engines = {{ e->class, e->instance }} - }; - struct drm_i915_gem_context_param param = { - .ctx_id = gem_context_create(i915), - .param = I915_CONTEXT_PARAM_ENGINES, - .value = to_user_pointer(&engines), - .size = sizeof(engines), - }; - igt_spin_t *spin[2]; - int64_t timeout; - - /* - * Suppose the user tries to hide a hanging batch by replacing - * the set of engines on the context so that it's not visible - * at the time of closure? Then we must act when they replace - * the engines! - */ - - gem_context_set_persistence(i915, param.ctx_id, false); - - gem_context_set_param(i915, ¶m); - spin[0] = igt_spin_new(i915, param.ctx_id); - - gem_context_set_param(i915, ¶m); - spin[1] = igt_spin_new(i915, param.ctx_id); - - gem_context_destroy(i915, param.ctx_id); - - timeout = reset_timeout_ms * NSEC_PER_MSEC; - igt_assert_eq(gem_wait(i915, spin[1]->handle, &timeout), 0); - - timeout = reset_timeout_ms * NSEC_PER_MSEC; - igt_assert_eq(gem_wait(i915, spin[0]->handle, &timeout), 0); - - igt_spin_free(i915, spin[1]); - igt_spin_free(i915, spin[0]); - gem_quiescent_gpu(i915); -} - -static void race_set_engines(int i915, int in, int out) -{ - I915_DEFINE_CONTEXT_PARAM_ENGINES(engines, 1) = { - .engines = {} - }; - struct drm_i915_gem_context_param param = { - .param = I915_CONTEXT_PARAM_ENGINES, - .value = to_user_pointer(&engines), - .size = sizeof(engines), - }; - igt_spin_t *spin; - - spin = igt_spin_new(i915); - igt_spin_end(spin); - - while (read(in, ¶m.ctx_id, sizeof(param.ctx_id)) > 0) { - if (!param.ctx_id) - break; - - __gem_context_set_param(i915, ¶m); - - spin->execbuf.rsvd1 = param.ctx_id; - __gem_execbuf(i915, &spin->execbuf); - - write(out, ¶m.ctx_id, sizeof(param.ctx_id)); - } - - igt_spin_free(i915, spin); -} - -static void close_replace_race(int i915) -{ - const int ncpus = sysconf(_SC_NPROCESSORS_ONLN); - int fence = -1; - int out[2], in[2]; - - cleanup(i915); - - /* - * If we time the submission of a hanging batch to one set of engines - * and then simultaneously replace the engines in one thread, and - * close the context in another, it might be possible for the kernel - * to lose track of the old engines believing that the non-persisten - * context is already closed and the hanging requests cancelled. - * - * Our challenge is try and expose any such race condition. - */ - - igt_assert(pipe(out) == 0); - igt_assert(pipe(in) == 0); - igt_fork(child, ncpus) { - close(out[1]); - close(in[0]); - race_set_engines(i915, out[0], in[1]); - } - for (int i = 0; i < ncpus; i++) - close(out[0]); - - igt_until_timeout(5) { - igt_spin_t *spin; - uint32_t ctx; - - ctx = gem_context_clone_with_engines(i915, 0); - gem_context_set_persistence(i915, ctx, false); - - spin = igt_spin_new(i915, ctx, .flags = IGT_SPIN_FENCE_OUT); - for (int i = 0; i < ncpus; i++) - write(out[1], &ctx, sizeof(ctx)); - - gem_context_destroy(i915, ctx); - for (int i = 0; i < ncpus; i++) - read(in[0], &ctx, sizeof(ctx)); - - if (fence < 0) { - fence = spin->out_fence; - } else { - int tmp; - - tmp = sync_fence_merge(fence, spin->out_fence); - close(fence); - close(spin->out_fence); - - fence = tmp; - } - spin->out_fence = -1; - } - close(in[0]); - - for (int i = 0; i < ncpus; i++) { - uint32_t end = 0; - - write(out[1], &end, sizeof(end)); - } - close(out[1]); - - if (sync_fence_wait(fence, MSEC_PER_SEC / 2)) { - igt_debugfs_dump(i915, "i915_engine_info"); - igt_assert(sync_fence_wait(fence, MSEC_PER_SEC / 2) == 0); - } - close(fence); - - igt_waitchildren(); - gem_quiescent_gpu(i915); -} - -static void replace_engines_hostile(int i915, - const struct intel_execution_engine2 *e) -{ - I915_DEFINE_CONTEXT_PARAM_ENGINES(engines, 1) = { - .engines = {{ e->class, e->instance }} - }; - struct drm_i915_gem_context_param param = { - .ctx_id = gem_context_create(i915), - .param = I915_CONTEXT_PARAM_ENGINES, - .value = to_user_pointer(&engines), - .size = sizeof(engines), - }; - int64_t timeout = reset_timeout_ms * NSEC_PER_MSEC; - igt_spin_t *spin; - - /* - * Suppose the user tries to hide a hanging batch by replacing - * the set of engines on the context so that it's not visible - * at the time of closure? Then we must act when they replace - * the engines! - */ - - gem_context_set_persistence(i915, param.ctx_id, false); - - gem_context_set_param(i915, ¶m); - spin = igt_spin_new(i915, param.ctx_id, - .flags = IGT_SPIN_NO_PREEMPTION); - - param.size = 8; - gem_context_set_param(i915, ¶m); - gem_context_destroy(i915, param.ctx_id); - - igt_assert_eq(gem_wait(i915, spin->handle, &timeout), 0); - - igt_spin_free(i915, spin); - gem_quiescent_gpu(i915); -} - static void do_test(void (*test)(int i915, unsigned int engine), int i915, unsigned int engine, const char *name) @@ -1422,31 +1237,6 @@ igt_main smoketest(i915); } - /* Check interactions with set-engines */ - igt_subtest_group { - const struct intel_execution_engine2 *e; - - igt_fixture - gem_require_contexts(i915); - - igt_subtest_with_dynamic("replace") { - __for_each_physical_engine(i915, e) { - igt_dynamic_f("%s", e->name) - replace_engines(i915, e); - } - } - - igt_subtest_with_dynamic("replace-hostile") { - __for_each_physical_engine(i915, e) { - igt_dynamic_f("%s", e->name) - replace_engines_hostile(i915, e); - } - } - - igt_subtest("close-replace-race") - close_replace_race(i915); - } - igt_fixture { close(i915); } -- cgit v1.2.3