Add Valgrind annotations

Add basic Valgrind annotations to libibverbs and libmthca (disabled by
default, can be enabled by configuring with --with-valgrind).  These
reduce false positive warnings from the Valgrind memcheck module.

Based on work and suggestions from Rainer Keller <keller@hlrs.de> and
Jeff Squyres <jsquyres@cisco.com>.

Signed-off-by: Roland Dreier <rolandd@cisco.com>

diff --git a/ChangeLog b/ChangeLog
index eceb76e1042a2db0cd166a219958ac6977d06246..eb956a679dfc2ecb02e0ebc519e1f92ca3cfca91 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,27 @@
+2006-10-03  Roland Dreier  <rdreier@cisco.com>
+
+       * src/cmd.c (ibv_cmd_get_context_v2, ibv_cmd_get_context)
+       (ibv_cmd_query_device, ibv_cmd_query_port, ibv_cmd_alloc_pd)
+       (ibv_cmd_reg_mr, ibv_cmd_create_cq_v2, ibv_cmd_create_cq)
+       (ibv_cmd_poll_cq, ibv_cmd_resize_cq, ibv_cmd_destroy_cq)
+       (ibv_cmd_create_srq, ibv_cmd_create_qp, ibv_cmd_post_send)
+       (ibv_cmd_post_recv, ibv_cmd_post_srq_recv, ibv_cmd_create_ah)
+       (ibv_cmd_destroy_qp): Annotate so that Valgrind knows responses
+       are defined after write() succeeds.  The kernel writes into the
+       response structure directly, so without these, Valgrind thinks
+       that response structures are undefined memory.
+
+       * src/ibverbs.h: Add wrapper for VALGRIND_MAKE_MEM_DEFINED so that
+       it can be used in .c files without worrying about whether Valgrind
+       is installed or enabled.
+
+       * configure.in: Add support for Valgrind annotation (enabled with
+       --with-valgrind option to configure).
+
+       * src/cmd.c (ibv_cmd_query_port, ibv_cmd_create_cq,
+       ibv_cmd_modify_qp): Set reserved fields to 0 to avoid future
+       problems and also to make Valgrind a little quieter.
+
 2006-07-04  Roland Dreier  <rdreier@cisco.com>
 
        * include/infiniband/arch.h: Fix typo in sparc mb()

diff --git a/configure.in b/configure.in
index d9becd642e4ac65dec75772ec51a006f68343c5e..73dbc082a115ff87c384d94100994c5fa27414a4 100644 (file)
--- a/configure.in
+++ b/configure.in
@@ -9,6 +9,13 @@ AM_INIT_AUTOMAKE(libibverbs, 1.0.4)
 
 AM_PROG_LIBTOOL
 
+AC_ARG_WITH([valgrind],
+    AC_HELP_STRING([--with-valgrind],
+        [Enable Valgrind annotations (small runtime overhead, default NO)]))
+if test "x$with_valgrind" != "xyes"; then
+    AC_DEFINE([NVALGRIND], 1, [disable Valgrind annotations])
+fi
+
 dnl Checks for programs
 AC_PROG_CC
 
@@ -24,6 +31,7 @@ dnl Checks for header files.
 AC_CHECK_HEADER(sysfs/libsysfs.h, [],
     AC_MSG_ERROR([<sysfs/libsysfs.h> not found.  libibverbs requires libsysfs.]))
 AC_HEADER_STDC
+AC_CHECK_HEADERS(valgrind/memcheck.h)
 
 dnl Checks for typedefs, structures, and compiler characteristics.
 AC_C_CONST

diff --git a/src/cmd.c b/src/cmd.c
index bd339516cb761b4e0e5644f5a7bd3568c51c34c4..f122ce9a28f7368064dee7b3ef913dec91664145 100644 (file)
--- a/src/cmd.c
+++ b/src/cmd.c
@@ -72,6 +72,8 @@ static int ibv_cmd_get_context_v2(struct ibv_context *context,
        if (write(context->cmd_fd, cmd, cmd_size) != cmd_size)
                return errno;
 
+       VALGRIND_MAKE_MEM_DEFINED(resp, resp_size);
+
        context->async_fd         = resp->async_fd;
        context->num_comp_vectors = 1;
        t->channel.fd             = cq_fd;
@@ -92,6 +94,8 @@ int ibv_cmd_get_context(struct ibv_context *context, struct ibv_get_context *cmd
        if (write(context->cmd_fd, cmd, cmd_size) != cmd_size)
                return errno;
 
+       VALGRIND_MAKE_MEM_DEFINED(resp, resp_size);
+
        context->async_fd         = resp->async_fd;
        context->num_comp_vectors = resp->num_comp_vectors;
 
@@ -110,6 +114,8 @@ int ibv_cmd_query_device(struct ibv_context *context,
        if (write(context->cmd_fd, cmd, cmd_size) != cmd_size)
                return errno;
 
+       VALGRIND_MAKE_MEM_DEFINED(&resp, sizeof resp);
+
        memset(device_attr->fw_ver, 0, sizeof device_attr->fw_ver);
        *raw_fw_ver                            = resp.fw_ver;
        device_attr->node_guid                 = resp.node_guid;
@@ -163,10 +169,13 @@ int ibv_cmd_query_port(struct ibv_context *context, uint8_t port_num,
 
        IBV_INIT_CMD_RESP(cmd, cmd_size, QUERY_PORT, &resp, sizeof resp);
        cmd->port_num = port_num;
+       memset(cmd->reserved, 0, sizeof cmd->reserved);
 
        if (write(context->cmd_fd, cmd, cmd_size) != cmd_size)
                return errno;
 
+       VALGRIND_MAKE_MEM_DEFINED(&resp, sizeof resp);
+
        port_attr->state           = resp.state;
        port_attr->max_mtu         = resp.max_mtu;
        port_attr->active_mtu      = resp.active_mtu;
@@ -199,6 +208,8 @@ int ibv_cmd_alloc_pd(struct ibv_context *context, struct ibv_pd *pd,
        if (write(context->cmd_fd, cmd, cmd_size) != cmd_size)
                return errno;
 
+       VALGRIND_MAKE_MEM_DEFINED(resp, resp_size);
+
        pd->handle = resp->pd_handle;
 
        return 0;
@@ -235,6 +246,8 @@ int ibv_cmd_reg_mr(struct ibv_pd *pd, void *addr, size_t length,
        if (write(pd->context->cmd_fd, cmd, cmd_size) != cmd_size)
                return errno;
 
+       VALGRIND_MAKE_MEM_DEFINED(&resp, sizeof resp);
+
        mr->handle  = resp.mr_handle;
        mr->lkey    = resp.lkey;
        mr->rkey    = resp.rkey;
@@ -275,6 +288,8 @@ static int ibv_cmd_create_cq_v2(struct ibv_context *context, int cqe,
        if (write(context->cmd_fd, cmd, cmd_size) != cmd_size)
                return errno;
 
+       VALGRIND_MAKE_MEM_DEFINED(resp, sizeof resp_size);
+
        cq->handle = resp->cq_handle;
        cq->cqe    = resp->cqe;
 
@@ -301,6 +316,8 @@ int ibv_cmd_create_cq(struct ibv_context *context, int cqe,
        if (write(context->cmd_fd, cmd, cmd_size) != cmd_size)
                return errno;
 
+       VALGRIND_MAKE_MEM_DEFINED(resp, resp_size);
+
        cq->handle = resp->cq_handle;
        cq->cqe    = resp->cqe;
 
@@ -329,6 +346,8 @@ int ibv_cmd_poll_cq(struct ibv_cq *ibcq, int ne, struct ibv_wc *wc)
                goto out;
        }
 
+       VALGRIND_MAKE_MEM_DEFINED(resp, rsize);
+
        for (i = 0; i < resp->count; i++) {
                wc[i].wr_id          = resp->wc[i].wr_id;
                wc[i].status         = resp->wc[i].status;
@@ -378,6 +397,8 @@ int ibv_cmd_resize_cq(struct ibv_cq *cq, int cqe,
        if (write(cq->context->cmd_fd, cmd, cmd_size) != cmd_size)
                return errno;
 
+       VALGRIND_MAKE_MEM_DEFINED(&resp, sizeof resp);
+
        cq->cqe = resp.cqe;
 
        return 0;
@@ -410,6 +431,8 @@ int ibv_cmd_destroy_cq(struct ibv_cq *cq)
        if (write(cq->context->cmd_fd, &cmd, sizeof cmd) != sizeof cmd)
                return errno;
 
+       VALGRIND_MAKE_MEM_DEFINED(&resp, sizeof resp);
+
        pthread_mutex_lock(&cq->mutex);
        while (cq->comp_events_completed  != resp.comp_events_reported ||
               cq->async_events_completed != resp.async_events_reported)
@@ -434,6 +457,8 @@ int ibv_cmd_create_srq(struct ibv_pd *pd,
        if (write(pd->context->cmd_fd, cmd, cmd_size) != cmd_size)
                return errno;
 
+       VALGRIND_MAKE_MEM_DEFINED(resp, resp_size);
+
        srq->handle = resp->srq_handle;
 
        if (abi_ver > 5) {
@@ -574,10 +599,13 @@ int ibv_cmd_create_qp(struct ibv_pd *pd,
        cmd->sq_sig_all      = attr->sq_sig_all;
        cmd->qp_type         = attr->qp_type;
        cmd->is_srq          = !!attr->srq;
+       cmd->reserved        = 0;
 
        if (write(pd->context->cmd_fd, cmd, cmd_size) != cmd_size)
                return errno;
 
+       VALGRIND_MAKE_MEM_DEFINED(resp, resp_size);
+
        qp->handle                = resp->qp_handle;
        qp->qp_num                = resp->qpn;
 
@@ -742,6 +770,8 @@ int ibv_cmd_modify_qp(struct ibv_qp *qp, struct ibv_qp_attr *attr,
        cmd->alt_dest.is_global     = attr->alt_ah_attr.is_global;
        cmd->alt_dest.port_num      = attr->alt_ah_attr.port_num;
 
+       cmd->reserved[0] = cmd->reserved[1] = 0;
+
        if (write(qp->context->cmd_fd, cmd, cmd_size) != cmd_size)
                return errno;
 
@@ -837,6 +867,8 @@ int ibv_cmd_post_send(struct ibv_qp *ibqp, struct ibv_send_wr *wr,
        if (write(ibqp->context->cmd_fd, cmd, cmd_size) != cmd_size)
                ret = errno;
 
+       VALGRIND_MAKE_MEM_DEFINED(&resp, sizeof resp);
+
        wr_count = resp.bad_wr;
        if (wr_count) {
                i = wr;
@@ -895,6 +927,8 @@ int ibv_cmd_post_recv(struct ibv_qp *ibqp, struct ibv_recv_wr *wr,
        if (write(ibqp->context->cmd_fd, cmd, cmd_size) != cmd_size)
                ret = errno;
 
+       VALGRIND_MAKE_MEM_DEFINED(&resp, sizeof resp);
+
        wr_count = resp.bad_wr;
        if (wr_count) {
                i = wr;
@@ -953,6 +987,8 @@ int ibv_cmd_post_srq_recv(struct ibv_srq *srq, struct ibv_recv_wr *wr,
        if (write(srq->context->cmd_fd, cmd, cmd_size) != cmd_size)
                ret = errno;
 
+       VALGRIND_MAKE_MEM_DEFINED(&resp, sizeof resp);
+
        wr_count = resp.bad_wr;
        if (wr_count) {
                i = wr;
@@ -988,6 +1024,8 @@ int ibv_cmd_create_ah(struct ibv_pd *pd, struct ibv_ah *ah,
        if (write(pd->context->cmd_fd, &cmd, sizeof cmd) != sizeof cmd)
                return errno;
 
+       VALGRIND_MAKE_MEM_DEFINED(&resp, sizeof resp);
+
        ah->handle = resp.handle;
 
        return 0;
@@ -1020,6 +1058,8 @@ int ibv_cmd_destroy_qp(struct ibv_qp *qp)
        if (write(qp->context->cmd_fd, &cmd, sizeof cmd) != sizeof cmd)
                return errno;
 
+       VALGRIND_MAKE_MEM_DEFINED(&resp, sizeof resp);
+
        pthread_mutex_lock(&qp->mutex);
        while (qp->events_completed != resp.events_reported)
                pthread_cond_wait(&qp->cond, &qp->mutex);

diff --git a/src/ibverbs.h b/src/ibverbs.h
index 2849da770b546a7ab166c74a96514788cac940d3..d6b9fe7788d5c5a7a3101d77c8ce912820bb9b28 100644 (file)
--- a/src/ibverbs.h
+++ b/src/ibverbs.h
@@ -39,6 +39,20 @@
 
 #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)
+#endif
+
 #define HIDDEN         __attribute__((visibility ("hidden")))
 
 #define INIT           __attribute__((constructor))