sessions: support MPI_TAG_UB

related to open-mpi#9097 test https://github.com/open-mpi/ompi-tests-public/blob/master/sessions/sessions_test16.c now passes. Signed-off-by: Howard Pritchard <[email protected]>
wckzhang · May 10, 2022 · 35dbfe0 · 35dbfe0
1 parent 90a0550
commit 35dbfe0
Show file tree

Hide file tree

Showing 5 changed files with 110 additions and 45 deletions.
diff --git a/ompi/attribute/attribute.h b/ompi/attribute/attribute.h
@@ -515,11 +515,22 @@ int ompi_attr_delete_all(ompi_attribute_type_t type, void *object,
 /**
  * \internal
  *
- * Create all the predefined attributes
+ * Create all the predefined attribute keys
+ * @note This routine is invoked when creating a session 
+ *       so must be thread safe.
  *
  * @returns OMPI_SUCCESS
  */
-int ompi_attr_create_predefined(void);
+int ompi_attr_create_predefined_keyvals(void);
+
+/**
+ * \internal
+ *
+ * Cache predefined attribute keys used in the World Model
+ *
+ * @returns OMPI_SUCCESS
+ */
+int ompi_attr_set_predefined_keyvals_for_wm(void);
 
 /**
  * \internal

diff --git a/ompi/attribute/attribute_predefined.c b/ompi/attribute/attribute_predefined.c
@@ -16,6 +16,8 @@
  * Copyright (c) 2020      Intel, Inc.  All rights reserved.
  * Copyright (c) 2022      Amazon.com, Inc. or its affiliates.
  *                         All Rights reserved.
+ * Copyright (c) 2022      Triad National Security, LLC. All rights
+ *                         reserved.
  * $COPYRIGHT$
  *
  * Additional copyrights may follow
@@ -95,6 +97,8 @@
 #include "ompi/mca/pml/pml.h"
 #include "ompi/runtime/ompi_rte.h"
 
+static bool attrs_predefined_initialized = false;
+
 /*
  * Private functions
  */
@@ -106,33 +110,53 @@ static int free_win(int keyval);
 
 static int set_f(int keyval, MPI_Fint value);
 
-
-int ompi_attr_create_predefined(void)
+/*
+ * We do not need a lock here as this function is invoked when the 
+ * instance_lock mutex is held.
+ */
+int ompi_attr_create_predefined_keyvals(void)
 {
-    int ret;
-
-    /* Create all the keyvals */
-
-    /* DO NOT CHANGE THE ORDER OF CREATING THESE KEYVALS!  This order
-       strictly adheres to the order in mpi.h.  If you change the
-       order here, you must change the order in mpi.h as well! */
-
-    if (OMPI_SUCCESS != (ret = create_comm(MPI_TAG_UB, true)) ||
-        OMPI_SUCCESS != (ret = create_comm(MPI_HOST, true)) ||
-        OMPI_SUCCESS != (ret = create_comm(MPI_IO, true)) ||
-        OMPI_SUCCESS != (ret = create_comm(MPI_WTIME_IS_GLOBAL, true)) ||
-        OMPI_SUCCESS != (ret = create_comm(MPI_APPNUM, true)) ||
-        OMPI_SUCCESS != (ret = create_comm(MPI_LASTUSEDCODE, false)) ||
-        OMPI_SUCCESS != (ret = create_comm(MPI_UNIVERSE_SIZE, true)) ||
-        OMPI_SUCCESS != (ret = create_win(MPI_WIN_BASE)) ||
-        OMPI_SUCCESS != (ret = create_win(MPI_WIN_SIZE)) ||
-        OMPI_SUCCESS != (ret = create_win(MPI_WIN_DISP_UNIT)) ||
-        OMPI_SUCCESS != (ret = create_win(MPI_WIN_CREATE_FLAVOR)) ||
-        OMPI_SUCCESS != (ret = create_win(MPI_WIN_MODEL)) ||
-        OMPI_SUCCESS != (ret = create_comm(MPI_FT, false)) || /* not #if conditional on OPAL_ENABLE_FT_MPI for ABI */
-        0) {
-        return ret;
+    int ret = OMPI_SUCCESS, rc;
+
+    if (false == attrs_predefined_initialized) {
+
+        attrs_predefined_initialized = true;
+
+        /* Create all the keyvals */
+
+        /* DO NOT CHANGE THE ORDER OF CREATING THESE KEYVALS!  This order
+            strictly adheres to the order in mpi.h.  If you change the
+            order here, you must change the order in mpi.h as well! */
+
+        if (OMPI_SUCCESS != (rc = create_comm(MPI_TAG_UB, true)) ||
+            OMPI_SUCCESS != (rc = create_comm(MPI_HOST, true)) ||
+            OMPI_SUCCESS != (rc = create_comm(MPI_IO, true)) ||
+            OMPI_SUCCESS != (rc = create_comm(MPI_WTIME_IS_GLOBAL, true)) ||
+            OMPI_SUCCESS != (rc = create_comm(MPI_APPNUM, true)) ||
+            OMPI_SUCCESS != (rc = create_comm(MPI_LASTUSEDCODE, false)) ||
+            OMPI_SUCCESS != (rc = create_comm(MPI_UNIVERSE_SIZE, true)) ||
+            OMPI_SUCCESS != (rc = create_win(MPI_WIN_BASE)) ||
+            OMPI_SUCCESS != (rc = create_win(MPI_WIN_SIZE)) ||
+            OMPI_SUCCESS != (rc = create_win(MPI_WIN_DISP_UNIT)) ||
+            OMPI_SUCCESS != (rc = create_win(MPI_WIN_CREATE_FLAVOR)) ||
+            OMPI_SUCCESS != (rc = create_win(MPI_WIN_MODEL)) ||
+            OMPI_SUCCESS != (rc = create_comm(MPI_FT, false)) || /* not #if conditional on OPAL_ENABLE_FT_MPI for ABI */
+            0) {
+            ret = rc;
+        }
+
     }
+
+    return ret;
+}
+
+/*
+ * This method is only invoked during MPI initialization using the world model
+ * (MPI_Init/MPI_Init_thread) so does not need to be thread safe.
+ */
+int ompi_attr_set_predefined_keyvals_for_wm(void)
+{
+    int ret = OMPI_SUCCESS;
 
     /* Set default values for everything except MPI_UNIVERSE_SIZE */
 
@@ -165,26 +189,38 @@ int ompi_attr_create_predefined(void)
 }
 
 
+/*
+ * We do not need a lock here as this function is invoked when the 
+ * destructor for attr_subsys is invoked.
+ */
+
 int ompi_attr_free_predefined(void)
 {
-    int ret;
-
-    if (OMPI_SUCCESS != (ret = free_comm(MPI_TAG_UB)) ||
-        OMPI_SUCCESS != (ret = free_comm(MPI_HOST)) ||
-        OMPI_SUCCESS != (ret = free_comm(MPI_IO)) ||
-        OMPI_SUCCESS != (ret = free_comm(MPI_WTIME_IS_GLOBAL)) ||
-        OMPI_SUCCESS != (ret = free_comm(MPI_APPNUM)) ||
-        OMPI_SUCCESS != (ret = free_comm(MPI_LASTUSEDCODE)) ||
-        OMPI_SUCCESS != (ret = free_comm(MPI_UNIVERSE_SIZE)) ||
-        OMPI_SUCCESS != (ret = free_comm(MPI_FT)) || /* not #if conditional on OPAL_ENABLE_FT_MPI for ABI */
-        OMPI_SUCCESS != (ret = free_win(MPI_WIN_BASE)) ||
-        OMPI_SUCCESS != (ret = free_win(MPI_WIN_SIZE)) ||
-        OMPI_SUCCESS != (ret = free_win(MPI_WIN_DISP_UNIT)) ||
-        OMPI_SUCCESS != (ret = free_win(MPI_WIN_CREATE_FLAVOR)) ||
-        OMPI_SUCCESS != (ret = free_win(MPI_WIN_MODEL))) {
-        return ret;
+    int ret = OMPI_SUCCESS, rc;
+
+    if (true == attrs_predefined_initialized) {
+
+        attrs_predefined_initialized = false;
+
+        if (OMPI_SUCCESS != (rc = free_comm(MPI_TAG_UB)) ||
+            OMPI_SUCCESS != (rc = free_comm(MPI_HOST)) ||
+            OMPI_SUCCESS != (rc = free_comm(MPI_IO)) ||
+            OMPI_SUCCESS != (rc = free_comm(MPI_WTIME_IS_GLOBAL)) ||
+            OMPI_SUCCESS != (rc = free_comm(MPI_APPNUM)) ||
+            OMPI_SUCCESS != (rc = free_comm(MPI_LASTUSEDCODE)) ||
+            OMPI_SUCCESS != (rc = free_comm(MPI_UNIVERSE_SIZE)) ||
+            OMPI_SUCCESS != (rc = free_comm(MPI_FT)) || /* not #if conditional on OPAL_ENABLE_FT_MPI for ABI */
+            OMPI_SUCCESS != (rc = free_win(MPI_WIN_BASE)) ||
+            OMPI_SUCCESS != (rc = free_win(MPI_WIN_SIZE)) ||
+            OMPI_SUCCESS != (rc = free_win(MPI_WIN_DISP_UNIT)) ||
+            OMPI_SUCCESS != (rc = free_win(MPI_WIN_CREATE_FLAVOR)) ||
+            OMPI_SUCCESS != (rc = free_win(MPI_WIN_MODEL))) {
+            ret = rc;
+        }
+
     }
-    return OMPI_SUCCESS;
+
+    return ret;
 }
 
 

diff --git a/ompi/communicator/comm.c b/ompi/communicator/comm.c
@@ -1331,6 +1331,17 @@ int ompi_comm_create_from_group (ompi_group_t *group, const char *tag, opal_info
 
     newcomp->instance = group->grp_instance;
 
+    /*
+     * setup predefined keyvals - see MPI Standard for predefined keyvals cached on 
+     * communicators created via MPI_Comm_from_group or MPI_Intercomm_create_from_groups
+     */
+    ompi_attr_hash_init(&newcomp->c_keyhash);
+    ompi_attr_set_int(COMM_ATTR,
+                      newcomp,
+                      &newcomp->c_keyhash,
+                      MPI_TAG_UB, mca_pml.pml_max_tag,
+                      true);
+
     *newcomm = newcomp;
     return MPI_SUCCESS;
 }

diff --git a/ompi/communicator/comm_init.c b/ompi/communicator/comm_init.c
@@ -292,7 +292,7 @@ int ompi_comm_init_mpi3 (void)
     /*
      * finally here we set the predefined attribute keyvals
      */
-    ompi_attr_create_predefined();
+    ompi_attr_set_predefined_keyvals_for_wm();
 
     OBJ_RETAIN(&ompi_mpi_errors_are_fatal.eh);
     /* During dyn_init, the comm_parent error handler will be set to the same

diff --git a/ompi/instance/instance.c b/ompi/instance/instance.c
@@ -595,6 +595,13 @@ static int ompi_mpi_instance_init_common (void)
         return ompi_instance_print_error ("ompi_comm_init() failed", ret);
     }
 
+    /* Construct predefined keyvals */
+
+    if (OMPI_SUCCESS != (ret = ompi_attr_create_predefined_keyvals())) {
+        opal_mutex_unlock (&instance_lock);
+        return ompi_instance_print_error ("ompi_attr_create_predefined_keyvals() failed", ret);
+    }
+
     if (mca_pml_base_requires_world ()) {
         /* need to set up comm world for this instance -- XXX -- FIXME -- probably won't always
          * be the case. */