Merge pull request #7931 from bosilca/fix/7928

bosilca · web-flow · commit 1139d9ecae72 · 2020-07-18T17:35:39.000-04:00
Fix the BTL API conversion for the SMCUDA BTL
diff --git a/ompi/mca/pml/ob1/pml_ob1_recvfrag.c b/ompi/mca/pml/ob1/pml_ob1_recvfrag.c
@@ -624,10 +624,10 @@ void mca_pml_ob1_recv_frag_callback_frag (mca_btl_base_module_t *btl,
         assert(btl->btl_flags & MCA_BTL_FLAGS_CUDA_COPY_ASYNC_RECV);
 
         /* This will trigger the opal_convertor_pack to start asynchronous copy. */
-        mca_pml_ob1_recv_request_frag_copy_start(recvreq,btl,segments,descriptor->des_segment_count,des);
+        mca_pml_ob1_recv_request_frag_copy_start(recvreq, btl, segments, descriptor->des_segment_count, NULL);
 
         /* Let BTL know that it CANNOT free the frag */
-        descriptor->des_flags |= MCA_BTL_DES_FLAGS_CUDA_COPY_ASYNC;
+        //TODO: GB: descriptor->des_flags |= MCA_BTL_DES_FLAGS_CUDA_COPY_ASYNC;
 
         return;
     }
diff --git a/ompi/mca/pml/ob1/pml_ob1_recvreq.c b/ompi/mca/pml/ob1/pml_ob1_recvreq.c
@@ -577,7 +577,7 @@ void mca_pml_ob1_recv_request_progress_frag( mca_pml_ob1_recv_request_t* recvreq
  */
 void mca_pml_ob1_recv_request_frag_copy_start( mca_pml_ob1_recv_request_t* recvreq,
                                                mca_btl_base_module_t* btl,
-                                               mca_btl_base_segment_t* segments,
+                                               const mca_btl_base_segment_t* segments,
                                                size_t num_segments,
                                                mca_btl_base_descriptor_t* des)
 {
diff --git a/ompi/mca/pml/ob1/pml_ob1_recvreq.h b/ompi/mca/pml/ob1/pml_ob1_recvreq.h
@@ -332,7 +332,7 @@ void mca_pml_ob1_recv_request_progress_frag(
 void mca_pml_ob1_recv_request_frag_copy_start(
     mca_pml_ob1_recv_request_t* req,
     struct mca_btl_base_module_t* btl,
-    mca_btl_base_segment_t* segments,
+    const mca_btl_base_segment_t* segments,
     size_t num_segments,
     mca_btl_base_descriptor_t* des);
 
diff --git a/opal/mca/btl/smcuda/btl_smcuda.c b/opal/mca/btl/smcuda/btl_smcuda.c
@@ -1197,17 +1197,16 @@ static void mca_btl_smcuda_send_cuda_ipc_request(struct mca_btl_base_module_t* b
     if (endpoint->ipcstate != IPC_INIT) {
         OPAL_THREAD_UNLOCK(&endpoint->endpoint_lock);
         return;
-    } else {
-        endpoint->ipctries++;
-        if (endpoint->ipctries > MAXTRIES) {
-            endpoint->ipcstate = IPC_BAD;
-            OPAL_THREAD_UNLOCK(&endpoint->endpoint_lock);
-            return;
-        }
-        /* All is good.  Set up state and continue. */
-        endpoint->ipcstate = IPC_SENT;
+    }
+    endpoint->ipctries++;
+    if (endpoint->ipctries > MAXTRIES) {
+        endpoint->ipcstate = IPC_BAD;
         OPAL_THREAD_UNLOCK(&endpoint->endpoint_lock);
+        return;
     }
+    /* All is good.  Set up state and continue. */
+    endpoint->ipcstate = IPC_SENT;
+    OPAL_THREAD_UNLOCK(&endpoint->endpoint_lock);
 
     if ( mca_btl_smcuda_component.num_outstanding_frags * 2 > (int) mca_btl_smcuda_component.fifo_size ) {
         mca_btl_smcuda_component_progress();
diff --git a/opal/mca/btl/smcuda/btl_smcuda_component.c b/opal/mca/btl/smcuda/btl_smcuda_component.c
@@ -3,7 +3,7 @@
  * Copyright (c) 2004-2011 The Trustees of Indiana University and Indiana
  *                         University Research and Technology
  *                         Corporation.  All rights reserved.
- * Copyright (c) 2004-2009 The University of Tennessee and The University
+ * Copyright (c) 2004-2020 The University of Tennessee and The University
  *                         of Tennessee Research Foundation.  All rights
  *                         reserved.
  * Copyright (c) 2004-2005 High Performance Computing Center Stuttgart,
@@ -42,6 +42,7 @@
 #include <sys/stat.h>  /* for mkfifo */
 #endif  /* HAVE_SYS_STAT_H */
 
+#include "opal/mca/hwloc/base/base.h"
 #include "opal/mca/shmem/base/base.h"
 #include "opal/mca/shmem/shmem.h"
 #include "opal/util/bit_ops.h"
@@ -135,8 +136,15 @@ static inline unsigned int mca_btl_smcuda_param_register_uint(
     return *storage;
 }
 
-static int mca_btl_smcuda_component_verify(void) {
-
+static int mca_btl_smcuda_component_verify(void)
+{
+    /* We canot support async memcpy right now */
+    if( (mca_btl_smcuda.super.btl_flags & MCA_BTL_FLAGS_CUDA_COPY_ASYNC_RECV) ||
+        (mca_btl_smcuda.super.btl_flags & MCA_BTL_FLAGS_CUDA_COPY_ASYNC_SEND) ) {
+        opal_output_verbose(10, opal_btl_base_framework.framework_output,
+                            "btl: smcuda: disable all asynchronous memcpy support");
+    }
+    mca_btl_smcuda.super.btl_flags &= ~(MCA_BTL_FLAGS_CUDA_COPY_ASYNC_RECV | MCA_BTL_FLAGS_CUDA_COPY_ASYNC_SEND);
     return mca_btl_base_param_verify(&mca_btl_smcuda.super);
 }
 
@@ -679,20 +687,15 @@ static void mca_btl_smcuda_send_cuda_ipc_ack(struct mca_btl_base_module_t* btl,
  * BTL.  It handles smcuda specific control messages that are triggered
  * when GPU memory transfers are initiated. */
 static void btl_smcuda_control(mca_btl_base_module_t* btl,
-                               mca_btl_base_tag_t tag,
-                               mca_btl_base_descriptor_t* des, void* cbdata)
+                               const mca_btl_base_receive_descriptor_t *descriptor)
 {
     int mydevnum, ipcaccess, res;
     ctrlhdr_t ctrlhdr;
     opal_proc_t *ep_proc;
-    struct mca_btl_base_endpoint_t *endpoint;
     mca_btl_smcuda_t *smcuda_btl = (mca_btl_smcuda_t *)btl;
-    mca_btl_smcuda_frag_t *frag = (mca_btl_smcuda_frag_t *)des;
-    mca_btl_base_segment_t* segments = des->des_segments;
+    const mca_btl_base_segment_t* segments = descriptor->des_segments;
+    struct mca_btl_base_endpoint_t *endpoint = descriptor->endpoint;
 
-    /* Use the rank of the peer that sent the data to get to the endpoint
-     * structure.  This is needed for PML callback. */
-    endpoint = mca_btl_smcuda_component.sm_peers[frag->hdr->my_smp_rank];
     ep_proc = endpoint->proc_opal;
 
     /* Copy out control message payload to examine it */
@@ -764,7 +767,6 @@ static void btl_smcuda_control(mca_btl_base_module_t* btl,
                 }
             }
 
-            assert(endpoint->peer_smp_rank == frag->hdr->my_smp_rank);
             opal_output_verbose(10, mca_btl_smcuda_component.cuda_ipc_output,
                                 "Analyzed CUDA IPC request: myrank=%d, mydev=%d, peerrank=%d, "
                                 "peerdev=%d --> ACCESS=%d",
@@ -872,6 +874,13 @@ mca_btl_smcuda_component_init(int *num_btls,
      * shared-memory segment. this routine sets component sm_max_procs. */
     calc_sm_max_procs(num_local_procs);
 
+    /* Before we can safely create the backend file we need to know minimal
+     * information about the local node. We need at least a size of a cache line
+     * as we align the data in the backing file to it. The simplest way for now is
+     * to force the HWLOC initialization.
+     */
+    opal_hwloc_base_get_topology();
+
     /* This is where the modex will live some day. For now, just have local rank
      * 0 create a rendezvous file containing the backing store info, so the
      * other local procs can read from it during add_procs. The rest will just
@@ -999,7 +1008,6 @@ int mca_btl_smcuda_component_progress(void)
     /* local variables */
     mca_btl_base_segment_t seg;
     mca_btl_smcuda_frag_t *frag;
-    mca_btl_smcuda_frag_t Frag;
     sm_fifo_t *fifo = NULL;
     mca_btl_smcuda_hdr_t *hdr;
     int my_smp_rank = mca_btl_smcuda_component.my_smp_rank;
@@ -1046,7 +1054,6 @@ int mca_btl_smcuda_component_progress(void)
         switch(((uintptr_t)hdr) & MCA_BTL_SMCUDA_FRAG_TYPE_MASK) {
             case MCA_BTL_SMCUDA_FRAG_SEND:
             {
-                mca_btl_active_message_callback_t* reg;
                 /* change the address from address relative to the shared
                  * memory address, to a true virtual address */
                 hdr = (mca_btl_smcuda_hdr_t *) RELATIVE2VIRTUAL(hdr);
@@ -1058,17 +1065,16 @@ int mca_btl_smcuda_component_progress(void)
                                 my_smp_rank, peer_smp_rank, j, FIFO_MAP(peer_smp_rank));
                 }
 #endif
-                /* recv upcall */
-                reg = mca_btl_base_active_message_trigger + hdr->tag;
                 seg.seg_addr.pval = ((char *)hdr) + sizeof(mca_btl_smcuda_hdr_t);
                 seg.seg_len = hdr->len;
-                Frag.base.des_segment_count = 1;
-                Frag.base.des_segments = &seg;
-#if OPAL_CUDA_SUPPORT
-                Frag.hdr = hdr;  /* needed for peer rank in control messages */
-#endif /* OPAL_CUDA_SUPPORT */
-                reg->cbfunc(&mca_btl_smcuda.super, hdr->tag, &(Frag.base),
-                            reg->cbdata);
+
+                mca_btl_active_message_callback_t *reg = mca_btl_base_active_message_trigger + hdr->tag;
+                mca_btl_base_receive_descriptor_t recv_desc = {.endpoint = mca_btl_smcuda_component.sm_peers[peer_smp_rank],
+                                                               .des_segments = &seg,
+                                                               .des_segment_count = 1,
+                                                               .tag = hdr->tag,
+                                                               .cbdata = reg->cbdata};
+                reg->cbfunc(&mca_btl_smcuda.super, &recv_desc);
                 /* return the fragment */
                 MCA_BTL_SMCUDA_FIFO_WRITE(
                         mca_btl_smcuda_component.sm_peers[peer_smp_rank],
@@ -1101,27 +1107,27 @@ int mca_btl_smcuda_component_progress(void)
                 }
                 goto recheck_peer;
             }
-            default:
-                /* unknown */
-                /*
-                 * This code path should presumably never be called.
-                 * It's unclear if it should exist or, if so, how it should be written.
-                 * If we want to return it to the sending process,
-                 * we have to figure out who the sender is.
-                 * It seems we need to subtract the mask bits.
-                 * Then, hopefully this is an sm header that has an smp_rank field.
-                 * Presumably that means the received header was relative.
-                 * Or, maybe this code should just be removed.
-                 */
-                opal_output(0, "mca_btl_smcuda_component_progress read an unknown type of header");
-                hdr = (mca_btl_smcuda_hdr_t *) RELATIVE2VIRTUAL(hdr);
-                peer_smp_rank = hdr->my_smp_rank;
-                hdr = (mca_btl_smcuda_hdr_t*)((uintptr_t)hdr->frag |
-                        MCA_BTL_SMCUDA_FRAG_STATUS_MASK);
-                MCA_BTL_SMCUDA_FIFO_WRITE(
-                        mca_btl_smcuda_component.sm_peers[peer_smp_rank],
-                        my_smp_rank, peer_smp_rank, hdr, false, true, rc);
-                break;
+        default:
+            /* unknown */
+            /*
+             * This code path should presumably never be called.
+             * It's unclear if it should exist or, if so, how it should be written.
+             * If we want to return it to the sending process,
+             * we have to figure out who the sender is.
+             * It seems we need to subtract the mask bits.
+             * Then, hopefully this is an sm header that has an smp_rank field.
+             * Presumably that means the received header was relative.
+             * Or, maybe this code should just be removed.
+             */
+            opal_output(0, "mca_btl_smcuda_component_progress read an unknown type of header");
+            hdr = (mca_btl_smcuda_hdr_t *) RELATIVE2VIRTUAL(hdr);
+            peer_smp_rank = hdr->my_smp_rank;
+            hdr = (mca_btl_smcuda_hdr_t*)((uintptr_t)hdr->frag |
+                                          MCA_BTL_SMCUDA_FRAG_STATUS_MASK);
+            MCA_BTL_SMCUDA_FIFO_WRITE(
+                    mca_btl_smcuda_component.sm_peers[peer_smp_rank],
+                    my_smp_rank, peer_smp_rank, hdr, false, true, rc);
+            break;
         }
     }
     (void)rc; /* this is safe to ignore as the message is requeued till success */
diff --git a/opal/mca/common/cuda/common_cuda.c b/opal/mca/common/cuda/common_cuda.c
@@ -182,7 +182,7 @@ static int cuda_event_ipc_first_avail, cuda_event_dtoh_first_avail, cuda_event_h
 static int cuda_event_ipc_first_used, cuda_event_dtoh_first_used, cuda_event_htod_first_used;
 
 /* Number of status items currently in use */
-static int cuda_event_ipc_num_used, cuda_event_dtoh_num_used, cuda_event_htod_num_used;
+static volatile int cuda_event_ipc_num_used, cuda_event_dtoh_num_used, cuda_event_htod_num_used;
 
 /* Size of array holding events */
 int cuda_event_max = 400;
@@ -1507,6 +1507,9 @@ void *mca_common_cuda_get_htod_stream(void) {
 int progress_one_cuda_ipc_event(struct mca_btl_base_descriptor_t **frag) {
     CUresult result;
 
+    if( OPAL_LIKELY(0 == cuda_event_ipc_num_used) )
+        return 0;
+
     OPAL_THREAD_LOCK(&common_cuda_ipc_lock);
     if (cuda_event_ipc_num_used > 0) {
         opal_output_verbose(20, mca_common_cuda_output,

Original file line number	Diff line number	Diff line change
`@@ -577,7 +577,7 @@ void mca_pml_ob1_recv_request_progress_frag( mca_pml_ob1_recv_request_t* recvreq`
`577`	`577`	`*/`
`578`	`578`	`void mca_pml_ob1_recv_request_frag_copy_start( mca_pml_ob1_recv_request_t* recvreq,`
`579`	`579`	`mca_btl_base_module_t* btl,`
`580`		`- mca_btl_base_segment_t* segments,`
	`580`	`+ const mca_btl_base_segment_t* segments,`
`581`	`581`	`size_t num_segments,`
`582`	`582`	`mca_btl_base_descriptor_t* des)`
`583`	`583`	`{`