[ofa-general] RE: [PATCH] librdmacm 1/2: add valgrind support to auto-tools configuration file

[Sean Hefty]

librdmacm: add valgrind support.

Signed-off-by: Dotan Barak <dotanb at dev.mellanox.co.il>
Signed-off-by: Sean Hefty <sean.hefty at intel.com>
---
Changes from the posted patches:

* I combined both patches into a single patch.
* I tried to keep the config file simple and went with the option of
  only including memcheck.h if valgrind support was requested.
* The check for memcheck.h is not done if disable_libcheck is true.
* VALGRIND_MAKE_MEM_DEFINED is only defined if memcheck.h is not
  included.  I would rather fail the build if memcheck.h does not
  define this, than print a warning and define it ourselves.

If there's a problem with any of these choices, please let me know.

 configure.in |   18 ++++++++++++++++++
 src/cma.c    |   20 ++++++++++++++++++++
 2 files changed, 38 insertions(+), 0 deletions(-)

diff --git a/configure.in b/configure.in
index 7ecaaf1..1b307b7 100644
--- a/configure.in
+++ b/configure.in
@@ -9,6 +9,18 @@ AM_INIT_AUTOMAKE(librdmacm, 1.0.2)
 
 AM_PROG_LIBTOOL
 
+AC_ARG_WITH([valgrind],
+    AC_HELP_STRING([--with-valgrind],
+		   [Enable valgrind annotations - default NO]))
+
+if test "$with_valgrind" != "" && test "$with_valgrind" != "no"; then
+	AC_DEFINE([INCLUDE_VALGRIND], 1,
+		  [Define to 1 to enable valgrind annotations])
+	if test -d $with_valgrind; then
+		CPPFLAGS="$CPPLFAGS -I$with_valgrind/include"
+	fi
+fi
+
 AC_ARG_ENABLE(libcheck, [  --disable-libcheck      do not test for presence of ib libraries],
 [       if test "$enableval" = "no"; then
                 disable_libcheck=yes
@@ -33,6 +45,12 @@ AC_HEADER_STDC
 if test "$disable_libcheck" != "yes"; then
 AC_CHECK_HEADER(infiniband/verbs.h, [],
     AC_MSG_ERROR([<infiniband/verbs.h> not found.  Is libibverbs installed?]))
+
+if test "$with_valgrind" != "" && test "$with_valgrind" != "no"; then
+AC_CHECK_HEADER(valgrind/memcheck.h, [],
+    AC_MSG_ERROR([valgrind requested but <valgrind/memcheck.h> not found.]))
+fi
+
 fi
 
 AC_CACHE_CHECK(whether ld accepts --version-script, ac_cv_version_script,
diff --git a/src/cma.c b/src/cma.c
index 32edc1f..db336da 100644
--- a/src/cma.c
+++ b/src/cma.c
@@ -55,6 +55,12 @@
 #include <rdma/rdma_cma.h>
 #include <rdma/rdma_cma_abi.h>
 
+#ifdef INCLUDE_VALGRIND
+#  include <valgrind/memcheck.h>
+#else
+#   define VALGRIND_MAKE_MEM_DEFINED(addr,len)
+#endif
+
 #define PFX "librdmacm: "
 
 #if __BYTE_ORDER == __LITTLE_ENDIAN
@@ -383,6 +389,8 @@ int rdma_create_id(struct rdma_event_channel *channel,
 	if (ret != size)
 		goto err;
 
+	VALGRIND_MAKE_MEM_DEFINED(resp, sizeof *resp);
+
 	id_priv->handle = resp->id;
 	*id = &id_priv->id;
 	return 0;
@@ -405,6 +413,8 @@ static int ucma_destroy_kern_id(int fd, uint32_t handle)
 	if (ret != size)
 		return (ret > 0) ? -ENODATA : ret;
 
+	VALGRIND_MAKE_MEM_DEFINED(resp, sizeof *resp);
+
 	return resp->events_reported;
 }
 
@@ -458,6 +468,8 @@ static int ucma_query_route(struct rdma_cm_id *id)
 	if (ret != size)
 		return (ret > 0) ? -ENODATA : ret;
 
+	VALGRIND_MAKE_MEM_DEFINED(resp, sizeof *resp);
+
 	if (resp->num_paths) {
 		id->route.path_rec = malloc(sizeof *id->route.path_rec *
 					    resp->num_paths);
@@ -583,6 +595,8 @@ static int rdma_init_qp_attr(struct rdma_cm_id *id, struct ibv_qp_attr *qp_attr,
 	if (ret != size)
 		return (ret > 0) ? -ENODATA : ret;
 
+	VALGRIND_MAKE_MEM_DEFINED(resp, sizeof *resp);
+
 	ibv_copy_qp_attr_from_kern(qp_attr, resp);
 	*qp_attr_mask = resp->qp_attr_mask;
 	return 0;
@@ -1010,6 +1024,8 @@ int rdma_join_multicast(struct rdma_cm_id *id, struct sockaddr *addr,
 		goto err2;
 	}
 
+	VALGRIND_MAKE_MEM_DEFINED(resp, sizeof *resp);
+
 	mc->handle = resp->id;
 	return 0;
 err2:
@@ -1061,6 +1077,8 @@ int rdma_leave_multicast(struct rdma_cm_id *id, struct sockaddr *addr)
 		goto free;
 	}
 
+	VALGRIND_MAKE_MEM_DEFINED(resp, sizeof *resp);
+
 	pthread_mutex_lock(&id_priv->mut);
 	while (mc->events_completed < resp->events_reported)
 		pthread_cond_wait(&mc->cond, &id_priv->mut);
@@ -1256,6 +1274,8 @@ retry:
 		return (ret > 0) ? -ENODATA : ret;
 	}
 	
+	VALGRIND_MAKE_MEM_DEFINED(resp, sizeof *resp);
+
 	evt->event.event = resp->event;
 	switch (resp->event) {
 	case RDMA_CM_EVENT_ADDR_RESOLVED: