[Intel-gfx] [PATCH] igt/gem_request_retire: Provoke context destruction with active VMAs
Chris Wilson
chris at chris-wilson.co.uk
Thu Nov 19 08:58:02 PST 2015
On Thu, Nov 19, 2015 at 03:06:05PM +0000, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
>
> Test designed to trigger the
> WARN_ON(!list_empty(&ppgtt->base.active_list))
> in i915_gem_context_clean.
>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
Wouldn't this be a better fit in gem_ctx_somethingorother
gem_ctx/close-active
> ---
> Some potentially hairy batch buffer building code since I don't have
> a lot of experience in that area. :)
> ---
> tests/.gitignore | 1 +
> tests/Makefile.sources | 1 +
> tests/gem_request_retire.c | 279 +++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 281 insertions(+)
> create mode 100644 tests/gem_request_retire.c
>
> diff --git a/tests/.gitignore b/tests/.gitignore
> index 80af9a718f06..85936ea45c9f 100644
> --- a/tests/.gitignore
> +++ b/tests/.gitignore
> @@ -91,6 +91,7 @@ gem_render_copy
> gem_render_copy_redux
> gem_render_linear_blits
> gem_render_tiled_blits
> +gem_request_retire
> gem_reset_stats
> gem_ring_sync_copy
> gem_ring_sync_loop
> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> index 8fb2de8b4665..ff178f7a2df4 100644
> --- a/tests/Makefile.sources
> +++ b/tests/Makefile.sources
> @@ -53,6 +53,7 @@ TESTS_progs_M = \
> gem_reloc_overflow \
> gem_reloc_vs_gpu \
> gem_render_copy_redux \
> + gem_request_retire \
> gem_reset_stats \
> gem_ringfill \
> gem_set_tiling_vs_blt \
> diff --git a/tests/gem_request_retire.c b/tests/gem_request_retire.c
> new file mode 100644
> index 000000000000..710a95f7b6ed
> --- /dev/null
> +++ b/tests/gem_request_retire.c
> @@ -0,0 +1,279 @@
> +/*
> + * Copyright © 2015 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.
> + *
> + * Authors:
> + * Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> + *
> + */
> +
> +/** @file gem_request_retire
> + *
> + * Collection of tests targeting request retirement code paths.
> + */
> +
> +#include "igt.h"
> +#include <stdlib.h>
> +#include <stdio.h>
> +#include <string.h>
> +#include <fcntl.h>
> +#include <inttypes.h>
> +#include <errno.h>
> +#include <sys/stat.h>
> +#include <sys/time.h>
> +#include <sys/mman.h>
> +#include <signal.h>
> +#include <pthread.h>
> +#include <time.h>
> +
> +#include "drm.h"
> +#include "i915_drm.h"
> +
> +#include "intel_bufmgr.h"
> +
> +#define WIDTH 4096
> +#define HEIGHT 4096
> +#define BO_SIZE (WIDTH * HEIGHT * sizeof(uint32_t))
> +
> +static uint32_t
> +blit(int fd, uint32_t dst, uint32_t src, uint32_t ctx_id)
> +{
> + const unsigned int copies = 1000;
> + uint32_t batch[12 * copies + 2];
> + struct drm_i915_gem_relocation_entry reloc[2 * copies];
> + struct drm_i915_gem_exec_object2 obj[3];
> + struct drm_i915_gem_execbuffer2 exec;
> + uint32_t handle;
> + int ret;
> + unsigned int i = 0, j;
> + unsigned int blit_len = 0;
> +
> + for (j = 0; j < copies; j++) {
> + batch[i++] = XY_SRC_COPY_BLT_CMD |
> + XY_SRC_COPY_BLT_WRITE_ALPHA |
> + XY_SRC_COPY_BLT_WRITE_RGB;
> + if (intel_gen(intel_get_drm_devid(fd)) >= 8)
> + batch[i - 1] |= 8;
> + else
> + batch[i - 1] |= 6;
> +
> + batch[i++] = (3 << 24) | /* 32 bits */
> + (0xcc << 16) | /* copy ROP */
> + WIDTH*4;
> + batch[i++] = 0; /* dst x1,y1 */
> + batch[i++] = (HEIGHT << 16) | WIDTH; /* dst x2,y2 */
> + batch[i++] = 0; /* dst reloc */
> + if (intel_gen(intel_get_drm_devid(fd)) >= 8)
> + batch[i++] = 0;
> + batch[i++] = 0; /* src x1,y1 */
> + batch[i++] = WIDTH*4;
> + batch[i++] = 0; /* src reloc */
> + if (intel_gen(intel_get_drm_devid(fd)) >= 8)
> + batch[i++] = 0;
> +
> + while (i % 4)
> + batch[i++] = MI_NOOP;
> +
> + if (blit_len == 0)
> + blit_len = i;
> + }
> +
> + batch[i++] = MI_BATCH_BUFFER_END;
> + batch[i++] = MI_NOOP;
> +
> + handle = gem_create(fd, sizeof(batch));
> + gem_write(fd, handle, 0, batch, sizeof(batch));
> +
> + for (j = 0; j < copies; j++) {
> + unsigned int r0 = j * 2;
> + unsigned int r1 = r0 + 1;
> +
> + reloc[r0].target_handle = dst;
> + reloc[r0].delta = 0;
> + reloc[r0].offset = j * blit_len + 4 * sizeof(uint32_t);
> + reloc[r0].presumed_offset = 0;
> + reloc[r0].read_domains = I915_GEM_DOMAIN_RENDER;
> + reloc[r0].write_domain = I915_GEM_DOMAIN_RENDER;
> +
> + reloc[r1].target_handle = src;
> + reloc[r1].delta = 0;
> + reloc[r1].offset = j * blit_len + 7 * sizeof(uint32_t);
> + if (intel_gen(intel_get_drm_devid(fd)) >= 8)
> + reloc[r1].offset += sizeof(uint32_t);
> + reloc[r1].presumed_offset = 0;
> + reloc[r1].read_domains = I915_GEM_DOMAIN_RENDER;
> + reloc[r1].write_domain = 0;
Just fill the relocation array as you generate the batch.
> + }
> +
> + memset(obj, 0, sizeof(obj));
> + exec.buffer_count = 0;
> + obj[exec.buffer_count++].handle = dst;
> + if (src != dst)
> + obj[exec.buffer_count++].handle = src;
> + obj[exec.buffer_count].handle = handle;
> + obj[exec.buffer_count].relocation_count = 2 * copies;
> + obj[exec.buffer_count].relocs_ptr = (uintptr_t)reloc;
> + exec.buffer_count++;
> + exec.buffers_ptr = (uintptr_t)obj;
> +
memset(&exec, 0, sizeof(exec));
instead of:
> + exec.batch_start_offset = 0;
> + exec.batch_len = i * sizeof(uint32_t);
> + exec.DR1 = exec.DR4 = 0;
> + exec.num_cliprects = 0;
> + exec.cliprects_ptr = 0;
> + exec.flags = I915_EXEC_BLT;
> + i915_execbuffer2_set_context_id(exec, ctx_id);
> + exec.rsvd2 = 0;
> +
> + ret = drmIoctl(fd, DRM_IOCTL_I915_GEM_EXECBUFFER2, &exec);
> + igt_assert_eq(ret, 0);
gem_execbuf(fd, &exec);
> +
> + return handle;
> +}
> +
> +static uint32_t
> +store(int fd, uint32_t dst, uint32_t src, uint32_t ctx_id)
> +{
> + uint32_t batch[6];
> + struct drm_i915_gem_relocation_entry reloc[2];
> + struct drm_i915_gem_exec_object2 obj[3];
> + struct drm_i915_gem_execbuffer2 exec;
> + uint32_t handle;
> + int ret, i = 0;
> +
> + batch[i++] = MI_STORE_DWORD_IMM;
> + batch[i++] = 0; /* addrhigh */
> + batch[i++] = 0; /* addrlow */
> + batch[i++] = 0; /* val */
> + batch[i++] = MI_BATCH_BUFFER_END;
> + batch[i++] = MI_BATCH_BUFFER_END;
> +
> + handle = gem_create(fd, 4096);
> + gem_write(fd, handle, 0, batch, sizeof(batch));
> +
> + reloc[0].target_handle = dst;
> + reloc[0].delta = 0;
> + reloc[0].offset = 2 * sizeof(batch[0]);
> + reloc[0].presumed_offset = 0;
> + reloc[0].read_domains = I915_GEM_DOMAIN_RENDER;
> + reloc[0].write_domain = I915_GEM_DOMAIN_RENDER;
> +
> + reloc[1].target_handle = src;
> + reloc[1].delta = 0;
> + reloc[1].offset = 3 * sizeof(batch[0]);
> + reloc[1].presumed_offset = 0;
> + reloc[1].read_domains = I915_GEM_DOMAIN_RENDER;
> + reloc[1].write_domain = 0;
> +
> + memset(obj, 0, sizeof(obj));
> + exec.buffer_count = 0;
> + obj[exec.buffer_count++].handle = dst;
> + if (src != dst)
> + obj[exec.buffer_count++].handle = src;
> + obj[exec.buffer_count].handle = handle;
> + obj[exec.buffer_count].relocation_count = 2;
> + obj[exec.buffer_count].relocs_ptr = (uintptr_t)reloc;
> + exec.buffer_count++;
> + exec.buffers_ptr = (uintptr_t)obj;
> +
> + exec.batch_start_offset = 0;
> + exec.batch_len = i * sizeof(uint32_t);
> + exec.DR1 = exec.DR4 = 0;
> + exec.num_cliprects = 0;
> + exec.cliprects_ptr = 0;
> + exec.flags = I915_EXEC_RENDER;
> + i915_execbuffer2_set_context_id(exec, ctx_id);
> + exec.rsvd2 = 0;
> +
> + ret = drmIoctl(fd, DRM_IOCTL_I915_GEM_EXECBUFFER2, &exec);
> + igt_assert_eq(ret, 0);
For the purposes of your test, you don't even need to execute anything,
just associate the dst handle with an empty batch on the render ring.
You don't even need a fake relocation since the test already requires a
fairly recent kernel.
> +
> + return handle;
> +}
> +
> +/*
> + * A single bo is operated from batchbuffers submitted from two contexts and on
> + * different rings.
> + * One execbuf finishes way ahead of the other at which point the respective
> + * context is destroyed.
> + */
> +static void
> +test_retire_vma_not_inactive(int fd)
> +{
> + uint32_t ctx_id;
> + uint32_t src, dst, dst2;
> + uint32_t blit_bb, store_bb;
> +
> + igt_require(HAS_BLT_RING(intel_get_drm_devid(fd)));
> +
> + ctx_id = gem_context_create(fd);
> + igt_assert(ctx_id);
> +
> + /* Create some bos batch buffers will operate on. */
> + src = gem_create(fd, BO_SIZE);
> + igt_assert(src);
> + dst = gem_create(fd, BO_SIZE);
> + igt_assert(dst);
> + dst2 = gem_create(fd, 4096);
> + igt_assert(dst2);
> +
> + /* Submit a long running batch. */
> + blit_bb = blit(fd, dst, src, 0);
> + /* Submit a quick batch referencing the same object. */
> + store_bb = store(fd, dst2, src, ctx_id);
> + /* Wait for the quick batch to complete. */
> + gem_sync(fd, store_bb);
> + gem_sync(fd, dst2);
> + gem_close(fd, store_bb);
> + /* Do it again to ensure in kernel retirement is triggered. */
Circumstantial!
> + store_bb = store(fd, dst2, src, ctx_id);
> + gem_sync(fd, store_bb);
> + gem_sync(fd, dst2);
> + gem_close(fd, store_bb);
> + gem_close(fd, dst2);
> +
> + /* Now destroy the context in which the quick batch was submitted. */
> + gem_context_destroy(fd, ctx_id);
Feels very fragile. Isn't the bug something like
batch = big_render_copy(ctx);
context_close(ctx);
gem_sync(batch);
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
More information about the Intel-gfx
mailing list