Some memchecker cleanup and others.

awlauria · awlauria · commit 04a3a28a7456 · 2020-03-05T16:44:18.000-05:00
- Port memchecker call from a1d502c. - Remove unused memcheck macro variables. - Some code readability improvements. - Remove some stray +1's in dynamic comm cleanup. - Re-add OPAL_ENABLE_DEBUG macro to osc header. - Cleanup some printf's, and includes. - Refactor cleanup of dpm_disconnect_objs. Signed-off-by: Austen Lauria <awlauria@us.ibm.com>
diff --git a/ompi/dpm/dpm.c b/ompi/dpm/dpm.c
@@ -1114,6 +1114,16 @@ int ompi_dpm_finalize(void)
     return OMPI_SUCCESS;
 }
 
+static void cleanup_dpm_disconnect_objs(ompi_dpm_disconnect_obj **objs, int count)
+{
+    for(int i = 0; i < count; i++) {
+        if (NULL != objs[i]->reqs) {
+            free(objs[i]->reqs);
+        }
+        free(objs[i]);
+    }
+    free(objs);
+}
 
 /**********************************************************************/
 /**********************************************************************/
@@ -1127,7 +1137,7 @@ int ompi_dpm_dyn_finalize(void)
     ompi_communicator_t *comm=NULL;
 
     if (1 <ompi_comm_num_dyncomm) {
-        objs = (ompi_dpm_disconnect_obj**)malloc((ompi_comm_num_dyncomm+1)*
+        objs = (ompi_dpm_disconnect_obj**)malloc(ompi_comm_num_dyncomm *
                                sizeof(ompi_dpm_disconnect_obj*));
         if (NULL == objs) {
             return OMPI_ERR_OUT_OF_RESOURCE;
@@ -1138,23 +1148,15 @@ int ompi_dpm_dyn_finalize(void)
             comm = (ompi_communicator_t*)opal_pointer_array_get_item(&ompi_mpi_communicators,i);
             if (NULL != comm &&  OMPI_COMM_IS_DYNAMIC(comm)) {
                 objs[j++] = disconnect_init(comm);
-                // printf ("%d/%d = %p\n", j-1, ompi_comm_num_dyncomm + 1, objs[j-1]);
             }
         }
 
-        if (j != ompi_comm_num_dyncomm+1) {
-            for(i=0; i<j; i++) {
-                if (NULL != objs[i]->reqs) {
-                    free(objs[i]->reqs);
-                }
-                free(objs[i]);
-            }
-            free(objs);
+        if (j != ompi_comm_num_dyncomm) {
+            cleanup_dpm_disconnect_objs(objs, j);
             return OMPI_ERROR;
         }
 
-        disconnect_waitall(ompi_comm_num_dyncomm+1, objs);
-        free(objs);
+        disconnect_waitall(ompi_comm_num_dyncomm, objs);
     }
 
     return OMPI_SUCCESS;
@@ -1181,7 +1183,6 @@ static ompi_dpm_disconnect_obj *disconnect_init(ompi_communicator_t *comm)
     int i;
 
     obj = (ompi_dpm_disconnect_obj*)calloc(1,sizeof(ompi_dpm_disconnect_obj));
-    // printf("obj = %p\n", obj);
     if (NULL == obj) {
         opal_output(0, "Could not allocate disconnect object");
         return NULL;
@@ -1274,14 +1275,7 @@ static int disconnect_waitall (int count, ompi_dpm_disconnect_obj **objs)
     ret = ompi_request_wait_all(2*totalcount, reqs, MPI_STATUSES_IGNORE);
 
     /* Finally, free everything */
-    for (i=0; i< count; i++ ) {
-        if (NULL != objs[i]->reqs ) {
-            free(objs[i]->reqs );
-        }
-        // printf("%d/%d free %p\n", i, count, objs[i]);
-        free(objs[i]);
-    }
-
+    cleanup_dpm_disconnect_objs(objs, count);
     free(reqs);
 
     return ret;
diff --git a/ompi/include/ompi/memchecker.h b/ompi/include/ompi/memchecker.h
@@ -131,7 +131,7 @@ static inline int memchecker_call (int (*f)(void *, size_t), const void * addr,
                 size_t blength = opal_datatype_basicDatatypes[pElem->elem.common.type]->size;
                 if ((size_t)pElem->elem.extent == blength) {
                     /* block is made of contiguous basic datatype */
-                    f( (void *)(source_base + pElem->elem.disp), pElem->elem.count*pElem->elem.extent );
+                    f( (void *)(source_base + pElem->elem.disp), pElem->elem.count*blength );
                 } else {
                     uint32_t j;
                     ptrdiff_t offset;
diff --git a/ompi/mca/osc/pt2pt/osc_pt2pt_header.h b/ompi/mca/osc/pt2pt/osc_pt2pt_header.h
@@ -100,7 +100,7 @@ typedef struct ompi_osc_pt2pt_header_get_t ompi_osc_pt2pt_header_get_t;
 
 struct ompi_osc_pt2pt_header_complete_t {
     ompi_osc_pt2pt_header_base_t base;
-#if OPAL_ENABLE_HETEROGENEOUS_SUPPORT
+#if OPAL_ENABLE_HETEROGENEOUS_SUPPORT || OPAL_ENABLE_DEBUG
     uint8_t padding[2];
 #endif
     int frag_count;
@@ -123,7 +123,7 @@ typedef struct ompi_osc_pt2pt_header_post_t ompi_osc_pt2pt_header_post_t;
 
 struct ompi_osc_pt2pt_header_lock_t {
     ompi_osc_pt2pt_header_base_t base;
-#if OPAL_ENABLE_HETEROGENEOUS_SUPPORT
+#if OPAL_ENABLE_HETEROGENEOUS_SUPPORT || OPAL_ENABLE_DEBUG
     uint8_t padding[2];
 #endif
     int32_t lock_type;
@@ -133,7 +133,7 @@ typedef struct ompi_osc_pt2pt_header_lock_t ompi_osc_pt2pt_header_lock_t;
 
 struct ompi_osc_pt2pt_header_lock_ack_t {
     ompi_osc_pt2pt_header_base_t base;
-#if OPAL_ENABLE_HETEROGENEOUS_SUPPORT
+#if OPAL_ENABLE_HETEROGENEOUS_SUPPORT || OPAL_ENABLE_DEBUG
     uint8_t padding[2];
 #endif
     uint32_t source;
@@ -143,7 +143,7 @@ typedef struct ompi_osc_pt2pt_header_lock_ack_t ompi_osc_pt2pt_header_lock_ack_t
 
 struct ompi_osc_pt2pt_header_unlock_t {
     ompi_osc_pt2pt_header_base_t base;
-#if OPAL_ENABLE_HETEROGENEOUS_SUPPORT
+#if OPAL_ENABLE_HETEROGENEOUS_SUPPORT || OPAL_ENABLE_DEBUG
     uint8_t padding[2];
 #endif
     int32_t lock_type;
@@ -154,7 +154,7 @@ typedef struct ompi_osc_pt2pt_header_unlock_t ompi_osc_pt2pt_header_unlock_t;
 
 struct ompi_osc_pt2pt_header_unlock_ack_t {
     ompi_osc_pt2pt_header_base_t base;
-#if OPAL_ENABLE_HETEROGENEOUS_SUPPORT
+#if OPAL_ENABLE_HETEROGENEOUS_SUPPORT || OPAL_ENABLE_DEBUG
     uint8_t padding[6];
 #endif
     uint64_t lock_ptr;
@@ -163,7 +163,7 @@ typedef struct ompi_osc_pt2pt_header_unlock_ack_t ompi_osc_pt2pt_header_unlock_a
 
 struct ompi_osc_pt2pt_header_flush_t {
     ompi_osc_pt2pt_header_base_t base;
-#if OPAL_ENABLE_HETEROGENEOUS_SUPPORT
+#if OPAL_ENABLE_HETEROGENEOUS_SUPPORT || OPAL_ENABLE_DEBUG
     uint8_t padding[2];
 #endif
     uint32_t frag_count;
@@ -173,7 +173,7 @@ typedef struct ompi_osc_pt2pt_header_flush_t ompi_osc_pt2pt_header_flush_t;
 
 struct ompi_osc_pt2pt_header_flush_ack_t {
     ompi_osc_pt2pt_header_base_t base;
-#if OPAL_ENABLE_HETEROGENEOUS_SUPPORT
+#if OPAL_ENABLE_HETEROGENEOUS_SUPPORT || OPAL_ENABLE_DEBUG
     uint8_t padding[6];
 #endif
     uint64_t lock_ptr;
diff --git a/ompi/mca/pml/ob1/pml_ob1_cuda.c b/ompi/mca/pml/ob1/pml_ob1_cuda.c
@@ -36,7 +36,6 @@
 #include "pml_ob1_recvreq.h"
 #include "pml_ob1_sendreq.h"
 #include "ompi/mca/bml/base/base.h"
-#include "ompi/memchecker.h"
 
 size_t mca_pml_ob1_rdma_cuda_btls(
     mca_bml_base_endpoint_t* bml_endpoint,
diff --git a/ompi/mca/pml/ob1/pml_ob1_recvfrag.c b/ompi/mca/pml/ob1/pml_ob1_recvfrag.c
@@ -40,7 +40,6 @@
 #include "ompi/communicator/communicator.h"
 #include "ompi/mca/pml/pml.h"
 #include "ompi/peruse/peruse-internal.h"
-#include "ompi/memchecker.h"
 #include "ompi/runtime/ompi_spc.h"
 
 #include "pml_ob1.h"
diff --git a/ompi/mca/pml/ob1/pml_ob1_recvreq.c b/ompi/mca/pml/ob1/pml_ob1_recvreq.c
@@ -40,7 +40,7 @@
 #include "pml_ob1_sendreq.h"
 #include "pml_ob1_rdmafrag.h"
 #include "ompi/mca/bml/base/base.h"
-#include "ompi/memchecker.h"
+
 #if OPAL_CUDA_SUPPORT
 #include "opal/datatype/opal_datatype_cuda.h"
 #include "opal/mca/common/cuda/common_cuda.h"
diff --git a/ompi/mca/pml/ob1/pml_ob1_sendreq.c b/ompi/mca/pml/ob1/pml_ob1_sendreq.c
@@ -39,7 +39,6 @@
 #include "pml_ob1_rdmafrag.h"
 #include "pml_ob1_recvreq.h"
 #include "ompi/mca/bml/base/base.h"
-#include "ompi/memchecker.h"
 
 OBJ_CLASS_INSTANCE(mca_pml_ob1_send_range_t, opal_free_list_item_t,
         NULL, NULL);
diff --git a/ompi/mca/pml/ob1/pml_ob1_sendreq.h b/ompi/mca/pml/ob1/pml_ob1_sendreq.h
@@ -33,6 +33,7 @@
 #include "pml_ob1_rdma.h"
 #include "pml_ob1_rdmafrag.h"
 #include "ompi/mca/bml/bml.h"
+#include "ompi/memchecker.h"
 
 BEGIN_C_DECLS
 
@@ -214,6 +215,16 @@ do {
 
 static inline void mca_pml_ob1_send_request_fini (mca_pml_ob1_send_request_t *sendreq)
 {
+
+  /* make buffer defined when the request is completed,
+     and before releasing the objects. */
+     MEMCHECKER(
+            memchecker_call(&opal_memchecker_base_mem_defined,
+                            sendreq->req_send.req_base.req_addr,
+                            sendreq->req_send.req_base.req_count,
+                            sendreq->req_send.req_base.req_datatype);
+     );
+
     /*  Let the base handle the reference counts */
     MCA_PML_BASE_SEND_REQUEST_FINI((&(sendreq)->req_send));
     assert( NULL == sendreq->rdma_frag );
diff --git a/ompi/mca/pml/ob1/pml_ob1_start.c b/ompi/mca/pml/ob1/pml_ob1_start.c
@@ -26,7 +26,6 @@
 #include "pml_ob1.h"
 #include "pml_ob1_recvreq.h"
 #include "pml_ob1_sendreq.h"
-#include "ompi/memchecker.h"
 
 
 int mca_pml_ob1_start(size_t count, ompi_request_t** requests)
diff --git a/ompi/mpi/c/exscan.c b/ompi/mpi/c/exscan.c
@@ -51,12 +51,10 @@ int MPI_Exscan(const void *sendbuf, void *recvbuf, int count,
 
     MEMCHECKER(
         memchecker_datatype(datatype);
-        if (0 != ompi_comm_rank(comm)) {
-            if (MPI_IN_PLACE != sendbuf) {
-                memchecker_call(&opal_memchecker_base_isdefined, sendbuf, count, datatype);
-            } else {
-                memchecker_call(&opal_memchecker_base_isdefined, recvbuf, count, datatype);
-            }
+        if (MPI_IN_PLACE != sendbuf) {
+            memchecker_call(&opal_memchecker_base_isdefined, sendbuf, count, datatype);
+        } else {
+            memchecker_call(&opal_memchecker_base_isdefined, recvbuf, count, datatype);
         }
         memchecker_call(&opal_memchecker_base_isaddressable, recvbuf, count, datatype);
         memchecker_comm(comm);
diff --git a/ompi/mpi/c/iexscan.c b/ompi/mpi/c/iexscan.c
@@ -52,12 +52,10 @@ int MPI_Iexscan(const void *sendbuf, void *recvbuf, int count,
 
     MEMCHECKER(
         memchecker_datatype(datatype);
-        if (0 != ompi_comm_rank(comm)) {
-            if (MPI_IN_PLACE != sendbuf) {
-                memchecker_call(&opal_memchecker_base_isdefined, sendbuf, count, datatype);
-            } else {
-                memchecker_call(&opal_memchecker_base_isdefined, recvbuf, count, datatype);
-            }
+        if (MPI_IN_PLACE != sendbuf) {
+            memchecker_call(&opal_memchecker_base_isdefined, sendbuf, count, datatype);
+        } else {
+            memchecker_call(&opal_memchecker_base_isdefined, recvbuf, count, datatype);
         }
         memchecker_call(&opal_memchecker_base_isaddressable, recvbuf, count, datatype);
         memchecker_comm(comm);
diff --git a/ompi/mpi/c/ineighbor_allgather.c b/ompi/mpi/c/ineighbor_allgather.c
@@ -57,10 +57,8 @@ int MPI_Ineighbor_allgather(const void *sendbuf, int sendcount, MPI_Datatype sen
     SPC_RECORD(OMPI_SPC_INEIGHBOR_ALLGATHER, 1);
 
     MEMCHECKER(
-        int rank;
         ptrdiff_t ext;
 
-        rank = ompi_comm_rank(comm);
         ompi_datatype_type_extent(recvtype, &ext);
 
         memchecker_datatype(recvtype);
diff --git a/ompi/mpi/c/ineighbor_allgatherv.c b/ompi/mpi/c/ineighbor_allgatherv.c
@@ -57,10 +57,8 @@ int MPI_Ineighbor_allgatherv(const void *sendbuf, int sendcount, MPI_Datatype se
     SPC_RECORD(OMPI_SPC_INEIGHBOR_ALLGATHERV, 1);
 
     MEMCHECKER(
-        int rank;
         ptrdiff_t ext;
 
-        rank = ompi_comm_rank(comm);
         size = ompi_comm_size(comm);
         ompi_datatype_type_extent(recvtype, &ext);
 
diff --git a/ompi/mpi/c/neighbor_allgather.c b/ompi/mpi/c/neighbor_allgather.c
@@ -56,10 +56,8 @@ int MPI_Neighbor_allgather(const void *sendbuf, int sendcount, MPI_Datatype send
     SPC_RECORD(OMPI_SPC_NEIGHBOR_ALLGATHER, 1);
 
     MEMCHECKER(
-        int rank;
         ptrdiff_t ext;
 
-        rank = ompi_comm_rank(comm);
         ompi_datatype_type_extent(recvtype, &ext);
 
         memchecker_datatype(recvtype);
diff --git a/ompi/mpi/c/neighbor_allgatherv.c b/ompi/mpi/c/neighbor_allgatherv.c
@@ -57,10 +57,8 @@ int MPI_Neighbor_allgatherv(const void *sendbuf, int sendcount, MPI_Datatype sen
     SPC_RECORD(OMPI_SPC_NEIGHBOR_ALLGATHERV, 1);
 
     MEMCHECKER(
-        int rank;
         ptrdiff_t ext;
 
-        rank = ompi_comm_rank(comm);
         mca_topo_base_neighbor_count (comm, &in_size, &out_size);
         ompi_datatype_type_extent(recvtype, &ext);
 
diff --git a/ompi/mpiext/pcollreq/c/neighbor_allgather_init.c b/ompi/mpiext/pcollreq/c/neighbor_allgather_init.c
@@ -58,10 +58,8 @@ int MPIX_Neighbor_allgather_init(const void *sendbuf, int sendcount, MPI_Datatyp
     SPC_RECORD(OMPI_SPC_NEIGHBOR_ALLGATHER_INIT, 1);
 
     MEMCHECKER(
-        int rank;
         ptrdiff_t ext;
 
-        rank = ompi_comm_rank(comm);
         ompi_datatype_type_extent(recvtype, &ext);
 
         memchecker_datatype(recvtype);
diff --git a/ompi/mpiext/pcollreq/c/neighbor_allgatherv_init.c b/ompi/mpiext/pcollreq/c/neighbor_allgatherv_init.c
@@ -59,10 +59,8 @@ int MPIX_Neighbor_allgatherv_init(const void *sendbuf, int sendcount, MPI_Dataty
     SPC_RECORD(OMPI_SPC_NEIGHBOR_ALLGATHERV_INIT, 1);
 
     MEMCHECKER(
-        int rank;
         ptrdiff_t ext;
 
-        rank = ompi_comm_rank(comm);
         size = ompi_comm_size(comm);
         ompi_datatype_type_extent(recvtype, &ext);
 
diff --git a/opal/mca/mpool/base/mpool_base_tree.c b/opal/mca/mpool/base/mpool_base_tree.c
@@ -43,11 +43,11 @@ static char *leak_msg = NULL;
 static int condition(void *value);
 static void action(void *key, void *value);
 
-static void mpbtcons(mca_mpool_base_tree_item_t *item) {
+static void opal_mca_mpool_base_tree_constructor(mca_mpool_base_tree_item_t *item) {
     item->key = NULL;
 }
 
-OBJ_CLASS_INSTANCE(mca_mpool_base_tree_item_t, opal_free_list_item_t, mpbtcons, NULL);
+OBJ_CLASS_INSTANCE(mca_mpool_base_tree_item_t, opal_free_list_item_t, opal_mca_mpool_base_tree_constructor, NULL);
 
 /*
  * use globals for the tree and the tree_item free list..
