On 4/23/2015 10:43 AM, Chris Wilson wrote:
On Thu, Apr 23, 2015 at 10:30:01AM +0100, daniele.ceraolospurio@xxxxxxxxx wrote:
From: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx>
From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
Using imported objects should not leak i915 vmas (and vms).
In practice this simulates Xorg importing fbcon and leaking (or not) one vma
per Xorg startup cycle.
v2: use low-level ioctl wrappers and bo offset to check the leak (Chris)
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx> (v2)
Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
---
tests/gem_ppgtt.c | 99 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 99 insertions(+)
diff --git a/tests/gem_ppgtt.c b/tests/gem_ppgtt.c
index 5bf773c..b865af3 100644
--- a/tests/gem_ppgtt.c
+++ b/tests/gem_ppgtt.c
@@ -48,6 +48,22 @@
#define HEIGHT 512
#define SIZE (HEIGHT*STRIDE)
+static bool uses_full_ppgtt(int fd)
+{
+ struct drm_i915_getparam gp;
+ int val = 0;
+
+ memset(&gp, 0, sizeof(gp));
+ gp.param = 18; /* HAS_ALIASING_PPGTT */
+ gp.value = &val;
+
+ if (drmIoctl(fd, DRM_IOCTL_I915_GETPARAM, &gp))
+ return 0;
+
+ errno = 0;
+ return val > 1;
+}
+
static drm_intel_bo *create_bo(drm_intel_bufmgr *bufmgr,
uint32_t pixel)
{
@@ -200,6 +216,86 @@ static void surfaces_check(dri_bo **bo, int count, uint32_t expected)
}
}
+
+static uint64_t exec_and_get_offset(int fd, uint32_t batch, uint32_t bo)
+{
+ struct drm_i915_gem_execbuffer2 execbuf;
+ struct drm_i915_gem_exec_object2 exec[2];
+ struct drm_i915_gem_relocation_entry reloc[1];
+ uint32_t buf[6], i = 0;
+
+ /* use a simple MI_STORE_DWORD_IMM to write something on the bo.
+ * We just want to get a VMA
+ */
+ buf[i++] = MI_STORE_DWORD_IMM | 2;
+ buf[i++] = 0;
+ buf[i++] = 0;
+ buf[i++] = 0xdeadbeef;
+
+ reloc->offset = 1 * sizeof(uint32_t);
+ reloc->delta = 0;
+ reloc->target_handle = bo;
+ reloc->read_domains = I915_GEM_DOMAIN_INSTRUCTION;
+ reloc->write_domain = I915_GEM_DOMAIN_INSTRUCTION;
+ reloc->presumed_offset = 0;
A relocation (and the STORE) is not required. The kernel will do all
reservations (of exec_object[]) before doing relocations. Not relocating
makes life more predictable (fewer error conditions may strike).
+static void flink_and_close(void)
+{
+ uint32_t fd, fd2;
+ uint32_t batch, bo, flinked_bo, new_bo, name;
+ uint64_t offset, offset_new;
+
+ fd = drm_open_any();
+ igt_require(uses_full_ppgtt(fd));
The test equally applies to !full-ppgtt. The bug we saw isn't possible,
but the interface expectations are the same.
I've tried the test with aliasing ppgtt, but the flinked buffer gets the
same offset as the original one, so the new_bo will get a different
offset indipendently from the vma leak and the assert will always fail.
Are there any other ways to check the vma leak in !full-ppgtt mode?
Thanks,
Daniele
+ bo = gem_create(fd, 4096);
+ name = gem_flink(fd, bo);
+
+ fd2 = drm_open_any();
+ batch = gem_create(fd2, 4096);
+
+ flinked_bo = gem_open(fd2, name);
+ offset = exec_and_get_offset(fd2, batch, flinked_bo);
+ gem_close(fd2, flinked_bo);
+ gem_sync(fd2, batch);
+
+ /* the flinked bo VMA should have been cleared now, so a new bo of the
+ * same size should get the same offset
+ */
+ new_bo = gem_create(fd2, 4096);
+ offset_new = exec_and_get_offset(fd2, batch, new_bo);
+ gem_close(fd2, new_bo);
+
+ igt_assert(offset == offset_new);
igt_assert_eq
Nice test.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
