On Wed, Mar 22, 2017 at 7:54 AM, Rob Clark <robdcl...@gmail.com> wrote:
> From: Rob Clark <robcl...@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/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
>
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Reply via email to