dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [RFC] freedreno: valgrind support
@ 2017-03-22 11:54 Rob Clark
       [not found] ` <20170322115434.28769-1-robdclark-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  0 siblings, 1 reply; 2+ messages in thread
From: Rob Clark @ 2017-03-22 11:54 UTC (permalink / raw)
  To: freedreno; +Cc: Kristian Høgsberg, Rob Clark, dri-devel

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

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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 2+ messages in thread
* Re: [RFC] freedreno: valgrind support
       [not found] ` <20170322115434.28769-1-robdclark-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2017-03-22 15:24   ` Rob Clark
  0 siblings, 0 replies; 2+ messages in thread
From: Rob Clark @ 2017-03-22 15:24 UTC (permalink / raw)
  To: open list:DRM DRIVER FOR MSM ADRENO GPU
  Cc: Kristian Høgsberg, Rob Clark,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org,
	Chris Wilson

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/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

^ permalink raw reply	[flat|nested] 2+ messages in thread
end of thread, other threads:[~2017-03-22 15:24 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-03-22 11:54 [RFC] freedreno: valgrind support Rob Clark
     [not found] ` <20170322115434.28769-1-robdclark-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2017-03-22 15:24   ` Rob Clark

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).