pml/ob1: fix bugs in static request objects

This commit fixes several bugs in the static request objects used by
ob1 for blocking send/receive operations.

 - Fix memory leak when using MPI_THREAD_MULTIPLE. Requests were
   allocated off the free list but were destructed and NOT returned.

 - Fix double-destruct of static objects. There is no reason to
   CONSTRUCT/DESTUCT the static object for each send/receive
   operation. This adds overhead and no benefit. To keep the code
   clean helper functions have been added to finalize ob1 send/receive
   requests.

 - Remove now unnecessary include of alloca.h.

Signed-off-by: Nathan Hjelm <[email protected]>
(cherry picked from commit open-mpi/ompi@9a8a876)

Author: hjelmn

ompi/mca/pml/ob1/pml_ob1_component.c

@@ -292,6 +292,18 @@ int mca_pml_ob1_component_fini(void)
         return OMPI_SUCCESS; /* never selected.. return success.. */
     mca_pml_ob1.enabled = false;  /* not anymore */
 
+    /* return the static receive/send requests to the respective free list and
+     * let the free list handle destruction. */
+    if( NULL != mca_pml_ob1_recvreq ) {
+        opal_free_list_return (&mca_pml_base_recv_requests, (opal_free_list_item_t *) mca_pml_ob1_recvreq);
+        mca_pml_ob1_recvreq = NULL;
+    }
+
+    if( NULL != mca_pml_ob1_sendreq ) {
+        opal_free_list_return (&mca_pml_base_send_requests, (opal_free_list_item_t *) mca_pml_ob1_sendreq);
+        mca_pml_ob1_sendreq = NULL;
+    }
+
     OBJ_DESTRUCT(&mca_pml_ob1.rdma_pending);
     OBJ_DESTRUCT(&mca_pml_ob1.pckt_pending);
     OBJ_DESTRUCT(&mca_pml_ob1.recv_pending);
@@ -304,15 +316,6 @@ int mca_pml_ob1_component_fini(void)
     OBJ_DESTRUCT(&mca_pml_ob1.lock);
     OBJ_DESTRUCT(&mca_pml_ob1.send_ranges);
 
-    if( NULL != mca_pml_ob1_recvreq ) {
-        OBJ_DESTRUCT(mca_pml_ob1_recvreq);
-        mca_pml_ob1_recvreq = NULL;
-    }
-    if( NULL != mca_pml_ob1_sendreq ) {
-        OBJ_DESTRUCT(mca_pml_ob1_sendreq);
-        mca_pml_ob1_sendreq = NULL;
-    }
-
     if( NULL != mca_pml_ob1.allocator ) {
         (void)mca_pml_ob1.allocator->alc_finalize(mca_pml_ob1.allocator);
         mca_pml_ob1.allocator = NULL;

ompi/mca/pml/ob1/pml_ob1_irecv.c

@@ -10,7 +10,7 @@
  *                         University of Stuttgart.  All rights reserved.
  * Copyright (c) 2004-2005 The Regents of the University of California.
  *                         All rights reserved.
- * Copyright (c) 2007-2014 Los Alamos National Security, LLC.  All rights
+ * Copyright (c) 2007-2015 Los Alamos National Security, LLC.  All rights
  *                         reserved.
  * Copyright (c) 2010-2012 Oracle and/or its affiliates.  All rights reserved.
  * Copyright (c) 2011      Sandia National Laboratories. All rights reserved.
@@ -28,9 +28,6 @@
 #include "pml_ob1_recvfrag.h"
 #include "ompi/peruse/peruse-internal.h"
 #include "ompi/message/message.h"
-#if HAVE_ALLOCA_H
-#include <alloca.h>
-#endif  /* HAVE_ALLOCA_H */
 
 mca_pml_ob1_recv_request_t *mca_pml_ob1_recvreq = NULL;
 
@@ -109,7 +106,6 @@ int mca_pml_ob1_recv(void *addr,
             mca_pml_ob1_recvreq = recvreq;
 #endif  /* !OMPI_ENABLE_THREAD_MULTIPLE */
         }
-    OBJ_CONSTRUCT(recvreq, mca_pml_ob1_recv_request_t);
 
     MCA_PML_OB1_RECV_REQUEST_INIT(recvreq, addr, count, datatype,
                                   src, tag, comm, false);
@@ -126,8 +122,12 @@ int mca_pml_ob1_recv(void *addr,
     }
 
     rc = recvreq->req_recv.req_base.req_ompi.req_status.MPI_ERROR;
-    MCA_PML_BASE_RECV_REQUEST_FINI(&recvreq->req_recv);
-    OBJ_DESTRUCT(recvreq);
+
+#if OMPI_ENABLE_THREAD_MULTIPLE
+    MCA_PML_OB1_RECV_REQUEST_RETURN(recvreq);
+#else
+    mca_pml_ob1_recv_request_fini (recvreq);
+#endif
 
     return rc;
 }

ompi/mca/pml/ob1/pml_ob1_isend.c

@@ -28,9 +28,6 @@
 #include "pml_ob1_sendreq.h"
 #include "pml_ob1_recvreq.h"
 #include "ompi/peruse/peruse-internal.h"
-#if HAVE_ALLOCA_H
-#include <alloca.h>
-#endif  /* HAVE_ALLOCA_H */
 
 mca_pml_ob1_send_request_t *mca_pml_ob1_sendreq = NULL;
 
@@ -232,7 +229,6 @@ int mca_pml_ob1_send(const void *buf,
             mca_pml_ob1_sendreq = sendreq;
 #endif  /* !OMPI_ENABLE_THREAD_MULTIPLE */
         }
-    OBJ_CONSTRUCT(sendreq, mca_pml_ob1_send_request_t);
     sendreq->req_send.req_base.req_proc = dst_proc;
     sendreq->rdma_frag = NULL;
 
@@ -252,9 +248,13 @@ int mca_pml_ob1_send(const void *buf,
         ompi_request_wait_completion(&sendreq->req_send.req_base.req_ompi);
 
         rc = sendreq->req_send.req_base.req_ompi.req_status.MPI_ERROR;
-        MCA_PML_BASE_SEND_REQUEST_FINI(&sendreq->req_send);
     }
-    OBJ_DESTRUCT(sendreq);
+
+#if OMPI_ENABLE_THREAD_MULTIPLE
+    MCA_PML_OB1_SEND_REQUEST_RETURN(sendreq);
+#else
+    mca_pml_ob1_send_request_fini (sendreq);
+#endif
 
     return rc;
 }

ompi/mca/pml/ob1/pml_ob1_recvreq.h

@@ -128,16 +128,21 @@ do {                                                                \
         ompi_request_complete( &(recvreq->req_recv.req_base.req_ompi), true );        \
     } while (0)
 
+static inline void mca_pml_ob1_recv_request_fini (mca_pml_ob1_recv_request_t *recvreq)
+{
+    MCA_PML_BASE_RECV_REQUEST_FINI(&recvreq->req_recv);
+    if ((recvreq)->local_handle) {
+        mca_bml_base_deregister_mem (recvreq->rdma_bml, recvreq->local_handle);
+        recvreq->local_handle = NULL;
+    }
+}
+
 /*
  *  Free the PML receive request
  */
 #define MCA_PML_OB1_RECV_REQUEST_RETURN(recvreq)                        \
     {                                                                   \
-        MCA_PML_BASE_RECV_REQUEST_FINI(&(recvreq)->req_recv);           \
-        if ((recvreq)->local_handle) {                                  \
-            mca_bml_base_deregister_mem ((recvreq)->rdma_bml, (recvreq)->local_handle); \
-            (recvreq)->local_handle = NULL;                             \
-        }                                                               \
+        mca_pml_ob1_recv_request_fini (recvreq);                        \
         opal_free_list_return (&mca_pml_base_recv_requests,             \
                                (opal_free_list_item_t*)(recvreq));      \
     }

ompi/mca/pml/ob1/pml_ob1_sendreq.h

@@ -215,18 +215,23 @@ do {
                             &(sendreq->req_send.req_base), PERUSE_SEND);             \
 } while(0)
 
+static inline void mca_pml_ob1_send_request_fini (mca_pml_ob1_send_request_t *sendreq)
+{
+    /*  Let the base handle the reference counts */
+    MCA_PML_BASE_SEND_REQUEST_FINI((&(sendreq)->req_send));
+    if (sendreq->rdma_frag) {
+        MCA_PML_OB1_RDMA_FRAG_RETURN (sendreq->rdma_frag);
+        sendreq->rdma_frag = NULL;
+    }
+}
+
 /*
  * Release resources associated with a request
  */
 
 #define MCA_PML_OB1_SEND_REQUEST_RETURN(sendreq)                        \
     do {                                                                \
-        /*  Let the base handle the reference counts */                 \
-        MCA_PML_BASE_SEND_REQUEST_FINI((&(sendreq)->req_send));         \
-        if (sendreq->rdma_frag) {                                       \
-            MCA_PML_OB1_RDMA_FRAG_RETURN (sendreq->rdma_frag);          \
-            sendreq->rdma_frag = NULL;                                  \
-        }                                                               \
+        mca_pml_ob1_send_request_fini (sendreq);                        \
         opal_free_list_return ( &mca_pml_base_send_requests,            \
                                 (opal_free_list_item_t*)sendreq);       \
     } while(0)