ofi: Delay monitor initialization

Delay initializing the import memory monitor from component
open to a function which should be called immediately before
the Libfabric domain is initialized.  Registering the import
memory monitor requires initializing the patcher memory hooks,
which is not necessary if the OFI fabrics were not selected.

Signed-off-by: Brian Barrett <[email protected]>

Author: bwbarrett

ompi/mca/mtl/ofi/mtl_ofi_component.c

@@ -887,6 +887,20 @@ ompi_mtl_ofi_component_init(bool enable_progress_threads,
         }
     }
 
+    /* this must be called during single threaded part of the code and
+     * before Libfabric configures its memory monitors.  Easiest to do
+     * that before domain open.  Silently ignore not-supported errors,
+     * as they are not critical to program correctness, but only
+     * indicate that LIbfabric will have to pick a different, possibly
+     * less optimial, monitor. */
+    ret = opal_common_ofi_inject_memory_monitor();
+    if (0 != ret && -FI_ENOSYS != ret) {
+        opal_output_verbose(1, opal_common_ofi.output,
+                            "Failed to inject Libfabric memory monitor: %s",
+                             fi_strerror(-ret));
+    }
+
+
     /**
      * Open fabric
      * The getinfo struct returns a fabric attribute struct that can be used to

opal/mca/btl/ofi/btl_ofi_component.c

@@ -446,6 +446,19 @@ static int mca_btl_ofi_init_device(struct fi_info *info)
      * to prevent races. */
     mca_btl_ofi_rcache_init(module);
 
+    /* for similar reasons to the rcache call, this must be called
+     * during single threaded part of the code and before Libfabric
+     * configures its memory monitors.  Easiest to do that before
+     * domain open.  Silently ignore not-supported errors, as they
+     * are not critical to program correctness, but only indicate
+     * that LIbfabric will have to pick a different, possibly less
+     * optimial, monitor. */
+    rc = opal_common_ofi_inject_memory_monitor();
+    if (0 != rc && -FI_ENOSYS != rc) {
+        BTL_VERBOSE(("Failed to inject Libfabric memory monitor: %s",
+                     fi_strerror(-rc)));
+    }
+
     linux_device_name = info->domain_attr->name;
     BTL_VERBOSE(
         ("initializing dev:%s provider:%s", linux_device_name, info->fabric_attr->prov_name));

opal/mca/common/ofi/common_ofi.c

@@ -36,20 +36,26 @@
 #include "opal/util/show_help.h"
 
 opal_common_ofi_module_t opal_common_ofi = {.prov_include = NULL,
-					    .prov_exclude = NULL,
-					    .registered = 0,
-					    .verbose = 0};
-
+                                            .prov_exclude = NULL,
+                                            .output = -1};
 static const char default_prov_exclude_list[] = "shm,sockets,tcp,udp,rstream,usnic";
 static opal_mutex_t opal_common_ofi_mutex = OPAL_MUTEX_STATIC_INIT;
-static bool opal_common_ofi_initialized = false;
 static int opal_common_ofi_init_ref_cnt = 0;
+static bool opal_common_ofi_installed_memory_monitor = false;
 
 #ifdef HAVE_STRUCT_FI_OPS_MEM_MONITOR
 
 /*
- * These no-op functions are necessary since libfabric does not allow null
- * function pointers here.
+ * Monitor object to inject into Libfabric to provide memory release
+ * notifications using our own memory hooks framework.  Monitors may
+ * use the subscribe/unsubscribe notifications to reduce unnecessary
+ * notifications, but are not required to do so.  Because patcher
+ * notifies about all releases, it is cheaper for us to not filter and
+ * this monitor can safely ignore subscribe/unsubscribe notifications.
+ *
+ * Libfabric requires the object to be fully defined.  Unlike most of
+ * Open MPI, it does not have NULL function pointer checks in calling
+ * code.
  */
 static int opal_common_ofi_monitor_start(struct fid_mem_monitor *monitor)
 {
@@ -80,7 +86,7 @@ static bool opal_common_ofi_monitor_valid(struct fid_mem_monitor *monitor,
 }
 
 static struct fid_mem_monitor *opal_common_ofi_monitor;
-static struct fid *opal_common_ofi_cache_fid;
+static struct fid *opal_common_ofi_cache_fid = NULL;
 static struct fi_ops_mem_monitor opal_common_ofi_export_ops = {
     .size = sizeof(struct fi_ops_mem_monitor),
     .start = opal_common_ofi_monitor_start,
@@ -90,6 +96,12 @@ static struct fi_ops_mem_monitor opal_common_ofi_export_ops = {
     .valid = opal_common_ofi_monitor_valid,
 };
 
+/**
+ * Callback function from Open MPI memory monitor
+ *
+ * Translation function between the callback function from Open MPI's
+ * memory notifier to the Libfabric memory monitor.
+ */
 static void opal_common_ofi_mem_release_cb(void *buf, size_t length,
                                            void *cbdata, bool from_alloc)
 {
@@ -99,87 +111,131 @@ static void opal_common_ofi_mem_release_cb(void *buf, size_t length,
 
 #endif /* HAVE_STRUCT_FI_OPS_MEM_MONITOR */
 
-int opal_common_ofi_open(void)
+int opal_common_ofi_inject_memory_monitor(void)
 {
-    int ret;
+    int ret = -FI_ENOSYS;
 
-    opal_common_ofi_init_ref_cnt++;
-    if (opal_common_ofi_initialized) {
-        return OPAL_SUCCESS;
-    }
 #ifdef HAVE_STRUCT_FI_OPS_MEM_MONITOR
+    uint32_t version;
 
-    mca_base_framework_open(&opal_memory_base_framework, 0);
+    OPAL_THREAD_LOCK(&opal_common_ofi_mutex);
+
+    if (opal_common_ofi_cache_fid != NULL) {
+        return 0;
+    }
+
+    /*
+     * While the memory import functionality was introduced in 1.13,
+     * some deadlock bugs exist in the 1.13 series.  Require version
+     * 1.14 before this code is activated.  Not activating the code
+     * should not break any functionality directly, but may lead to
+     * sub-optimal memory monitors being used in Libfabric, as Open
+     * MPI will almost certainly install a patcher first.
+     */
+    version = fi_version();
+    if (FI_VERSION_LT(version, FI_VERSION(1, 14))) {
+        ret = -FI_ENOSYS;
+        goto err;
+    }
+
+    ret = mca_base_framework_open(&opal_memory_base_framework, 0);
+    if (OPAL_SUCCESS != ret) {
+        ret = -FI_ENOSYS;
+	goto err;
+    }
     if ((OPAL_MEMORY_FREE_SUPPORT | OPAL_MEMORY_MUNMAP_SUPPORT)
         != (((OPAL_MEMORY_FREE_SUPPORT | OPAL_MEMORY_MUNMAP_SUPPORT))
         & opal_mem_hooks_support_level())) {
-        return OPAL_SUCCESS;
+        ret = -FI_ENOSYS;
+	goto err;
     }
 
     /*
-     * This cache object doesn't do much, but is necessary for the API to work.
-     * It is required to call the fi_import_fid API. This API was introduced in
-     * libfabric version 1.13.0 and "mr_cache" is a "well known" name (documented
-     * in libfabric) to indicate the type of object that we are trying to open.
+     * The monitor inject object has the well known name "mr_cache"
+     * and was introduced in Libfabric 1.13
      */
-    ret = fi_open(FI_VERSION(1,13), "mr_cache", NULL, 0, 0, &opal_common_ofi_cache_fid, NULL);
-    if (ret) {
+    ret = fi_open(FI_VERSION(1,13), "mr_cache", NULL, 0, 0,
+		  &opal_common_ofi_cache_fid, NULL);
+    if (0 != ret) {
         goto err;
     }
 
     opal_common_ofi_monitor = calloc(1, sizeof(*opal_common_ofi_monitor));
-    if (!opal_common_ofi_monitor) {
+    if (NULL == opal_common_ofi_monitor) {
+        ret = -FI_ENOMEM;
         goto err;
     }
 
     opal_common_ofi_monitor->fid.fclass = FI_CLASS_MEM_MONITOR;
     opal_common_ofi_monitor->export_ops = &opal_common_ofi_export_ops;
-    /*
-     * This import_fid call must occur before the libfabric provider creates
-     * its memory registration cache. This will typically occur during domain
-     * open as it is a domain level object. We put it early in initialization
-     * to guarantee this and share the import monitor between the ofi btl
-     * and ofi mtl.
-     */
-    ret = fi_import_fid(opal_common_ofi_cache_fid, &opal_common_ofi_monitor->fid, 0);
-    if (ret) {
+    ret = fi_import_fid(opal_common_ofi_cache_fid,
+			&opal_common_ofi_monitor->fid, 0);
+    if (0 != ret) {
         goto err;
     }
     opal_mem_hooks_register_release(opal_common_ofi_mem_release_cb, NULL);
-    opal_common_ofi_initialized = true;
+    opal_common_ofi_installed_memory_monitor = true;
+
+    ret = 0;
 
-    return OPAL_SUCCESS;
 err:
-    if (opal_common_ofi_cache_fid) {
-        fi_close(opal_common_ofi_cache_fid);
-    }
-    if (opal_common_ofi_monitor) {
-        free(opal_common_ofi_monitor);
+    if (0 != ret) {
+        if (opal_common_ofi_cache_fid) {
+            fi_close(opal_common_ofi_cache_fid);
+        }
+        if (opal_common_ofi_monitor) {
+            free(opal_common_ofi_monitor);
+        }
     }
 
-    return OPAL_ERROR;
-#else
-    opal_common_ofi_initialized = true;
-    return OPAL_SUCCESS;
+    OPAL_THREAD_UNLOCK(&opal_common_ofi_mutex);
 #endif
+
+    return ret;
 }
 
-int opal_common_ofi_close(void)
+static int opal_common_ofi_remove_memory_monitor(void)
 {
-    if (opal_common_ofi_initialized && !--opal_common_ofi_init_ref_cnt) {
 #ifdef HAVE_STRUCT_FI_OPS_MEM_MONITOR
+    if (opal_common_ofi_installed_memory_monitor) {
         opal_mem_hooks_unregister_release(opal_common_ofi_mem_release_cb);
         fi_close(opal_common_ofi_cache_fid);
         fi_close(&opal_common_ofi_monitor->fid);
         free(opal_common_ofi_monitor);
+	opal_common_ofi_installed_memory_monitor = false;
+    }
 #endif
-        opal_common_ofi_initialized = false;
+
+    return OPAL_SUCCESS;
+}
+
+int opal_common_ofi_open(void)
+{
+    if ((opal_common_ofi_init_ref_cnt++) > 0) {
+        return OPAL_SUCCESS;
     }
 
-    opal_common_ofi.registered--;
-    assert(opal_common_ofi.registered >= 0);
-    if (opal_common_ofi.registered == 0) {
-      opal_output_close(opal_common_ofi.output);
+    return OPAL_SUCCESS;
+}
+
+int opal_common_ofi_close(void)
+{
+    int ret;
+
+    if ((--opal_common_ofi_init_ref_cnt) > 0) {
+        return OPAL_SUCCESS;
+    }
+
+    ret = opal_common_ofi_remove_memory_monitor();
+    if (ret != OPAL_SUCCESS) {
+        return ret;
+    }
+
+    if (opal_common_ofi.output != -1) {
+        opal_output_close(opal_common_ofi.output);
+        if (ret != OPAL_SUCCESS) {
+            return ret;
+        }
     }
 
     return OPAL_SUCCESS;
@@ -209,18 +265,13 @@ int opal_common_ofi_mca_register(const mca_base_component_t *component)
     static int include_index;
     static int exclude_index;
     static int verbose_index;
+    int verbose;
     int param;
 
     if (fi_version() < FI_VERSION(1, 0)) {
         return OPAL_ERROR;
     }
 
-    opal_common_ofi.registered++;
-    if (opal_common_ofi.registered == 1) {
-      opal_common_ofi.output = opal_output_open(NULL);
-      opal_output_set_verbosity(opal_common_ofi.output, opal_common_ofi.verbose);
-    }
-
     OPAL_THREAD_LOCK(&opal_common_ofi_mutex);
 
     param = mca_base_var_find("opal", "opal_common", "ofi", "provider_include");
@@ -272,7 +323,7 @@ int opal_common_ofi_mca_register(const mca_base_component_t *component)
                                               MCA_BASE_VAR_TYPE_INT, NULL, 0,
                                               MCA_BASE_VAR_FLAG_SETTABLE, OPAL_INFO_LVL_3,
                                               MCA_BASE_VAR_SCOPE_LOCAL,
-                                              &opal_common_ofi.verbose);
+                                              &verbose);
     } else {
         verbose_index = param;
     }
@@ -289,6 +340,11 @@ int opal_common_ofi_mca_register(const mca_base_component_t *component)
                                       "verbose", 0);
     }
 
+    if (opal_common_ofi.output == -1) {
+        opal_common_ofi.output = opal_output_open(NULL);
+	opal_output_set_verbosity(opal_common_ofi.output, verbose);
+    }
+
     OPAL_THREAD_UNLOCK(&opal_common_ofi_mutex);
 
     return OPAL_SUCCESS;

opal/mca/common/ofi/common_ofi.h

@@ -26,8 +26,6 @@ BEGIN_C_DECLS
 typedef struct opal_common_ofi_module {
     char **prov_include;
     char **prov_exclude;
-    int verbose;
-    int registered;
     int output;
 } opal_common_ofi_module_t;
 
@@ -69,6 +67,18 @@ OPAL_DECLSPEC int opal_common_ofi_open(void);
  */
 OPAL_DECLSPEC int opal_common_ofi_close(void);
 
+/**
+ * Inject our memory hooks into Libfabric monitor
+ *
+ * Use Open MPI's memory hooks to provide monitor notifications to
+ * Libfabric via the external mr_cache facility.  This must be called
+ * before any domain is initialized (ie, before any Libfabric memory
+ * monitor is configured).
+ *
+ * @returns A libfabric error code is returned on error
+ */
+OPAL_DECLSPEC int opal_common_ofi_inject_memory_monitor(void);
+
 /**
  * Search function for provider names
  *