On Wed, Mar 22, 2017 at 7:54 AM, Rob Clark <robdclark@xxxxxxxxx> wrote:
> From: Rob Clark <robclark@xxxxxxxxxxxxxxx>
>
> ---
> 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/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
>
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
