diff mbox series

[i-g-t] i915: Drop gem_exec_reuse

Message ID 20200221193808.2849185-1-chris@chris-wilson.co.uk (mailing list archive)
State New, archived
Headers show
Series [i-g-t] i915: Drop gem_exec_reuse | expand

Commit Message

Chris Wilson Feb. 21, 2020, 7:38 p.m. UTC 
The test throws a large number of objects at the GPU across many
batches. It only serves to try and assess the scaling impact, without
doing any conformance checking, nor analysing said impact of scaling.
The only effective coverage it gives us is on multi-engine reuse, which
any of the stress tests (gem_exec_parallel, gem_exec_whisper) provide,
and they do validate!

I have no doubt that if we are presented with a concrete problem, or
bug, in this area we will be motivated to write a more precise test
case.

Closes: https://gitlab.freedesktop.org/drm/intel/issues/1289
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Martin Peres <martin.peres@linux.intel.com>
---
 tests/Makefile.sources                 |   3 -
 tests/i915/gem_exec_reuse.c            | 182 -------------------------
 tests/intel-ci/blacklist-pre-merge.txt |  20 ---
 tests/meson.build                      |   1 -
 4 files changed, 206 deletions(-)
 delete mode 100644 tests/i915/gem_exec_reuse.c

Comments

Martin Peres Feb. 24, 2020, 9:21 a.m. UTC | #1 
On 2020-02-21 21:38, Chris Wilson wrote:
> The test throws a large number of objects at the GPU across many
> batches. It only serves to try and assess the scaling impact, without
> doing any conformance checking, nor analysing said impact of scaling.
> The only effective coverage it gives us is on multi-engine reuse, which
> any of the stress tests (gem_exec_parallel, gem_exec_whisper) provide,
> and they do validate!
> 
> I have no doubt that if we are presented with a concrete problem, or
> bug, in this area we will be motivated to write a more precise test
> case.
> 
> Closes: https://gitlab.freedesktop.org/drm/intel/issues/1289
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Martin Peres <martin.peres@linux.intel.com>

I can't review easily that what Chris' statement that
gem_exec_parallel/gem_exec_whisper are testing a superset of
gem_exec_reuse, but I can agree with the logic:

Acked-by: Martin Peres <martin.peres@linux.intel.com>

> ---
>  tests/Makefile.sources                 |   3 -
>  tests/i915/gem_exec_reuse.c            | 182 -------------------------
>  tests/intel-ci/blacklist-pre-merge.txt |  20 ---
>  tests/meson.build                      |   1 -
>  4 files changed, 206 deletions(-)
>  delete mode 100644 tests/i915/gem_exec_reuse.c
> 
> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> index 76cf99da5..10913e86b 100644
> --- a/tests/Makefile.sources
> +++ b/tests/Makefile.sources
> @@ -247,9 +247,6 @@ gen9_exec_parse_SOURCES = i915/gen9_exec_parse.c
>  TESTS_progs += gem_exec_reloc
>  gem_exec_reloc_SOURCES = i915/gem_exec_reloc.c
>  
> -TESTS_progs += gem_exec_reuse
> -gem_exec_reuse_SOURCES = i915/gem_exec_reuse.c
> -
>  TESTS_progs += gem_exec_schedule
>  gem_exec_schedule_SOURCES = i915/gem_exec_schedule.c
>  
> diff --git a/tests/i915/gem_exec_reuse.c b/tests/i915/gem_exec_reuse.c
> deleted file mode 100644
> index 971eb4137..000000000
> --- a/tests/i915/gem_exec_reuse.c
> +++ /dev/null
> @@ -1,182 +0,0 @@
> -/*
> - * Copyright © 2017 Intel Corporation
> - *
> - * Permission is hereby granted, free of charge, to any person obtaining a
> - * copy of this software and associated documentation files (the "Software"),
> - * to deal in the Software without restriction, including without limitation
> - * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> - * and/or sell copies of the Software, and to permit persons to whom the
> - * Software is furnished to do so, subject to the following conditions:
> - *
> - * The above copyright notice and this permission notice (including the next
> - * paragraph) shall be included in all copies or substantial portions of the
> - * Software.
> - *
> - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> - * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> - * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> - * IN THE SOFTWARE.
> - */
> -
> -#include <limits.h>
> -#include <sys/resource.h>
> -
> -#include "igt.h"
> -#include "igt_aux.h"
> -
> -IGT_TEST_DESCRIPTION("Inspect scaling with large number of reused objects");
> -
> -struct noop {
> -	struct drm_i915_gem_exec_object2 *obj;
> -	uint32_t batch;
> -	uint32_t *handles;
> -	unsigned int nhandles;
> -	unsigned int max_age;
> -	int fd;
> -};
> -
> -static void noop(struct noop *n,
> -		 unsigned ring, unsigned ctx,
> -		 unsigned int count, unsigned int offset)
> -{
> -	struct drm_i915_gem_execbuffer2 execbuf;
> -	unsigned int i;
> -
> -	for (i = 0; i < count; i++)
> -		n->obj[i].handle = n->handles[(i + offset) & (n->nhandles-1)];
> -	n->obj[i].handle = n->batch;
> -
> -	memset(&execbuf, 0, sizeof(execbuf));
> -	execbuf.buffers_ptr = to_user_pointer(n->obj);
> -	execbuf.buffer_count = count + 1;
> -	execbuf.flags = ring | 1 << 12;
> -	execbuf.rsvd1 = ctx;
> -	gem_execbuf(n->fd, &execbuf);
> -}
> -
> -static uint64_t max_open_files(void)
> -{
> -	struct rlimit rlim;
> -
> -	if (getrlimit(RLIMIT_NOFILE, &rlim))
> -		rlim.rlim_cur = 64 << 10;
> -
> -	igt_info("Process limit for file descriptors is %lu\n",
> -		 (long)rlim.rlim_cur);
> -	return rlim.rlim_cur;
> -}
> -
> -static unsigned int max_nfd(void)
> -{
> -	uint64_t vfs = vfs_file_max();
> -	uint64_t fd = max_open_files();
> -	uint64_t min = fd < vfs ? fd : vfs;
> -	if (min > INT_MAX)
> -		min = INT_MAX;
> -	return min;
> -}
> -
> -igt_main
> -{
> -	struct noop no;
> -	unsigned engines[16];
> -	unsigned nengine;
> -	unsigned n;
> -
> -	igt_fixture {
> -		uint64_t gtt_size, max;
> -		uint32_t bbe = MI_BATCH_BUFFER_END;
> -
> -		igt_allow_unlimited_files();
> -
> -		no.fd = drm_open_driver(DRIVER_INTEL);
> -		igt_require_gem(no.fd);
> -
> -		igt_fork_hang_detector(no.fd);
> -
> -		gtt_size = (gem_aperture_size(no.fd) / 2) >> 12;
> -		if (gtt_size > INT_MAX / sizeof(*no.handles))
> -			gtt_size = INT_MAX / sizeof(*no.handles);
> -
> -		max = max_nfd() - 16;
> -		if (max < gtt_size)
> -			gtt_size = max;
> -
> -		no.nhandles = 1 << (igt_fls(gtt_size) - 1);
> -		intel_require_memory(no.nhandles, 4096, CHECK_RAM);
> -
> -		no.max_age = no.nhandles / 2;
> -
> -		no.handles = malloc(sizeof(*no.handles) * no.nhandles);
> -		for (n = 0; n < no.nhandles; n++)
> -			no.handles[n] = gem_create(no.fd, 4096);
> -
> -		no.obj = malloc(sizeof(struct drm_i915_gem_exec_object2) * (no.max_age + 1));
> -
> -		nengine = 0;
> -		for_each_physical_engine(e, no.fd)
> -			engines[nengine++] = eb_ring(e);
> -		igt_require(nengine);
> -
> -		no.batch = gem_create(no.fd, 4096);
> -		gem_write(no.fd, no.batch, 0, &bbe, sizeof(bbe));
> -	}
> -
> -	igt_subtest_f("single") {
> -		unsigned int timeout = 5;
> -		unsigned long age = 0;
> -
> -		igt_until_timeout(timeout)
> -			for (n = 0; n < nengine; n++)
> -				noop(&no, engines[n], 0, 0, age++);
> -		gem_sync(no.fd, no.batch);
> -		igt_info("Completed %lu cycles\n", age);
> -	}
> -
> -	igt_subtest_f("baggage") {
> -		unsigned int timeout = 5;
> -		unsigned long age = 0;
> -
> -		igt_until_timeout(timeout)
> -			for (n = 0; n < nengine; n++)
> -				noop(&no, engines[n], 0,
> -				     no.max_age, age++);
> -		gem_sync(no.fd, no.batch);
> -		igt_info("Completed %lu cycles\n", age);
> -	}
> -
> -	igt_subtest_f("contexts") {
> -		unsigned int timeout = 5;
> -		unsigned long ctx_age = 0;
> -		unsigned long obj_age = 0;
> -		const unsigned int ncontexts = 1024;
> -		uint32_t contexts[ncontexts];
> -
> -		gem_require_contexts(no.fd);
> -
> -		for (n = 0; n < ncontexts; n++)
> -			contexts[n] = gem_context_create(no.fd);
> -
> -		igt_until_timeout(timeout) {
> -			for (n = 0; n < nengine; n++) {
> -				noop(&no, engines[n],
> -				     contexts[ctx_age % ncontexts],
> -				     no.max_age, obj_age);
> -				obj_age++;
> -			}
> -			ctx_age++;
> -		}
> -		gem_sync(no.fd, no.batch);
> -		igt_info("Completed %lu cycles across %lu context switches\n",
> -			 obj_age, ctx_age);
> -
> -		for (n = 0; n < ncontexts; n++)
> -			gem_context_destroy(no.fd, contexts[n]);
> -	}
> -
> -	igt_fixture
> -		igt_stop_hang_detector();
> -}
> diff --git a/tests/intel-ci/blacklist-pre-merge.txt b/tests/intel-ci/blacklist-pre-merge.txt
> index be30bdfe2..070f3b32c 100644
> --- a/tests/intel-ci/blacklist-pre-merge.txt
> +++ b/tests/intel-ci/blacklist-pre-merge.txt
> @@ -182,23 +182,3 @@ igt@kms_plane@pixel-format-pipe-[a-d]-planes(-source-clamping)?
>  # Data acquired on 2020-02-20 by Martin Peres
>  ###############################################################################
>  igt@i915_pm_rpm@modeset-stress-extra-wait
> -
> -
> -###############################################################################
> -# These 2 tests are stressing the re-usability of objects. It does not look
> -# like we have had issues with this outside of the gen7 ppgtt issue, which
> -# does not counterbalance its overall execution time.
> -#
> -# - shard-skl: 2% (~5 minutes)
> -# - shard-kbl: 1% (~1.5 minutes)
> -# - shard-apl: 1.7% (~3 minutes)
> -# - shard-glk: 1% (2.5 minutes)
> -# - shard-icl: 0.5% (1 minute)
> -# - shard-tgl: 0.5% (1 minute)
> -#
> -# Issue: https://gitlab.freedesktop.org/drm/intel/issues/1289
> -#
> -# Data acquired on 2020-02-20 by Martin Peres
> -###############################################################################
> -igt@gem_exec_reuse@baggage
> -igt@gem_exec_reuse@contexts
> diff --git a/tests/meson.build b/tests/meson.build
> index 4ba973a5d..dcc5e7b6a 100644
> --- a/tests/meson.build
> +++ b/tests/meson.build
> @@ -148,7 +148,6 @@ i915_progs = [
>  	'gen7_exec_parse',
>  	'gen9_exec_parse',
>  	'gem_exec_reloc',
> -	'gem_exec_reuse',
>  	'gem_exec_schedule',
>  	'gem_exec_store',
>  	'gem_exec_suspend',
>
diff mbox series

Patch

diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index 76cf99da5..10913e86b 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -247,9 +247,6 @@  gen9_exec_parse_SOURCES = i915/gen9_exec_parse.c
 TESTS_progs += gem_exec_reloc
 gem_exec_reloc_SOURCES = i915/gem_exec_reloc.c
 
-TESTS_progs += gem_exec_reuse
-gem_exec_reuse_SOURCES = i915/gem_exec_reuse.c
-
 TESTS_progs += gem_exec_schedule
 gem_exec_schedule_SOURCES = i915/gem_exec_schedule.c
 
diff --git a/tests/i915/gem_exec_reuse.c b/tests/i915/gem_exec_reuse.c
deleted file mode 100644
index 971eb4137..000000000
--- a/tests/i915/gem_exec_reuse.c
+++ /dev/null
@@ -1,182 +0,0 @@ 
-/*
- * Copyright © 2017 Intel Corporation
- *
- * Permission is hereby granted, free of charge, to any person obtaining a
- * copy of this software and associated documentation files (the "Software"),
- * to deal in the Software without restriction, including without limitation
- * the rights to use, copy, modify, merge, publish, distribute, sublicense,
- * and/or sell copies of the Software, and to permit persons to whom the
- * Software is furnished to do so, subject to the following conditions:
- *
- * The above copyright notice and this permission notice (including the next
- * paragraph) shall be included in all copies or substantial portions of the
- * Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
- * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
- * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
- * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
- * IN THE SOFTWARE.
- */
-
-#include <limits.h>
-#include <sys/resource.h>
-
-#include "igt.h"
-#include "igt_aux.h"
-
-IGT_TEST_DESCRIPTION("Inspect scaling with large number of reused objects");
-
-struct noop {
-	struct drm_i915_gem_exec_object2 *obj;
-	uint32_t batch;
-	uint32_t *handles;
-	unsigned int nhandles;
-	unsigned int max_age;
-	int fd;
-};
-
-static void noop(struct noop *n,
-		 unsigned ring, unsigned ctx,
-		 unsigned int count, unsigned int offset)
-{
-	struct drm_i915_gem_execbuffer2 execbuf;
-	unsigned int i;
-
-	for (i = 0; i < count; i++)
-		n->obj[i].handle = n->handles[(i + offset) & (n->nhandles-1)];
-	n->obj[i].handle = n->batch;
-
-	memset(&execbuf, 0, sizeof(execbuf));
-	execbuf.buffers_ptr = to_user_pointer(n->obj);
-	execbuf.buffer_count = count + 1;
-	execbuf.flags = ring | 1 << 12;
-	execbuf.rsvd1 = ctx;
-	gem_execbuf(n->fd, &execbuf);
-}
-
-static uint64_t max_open_files(void)
-{
-	struct rlimit rlim;
-
-	if (getrlimit(RLIMIT_NOFILE, &rlim))
-		rlim.rlim_cur = 64 << 10;
-
-	igt_info("Process limit for file descriptors is %lu\n",
-		 (long)rlim.rlim_cur);
-	return rlim.rlim_cur;
-}
-
-static unsigned int max_nfd(void)
-{
-	uint64_t vfs = vfs_file_max();
-	uint64_t fd = max_open_files();
-	uint64_t min = fd < vfs ? fd : vfs;
-	if (min > INT_MAX)
-		min = INT_MAX;
-	return min;
-}
-
-igt_main
-{
-	struct noop no;
-	unsigned engines[16];
-	unsigned nengine;
-	unsigned n;
-
-	igt_fixture {
-		uint64_t gtt_size, max;
-		uint32_t bbe = MI_BATCH_BUFFER_END;
-
-		igt_allow_unlimited_files();
-
-		no.fd = drm_open_driver(DRIVER_INTEL);
-		igt_require_gem(no.fd);
-
-		igt_fork_hang_detector(no.fd);
-
-		gtt_size = (gem_aperture_size(no.fd) / 2) >> 12;
-		if (gtt_size > INT_MAX / sizeof(*no.handles))
-			gtt_size = INT_MAX / sizeof(*no.handles);
-
-		max = max_nfd() - 16;
-		if (max < gtt_size)
-			gtt_size = max;
-
-		no.nhandles = 1 << (igt_fls(gtt_size) - 1);
-		intel_require_memory(no.nhandles, 4096, CHECK_RAM);
-
-		no.max_age = no.nhandles / 2;
-
-		no.handles = malloc(sizeof(*no.handles) * no.nhandles);
-		for (n = 0; n < no.nhandles; n++)
-			no.handles[n] = gem_create(no.fd, 4096);
-
-		no.obj = malloc(sizeof(struct drm_i915_gem_exec_object2) * (no.max_age + 1));
-
-		nengine = 0;
-		for_each_physical_engine(e, no.fd)
-			engines[nengine++] = eb_ring(e);
-		igt_require(nengine);
-
-		no.batch = gem_create(no.fd, 4096);
-		gem_write(no.fd, no.batch, 0, &bbe, sizeof(bbe));
-	}
-
-	igt_subtest_f("single") {
-		unsigned int timeout = 5;
-		unsigned long age = 0;
-
-		igt_until_timeout(timeout)
-			for (n = 0; n < nengine; n++)
-				noop(&no, engines[n], 0, 0, age++);
-		gem_sync(no.fd, no.batch);
-		igt_info("Completed %lu cycles\n", age);
-	}
-
-	igt_subtest_f("baggage") {
-		unsigned int timeout = 5;
-		unsigned long age = 0;
-
-		igt_until_timeout(timeout)
-			for (n = 0; n < nengine; n++)
-				noop(&no, engines[n], 0,
-				     no.max_age, age++);
-		gem_sync(no.fd, no.batch);
-		igt_info("Completed %lu cycles\n", age);
-	}
-
-	igt_subtest_f("contexts") {
-		unsigned int timeout = 5;
-		unsigned long ctx_age = 0;
-		unsigned long obj_age = 0;
-		const unsigned int ncontexts = 1024;
-		uint32_t contexts[ncontexts];
-
-		gem_require_contexts(no.fd);
-
-		for (n = 0; n < ncontexts; n++)
-			contexts[n] = gem_context_create(no.fd);
-
-		igt_until_timeout(timeout) {
-			for (n = 0; n < nengine; n++) {
-				noop(&no, engines[n],
-				     contexts[ctx_age % ncontexts],
-				     no.max_age, obj_age);
-				obj_age++;
-			}
-			ctx_age++;
-		}
-		gem_sync(no.fd, no.batch);
-		igt_info("Completed %lu cycles across %lu context switches\n",
-			 obj_age, ctx_age);
-
-		for (n = 0; n < ncontexts; n++)
-			gem_context_destroy(no.fd, contexts[n]);
-	}
-
-	igt_fixture
-		igt_stop_hang_detector();
-}
diff --git a/tests/intel-ci/blacklist-pre-merge.txt b/tests/intel-ci/blacklist-pre-merge.txt
index be30bdfe2..070f3b32c 100644
--- a/tests/intel-ci/blacklist-pre-merge.txt
+++ b/tests/intel-ci/blacklist-pre-merge.txt
@@ -182,23 +182,3 @@  igt@kms_plane@pixel-format-pipe-[a-d]-planes(-source-clamping)?
 # Data acquired on 2020-02-20 by Martin Peres
 ###############################################################################
 igt@i915_pm_rpm@modeset-stress-extra-wait
-
-
-###############################################################################
-# These 2 tests are stressing the re-usability of objects. It does not look
-# like we have had issues with this outside of the gen7 ppgtt issue, which
-# does not counterbalance its overall execution time.
-#
-# - shard-skl: 2% (~5 minutes)
-# - shard-kbl: 1% (~1.5 minutes)
-# - shard-apl: 1.7% (~3 minutes)
-# - shard-glk: 1% (2.5 minutes)
-# - shard-icl: 0.5% (1 minute)
-# - shard-tgl: 0.5% (1 minute)
-#
-# Issue: https://gitlab.freedesktop.org/drm/intel/issues/1289
-#
-# Data acquired on 2020-02-20 by Martin Peres
-###############################################################################
-igt@gem_exec_reuse@baggage
-igt@gem_exec_reuse@contexts
diff --git a/tests/meson.build b/tests/meson.build
index 4ba973a5d..dcc5e7b6a 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -148,7 +148,6 @@  i915_progs = [
 	'gen7_exec_parse',
 	'gen9_exec_parse',
 	'gem_exec_reloc',
-	'gem_exec_reuse',
 	'gem_exec_schedule',
 	'gem_exec_store',
 	'gem_exec_suspend',