[Freedreno] [RFC] freedreno: valgrind support

[Rob Clark]

From: Rob Clark <robclark at freedesktop.org>

---
This is mostly an attempt at teaching valgrind about the bo cache pool,
so it would not think that gem objects returned to the bo cache were
leaked.  Unfortunately the list head node in the gem bo is used to
store the bo in the pool.  This is why I also have to disable/enable
error reporting, otherwise valgrind thinks iterating the list is
invalid access.

But DISABLE/ENABLE_ADDR_ERROR_REPORTING_IN_RANGE() is quite chatty,
and I end up with like 90MB worth of:

--6162-- memcheck: modify_ignore_ranges: add 0x7D87590 0x7D875DF
--6162-- memcheck:   now have 27 ranges:
--6162-- memcheck:      [0]  0000000000000000-000000000531f92f  NotIgnored
--6162-- memcheck:      [1]  000000000531f930-000000000531f97f  ClientReq
--6162-- memcheck:      [2]  000000000531f980-000000000538865f  NotIgnored
--6162-- memcheck:      [3]  0000000005388660-00000000053886af  ClientReq
--6162-- memcheck:      [4]  00000000053886b0-00000000053a1ccf  NotIgnored
--6162-- memcheck:      [5]  00000000053a1cd0-00000000053a1d1f  ClientReq
--6162-- memcheck:      [6]  00000000053a1d20-00000000054df5af  NotIgnored
--6162-- memcheck:      [7]  00000000054df5b0-00000000054df5ff  ClientReq
--6162-- memcheck:      [8]  00000000054df600-00000000055807ef  NotIgnored
--6162-- memcheck:      [9]  00000000055807f0-000000000558083f  ClientReq
--6162-- memcheck:      [10]  0000000005580840-0000000006ae611f  NotIgnored
--6162-- memcheck:      [11]  0000000006ae6120-0000000006ae616f  ClientReq
--6162-- memcheck:      [12]  0000000006ae6170-0000000006bb0d0f  NotIgnored
--6162-- memcheck:      [13]  0000000006bb0d10-0000000006bb0d5f  ClientReq
--6162-- memcheck:      [14]  0000000006bb0d60-000000000780350f  NotIgnored
--6162-- memcheck:      [15]  0000000007803510-000000000780355f  ClientReq
--6162-- memcheck:      [16]  0000000007803560-0000000007d8758f  NotIgnored
--6162-- memcheck:      [17]  0000000007d87590-0000000007d875df  ClientReq
--6162-- memcheck:      [18]  0000000007d875e0-0000000007decc5f  NotIgnored
--6162-- memcheck:      [19]  0000000007decc60-0000000007deccaf  ClientReq
--6162-- memcheck:      [20]  0000000007deccb0-000000000820eb7f  NotIgnored
--6162-- memcheck:      [21]  000000000820eb80-000000000820ebcf  ClientReq
--6162-- memcheck:      [22]  000000000820ebd0-00000000095b9c6f  NotIgnored
--6162-- memcheck:      [23]  00000000095b9c70-00000000095b9cbf  ClientReq
--6162-- memcheck:      [24]  00000000095b9cc0-000000000963fdef  NotIgnored
--6162-- memcheck:      [25]  000000000963fdf0-000000000963fe3f  ClientReq
--6162-- memcheck:      [26]  000000000963fe40-ffffffffffffffff  NotIgnored

Not really a valgrind expert, so not sure if there is a better way to
handle this.

I did notice that intel was using valgrind to track the mmap's, and
even track coherency of buffers, which is kinda clever.  I should
probably do at least some of that sometime, but that isn't exactly
what I'm trying to do here.

Note that if the pipe_screen was destroyed, then the fd_device would
be destroyed and the cached bo's freed.  Although that seems not to
be reliable.  In particular, I'm looking at a memory leak in glmark2,
which does destroy/recreate EGL contexts.  But the screen does not
appear to be destoyed.  Something like:

  glmark2 -b :duration=1 --run-forever

will reproduce.  Bo's that end up in the cache make valgrind think
that buffers associated with the previous context have been leaked.

 freedreno/Makefile.am          |  1 +
 freedreno/freedreno_bo_cache.c | 17 +++++++++++++++++
 2 files changed, 18 insertions(+)

diff --git a/freedreno/Makefile.am b/freedreno/Makefile.am
index 0771d14..cbb0d03 100644
--- a/freedreno/Makefile.am
+++ b/freedreno/Makefile.am
@@ -5,6 +5,7 @@ AM_CFLAGS = \
 	$(WARN_CFLAGS) \
 	-I$(top_srcdir) \
 	$(PTHREADSTUBS_CFLAGS) \
+	$(VALGRIND_CFLAGS) \
 	-I$(top_srcdir)/include/drm
 
 libdrm_freedreno_la_LTLIBRARIES = libdrm_freedreno.la
diff --git a/freedreno/freedreno_bo_cache.c b/freedreno/freedreno_bo_cache.c
index 7becb0d..0f8ff10 100644
--- a/freedreno/freedreno_bo_cache.c
+++ b/freedreno/freedreno_bo_cache.c
@@ -33,6 +33,20 @@
 #include "freedreno_drmif.h"
 #include "freedreno_priv.h"
 
+#ifdef HAVE_VALGRIND
+#include <memcheck.h>
+#  define VG_RELEASE(__ptr) do { \
+		VALGRIND_MAKE_MEM_NOACCESS((__ptr), sizeof(*(__ptr))); \
+		VALGRIND_DISABLE_ADDR_ERROR_REPORTING_IN_RANGE((__ptr), sizeof(*(__ptr))); \
+	} while (0);
+#  define VG_OBTAIN(__ptr) do { \
+		VALGRIND_ENABLE_ADDR_ERROR_REPORTING_IN_RANGE((__ptr), sizeof(*(__ptr))); \
+		VALGRIND_MAKE_MEM_DEFINED((__ptr), sizeof(*(__ptr))); \
+	} while (0);
+#else
+#  define VG_RELEASE(__ptr)  do { } while (0)
+#  define VG_OBTAIN(__ptr)   do { } while (0)
+#endif
 
 drm_private void bo_del(struct fd_bo *bo);
 drm_private extern pthread_mutex_t table_lock;
@@ -102,6 +116,7 @@ fd_bo_cache_cleanup(struct fd_bo_cache *cache, time_t time)
 			if (time && ((time - bo->free_time) <= 1))
 				break;
 
+			VG_OBTAIN(bo);
 			list_del(&bo->list);
 			bo_del(bo);
 		}
@@ -177,6 +192,7 @@ retry:
 		*size = bucket->size;
 		bo = find_in_bucket(bucket, flags);
 		if (bo) {
+			VG_OBTAIN(bo);
 			if (bo->funcs->madvise(bo, TRUE) <= 0) {
 				/* we've lost the backing pages, delete and try again: */
 				pthread_mutex_lock(&table_lock);
@@ -207,6 +223,7 @@ fd_bo_cache_free(struct fd_bo_cache *cache, struct fd_bo *bo)
 		clock_gettime(CLOCK_MONOTONIC, &time);
 
 		bo->free_time = time.tv_sec;
+		VG_RELEASE(bo);
 		list_addtail(&bo->list, &bucket->list);
 		fd_bo_cache_cleanup(cache, time.tv_sec);
 
-- 
2.9.3