On Wed, Mar 22, 2017 at 7:54 AM, Rob Clark robdclark@gmail.com wrote:
From: Rob Clark robclark@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
heh, well just dropping '-v' arg to valgrind drops the extended spam that enable/disable error checking triggers.
It does seem like maybe I could move the list node to the head of the block and pretend this is a custom allocator (VALGRIND_MEMPOOL_*), but that also seems like a bit of a hack..
BR, -R
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/drmlibdrm_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