On 02/09/2012 11:43 AM, Chris Wilson wrote: > In particular, declare the hidden CPU mmaps to valgrind so that it knows > about those memory regions. > > References: https://bugs.freedesktop.org/show_bug.cgi?id=35071 > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk> Acked-by: Ben Widawsky <ben at bwidawsk.net> > --- > configure.ac | 3 ++ > intel/Makefile.am | 1 + > intel/intel_bufmgr_gem.c | 52 +++++++++++++++++++++++++++++++++------------ > 3 files changed, 42 insertions(+), 14 deletions(-) > > diff --git a/configure.ac b/configure.ac > index 773167f..3ca7b79 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -258,6 +258,9 @@ if test "x$INTEL" != "xno" -o "x$RADEON" != "xno"; then > fi > fi > > +PKG_CHECK_MODULES(VALGRIND, [valgrind], > + AC_DEFINE([HAVE_VALGRIND], 1, [Use valgrind intrinsics to suppress false warings]),) > + > AM_CONDITIONAL(HAVE_INTEL, [test "x$INTEL" != "xno"]) > AM_CONDITIONAL(HAVE_RADEON, [test "x$RADEON" != "xno"]) > if test "x$RADEON" = xyes; then > diff --git a/intel/Makefile.am b/intel/Makefile.am > index 581c8c0..e313614 100644 > --- a/intel/Makefile.am > +++ b/intel/Makefile.am > @@ -28,6 +28,7 @@ AM_CFLAGS = \ > -I$(top_srcdir)/intel \ > $(PTHREADSTUBS_CFLAGS) \ > $(PCIACCESS_CFLAGS) \ > + $(VALGRIND_CFLAGS) \ > -I$(top_srcdir)/include/drm > > libdrm_intel_la_LTLIBRARIES = libdrm_intel.la > diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c > index 2b4fab1..4d30e62 100644 > --- a/intel/intel_bufmgr_gem.c > +++ b/intel/intel_bufmgr_gem.c > @@ -62,6 +62,16 @@ > > #include "i915_drm.h" > > +#ifdef HAVE_VALGRIND > +#include <valgrind.h> > +#include <memcheck.h> > +#define VG(x) x > +#else > +#define VG(x) > +#endif > + > +#define VG_CLEAR(s) VG(memset(&s, 0, sizeof(s))) > + > #define DBG(...) do { \ > if (bufmgr_gem->bufmgr.debug) \ > fprintf(stderr, __VA_ARGS__); \ > @@ -538,7 +548,7 @@ drm_intel_gem_bo_busy(drm_intel_bo *bo) > struct drm_i915_gem_busy busy; > int ret; > > - memset(&busy, 0, sizeof(busy)); > + VG_CLEAR(busy); > busy.handle = bo_gem->gem_handle; > > ret = drmIoctl(bufmgr_gem->fd, DRM_IOCTL_I915_GEM_BUSY, &busy); > @@ -552,6 +562,7 @@ drm_intel_gem_bo_madvise_internal(drm_intel_bufmgr_gem *bufmgr_gem, > { > struct drm_i915_gem_madvise madv; > > + VG_CLEAR(madv); > madv.handle = bo_gem->gem_handle; > madv.madv = state; > madv.retained = 1; > @@ -679,7 +690,8 @@ retry: > return NULL; > > bo_gem->bo.size = bo_size; > - memset(&create, 0, sizeof(create)); > + > + VG_CLEAR(create); > create.size = bo_size; > > ret = drmIoctl(bufmgr_gem->fd, > @@ -835,7 +847,7 @@ drm_intel_bo_gem_create_from_name(drm_intel_bufmgr *bufmgr, > if (!bo_gem) > return NULL; > > - memset(&open_arg, 0, sizeof(open_arg)); > + VG_CLEAR(open_arg); > open_arg.name = handle; > ret = drmIoctl(bufmgr_gem->fd, > DRM_IOCTL_GEM_OPEN, > @@ -858,7 +870,7 @@ drm_intel_bo_gem_create_from_name(drm_intel_bufmgr *bufmgr, > bo_gem->global_name = handle; > bo_gem->reusable = false; > > - memset(&get_tiling, 0, sizeof(get_tiling)); > + VG_CLEAR(get_tiling); > get_tiling.handle = bo_gem->gem_handle; > ret = drmIoctl(bufmgr_gem->fd, > DRM_IOCTL_I915_GEM_GET_TILING, > @@ -889,6 +901,7 @@ drm_intel_gem_bo_free(drm_intel_bo *bo) > > DRMLISTDEL(&bo_gem->vma_list); > if (bo_gem->mem_virtual) { > + VG(VALGRIND_FREELIKE_BLOCK(bo_gem->mem_virtual, 0)); > munmap(bo_gem->mem_virtual, bo_gem->bo.size); > bufmgr_gem->vma_count--; > } > @@ -898,7 +911,7 @@ drm_intel_gem_bo_free(drm_intel_bo *bo) > } > > /* Close this object */ > - memset(&close, 0, sizeof(close)); > + VG_CLEAR(close); > close.handle = bo_gem->gem_handle; > ret = drmIoctl(bufmgr_gem->fd, DRM_IOCTL_GEM_CLOSE, &close); > if (ret != 0) { > @@ -1103,7 +1116,7 @@ static int drm_intel_gem_bo_map(drm_intel_bo *bo, int write_enable) > DBG("bo_map: %d (%s), map_count=%d\n", > bo_gem->gem_handle, bo_gem->name, bo_gem->map_count); > > - memset(&mmap_arg, 0, sizeof(mmap_arg)); > + VG_CLEAR(mmap_arg); > mmap_arg.handle = bo_gem->gem_handle; > mmap_arg.offset = 0; > mmap_arg.size = bo->size; > @@ -1120,12 +1133,14 @@ static int drm_intel_gem_bo_map(drm_intel_bo *bo, int write_enable) > pthread_mutex_unlock(&bufmgr_gem->lock); > return ret; > } > + VG(VALGRIND_MALLOCLIKE_BLOCK(mmap_arg.addr_ptr, mmap_arg.size, 0, 1)); > bo_gem->mem_virtual = (void *)(uintptr_t) mmap_arg.addr_ptr; > } > DBG("bo_map: %d (%s) -> %p\n", bo_gem->gem_handle, bo_gem->name, > bo_gem->mem_virtual); > bo->virtual = bo_gem->mem_virtual; > > + VG_CLEAR(set_domain); > set_domain.handle = bo_gem->gem_handle; > set_domain.read_domains = I915_GEM_DOMAIN_CPU; > if (write_enable) > @@ -1168,7 +1183,7 @@ int drm_intel_gem_bo_map_gtt(drm_intel_bo *bo) > DBG("bo_map_gtt: mmap %d (%s), map_count=%d\n", > bo_gem->gem_handle, bo_gem->name, bo_gem->map_count); > > - memset(&mmap_arg, 0, sizeof(mmap_arg)); > + VG_CLEAR(mmap_arg); > mmap_arg.handle = bo_gem->gem_handle; > > /* Get the fake offset back... */ > @@ -1211,6 +1226,7 @@ int drm_intel_gem_bo_map_gtt(drm_intel_bo *bo) > bo_gem->gtt_virtual); > > /* Now move it to the GTT domain so that the CPU caches are flushed */ > + VG_CLEAR(set_domain); > set_domain.handle = bo_gem->gem_handle; > set_domain.read_domains = I915_GEM_DOMAIN_GTT; > set_domain.write_domain = I915_GEM_DOMAIN_GTT; > @@ -1232,7 +1248,6 @@ static int drm_intel_gem_bo_unmap(drm_intel_bo *bo) > { > drm_intel_bufmgr_gem *bufmgr_gem = (drm_intel_bufmgr_gem *) bo->bufmgr; > drm_intel_bo_gem *bo_gem = (drm_intel_bo_gem *) bo; > - struct drm_i915_gem_sw_finish sw_finish; > int ret = 0; > > if (bo == NULL) > @@ -1250,11 +1265,14 @@ static int drm_intel_gem_bo_unmap(drm_intel_bo *bo) > } > > if (bo_gem->mapped_cpu_write) { > + struct drm_i915_gem_sw_finish sw_finish; > + > /* Cause a flush to happen if the buffer's pinned for > * scanout, so the results show up in a timely manner. > * Unlike GTT set domains, this only does work if the > * buffer should be scanout-related. > */ > + VG_CLEAR(sw_finish); > sw_finish.handle = bo_gem->gem_handle; > ret = drmIoctl(bufmgr_gem->fd, > DRM_IOCTL_I915_GEM_SW_FINISH, > @@ -1291,7 +1309,7 @@ drm_intel_gem_bo_subdata(drm_intel_bo *bo, unsigned long offset, > struct drm_i915_gem_pwrite pwrite; > int ret; > > - memset(&pwrite, 0, sizeof(pwrite)); > + VG_CLEAR(pwrite); > pwrite.handle = bo_gem->gem_handle; > pwrite.offset = offset; > pwrite.size = size; > @@ -1316,6 +1334,7 @@ drm_intel_gem_get_pipe_from_crtc_id(drm_intel_bufmgr *bufmgr, int crtc_id) > struct drm_i915_get_pipe_from_crtc_id get_pipe_from_crtc_id; > int ret; > > + VG_CLEAR(get_pipe_from_crtc_id); > get_pipe_from_crtc_id.crtc_id = crtc_id; > ret = drmIoctl(bufmgr_gem->fd, > DRM_IOCTL_I915_GET_PIPE_FROM_CRTC_ID, > @@ -1342,7 +1361,7 @@ drm_intel_gem_bo_get_subdata(drm_intel_bo *bo, unsigned long offset, > struct drm_i915_gem_pread pread; > int ret; > > - memset(&pread, 0, sizeof(pread)); > + VG_CLEAR(pread); > pread.handle = bo_gem->gem_handle; > pread.offset = offset; > pread.size = size; > @@ -1382,6 +1401,7 @@ drm_intel_gem_bo_start_gtt_access(drm_intel_bo *bo, int write_enable) > struct drm_i915_gem_set_domain set_domain; > int ret; > > + VG_CLEAR(set_domain); > set_domain.handle = bo_gem->gem_handle; > set_domain.read_domains = I915_GEM_DOMAIN_GTT; > set_domain.write_domain = write_enable ? I915_GEM_DOMAIN_GTT : 0; > @@ -1690,6 +1710,7 @@ drm_intel_gem_bo_exec(drm_intel_bo *bo, int used, > */ > drm_intel_add_validate_buffer(bo); > > + VG_CLEAR(execbuf); > execbuf.buffers_ptr = (uintptr_t) bufmgr_gem->exec_objects; > execbuf.buffer_count = bufmgr_gem->exec_count; > execbuf.batch_start_offset = 0; > @@ -1769,6 +1790,7 @@ drm_intel_gem_bo_mrb_exec2(drm_intel_bo *bo, int used, > */ > drm_intel_add_validate_buffer2(bo, 0); > > + VG_CLEAR(execbuf); > execbuf.buffers_ptr = (uintptr_t)bufmgr_gem->exec2_objects; > execbuf.buffer_count = bufmgr_gem->exec_count; > execbuf.batch_start_offset = 0; > @@ -1833,7 +1855,7 @@ drm_intel_gem_bo_pin(drm_intel_bo *bo, uint32_t alignment) > struct drm_i915_gem_pin pin; > int ret; > > - memset(&pin, 0, sizeof(pin)); > + VG_CLEAR(pin); > pin.handle = bo_gem->gem_handle; > pin.alignment = alignment; > > @@ -1855,7 +1877,7 @@ drm_intel_gem_bo_unpin(drm_intel_bo *bo) > struct drm_i915_gem_unpin unpin; > int ret; > > - memset(&unpin, 0, sizeof(unpin)); > + VG_CLEAR(unpin); > unpin.handle = bo_gem->gem_handle; > > ret = drmIoctl(bufmgr_gem->fd, DRM_IOCTL_I915_GEM_UNPIN, &unpin); > @@ -1941,16 +1963,18 @@ drm_intel_gem_bo_flink(drm_intel_bo *bo, uint32_t * name) > { > drm_intel_bufmgr_gem *bufmgr_gem = (drm_intel_bufmgr_gem *) bo->bufmgr; > drm_intel_bo_gem *bo_gem = (drm_intel_bo_gem *) bo; > - struct drm_gem_flink flink; > int ret; > > if (!bo_gem->global_name) { > - memset(&flink, 0, sizeof(flink)); > + struct drm_gem_flink flink; > + > + VG_CLEAR(flink); > flink.handle = bo_gem->gem_handle; > > ret = drmIoctl(bufmgr_gem->fd, DRM_IOCTL_GEM_FLINK, &flink); > if (ret != 0) > return -errno; > + > bo_gem->global_name = flink.name; > bo_gem->reusable = false; >
