btl/uct: bug fixes and general improvements

hjelmn · hjelmn · commit 39be6ec15c20 · 2018-10-09T15:15:45.000-06:00
This commit updates the uct btl to change the transports parameter
into a priority list. The dc_mlx5, rc_mlx5, and ud transports to the
priority list. This will give better out of the box performance for
multi-threaded codes beacuse the *_mlx5 transports can avoid the mlx5
lock inside libmlx5_rdmav2.

This commit also fixes a number of leaks and a possible deadlock when
using RDMA.

Signed-off-by: Nathan Hjelm &lt;hjelmn@lanl.gov&gt;
diff --git a/opal/mca/btl/uct/btl_uct.h b/opal/mca/btl/uct/btl_uct.h
@@ -106,9 +106,6 @@ struct mca_btl_uct_module_t {
     /** large registered frags for packing non-contiguous data */
     opal_free_list_t max_frags;
 
-    /** RDMA completions */
-    opal_free_list_t rdma_completions;
-
     /** frags that were waiting on connections that are now ready to send */
     opal_list_t pending_frags;
 };
diff --git a/opal/mca/btl/uct/btl_uct_amo.c b/opal/mca/btl/uct/btl_uct_amo.c
@@ -104,8 +104,10 @@ int mca_btl_uct_afop (struct mca_btl_base_module_t *btl, struct mca_btl_base_end
         rc = OPAL_SUCCESS;
     } else if (UCS_OK == ucs_status) {
         rc = 1;
+        mca_btl_uct_uct_completion_release (comp);
     } else {
         rc = OPAL_ERR_OUT_OF_RESOURCE;
+        mca_btl_uct_uct_completion_release (comp);
     }
 
     uct_rkey_release (&rkey);
@@ -176,8 +178,10 @@ int mca_btl_uct_acswap (struct mca_btl_base_module_t *btl, struct mca_btl_base_e
         rc = OPAL_SUCCESS;
     } else if (UCS_OK == ucs_status) {
         rc = 1;
+        mca_btl_uct_uct_completion_release (comp);
     } else {
         rc = OPAL_ERR_OUT_OF_RESOURCE;
+        mca_btl_uct_uct_completion_release (comp);
     }
 
     uct_rkey_release (&rkey);
diff --git a/opal/mca/btl/uct/btl_uct_component.c b/opal/mca/btl/uct/btl_uct_component.c
@@ -29,6 +29,10 @@
 #include "opal/mca/btl/base/base.h"
 #include "opal/mca/hwloc/base/base.h"
 #include "opal/util/argv.h"
+#include "opal/memoryhooks/memory.h"
+#include "opal/mca/memory/base/base.h"
+#include <ucm/api/ucm.h>
+
 #include "opal/util/printf.h"
 
 #include <string.h>
@@ -49,13 +53,13 @@ static int mca_btl_uct_component_register(void)
                                            MCA_BASE_VAR_FLAG_SETTABLE, OPAL_INFO_LVL_3, MCA_BASE_VAR_SCOPE_LOCAL,
                                            &mca_btl_uct_component.memory_domains);
 
-    mca_btl_uct_component.allowed_transports = "any";
+    mca_btl_uct_component.allowed_transports = "dc_mlx5,rc_mlx5,ud,any";
     (void) mca_base_component_var_register(&mca_btl_uct_component.super.btl_version,
-                                           "transports", "Comma-delimited list of transports of the form to use."
-                                           " The list of transports available can be queried using ucx_info. Special"
-                                           "values: any (any available) (default: any)", MCA_BASE_VAR_TYPE_STRING,
-                                           NULL, 0, MCA_BASE_VAR_FLAG_SETTABLE, OPAL_INFO_LVL_3, MCA_BASE_VAR_SCOPE_LOCAL,
-                                           &mca_btl_uct_component.allowed_transports);
+                                           "transports", "Comma-delimited list of transports to use sorted by increasing "
+                                           "priority. The list of transports available can be queried using ucx_info. Special"
+                                           "values: any (any available) (default: dc_mlx5,rc_mlx5,ud,any)",
+                                           MCA_BASE_VAR_TYPE_STRING, NULL, 0, MCA_BASE_VAR_FLAG_SETTABLE, OPAL_INFO_LVL_3,
+                                           MCA_BASE_VAR_SCOPE_LOCAL, &mca_btl_uct_component.allowed_transports);
 
     mca_btl_uct_component.num_contexts_per_module = 0;
     (void) mca_base_component_var_register(&mca_btl_uct_component.super.btl_version,
@@ -95,6 +99,11 @@ static int mca_btl_uct_component_register(void)
                                         &module->super);
 }
 
+static void mca_btl_uct_mem_release_cb(void *buf, size_t length, void *cbdata, bool from_alloc)
+{
+    ucm_vm_munmap(buf, length);
+}
+
 static int mca_btl_uct_component_open(void)
 {
     if (0 == mca_btl_uct_component.num_contexts_per_module) {
@@ -114,6 +123,15 @@ static int mca_btl_uct_component_open(void)
         }
     }
 
+    if (mca_btl_uct_component.num_contexts_per_module > MCA_BTL_UCT_MAX_WORKERS) {
+        mca_btl_uct_component.num_contexts_per_module = MCA_BTL_UCT_MAX_WORKERS;
+    }
+
+    if (mca_btl_uct_component.disable_ucx_memory_hooks) {
+        ucm_set_external_event(UCM_EVENT_VM_UNMAPPED);
+        opal_mem_hooks_register_release(mca_btl_uct_mem_release_cb, NULL);
+    }
+
     return OPAL_SUCCESS;
 }
 
@@ -123,6 +141,10 @@ static int mca_btl_uct_component_open(void)
  */
 static int mca_btl_uct_component_close(void)
 {
+    if (mca_btl_uct_component.disable_ucx_memory_hooks) {
+        opal_mem_hooks_unregister_release (mca_btl_uct_mem_release_cb);
+    }
+
     return OPAL_SUCCESS;
 }
 
@@ -249,7 +271,6 @@ static mca_btl_uct_module_t *mca_btl_uct_alloc_module (const char *md_name, mca_
     OBJ_CONSTRUCT(&module->short_frags, opal_free_list_t);
     OBJ_CONSTRUCT(&module->eager_frags, opal_free_list_t);
     OBJ_CONSTRUCT(&module->max_frags, opal_free_list_t);
-    OBJ_CONSTRUCT(&module->rdma_completions, opal_free_list_t);
     OBJ_CONSTRUCT(&module->pending_frags, opal_list_t);
     OBJ_CONSTRUCT(&module->lock, opal_mutex_t);
 
diff --git a/opal/mca/btl/uct/btl_uct_device_context.h b/opal/mca/btl/uct/btl_uct_device_context.h
@@ -23,7 +23,7 @@
  * @param[in] tl         btl uct tl pointer
  * @param[in] context_id identifier for this context (0..MCA_BTL_UCT_MAX_WORKERS-1)
  */
-mca_btl_uct_device_context_t *mca_btl_uct_context_create (mca_btl_uct_module_t *module, mca_btl_uct_tl_t *tl, int context_id);
+mca_btl_uct_device_context_t *mca_btl_uct_context_create (mca_btl_uct_module_t *module, mca_btl_uct_tl_t *tl, int context_id, bool enable_progress);
 
 /**
  * @brief Destroy a device context and release all resources
@@ -91,8 +91,9 @@ mca_btl_uct_module_get_tl_context_specific (mca_btl_uct_module_t *module, mca_bt
     if (OPAL_UNLIKELY(NULL == context)) {
         mca_btl_uct_device_context_t *new_context;
 
-        new_context = mca_btl_uct_context_create (module, tl, context_id);
-        if (!opal_atomic_compare_exchange_strong_ptr ((opal_atomic_intptr_t *) &tl->uct_dev_contexts[context_id], &context, new_context)) {
+        new_context = mca_btl_uct_context_create (module, tl, context_id, true);
+        if (!opal_atomic_compare_exchange_strong_ptr ((opal_atomic_intptr_t *) &tl->uct_dev_contexts[context_id],
+                                                      (intptr_t *) &context, (intptr_t) new_context)) {
             mca_btl_uct_context_destroy (new_context);
         } else {
             context = new_context;
diff --git a/opal/mca/btl/uct/btl_uct_module.c b/opal/mca/btl/uct/btl_uct_module.c
@@ -31,15 +31,6 @@
 #include "btl_uct_endpoint.h"
 #include "btl_uct_am.h"
 
-#include "opal/memoryhooks/memory.h"
-#include "opal/mca/memory/base/base.h"
-#include <ucm/api/ucm.h>
-
-static void mca_btl_uct_mem_release_cb(void *buf, size_t length, void *cbdata, bool from_alloc)
-{
-    ucm_vm_munmap(buf, length);
-}
-
 struct mca_btl_base_endpoint_t *mca_btl_uct_get_ep (struct mca_btl_base_module_t *module, opal_proc_t *proc)
 {
     mca_btl_uct_module_t *uct_module = (mca_btl_uct_module_t *) module;
@@ -111,18 +102,6 @@ static int mca_btl_uct_add_procs (mca_btl_base_module_t *btl,
                                       NULL, 0, uct_module->rcache, NULL, NULL);
         }
 
-        if (rdma_tl) {
-            rc = opal_free_list_init (&uct_module->rdma_completions, sizeof (mca_btl_uct_uct_completion_t),
-                                      opal_cache_line_size, OBJ_CLASS(mca_btl_uct_uct_completion_t),
-                                      0, opal_cache_line_size, 0, 4096, 128, NULL, 0, NULL, NULL,
-                                      NULL);
-        }
-
-        if (mca_btl_uct_component.disable_ucx_memory_hooks) {
-            ucm_set_external_event(UCM_EVENT_VM_UNMAPPED);
-            opal_mem_hooks_register_release(mca_btl_uct_mem_release_cb, NULL);
-        }
-
         uct_module->initialized = true;
     }
 
@@ -288,10 +267,6 @@ int mca_btl_uct_finalize (mca_btl_base_module_t* btl)
     mca_btl_uct_endpoint_t *endpoint;
     uint64_t key;
 
-    if (mca_btl_uct_component.disable_ucx_memory_hooks) {
-        opal_mem_hooks_unregister_release (mca_btl_uct_mem_release_cb);
-    }
-
     /* clean up any leftover endpoints */
     OPAL_HASH_TABLE_FOREACH(key, uint64, endpoint, &uct_module->id_to_endpoint) {
         OBJ_RELEASE(endpoint);
@@ -300,7 +275,6 @@ int mca_btl_uct_finalize (mca_btl_base_module_t* btl)
     OBJ_DESTRUCT(&uct_module->short_frags);
     OBJ_DESTRUCT(&uct_module->eager_frags);
     OBJ_DESTRUCT(&uct_module->max_frags);
-    OBJ_DESTRUCT(&uct_module->rdma_completions);
     OBJ_DESTRUCT(&uct_module->pending_frags);
     OBJ_DESTRUCT(&uct_module->lock);
 
diff --git a/opal/mca/btl/uct/btl_uct_rdma.c b/opal/mca/btl/uct/btl_uct_rdma.c
@@ -30,13 +30,14 @@ static void mca_btl_uct_uct_completion_construct (mca_btl_uct_uct_completion_t *
 
 OBJ_CLASS_INSTANCE(mca_btl_uct_uct_completion_t, opal_free_list_item_t, mca_btl_uct_uct_completion_construct, NULL);
 
+
 mca_btl_uct_uct_completion_t *
 mca_btl_uct_uct_completion_alloc (mca_btl_uct_module_t *uct_btl, mca_btl_base_endpoint_t *endpoint,
                                   void *local_address, mca_btl_base_registration_handle_t *local_handle,
                                   mca_btl_uct_device_context_t *dev_context, mca_btl_base_rdma_completion_fn_t cbfunc,
                                   void *cbcontext, void *cbdata)
 {
-    mca_btl_uct_uct_completion_t *comp = (mca_btl_uct_uct_completion_t *) opal_free_list_get (&uct_btl->rdma_completions);
+    mca_btl_uct_uct_completion_t *comp = (mca_btl_uct_uct_completion_t *) opal_free_list_get (&dev_context->rdma_completions);
     if (OPAL_LIKELY(NULL != comp)) {
         comp->uct_comp.count = 1;
         comp->btl = &uct_btl->super;
@@ -55,8 +56,7 @@ mca_btl_uct_uct_completion_alloc (mca_btl_uct_module_t *uct_btl, mca_btl_base_en
 void mca_btl_uct_uct_completion_release (mca_btl_uct_uct_completion_t *comp)
 {
     if (comp) {
-        mca_btl_uct_module_t *uct_btl = (mca_btl_uct_module_t *) comp->btl;
-        opal_free_list_return (&uct_btl->rdma_completions, &comp->super);
+        opal_free_list_return (&comp->dev_context->rdma_completions, &comp->super);
     }
 }
 
@@ -122,6 +122,8 @@ int mca_btl_uct_get (mca_btl_base_module_t *btl, mca_btl_base_endpoint_t *endpoi
         mca_btl_uct_uct_completion_release (comp);
     } else if (UCS_INPROGRESS == ucs_status) {
         ucs_status = UCS_OK;
+    } else {
+        mca_btl_uct_uct_completion_release (comp);
     }
 
     BTL_VERBOSE(("get issued. status = %d", ucs_status));
@@ -157,6 +159,8 @@ int mca_btl_uct_put (mca_btl_base_module_t *btl, mca_btl_base_endpoint_t *endpoi
     ucs_status_t ucs_status;
     uct_rkey_bundle_t rkey;
     uct_ep_h ep_handle;
+    bool use_short = false;
+    bool use_bcopy = false;
     int rc;
 
     BTL_VERBOSE(("performing put operation. local address: %p, length: %lu", local_address, (unsigned long) size));
@@ -177,12 +181,19 @@ int mca_btl_uct_put (mca_btl_base_module_t *btl, mca_btl_base_endpoint_t *endpoi
 
     mca_btl_uct_context_lock (context);
 
+    /* determine what UCT prototol should be used */
+    if (size <= uct_btl->super.btl_put_local_registration_threshold) {
+        use_short = size <= uct_btl->rdma_tl->uct_iface_attr.cap.put.max_short;
+        use_bcopy = !use_short;
+    }
+
     do {
-        if (size <= uct_btl->rdma_tl->uct_iface_attr.cap.put.max_short) {
+        if (use_short) {
             ucs_status = uct_ep_put_short (ep_handle, local_address, size, remote_address, rkey.rkey);
-        } else if (size <= uct_btl->super.btl_put_local_registration_threshold) {
+        } else if (use_bcopy) {
             ssize_t tmp = uct_ep_put_bcopy (ep_handle, mca_btl_uct_put_pack,
-                                            &(mca_btl_uct_put_pack_args_t) {.local_address = local_address, .size = size},
+                                            &(mca_btl_uct_put_pack_args_t) {.local_address = local_address,
+                                                    .size = size},
                                             remote_address, rkey.rkey);
             ucs_status = (tmp == (ssize_t) size) ? UCS_OK : UCS_ERR_NO_RESOURCE;
         } else {
diff --git a/opal/mca/btl/uct/btl_uct_tl.c b/opal/mca/btl/uct/btl_uct_tl.c
diff --git a/opal/mca/btl/uct/btl_uct_types.h b/opal/mca/btl/uct/btl_uct_types.h
