[Intel-gfx] [PATCH v2 i-g-t] lib: Add basic support for valgrind annotations.

Maarten Lankhorst maarten.lankhorst at linux.intel.com
Thu Feb 9 10:09:41 UTC 2017


SIGRTMAX appears to be used by valgrind now for its internal tracking,
so avoid it in the helpers.

Also add some valgrind annotations in gem_mmap, to make sure that its
accesses are tracked correctly. I've also added gem_munmap for unmapping
calls done with gem_mmap__*, but there are a lot of places that don't use
it yet. Those tests need to be converted separately.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
---
Changes since v1:
- Add gem_munmap

 configure.ac         |  5 +++++
 lib/Makefile.am      |  1 +
 lib/igt_aux.c        |  2 +-
 lib/igt_draw.c       |  6 +++---
 lib/igt_dummyload.c  |  5 +++--
 lib/igt_fb.c         |  6 +++---
 lib/ioctl_wrappers.c | 25 +++++++++++++++++++++++++
 lib/ioctl_wrappers.h |  2 ++
 8 files changed, 43 insertions(+), 9 deletions(-)

diff --git a/configure.ac b/configure.ac
index 5bdd744a0750..562d5525e894 100644
--- a/configure.ac
+++ b/configure.ac
@@ -124,6 +124,11 @@ PKG_CHECK_MODULES(DRM, [libdrm])
 PKG_CHECK_MODULES(PCIACCESS, [pciaccess >= 0.10])
 PKG_CHECK_MODULES(KMOD, [libkmod])
 PKG_CHECK_MODULES(PROCPS, [libprocps])
+PKG_CHECK_MODULES(VALGRIND, [valgrind], [have_valgrind=yes], [have_valgrind=no])
+
+if test x$have_valgrind = xyes; then
+	AC_DEFINE(HAVE_VALGRIND, 1, [Enable valgrind annotation support.])
+fi
 
 case "$target_cpu" in
 	x86*|i?86)
diff --git a/lib/Makefile.am b/lib/Makefile.am
index 7030ea27dc27..c0ddf2986849 100644
--- a/lib/Makefile.am
+++ b/lib/Makefile.am
@@ -34,6 +34,7 @@ AM_CFLAGS = \
 	    $(XMLRPC_CFLAGS) \
 	    $(LIBUDEV_CFLAGS) \
 	    $(PIXMAN_CFLAGS) \
+	    $(VALGRIND_CFLAGS) \
 	    -DIGT_SRCDIR=\""$(abs_top_srcdir)/tests"\" \
 	    -DIGT_DATADIR=\""$(pkgdatadir)"\" \
 	    -DIGT_LOG_DOMAIN=\""$(subst _,-,$*)"\" \
diff --git a/lib/igt_aux.c b/lib/igt_aux.c
index 763e997c5b83..eb4ca640128a 100644
--- a/lib/igt_aux.c
+++ b/lib/igt_aux.c
@@ -402,7 +402,7 @@ void igt_fork_hang_detector(int fd)
 
 	igt_assert(fstat(fd, &st) == 0);
 
-	signal(SIGRTMAX, sig_abort);
+	signal(SIGRTMAX - 1, sig_abort);
 	igt_fork_helper(&hang_detector)
 		hang_detector_process(getppid(), st.st_rdev);
 }
diff --git a/lib/igt_draw.c b/lib/igt_draw.c
index 3afb8272b06d..29aec85075b9 100644
--- a/lib/igt_draw.c
+++ b/lib/igt_draw.c
@@ -269,7 +269,7 @@ static void draw_rect_mmap_cpu(int fd, struct buf_data *buf, struct rect *rect,
 
 	gem_sw_finish(fd, buf->handle);
 
-	igt_assert(munmap(ptr, buf->size) == 0);
+	igt_assert(gem_munmap(ptr, buf->size) == 0);
 }
 
 static void draw_rect_mmap_gtt(int fd, struct buf_data *buf, struct rect *rect,
@@ -284,7 +284,7 @@ static void draw_rect_mmap_gtt(int fd, struct buf_data *buf, struct rect *rect,
 
 	draw_rect_ptr_linear(ptr, buf->stride, rect, color, buf->bpp);
 
-	igt_assert(munmap(ptr, buf->size) == 0);
+	igt_assert(gem_munmap(ptr, buf->size) == 0);
 }
 
 static void draw_rect_mmap_wc(int fd, struct buf_data *buf, struct rect *rect,
@@ -317,7 +317,7 @@ static void draw_rect_mmap_wc(int fd, struct buf_data *buf, struct rect *rect,
 		break;
 	}
 
-	igt_assert(munmap(ptr, buf->size) == 0);
+	igt_assert(gem_munmap(ptr, buf->size) == 0);
 }
 
 static void draw_rect_pwrite_untiled(int fd, struct buf_data *buf,
diff --git a/lib/igt_dummyload.c b/lib/igt_dummyload.c
index 7ceb370b6fcb..019c1fbcf584 100644
--- a/lib/igt_dummyload.c
+++ b/lib/igt_dummyload.c
@@ -220,7 +220,8 @@ void igt_spin_batch_set_timeout(igt_spin_t *spin, int64_t ns)
 
 	igt_assert(!spin->timer);
 
-	if (spin_signo == SIGRTMAX)
+	/* SIGRTMAX is used by valgrind, SIGRTMAX - 1 by igt_fork_hang_detector */
+	if (spin_signo >= SIGRTMAX - 2)
 		spin_signo = SIGRTMIN;
 	spin->signo = ++spin_signo;
 
@@ -283,7 +284,7 @@ void igt_spin_batch_free(int fd, igt_spin_t *spin)
 		timer_delete(spin->timer);
 
 	igt_spin_batch_end(spin);
-	munmap(spin->batch, BATCH_SIZE);
+	gem_munmap(spin->batch, BATCH_SIZE);
 
 	gem_close(fd, spin->handle);
 	free(spin);
diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index bb87869f18ce..d2b7e9e36606 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -237,7 +237,7 @@ static int create_bo_for_fb(int fd, int width, int height, uint32_t format,
 			/* Ensure the framebuffer is preallocated */
 			ptr = gem_mmap__gtt(fd, bo, size, PROT_READ);
 			igt_assert(*ptr == 0);
-			munmap(ptr, size);
+			gem_munmap(ptr, size);
 
 			if (size_ret)
 				*size_ret = size;
@@ -988,7 +988,7 @@ static void destroy_cairo_surface__blit(void *arg)
 	struct igt_fb *fb = blit->fb;
 	unsigned int obj_tiling = igt_fb_mod_to_tiling(fb->tiling);
 
-	munmap(blit->linear.map, blit->linear.size);
+	gem_munmap(blit->linear.map, blit->linear.size);
 	fb->cairo_surface = NULL;
 
 	gem_set_domain(blit->fd, blit->linear.handle,
@@ -1095,7 +1095,7 @@ static void destroy_cairo_surface__gtt(void *arg)
 {
 	struct igt_fb *fb = arg;
 
-	munmap(cairo_image_surface_get_data(fb->cairo_surface), fb->size);
+	gem_munmap(cairo_image_surface_get_data(fb->cairo_surface), fb->size);
 	fb->cairo_surface = NULL;
 
 	if (fb->is_dumb)
diff --git a/lib/ioctl_wrappers.c b/lib/ioctl_wrappers.c
index cd0c24ba871a..f46b12362376 100644
--- a/lib/ioctl_wrappers.c
+++ b/lib/ioctl_wrappers.c
@@ -56,6 +56,15 @@
 #include "igt_debugfs.h"
 #include "config.h"
 
+#ifdef HAVE_VALGRIND
+#include <valgrind/valgrind.h>
+#include <valgrind/memcheck.h>
+
+#define VG(x) x
+#else
+#define VG(x)
+#endif
+
 #include "ioctl_wrappers.h"
 
 /**
@@ -636,6 +645,8 @@ void *__gem_mmap__gtt(int fd, uint32_t handle, uint64_t size, unsigned prot)
 	else
 		errno = 0;
 
+	VG(VALGRIND_MAKE_MEM_DEFINED(ptr, size));
+
 	return ptr;
 }
 
@@ -657,6 +668,16 @@ void *gem_mmap__gtt(int fd, uint32_t handle, uint64_t size, unsigned prot)
 	return ptr;
 }
 
+int gem_munmap(void *ptr, uint64_t size)
+{
+	int ret = munmap(ptr, size);
+
+	if (ret == 0)
+		VG(VALGRIND_MAKE_MEM_NOACCESS(ptr, size));
+
+	return ret;
+}
+
 struct local_i915_gem_mmap_v2 {
 	uint32_t handle;
 	uint32_t pad;
@@ -734,6 +755,8 @@ void *__gem_mmap__wc(int fd, uint32_t handle, uint64_t offset, uint64_t size, un
 	if (igt_ioctl(fd, LOCAL_IOCTL_I915_GEM_MMAP_v2, &arg))
 		return NULL;
 
+	VG(VALGRIND_MAKE_MEM_DEFINED(from_user_pointer(arg.addr_ptr), arg.size));
+
 	errno = 0;
 	return from_user_pointer(arg.addr_ptr);
 }
@@ -781,6 +804,8 @@ void *__gem_mmap__cpu(int fd, uint32_t handle, uint64_t offset, uint64_t size, u
 	if (igt_ioctl(fd, DRM_IOCTL_I915_GEM_MMAP, &mmap_arg))
 		return NULL;
 
+	VG(VALGRIND_MAKE_MEM_DEFINED(from_user_pointer(mmap_arg.addr_ptr), mmap_arg.size));
+
 	errno = 0;
 	return from_user_pointer(mmap_arg.addr_ptr);
 }
diff --git a/lib/ioctl_wrappers.h b/lib/ioctl_wrappers.h
index 64628df7e97f..ee1dca9709ba 100644
--- a/lib/ioctl_wrappers.h
+++ b/lib/ioctl_wrappers.h
@@ -86,6 +86,8 @@ void *__gem_mmap__gtt(int fd, uint32_t handle, uint64_t size, unsigned prot);
 void *__gem_mmap__cpu(int fd, uint32_t handle, uint64_t offset, uint64_t size, unsigned prot);
 void *__gem_mmap__wc(int fd, uint32_t handle, uint64_t offset, uint64_t size, unsigned prot);
 
+int gem_munmap(void *ptr, uint64_t size);
+
 /**
  * gem_require_stolen_support:
  * @fd: open i915 drm file descriptor
-- 
2.7.4




More information about the Intel-gfx mailing list