[Intel-gfx] [PATCH igt] gem_concurrent_blit: Don't call igt_require() outside of a subtest/fixture
Daniel Vetter
daniel at ffwll.ch
Mon Jan 11 00:00:13 PST 2016
On Fri, Jan 08, 2016 at 09:10:38AM +0000, Chris Wilson wrote:
> gem_concurrent_blit tries to ensure that it doesn't try and run a test
> that would grind the system to a halt, i.e. unexpectedly cause swap
> thrashing. It currently calls intel_require_memory(), but outside of
> the subtest (as the tests use fork, it cannot do requirement testing
> within the test children) - but intel_require_memory() calls
> igt_require() and triggers and abort. Wrapping that initial require
> within an igt_fixture() stops the abort(), but also prevents any further
> testing.
>
> This patch restructures the requirement checking to ordinary conditions,
> which though allowing the test to run, also prevents listing of subtests
> on machines which cannot handle them.
> ---
> lib/igt_aux.h | 2 ++
> lib/intel_os.c | 53 +++++++++++++++++++++++-------------
> tests/gem_concurrent_all.c | 67 +++++++++++++++++++++++++++++++++-------------
> 3 files changed, 85 insertions(+), 37 deletions(-)
>
> diff --git a/lib/igt_aux.h b/lib/igt_aux.h
> index 6e11ee6..5a88c2a 100644
> --- a/lib/igt_aux.h
> +++ b/lib/igt_aux.h
> @@ -88,6 +88,8 @@ uint64_t intel_get_total_swap_mb(void);
>
> #define CHECK_RAM 0x1
> #define CHECK_SWAP 0x2
> +int __intel_check_memory(uint32_t count, uint64_t size, unsigned mode,
> + uint64_t *out_required, uint64_t *out_total);
> void intel_require_memory(uint32_t count, uint64_t size, unsigned mode);
> int intel_num_objects_for_memory(uint32_t size, unsigned mode);
>
> diff --git a/lib/intel_os.c b/lib/intel_os.c
> index dba9e17..90f9bb3 100644
> --- a/lib/intel_os.c
> +++ b/lib/intel_os.c
> @@ -192,6 +192,38 @@ intel_get_total_swap_mb(void)
> return retval / (1024*1024);
> }
>
Please add the usual gtkdoc boilerplate here with a mention of
intel_check_memory. Ack with that.
-Daniel
> +int __intel_check_memory(uint32_t count, uint64_t size, unsigned mode,
> + uint64_t *out_required, uint64_t *out_total)
> +{
> +/* rough estimate of how many bytes the kernel requires to track each object */
> +#define KERNEL_BO_OVERHEAD 512
> + uint64_t required, total;
> +
> + required = count;
> + required *= size + KERNEL_BO_OVERHEAD;
> + required = ALIGN(required, 4096);
> +
> + igt_debug("Checking %'u surfaces of size %'llu bytes (total %'llu) against %s%s\n",
> + count, (long long)size, (long long)required,
> + mode & (CHECK_RAM | CHECK_SWAP) ? "RAM" : "",
> + mode & CHECK_SWAP ? " + swap": "");
> +
> + total = 0;
> + if (mode & (CHECK_RAM | CHECK_SWAP))
> + total += intel_get_avail_ram_mb();
> + if (mode & CHECK_SWAP)
> + total += intel_get_total_swap_mb();
> + total *= 1024 * 1024;
> +
> + if (out_required)
> + *out_required = required;
> +
> + if (out_total)
> + *out_total = total;
> +
> + return required < total;
> +}
> +
> /**
> * intel_require_memory:
> * @count: number of surfaces that will be created
> @@ -217,27 +249,10 @@ intel_get_total_swap_mb(void)
> */
> void intel_require_memory(uint32_t count, uint64_t size, unsigned mode)
> {
> -/* rough estimate of how many bytes the kernel requires to track each object */
> -#define KERNEL_BO_OVERHEAD 512
> uint64_t required, total;
>
> - required = count;
> - required *= size + KERNEL_BO_OVERHEAD;
> - required = ALIGN(required, 4096);
> -
> - igt_debug("Checking %'u surfaces of size %'llu bytes (total %'llu) against %s%s\n",
> - count, (long long)size, (long long)required,
> - mode & (CHECK_RAM | CHECK_SWAP) ? "RAM" : "",
> - mode & CHECK_SWAP ? " + swap": "");
> -
> - total = 0;
> - if (mode & (CHECK_RAM | CHECK_SWAP))
> - total += intel_get_avail_ram_mb();
> - if (mode & CHECK_SWAP)
> - total += intel_get_total_swap_mb();
> - total *= 1024 * 1024;
> -
> - igt_skip_on_f(total <= required,
> + igt_skip_on_f(!__intel_check_memory(count, size, mode,
> + &required, &total),
> "Estimated that we need %'llu bytes for the test, but only have %'llu bytes available (%s%s)\n",
> (long long)required, (long long)total,
> mode & (CHECK_RAM | CHECK_SWAP) ? "RAM" : "",
> diff --git a/tests/gem_concurrent_all.c b/tests/gem_concurrent_all.c
> index 0e873c4..9a2fb6d 100644
> --- a/tests/gem_concurrent_all.c
> +++ b/tests/gem_concurrent_all.c
> @@ -155,9 +155,9 @@ static bool can_create_stolen(void)
> static drm_intel_bo *
> (*create_func)(drm_intel_bufmgr *bufmgr, uint64_t size);
>
> -static void create_cpu_require(void)
> +static bool create_cpu_require(void)
> {
> - igt_require(create_func != create_stolen_bo);
> + return create_func != create_stolen_bo;
> }
>
> static drm_intel_bo *
> @@ -375,7 +375,7 @@ gpu_cmp_bo(drm_intel_bo *bo, uint32_t val, int width, int height, drm_intel_bo *
>
> const struct access_mode {
> const char *name;
> - void (*require)(void);
> + bool (*require)(void);
> void (*set_bo)(drm_intel_bo *bo, uint32_t val, int w, int h);
> void (*cmp_bo)(drm_intel_bo *bo, uint32_t val, int w, int h, drm_intel_bo *tmp);
> drm_intel_bo *(*create_bo)(drm_intel_bufmgr *bufmgr, int width, int height);
> @@ -1294,23 +1294,22 @@ run_basic_modes(const char *prefix,
> }
>
> static void
> -run_modes(const char *style, const struct access_mode *mode)
> +run_modes(const char *style, const struct access_mode *mode, unsigned allow_mem)
> {
> - if (mode->require)
> - mode->require();
> + if (mode->require && !mode->require())
> + return;
>
> - igt_debug("%s: using 2x%d buffers, each 1MiB\n", style, num_buffers);
> - intel_require_memory(2*num_buffers, 1024*1024, CHECK_RAM);
> + igt_debug("%s: using 2x%d buffers, each 1MiB\n",
> + style, num_buffers);
> + if (!__intel_check_memory(2*num_buffers, 1024*1024, allow_mem,
> + NULL, NULL))
> + return;
>
> - if (all) {
> - run_basic_modes(style, mode, "", run_single);
> -
> - igt_fork_signal_helper();
> - run_basic_modes(style, mode, "-interruptible", run_interruptible);
> - igt_stop_signal_helper();
> - }
> + run_basic_modes(style, mode, "", run_single);
>
> igt_fork_signal_helper();
> + run_basic_modes(style, mode, "-interruptible",
> + run_interruptible);
> run_basic_modes(style, mode, "-forked", run_forked);
> run_basic_modes(style, mode, "-bomb", run_bomb);
> igt_stop_signal_helper();
> @@ -1328,6 +1327,8 @@ igt_main
> { "stolen-", create_stolen_bo, can_create_stolen },
> { NULL, NULL }
> }, *c;
> + void *pinned;
> + uint64_t pin_sz;
> int i;
>
> igt_skip_on_simulation();
> @@ -1354,7 +1355,7 @@ igt_main
> if (c->require()) {
> snprintf(name, sizeof(name), "%s%s", c->name, "small");
> for (i = 0; i < ARRAY_SIZE(access_modes); i++)
> - run_modes(name, &access_modes[i]);
> + run_modes(name, &access_modes[i], CHECK_RAM);
> }
>
> igt_fixture {
> @@ -1364,7 +1365,7 @@ igt_main
> if (c->require()) {
> snprintf(name, sizeof(name), "%s%s", c->name, "thrash");
> for (i = 0; i < ARRAY_SIZE(access_modes); i++)
> - run_modes(name, &access_modes[i]);
> + run_modes(name, &access_modes[i], CHECK_RAM);
> }
>
> igt_fixture {
> @@ -1374,7 +1375,37 @@ igt_main
> if (c->require()) {
> snprintf(name, sizeof(name), "%s%s", c->name, "full");
> for (i = 0; i < ARRAY_SIZE(access_modes); i++)
> - run_modes(name, &access_modes[i]);
> + run_modes(name, &access_modes[i], CHECK_RAM);
> + }
> +
> + igt_fixture {
> + num_buffers = gem_mappable_aperture_size() / (1024 * 1024);
> + pin_sz = intel_get_avail_ram_mb() - num_buffers;
> +
> + igt_debug("Pinning %ld MiB\n", pin_sz);
> + pin_sz *= 1024 * 1024;
> +
> + if (posix_memalign(&pinned, 4096, pin_sz) ||
> + mlock(pinned, pin_sz) ||
> + madvise(pinned, pin_sz, MADV_DONTFORK)) {
> + free(pinned);
> + pinned = NULL;
> + }
> + igt_require(pinned);
> + }
> +
> + if (c->require()) {
> + snprintf(name, sizeof(name), "%s%s", c->name, "swap");
> + for (i = 0; i < ARRAY_SIZE(access_modes); i++)
> + run_modes(name, &access_modes[i], CHECK_RAM | CHECK_SWAP);
> + }
> +
> + igt_fixture {
> + if (pinned) {
> + munlock(pinned, pin_sz);
> + free(pinned);
> + pinned = NULL;
> + }
> }
> }
> }
> --
> 2.7.0.rc3
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
More information about the Intel-gfx
mailing list