coll/han/alltoallv: handle errors better

Previously if one rank returned an error, it would immediately exit
the collective with an error which likely causes that pid to
immediately exit with an error.  Unfortunately this causes immediate
segfaults as other ranks may still be accessing shared memory, often
causing cascading walls of segfault text.

I have moved the existing MPI_barrier so that all processes must pass
it before exit regardless of error condition.  This means that either
errors will hang, or all ranks will encounter an error.

Signed-off-by: Luke Robison <[email protected]>

Author: lrbison

ompi/mca/coll/han/coll_han_alltoallv.c

@@ -184,7 +184,7 @@ static inline int alltoallv_sendrecv_w_direct_for_debugging(
             have_completion = 0;
         } else {
             have_completion = 1;
-            ompi_request_wait_any( nreqs, requests, &jreq, MPI_STATUS_IGNORE );
+            rc = ompi_request_wait_any( nreqs, requests, &jreq, MPI_STATUS_IGNORE );
         }
         int ii_send_req = jreq >= jfirst_sendreq;
         if (have_completion) {
@@ -197,28 +197,41 @@ static inline int alltoallv_sendrecv_w_direct_for_debugging(
         requests[jreq] = &ompi_request_null.request;
         if (ii_send_req && jsends_posted < ntypes_send) {
             rc = ompi_datatype_create_contiguous( 1, (ompi_datatype_t*)send_types[jsends_posted], &yuck_ompi_dtype_from_opal );
-            ompi_datatype_commit(&yuck_ompi_dtype_from_opal);
-            MCA_PML_CALL(isend
+            if (rc) break;
+            rc = ompi_datatype_commit(&yuck_ompi_dtype_from_opal);
+            if (rc) break;
+            rc = MCA_PML_CALL(isend
                 (send_from_addrs[jsends_posted], (int)send_counts[jsends_posted], yuck_ompi_dtype_from_opal, jrank_sendto,
                 MCA_COLL_BASE_TAG_ALLTOALLV, MCA_PML_BASE_SEND_STANDARD,
                 comm, &requests[jreq]));
-            ompi_datatype_destroy( &yuck_ompi_dtype_from_opal );
+            if (rc) break;
+            rc = ompi_datatype_destroy( &yuck_ompi_dtype_from_opal );
+            if (rc) break;
+
             jsends_posted++;
         }
         if (!ii_send_req && jrecvs_posted < ntypes_recv ) {
             rc = ompi_datatype_create_contiguous( 1, (ompi_datatype_t*)recv_types[jrecvs_posted], &yuck_ompi_dtype_from_opal );
-            ompi_datatype_commit(&yuck_ompi_dtype_from_opal);
-            MCA_PML_CALL(irecv
+            if (rc) break;
+            rc = ompi_datatype_commit(&yuck_ompi_dtype_from_opal);
+            if (rc) break;
+            rc = MCA_PML_CALL(irecv
                 (recv_to_addrs[jrecvs_posted], (int)recv_counts[jrecvs_posted], yuck_ompi_dtype_from_opal, jrank_recvfrom,
                 MCA_COLL_BASE_TAG_ALLTOALLV,
                 comm, &requests[jreq]));
-            ompi_datatype_destroy( &yuck_ompi_dtype_from_opal );
+            if (rc) break;
+            rc = ompi_datatype_destroy( &yuck_ompi_dtype_from_opal );
+            if (rc) break;
             jrecvs_posted++;
         }
 
-
         if (rc) { break; };
     }
+    if (rc) {
+        opal_output_verbose(1, mca_coll_han_component.han_output,
+            "Failed in alltoallv_sendrecv_w_direct_for_debugging: jloop=%d, rc=%d\n",
+            jloop,rc);
+    }
     return rc;
 }
 
@@ -250,10 +263,16 @@ static int alltoallv_sendrecv_w(
         buf_items[jbuf] = opal_free_list_get(&mca_coll_han_component.pack_buffers);
         if (buf_items[jbuf] == NULL) {
             nbufs = jbuf - 1;
-            printf("Uh-oh, not enough buffers: %d\n",nbufs);
+            opal_output_verbose(20, mca_coll_han_component.han_output,
+                "Uh-oh, not enough buffers: %d\n",nbufs);
             break;
         }
     }
+    if (nbufs < 2) {
+        opal_output_verbose(1, mca_coll_han_component.han_output,
+                    "ERROR: Need at least 2 buffers from mca_coll_han_component.pack_buffers!");
+        return MPI_ERR_NO_MEM;
+    }
 
     size_t nreqs = nbufs;
     int jreq;
@@ -549,22 +568,26 @@ static int decide_to_use_smsc_alg(
     OBJ_CONSTRUCT( &convertor, opal_convertor_t );
     rc = opal_convertor_copy_and_prepare_for_recv(ompi_mpi_local_convertor,
         &rdtype->super, count_for_convertor, rbuf, 0, &convertor);
+    if (rc) goto cleanup1;
     bufs_on_device = opal_convertor_on_device(&convertor);
     need_bufs = opal_convertor_need_buffers(&convertor);
-    rc |= opal_convertor_cleanup(&convertor);
-    rc |= opal_convertor_copy_and_prepare_for_send(ompi_mpi_local_convertor,
+    rc = opal_convertor_cleanup(&convertor);
+    if (rc) goto cleanup1;
+    rc = opal_convertor_copy_and_prepare_for_send(ompi_mpi_local_convertor,
         &sdtype->super, count_for_convertor, sbuf, 0, &convertor);
+    if (rc) goto cleanup1;
     bufs_on_device |= opal_convertor_on_device(&convertor);
     opal_convertor_get_packed_size(&convertor, &packed_size_bytes);
     for (int jrank=0; jrank<comm_size; jrank++) {
         avg_send_bytes += packed_size_bytes/count_for_convertor * ompi_count_array_get(scounts,jrank);
     }
     need_bufs |= opal_convertor_need_buffers(&convertor);
 
-    rc |= opal_convertor_cleanup(&convertor);
+    rc = opal_convertor_cleanup(&convertor);
+cleanup1:
     OBJ_DESTRUCT( &convertor );
-
     if (rc != OMPI_SUCCESS) { return rc;}
+
     avg_send_bytes = avg_send_bytes / comm_size;
     reduce_buf_input[0] = !!(bufs_on_device);
     reduce_buf_input[1] = avg_send_bytes;
@@ -613,12 +636,17 @@ int mca_coll_han_alltoallv_using_smsc(
         struct ompi_communicator_t *comm,
         mca_coll_base_module_t *module)
 {
+    int rc;
+    void **send_from_addrs = NULL;
+    void **recv_to_addrs = NULL;
+    size_t *send_counts = NULL;
+    size_t *recv_counts = NULL;
+    opal_datatype_t **send_types = NULL;
+    opal_datatype_t **recv_types = NULL;
+    mca_coll_han_module_t *han_module = (mca_coll_han_module_t *)module;
 
     OPAL_OUTPUT_VERBOSE((90, mca_coll_han_component.han_output,
                             "Entering mca_coll_han_alltoall_using_smsc\n"));
-    int rc;
-
-    mca_coll_han_module_t *han_module = (mca_coll_han_module_t *)module;
 
     if (!mca_smsc || !mca_smsc_base_has_feature(MCA_SMSC_FEATURE_CAN_MAP)) {
         /* Assume all hosts take this path together :-\ */
@@ -654,13 +682,15 @@ int mca_coll_han_alltoallv_using_smsc(
                                              comm, han_module->previous_alltoallv_module);
     }
 
-    int w_rank = ompi_comm_rank(comm);
     int w_size = ompi_comm_size(comm);
 
     int use_smsc;
     rc = decide_to_use_smsc_alg(&use_smsc,
         sbuf, scounts, sdispls, sdtype, rbuf, rcounts, rdispls, rdtype, comm);
-    if (rc != 0) { return rc; }
+    if (rc != 0) {
+        opal_output_verbose(1, mca_coll_han_component.han_output,
+            "decide_to_use_smsc_alg failed during execution! rc=%d\n", rc);
+    }
     if (!use_smsc) {
         return han_module->previous_alltoallv(sbuf, scounts, sdispls, sdtype, rbuf, rcounts, rdispls, rdtype,
                                              comm, han_module->previous_alltoallv_module);
@@ -738,8 +768,8 @@ int mca_coll_han_alltoallv_using_smsc(
             low_gather_out, sizeof(low_gather_in), MPI_BYTE, low_comm,
             low_comm->c_coll->coll_allgather_module);
     if (rc != 0) {
-        OPAL_OUTPUT_VERBOSE((40, mca_coll_han_component.han_output,
-        "Allgather failed with %d\n",rc));
+        opal_output_verbose(1, mca_coll_han_component.han_output,
+            "During mca_coll_han_alltoallv_using_smsc: Allgather failed with rc=%d\n",rc);
         goto cleanup;
     }
 
@@ -806,12 +836,12 @@ int mca_coll_han_alltoallv_using_smsc(
         peers[jrank].recvtype = &peer_recv_types[jrank];
     }
 
-    void **send_from_addrs = malloc(sizeof(*send_from_addrs)*low_size);
-    void **recv_to_addrs = malloc(sizeof(*recv_to_addrs)*low_size);
-    size_t *send_counts = malloc(sizeof(*send_counts)*low_size);
-    size_t *recv_counts = malloc(sizeof(*recv_counts)*low_size);
-    opal_datatype_t **send_types = malloc(sizeof(*send_types)*low_size);
-    opal_datatype_t **recv_types = malloc(sizeof(*recv_types)*low_size);
+    send_from_addrs = malloc(sizeof(*send_from_addrs)*low_size);
+    recv_to_addrs = malloc(sizeof(*recv_to_addrs)*low_size);
+    send_counts = malloc(sizeof(*send_counts)*low_size);
+    recv_counts = malloc(sizeof(*recv_counts)*low_size);
+    send_types = malloc(sizeof(*send_types)*low_size);
+    recv_types = malloc(sizeof(*recv_types)*low_size);
 
     /****
      *  Main exchange loop
@@ -828,6 +858,11 @@ int mca_coll_han_alltoallv_using_smsc(
             ptrdiff_t peer_sextent;
 
             rc = opal_datatype_type_extent( peers[jlow].sendtype, &peer_sextent);
+            if (rc != 0) {
+                opal_output_verbose(1, mca_coll_han_component.han_output,
+                    "opal_datatype_type_extent returned error code = %d during mca_coll_han_alltoallv_using_smsc!\n",rc);
+                goto cleanup;
+            }
             void *from_addr = (uint8_t*)peers[jlow].sbuf + peers[jlow].counts[jrank_sendto].sdispl*peer_sextent;
 
             send_from_addrs[jlow] = from_addr;
@@ -847,20 +882,28 @@ send_types[jlow] = peers[jlow].sendtype;
             send_from_addrs, send_counts, send_types, jrank_sendto, ntypes_send,
             recv_to_addrs, recv_counts, recv_types, jrank_recvfrom, ntypes_recv,
             comm);
-        if (rc != 0) goto cleanup;
-    }
+        if (rc != 0) {
+            opal_output_verbose(1, mca_coll_han_component.han_output,
+                "alltoallv_sendrecv_w returned error code = %d!\n",rc);
+            goto cleanup;
+        }
 
-    free(send_from_addrs);
-    free(recv_to_addrs);
-    free(send_counts);
-    free(recv_counts);
-    free(send_types);
-    free(recv_types);
 
+    }
     rc=0;
+
+    cleanup:
     low_comm->c_coll->coll_barrier(low_comm, low_comm->c_coll->coll_barrier_module);
 
-cleanup:
+    if (send_from_addrs) {
+        free(send_from_addrs);
+        free(recv_to_addrs);
+        free(send_counts);
+        free(recv_counts);
+        free(send_types);
+        free(recv_types);
+    }
+
     for (int jlow=0; jlow<low_size; jlow++) {
         if (jlow != low_rank) {
             OBJ_DESTRUCT(&peer_send_types[jlow]);