ugni: fixed buffered sends and code cleanup

This commit was SVN r26401.

ompi/mca/btl/ugni/btl_ugni_endpoint.h

@@ -49,7 +49,7 @@ typedef struct mca_btl_base_endpoint_t {
     opal_list_t pending_list;
     opal_list_t pending_smsg_sends;
 
-    uint32_t smsg_progressing;
+    int32_t smsg_progressing;
 } mca_btl_base_endpoint_t;
 
 OBJ_CLASS_DECLARATION(mca_btl_base_endpoint_t);

ompi/mca/btl/ugni/btl_ugni_frag.h

@@ -50,6 +50,12 @@ typedef union mca_btl_ugni_frag_hdr_t {
     mca_btl_ugni_eager_ex_frag_hdr_t eager_ex;
 } mca_btl_ugni_frag_hdr_t;
 
+enum {
+    MCA_BTL_UGNI_FRAG_BUFFERED = 1,
+    MCA_BTL_UGNI_FRAG_COMPLETE = 2,
+    MCA_BTL_UGNI_FRAG_EAGER    = 4
+};
+
 typedef struct mca_btl_ugni_base_frag_t {
     mca_btl_base_descriptor_t base;
     mca_btl_base_segment_t segments[2];
@@ -60,8 +66,7 @@ typedef struct mca_btl_ugni_base_frag_t {
     mca_btl_ugni_reg_t *registration;
     ompi_free_list_t *my_list;
     uint32_t msg_id;
-    bool is_buffered;
+    uint32_t flags;
-    bool complete;
     void (*cbfunc) (struct mca_btl_ugni_base_frag_t*, int);
 } mca_btl_ugni_base_frag_t;
 
@@ -90,6 +95,7 @@ static inline int mca_btl_ugni_frag_alloc (mca_btl_base_endpoint_t *ep,
     if (OPAL_LIKELY(NULL != item)) {
         (*frag)->my_list  = list;
         (*frag)->endpoint = ep;
+        (*frag)->flags    = 0;
     }
 
     return rc;
@@ -110,7 +116,7 @@ static inline int mca_btl_ugni_frag_return (mca_btl_ugni_base_frag_t *frag)
 
 static inline void mca_btl_ugni_frag_complete (mca_btl_ugni_base_frag_t *frag, int rc) {
     /* call callback if specified */
-    frag->complete = true;
+    frag->flags |= MCA_BTL_UGNI_FRAG_COMPLETE;
 
     if (frag->base.des_flags & MCA_BTL_DES_SEND_ALWAYS_CALLBACK) {
         frag->base.des_cbfunc(&frag->endpoint->btl->super, frag->endpoint, &frag->base, rc);

ompi/mca/btl/ugni/btl_ugni_get.c

@@ -87,13 +87,19 @@ static void mca_btl_ugni_callback_eager_get (mca_btl_ugni_base_frag_t *frag, int
     BTL_VERBOSE(("eager get for rem_ctx %p complete", frag->hdr.eager.ctx));
 
     tmp.base.des_dst = tmp.segments;
-    tmp.base.des_dst_cnt = 2;
+    if (hdr_len) {
+        tmp.base.des_dst_cnt = 2;
 
-    tmp.segments[0].seg_addr.pval = frag->hdr.eager_ex.pml_header;
+        tmp.segments[0].seg_addr.pval = frag->hdr.eager_ex.pml_header;
-    tmp.segments[0].seg_len       = hdr_len;
+        tmp.segments[0].seg_len       = hdr_len;
+        tmp.segments[1].seg_addr.pval = frag->segments[0].seg_addr.pval;
+        tmp.segments[1].seg_len       = payload_len;
+    } else {
+        tmp.base.des_dst_cnt = 1;
 
-    tmp.segments[1].seg_addr.pval = frag->segments[0].seg_addr.pval;
+        tmp.segments[0].seg_addr.pval = frag->segments[0].seg_addr.pval;
-    tmp.segments[1].seg_len       = payload_len;
+        tmp.segments[0].seg_len       = payload_len;
+    }
 
     reg = mca_btl_base_active_message_trigger + tag;
     reg->cbfunc(&frag->endpoint->btl->super, tag, &(tmp.base), reg->cbdata);
@@ -108,6 +114,7 @@ int mca_btl_ugni_start_eager_get (mca_btl_base_endpoint_t *ep,
                                   mca_btl_ugni_eager_ex_frag_hdr_t hdr,
                                   mca_btl_ugni_base_frag_t *frag)
 {
+    mca_btl_ugni_reg_t *registration;
     int rc;
 
     if (OPAL_UNLIKELY(frag && frag->my_list == &ep->btl->rdma_int_frags)) {

ompi/mca/btl/ugni/btl_ugni_module.c

@@ -139,8 +139,6 @@ mca_btl_ugni_module_init (mca_btl_ugni_module_t *ugni_module,
         return ompi_common_rc_ugni_to_ompi (rc);
     }
 
-    ugni_module->next_frag_id = 0;
-
     return OMPI_SUCCESS;
 }
 
@@ -244,14 +242,27 @@ mca_btl_ugni_alloc(struct mca_btl_base_module_t *btl,
     frag->base.des_dst = frag->segments + 1;
     frag->base.des_dst_cnt = 1;
 
-    frag->hdr_size = (size <= mca_btl_ugni_component.smsg_max_data) ? sizeof (frag->hdr.send) :
-        sizeof (frag->hdr.eager);
-
     frag->segments[0].seg_addr.pval = NULL;
     frag->segments[0].seg_len       = 0;
     frag->segments[1].seg_addr.pval = frag->base.super.ptr;
     frag->segments[1].seg_len       = size;
 
+    frag->flags = MCA_BTL_UGNI_FRAG_BUFFERED;
+    if (size > mca_btl_ugni_component.smsg_max_data) {
+        mca_btl_ugni_reg_t *registration;
+
+        frag->hdr_size = sizeof (frag->hdr.eager);
+        frag->flags    |= MCA_BTL_UGNI_FRAG_EAGER;
+
+        registration = (mca_btl_ugni_reg_t *) frag->base.super.registration;
+
+        memcpy ((void *) frag->segments[1].seg_key.key64,
+                (void *)&registration->memory_hdl,
+                sizeof (registration->memory_hdl));
+    } else {
+        frag->hdr_size = sizeof (frag->hdr.send);
+    }
+
     return &frag->base;
 }
 

ompi/mca/btl/ugni/btl_ugni_prepare.h

@@ -39,8 +39,6 @@ mca_btl_ugni_prepare_src_send_inplace (struct mca_btl_base_module_t *btl,
         return NULL;
     }
 
-    frag->is_buffered = false;
-
     BTL_VERBOSE(("preparing src for send fragment. size = %u",
                  (unsigned int)(*size + reserve)));
 
@@ -52,6 +50,8 @@ mca_btl_ugni_prepare_src_send_inplace (struct mca_btl_base_module_t *btl,
             return NULL;
         }
 
+        frag->flags = MCA_BTL_UGNI_FRAG_EAGER;
+
         frag->registration = registration;
         memcpy ((void *) frag->segments[1].seg_key.key64,
                 (void *)&registration->memory_hdl,
@@ -89,23 +89,19 @@ mca_btl_ugni_prepare_src_send_buffered (struct mca_btl_base_module_t *btl,
     mca_btl_ugni_base_frag_t *frag = NULL;
     uint32_t iov_count = 1;
     struct iovec iov;
+    size_t max_size = *size;
     int rc;
 
-    /* buffer user data */
-    if (OPAL_LIKELY(!use_eager_get)) {
-        (void) MCA_BTL_UGNI_FRAG_ALLOC_SMSG(endpoint, frag);
-    } else {
-        (void) MCA_BTL_UGNI_FRAG_ALLOC_EAGER_SEND(endpoint, frag);
-    }
-
-    if (OPAL_UNLIKELY(NULL == frag)) {
-        return NULL;
-    }
-
-    frag->is_buffered = true;
-
     if (OPAL_UNLIKELY(true == use_eager_get)) {
+        (void) MCA_BTL_UGNI_FRAG_ALLOC_EAGER_SEND(endpoint, frag);
+        if (OPAL_UNLIKELY(NULL == frag)) {
+            return NULL;
+        }
+
+        frag->flags = MCA_BTL_UGNI_FRAG_EAGER;
+
         registration = (mca_btl_ugni_reg_t *) frag->base.super.registration;
+
         memcpy ((void *) frag->segments[1].seg_key.key64,
                 (void *)&registration->memory_hdl,
                 sizeof (registration->memory_hdl));
@@ -113,24 +109,35 @@ mca_btl_ugni_prepare_src_send_buffered (struct mca_btl_base_module_t *btl,
         frag->hdr_size = reserve + sizeof (frag->hdr.eager);
         frag->segments[0].seg_addr.pval = frag->hdr.eager_ex.pml_header;
     } else {
+        (void) MCA_BTL_UGNI_FRAG_ALLOC_SMSG(endpoint, frag);
+        if (OPAL_UNLIKELY(NULL == frag)) {
+            return NULL;
+        }
+
         frag->hdr_size = reserve + sizeof (frag->hdr.send);
         frag->segments[0].seg_addr.pval = frag->hdr.send_ex.pml_header;
     }
 
-    if (*size) {
+    frag->flags |= MCA_BTL_UGNI_FRAG_BUFFERED;
-        iov.iov_len  = *size;
+ 
-        iov.iov_base = (IOVBASE_TYPE *) frag->base.super.ptr;
+    iov.iov_len  = *size;
+    iov.iov_base = (IOVBASE_TYPE *) frag->base.super.ptr;
 
-        rc = opal_convertor_pack (convertor, &iov, &iov_count, size);
+    rc = opal_convertor_pack (convertor, &iov, &iov_count, &max_size);
-        if (OPAL_UNLIKELY(rc < 0)) {
+    if (OPAL_UNLIKELY(rc < 0)) {
-            mca_btl_ugni_frag_return (frag);
+        mca_btl_ugni_frag_return (frag);
-            return NULL;
+        return NULL;
-        }
+    }
+
+    if (max_size != *size) {
+        fprintf (stderr, "**** max_size = %d. iov.iov_len = %d\n", max_size,
+                 iov.iov_len);
+        abort();
     }
 
     frag->segments[0].seg_len       = reserve;
 
-    frag->segments[1].seg_addr.pval = *size ? frag->base.super.ptr : NULL;
+    frag->segments[1].seg_addr.pval = frag->base.super.ptr;
     frag->segments[1].seg_len       = *size;
 
     frag->base.des_src     = frag->segments;

ompi/mca/btl/ugni/btl_ugni_rdma.h

@@ -49,7 +49,7 @@ static inline int init_gni_post_desc(mca_btl_ugni_base_frag_t *frag,
 static inline int mca_btl_ugni_post_fma (mca_btl_ugni_base_frag_t *frag, gni_post_type_t op_type,
                                          mca_btl_base_segment_t *lcl_seg, mca_btl_base_segment_t *rem_seg)
 {
-    int rc;
+    gni_return_t rc;
 
     /* Post descriptor */
     init_gni_post_desc (frag, op_type, lcl_seg->seg_addr.lval,
@@ -59,17 +59,17 @@ static inline int mca_btl_ugni_post_fma (mca_btl_ugni_base_frag_t *frag, gni_pos
 
     rc = GNI_PostFma (frag->endpoint->rdma_ep_handle, &frag->post_desc.base);
     if (GNI_RC_SUCCESS != rc) {
-        assert(rc < 4);
+        BTL_VERBOSE(("GNI_PostFma failed with gni rc: %d", rc));
-        rc = OMPI_ERR_OUT_OF_RESOURCE;
+        return OMPI_ERR_OUT_OF_RESOURCE;
     }
 
-    return rc;
+    return OMPI_SUCCESS;
 }
 
 static inline int mca_btl_ugni_post_bte (mca_btl_ugni_base_frag_t *frag, gni_post_type_t op_type,
                                          mca_btl_base_segment_t *lcl_seg, mca_btl_base_segment_t *rem_seg)
 {
-    int rc;
+    gni_return_t rc;
 
     /* Post descriptor */
     init_gni_post_desc (frag, op_type, lcl_seg->seg_addr.lval,
@@ -79,11 +79,11 @@ static inline int mca_btl_ugni_post_bte (mca_btl_ugni_base_frag_t *frag, gni_pos
 
     rc = GNI_PostRdma (frag->endpoint->rdma_ep_handle, &frag->post_desc.base);
     if (GNI_RC_SUCCESS != rc) {
-        rc = ompi_common_rc_ugni_to_ompi (rc);
+        BTL_VERBOSE(("GNI_PostRdma failed with gni rc: %d", rc));
-        BTL_ERROR(("GNI_PostRdma failed with rc = %d", rc));
+        return OMPI_ERR_OUT_OF_RESOURCE;
     }
 
-    return rc;
+    return OMPI_SUCCESS;
 }
 
 #endif /* MCA_BTL_UGNI_RDMA_H */

ompi/mca/btl/ugni/btl_ugni_send.c

@@ -21,7 +21,7 @@ int mca_btl_ugni_send (struct mca_btl_base_module_t *btl,
 {
     mca_btl_ugni_base_frag_t *frag = (mca_btl_ugni_base_frag_t *) descriptor;
     size_t size = frag->segments[0].seg_len + frag->segments[1].seg_len;
-    bool use_eager_get = size > mca_btl_ugni_component.smsg_max_data;
+    bool use_eager_get = !!(frag->flags & MCA_BTL_UGNI_FRAG_EAGER);
     int flags_save = frag->base.des_flags;
     int rc;
 
@@ -30,7 +30,6 @@ int mca_btl_ugni_send (struct mca_btl_base_module_t *btl,
 
     /* tag and len are at the same location in eager and smsg frag hdrs */
     frag->hdr.send.lag = (tag << 24) | size;
-
     if (OPAL_UNLIKELY(use_eager_get)) {
         frag->hdr.eager.src_seg = frag->segments[1];
         frag->hdr.eager.ctx     = (void *) frag;
@@ -47,36 +46,39 @@ int mca_btl_ugni_send (struct mca_btl_base_module_t *btl,
 
     /* temporarily disable ownership and callback flags so we can reliably check the complete flag */
     frag->base.des_flags &= ~(MCA_BTL_DES_FLAGS_BTL_OWNERSHIP | MCA_BTL_DES_SEND_ALWAYS_CALLBACK);
-    frag->complete = false;
+    frag->flags &= ~MCA_BTL_UGNI_FRAG_COMPLETE;
 
     rc = ompi_mca_btl_ugni_smsg_send (frag, use_eager_get, &frag->hdr.send, frag->hdr_size,
-                                      frag->segments[1].seg_addr.pval, use_eager_get ? 0 : frag->segments[1].seg_len,
+                                      frag->segments[1].seg_addr.pval, use_eager_get ? 0 :
-                                      use_eager_get ? MCA_BTL_UGNI_TAG_GET_INIT : MCA_BTL_UGNI_TAG_SEND);
+                                      frag->segments[1].seg_len, use_eager_get ?
+                                      MCA_BTL_UGNI_TAG_GET_INIT : MCA_BTL_UGNI_TAG_SEND);
     if (OPAL_UNLIKELY(OMPI_SUCCESS != rc)) {
         return rc;
     }
 
-    if (OPAL_LIKELY(frag->complete)) {
+    if (OPAL_LIKELY(!use_eager_get)) {
-        /* fast path: remote side has received the frag */
+        if (OPAL_LIKELY(frag->flags & MCA_BTL_UGNI_FRAG_COMPLETE)) {
-        frag->base.des_flags = flags_save;
+            /* fast path: remote side has received the frag */
-        mca_btl_ugni_frag_complete (frag, OMPI_SUCCESS);
+            frag->base.des_flags = flags_save;
+            mca_btl_ugni_frag_complete (frag, OMPI_SUCCESS);
 
-        return 1;
+            return 1;
-    }
-
-    if (frag->is_buffered && (flags_save & MCA_BTL_DES_FLAGS_BTL_OWNERSHIP)) {
-        /* fast(ish) path: btl owned buffered frag. report send as complete */
-        frag->base.des_flags = flags_save & ~MCA_BTL_DES_SEND_ALWAYS_CALLBACK;
-
-        if (OPAL_LIKELY(flags_save & MCA_BTL_DES_SEND_ALWAYS_CALLBACK)) {
-            frag->base.des_cbfunc(&frag->endpoint->btl->super, frag->endpoint, &frag->base, rc);
         }
 
-        return 1;
+        if ((frag->flags & MCA_BTL_UGNI_FRAG_BUFFERED) && (flags_save & MCA_BTL_DES_FLAGS_BTL_OWNERSHIP)) {
+            /* fast(ish) path: btl owned buffered frag. report send as complete */
+            frag->base.des_flags = flags_save & ~MCA_BTL_DES_SEND_ALWAYS_CALLBACK;
+
+            if (OPAL_LIKELY(flags_save & MCA_BTL_DES_SEND_ALWAYS_CALLBACK)) {
+                frag->base.des_cbfunc(&frag->endpoint->btl->super, frag->endpoint, &frag->base, rc);
+            }
+
+            return 1;
+        }
     }
 
     /* slow(ish) path: remote side hasn't received the frag. call the frag's callback when
-       we get the local smsg/msgq completion */
+       we get the local smsg/msgq or remote rdma completion */
     frag->base.des_flags = flags_save | MCA_BTL_DES_SEND_ALWAYS_CALLBACK;
 
     return OMPI_SUCCESS;

ompi/mca/btl/ugni/btl_ugni_sendi.c

@@ -25,8 +25,6 @@ int mca_btl_ugni_sendi (struct mca_btl_base_module_t *btl,
 {
     const size_t length = header_size + payload_size;
     mca_btl_ugni_base_frag_t *frag;
-    uint32_t iov_count = 1;
-    struct iovec iov;
     size_t max_data;
     int rc;
 
@@ -63,7 +61,7 @@ int mca_btl_ugni_sendi (struct mca_btl_base_module_t *btl,
     memmove (frag->base.des_src[0].seg_addr.pval, header, header_size);
 
     /* send message */
-    if (OPAL_LIKELY(length <= mca_btl_ugni_component.smsg_max_data)) { 
+    if (OPAL_LIKELY(!(frag->flags & MCA_BTL_UGNI_FRAG_EAGER))) {
         rc = ompi_mca_btl_ugni_smsg_send (frag, false, &frag->hdr.send_ex, frag->hdr_size,
                                           frag->segments[1].seg_addr.pval, frag->segments[1].seg_len,
                                           MCA_BTL_UGNI_TAG_SEND);

ompi/mca/btl/ugni/btl_ugni_smsg.h

@@ -28,14 +28,13 @@ typedef enum {
 int mca_btl_ugni_smsg_process (mca_btl_base_endpoint_t *ep);
 int mca_btl_ugni_progress_remote_smsg (mca_btl_ugni_module_t *btl);
 
-static inline int mca_btl_ugni_smsg_next_local_completion (mca_btl_ugni_module_t *ugni_module, mca_btl_ugni_base_frag_t **frag)
+static inline int mca_btl_ugni_progress_local_smsg (mca_btl_ugni_module_t *ugni_module)
 {
+    mca_btl_ugni_base_frag_t *frag;
     gni_cq_entry_t event_data;
     gni_return_t rc;
     uint32_t msg_id;
 
-    *frag = NULL;
-
     rc = GNI_CqGetEvent (ugni_module->smsg_local_cq, &event_data);
     if (GNI_RC_NOT_DONE == rc) {
         return OMPI_SUCCESS;
@@ -57,22 +56,15 @@ static inline int mca_btl_ugni_smsg_next_local_completion (mca_btl_ugni_module_t
         return OMPI_SUCCESS;
     }
 
-    *frag = (mca_btl_ugni_base_frag_t *) opal_pointer_array_get_item (&ugni_module->pending_smsg_frags_bb, msg_id);
+    frag = (mca_btl_ugni_base_frag_t *) opal_pointer_array_get_item (&ugni_module->pending_smsg_frags_bb,
-    assert (NULL != *frag);
+                                                                     msg_id);
-
+    assert (NULL != frag);
-    return GNI_CQ_STATUS_OK(event_data) ? OMPI_SUCCESS : OMPI_ERROR;
+    if (OPAL_UNLIKELY(NULL == frag)) {
-}
+        return OMPI_ERROR;
-
-static inline int mca_btl_ugni_progress_local_smsg (mca_btl_ugni_module_t *ugni_module)
-{
-    mca_btl_ugni_base_frag_t *frag;
-    int rc;
-
-    rc = mca_btl_ugni_smsg_next_local_completion (ugni_module, &frag);
-    if (NULL != frag) {
-        mca_btl_ugni_frag_complete (frag, rc);
     }
 
+    mca_btl_ugni_frag_complete (frag, rc);
+
     return 1;
 }
 
@@ -88,23 +80,23 @@ static inline int ompi_mca_btl_ugni_smsg_send (mca_btl_ugni_base_frag_t *frag,
 
     (void) mca_btl_ugni_progress_local_smsg ((mca_btl_ugni_module_t *) frag->endpoint->btl);
 
-    if (OPAL_UNLIKELY(GNI_RC_SUCCESS != grc)) {
+    if (OPAL_LIKELY(GNI_RC_SUCCESS == grc)) {
-        /* see if we can free up some credits */
+        return OMPI_SUCCESS;
-        (void) mca_btl_ugni_progress_remote_smsg ((mca_btl_ugni_module_t *) frag->endpoint->btl);
-
-        if (OPAL_LIKELY(GNI_RC_NOT_DONE == grc)) {
-            BTL_VERBOSE(("out of credits"));
-
-            return OMPI_ERR_OUT_OF_RESOURCE;
-        }
-
-        BTL_ERROR(("GNI_SmsgSendWTag failed with rc = %d. handle = %d, hdr_len = %d, payload_len = %d",
-                   grc, (int) frag->endpoint->smsg_ep_handle, (int) hdr_len, (int) payload_len));
-
-        return OMPI_ERROR;
     }
 
-    return OMPI_SUCCESS;
+    /* see if we can free up some credits */
+    (void) mca_btl_ugni_progress_remote_smsg ((mca_btl_ugni_module_t *) frag->endpoint->btl);
+
+    if (OPAL_LIKELY(GNI_RC_NOT_DONE == grc)) {
+        BTL_VERBOSE(("out of credits"));
+
+        return OMPI_ERR_OUT_OF_RESOURCE;
+    }
+
+    BTL_ERROR(("GNI_SmsgSendWTag failed with rc = %d. handle = %lu, hdr_len = %d, payload_len = %d",
+               grc, (uintptr_t) frag->endpoint->smsg_ep_handle, (int) hdr_len, (int) payload_len));
+
+    return OMPI_ERROR;
 }
 
 #endif /* MCA_BTL_UGNI_SMSG_H */