| Message ID | 1475182076-5411-3-git-send-email-jgunthorpe@obsidianresearch.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
On 09/29/2016 01:47 PM, Jason Gunthorpe wrote: > The cmake build system guarantees that valgrind/memcheck.h is present. > If the system does not have it, or valgrind is disabled, then it is > replaced with a dummy header full of empty stubs. Independent of this patch: Valgrind has a stable ABI so I propose to add a copy of the memcheck.h header file to the rdma-core source tree. Many other open source projects already do this. Bart. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Sep 29, 2016 at 02:01:26PM -0700, Bart Van Assche wrote: > Independent of this patch: Valgrind has a stable ABI so I propose to add a > copy of the memcheck.h header file to the rdma-core source tree. Many other > open source projects already do this. Interesting idea, what do you see as the benefit? FWIW, I haven't had any problems getting the valgrind headers on any supported distro. I think I'd rather make the absence of valgrind a hard error (right now cmake makes a warning in the summary) than worry about another copied header... Jason -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 09/29/2016 03:34 PM, Jason Gunthorpe wrote: > On Thu, Sep 29, 2016 at 02:01:26PM -0700, Bart Van Assche wrote: >> Independent of this patch: Valgrind has a stable ABI so I propose to add a >> copy of the memcheck.h header file to the rdma-core source tree. Many other >> open source projects already do this. > > Interesting idea, what do you see as the benefit? Hello Jason, That would allow to annotate rdma-core code correctly for Valgrind without introducing a dependency on the valgrind-devel package. As you know most Linux distro's by default do not install the valgrind-devel package. Bart. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Sep 29, 2016 at 04:48:25PM -0700, Bart Van Assche wrote: > That would allow to annotate rdma-core code correctly for Valgrind without > introducing a dependency on the valgrind-devel package. Sure, but we are recommending distros build their official packages with valgrind, the sample packaging I built includes valgrind as a dependency, the README.md instructs to install valgrind, and cmake warns if valgrind is not present. I would never encourage a distro to build official packages using a built-in valgrind header, that is a great way to accidently break a new or obscure architecture. > As you know most Linux distro's by default do not install the > valgrind-devel package. Are you worried about users? Like I said, we could make valgrind hard required. It is not hard to install valgrind, and everyone already needs to install other non-default things like libnl3-devel and cmake. Jason -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 09/29/2016 01:47 PM, Jason Gunthorpe wrote: > The cmake build system guarantees that valgrind/memcheck.h is present. > If the system does not have it, or valgrind is disabled, then it is > replaced with a dummy header full of empty stubs. > > Thus all the copy&paste boiler plate is consolidated into buildlib. > > Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> > --- > buildlib/config.h.in | 4 ---- > libibcm/src/cm.c | 11 +---------- > libibumad/src/umad.c | 14 +------------- > libibverbs/src/ibverbs.h | 14 +------------- > libmlx4/src/mlx4.h | 18 +----------------- > libmlx5/src/mlx5.h | 18 +----------------- > libmthca/src/mthca.h | 18 +----------------- > librdmacm/src/cma.h | 11 +---------- > srp_daemon/srp_daemon/srp_daemon.h | 5 ----- > 9 files changed, 7 insertions(+), 106 deletions(-) For the srp_daemon changes: Reviewed-by: Bart Van Assche <bart.vanassche@sandisk.com> -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 9/29/2016 4:47 PM, Jason Gunthorpe wrote: > libibumad/src/umad.c | 14 +------------- For libibumad bit, Acked-by: Hal Rosenstock <hal@mellanox.com> -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/buildlib/config.h.in b/buildlib/config.h.in index 0a87457e6d7a..a02be9cfb0f7 100644 --- a/buildlib/config.h.in +++ b/buildlib/config.h.in @@ -21,10 +21,6 @@ // FIXME: Remove this, The cmake version hard-requires new style CLOEXEC support #define STREAM_CLOEXEC "e" -// FIXME: Remove this, cmake always provides a valgrind/memcheck.h -#define HAVE_VALGRIND_MEMCHECK_H 1 -#define INCLUDE_VALGRIND 1 - #define IBV_CONFIG_DIR "@CONFIG_DIR@" #define RS_CONF_DIR "@CMAKE_INSTALL_FULL_SYSCONFDIR@/rdma/rsocket" #define IWPM_CONFIG_FILE "@CMAKE_INSTALL_FULL_SYSCONFDIR@/iwpmd.conf" diff --git a/libibcm/src/cm.c b/libibcm/src/cm.c index ebe36bef89e6..f775923aa73c 100644 --- a/libibcm/src/cm.c +++ b/libibcm/src/cm.c @@ -49,16 +49,7 @@ #include <infiniband/driver.h> #include <infiniband/marshall.h> -#ifdef INCLUDE_VALGRIND -# include <valgrind/memcheck.h> -# ifndef VALGRIND_MAKE_MEM_DEFINED -# warning "Valgrind requested, but VALGRIND_MAKE_MEM_DEFINED undefined" -# endif -#endif - -#ifndef VALGRIND_MAKE_MEM_DEFINED -# define VALGRIND_MAKE_MEM_DEFINED(addr,len) -#endif +#include <valgrind/memcheck.h> #define PFX "libibcm: " diff --git a/libibumad/src/umad.c b/libibumad/src/umad.c index 176d112e125b..0c969f1eb8ad 100644 --- a/libibumad/src/umad.c +++ b/libibumad/src/umad.c @@ -52,19 +52,7 @@ #define IB_OPENIB_OUI (0x001405) -#ifdef HAVE_VALGRIND_MEMCHECK_H - -# include <valgrind/memcheck.h> - -# ifndef VALGRIND_MAKE_MEM_DEFINED -# warning "Valgrind support requested, but VALGRIND_MAKE_MEM_DEFINED not available" -# endif - -#endif /* HAVE_VALGRIND_MEMCHECK_H */ - -#ifndef VALGRIND_MAKE_MEM_DEFINED -# define VALGRIND_MAKE_MEM_DEFINED(addr,len) -#endif +#include <valgrind/memcheck.h> typedef struct ib_user_mad_reg_req { uint32_t id; diff --git a/libibverbs/src/ibverbs.h b/libibverbs/src/ibverbs.h index 062a490915b0..7892af4ad247 100644 --- a/libibverbs/src/ibverbs.h +++ b/libibverbs/src/ibverbs.h @@ -38,19 +38,7 @@ #include <infiniband/driver.h> -#ifdef HAVE_VALGRIND_MEMCHECK_H - -# include <valgrind/memcheck.h> - -# ifndef VALGRIND_MAKE_MEM_DEFINED -# warning "Valgrind support requested, but VALGRIND_MAKE_MEM_DEFINED not available" -# endif - -#endif /* HAVE_VALGRIND_MEMCHECK_H */ - -#ifndef VALGRIND_MAKE_MEM_DEFINED -# define VALGRIND_MAKE_MEM_DEFINED(addr, len) 0 -#endif +#include <valgrind/memcheck.h> #define HIDDEN __attribute__((visibility ("hidden"))) diff --git a/libmlx4/src/mlx4.h b/libmlx4/src/mlx4.h index feea3386f9a7..4551141d2007 100644 --- a/libmlx4/src/mlx4.h +++ b/libmlx4/src/mlx4.h @@ -47,23 +47,7 @@ #define uninitialized_var(x) x = x #endif -#ifdef HAVE_VALGRIND_MEMCHECK_H - -# include <valgrind/memcheck.h> - -# if !defined(VALGRIND_MAKE_MEM_DEFINED) || !defined(VALGRIND_MAKE_MEM_UNDEFINED) -# warning "Valgrind support requested, but VALGRIND_MAKE_MEM_(UN)DEFINED not available" -# endif - -#endif /* HAVE_VALGRIND_MEMCHECK_H */ - -#ifndef VALGRIND_MAKE_MEM_DEFINED -# define VALGRIND_MAKE_MEM_DEFINED(addr,len) -#endif - -#ifndef VALGRIND_MAKE_MEM_UNDEFINED -# define VALGRIND_MAKE_MEM_UNDEFINED(addr,len) -#endif +#include <valgrind/memcheck.h> #ifndef rmb # define rmb() mb() diff --git a/libmlx5/src/mlx5.h b/libmlx5/src/mlx5.h index 6c4e83002028..aa8b6feceb4e 100644 --- a/libmlx5/src/mlx5.h +++ b/libmlx5/src/mlx5.h @@ -52,23 +52,7 @@ #define uninitialized_var(x) x = x #endif -#ifdef HAVE_VALGRIND_MEMCHECK_H - -# include <valgrind/memcheck.h> - -# if !defined(VALGRIND_MAKE_MEM_DEFINED) || !defined(VALGRIND_MAKE_MEM_UNDEFINED) -# warning "Valgrind support requested, but VALGRIND_MAKE_MEM_(UN)DEFINED not available" -# endif - -#endif /* HAVE_VALGRIND_MEMCHECK_H */ - -#ifndef VALGRIND_MAKE_MEM_DEFINED -# define VALGRIND_MAKE_MEM_DEFINED(addr, len) -#endif - -#ifndef VALGRIND_MAKE_MEM_UNDEFINED -# define VALGRIND_MAKE_MEM_UNDEFINED(addr, len) -#endif +#include <valgrind/memcheck.h> #ifndef rmb # define rmb() mb() diff --git a/libmthca/src/mthca.h b/libmthca/src/mthca.h index bd1e7a2b1dc0..18d52291343f 100644 --- a/libmthca/src/mthca.h +++ b/libmthca/src/mthca.h @@ -39,23 +39,7 @@ #include <infiniband/driver.h> #include <infiniband/arch.h> -#ifdef HAVE_VALGRIND_MEMCHECK_H - -# include <valgrind/memcheck.h> - -# if !defined(VALGRIND_MAKE_MEM_DEFINED) || !defined(VALGRIND_MAKE_MEM_UNDEFINED) -# warning "Valgrind support requested, but VALGRIND_MAKE_MEM_(UN)DEFINED not available" -# endif - -#endif /* HAVE_VALGRIND_MEMCHECK_H */ - -#ifndef VALGRIND_MAKE_MEM_DEFINED -# define VALGRIND_MAKE_MEM_DEFINED(addr,len) -#endif - -#ifndef VALGRIND_MAKE_MEM_UNDEFINED -# define VALGRIND_MAKE_MEM_UNDEFINED(addr,len) -#endif +#include <valgrind/memcheck.h> #ifndef rmb # define rmb() mb() diff --git a/librdmacm/src/cma.h b/librdmacm/src/cma.h index d6005538227f..16a55a67af9e 100644 --- a/librdmacm/src/cma.h +++ b/librdmacm/src/cma.h @@ -47,16 +47,7 @@ #include <ccan/minmax.h> -#ifdef INCLUDE_VALGRIND -# include <valgrind/memcheck.h> -# ifndef VALGRIND_MAKE_MEM_DEFINED -# warning "Valgrind requested, but VALGRIND_MAKE_MEM_DEFINED undefined" -# endif -#endif - -#ifndef VALGRIND_MAKE_MEM_DEFINED -# define VALGRIND_MAKE_MEM_DEFINED(addr,len) -#endif +#include <valgrind/memcheck.h> #define PFX "librdmacm: " diff --git a/srp_daemon/srp_daemon/srp_daemon.h b/srp_daemon/srp_daemon/srp_daemon.h index 6d1575519bc3..d6a2d8a84728 100644 --- a/srp_daemon/srp_daemon/srp_daemon.h +++ b/srp_daemon/srp_daemon/srp_daemon.h @@ -408,12 +408,7 @@ typedef struct { char filler[MAD_BLOCK_SIZE]; } srp_ib_user_mad_t; -#if defined(HAVE_VALGRIND_DRD_H) && defined(ENABLE_VALGRIND) #include <valgrind/drd.h> -#endif -#ifndef ANNOTATE_BENIGN_RACE_SIZED -#define ANNOTATE_BENIGN_RACE_SIZED(a, b, c) do { } while(0) -#endif #define pr_human(arg...) \ do { \
The cmake build system guarantees that valgrind/memcheck.h is present. If the system does not have it, or valgrind is disabled, then it is replaced with a dummy header full of empty stubs. Thus all the copy&paste boiler plate is consolidated into buildlib. Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> --- buildlib/config.h.in | 4 ---- libibcm/src/cm.c | 11 +---------- libibumad/src/umad.c | 14 +------------- libibverbs/src/ibverbs.h | 14 +------------- libmlx4/src/mlx4.h | 18 +----------------- libmlx5/src/mlx5.h | 18 +----------------- libmthca/src/mthca.h | 18 +----------------- librdmacm/src/cma.h | 11 +---------- srp_daemon/srp_daemon/srp_daemon.h | 5 ----- 9 files changed, 7 insertions(+), 106 deletions(-)