oshmem: memheap: refactor component selection code

alex-mikheev · alex-mikheev · commit 83c2ab76a59e · 2017-01-16T13:48:58.000+02:00
Do not call component's init function until the component has been
selected.

Use mca_base_select() instead of the custom component selection code.

Signed-off-by: Alex Mikheev &lt;alexm@mellanox.com&gt;
diff --git a/oshmem/mca/memheap/base/base.h b/oshmem/mca/memheap/base/base.h
@@ -31,16 +31,13 @@ OSHMEM_DECLSPEC int mca_memheap_base_select(void);
 /*
  * Globals
  */
-OSHMEM_DECLSPEC extern struct mca_memheap_base_module_t* mca_memheap_base_module_initialized;
 
 /* only used within base -- no need to DECLSPEC */
 #define MEMHEAP_BASE_MIN_ORDER         3                                /* forces 64 bit alignment */
 #define MEMHEAP_BASE_PAGE_ORDER        21
 #define MEMHEAP_BASE_PRIVATE_SIZE      (1ULL << MEMHEAP_BASE_PAGE_ORDER) /* should be at least the same as a huge page size */
 #define MEMHEAP_BASE_MIN_SIZE          (1ULL << MEMHEAP_BASE_PAGE_ORDER)    /* must fit into at least one huge page */
 
-extern char* mca_memheap_base_include;
-extern char* mca_memheap_base_exclude;
 extern int mca_memheap_base_already_opened;
 extern int mca_memheap_base_key_exchange;
 
diff --git a/oshmem/mca/memheap/base/memheap_base_frame.c b/oshmem/mca/memheap/base/memheap_base_frame.c
@@ -33,10 +33,7 @@
 
 int mca_memheap_base_output = -1;
 int mca_memheap_base_key_exchange = 1;
-char* mca_memheap_base_include = NULL;
-char* mca_memheap_base_exclude = NULL;
 opal_list_t mca_memheap_base_components_opened = {{0}};
-struct mca_memheap_base_module_t* mca_memheap_base_module_initialized = NULL;
 int mca_memheap_base_already_opened = 0;
 mca_memheap_map_t mca_memheap_base_map = {{{{0}}}};
 
@@ -55,39 +52,6 @@ static int mca_memheap_base_register(mca_base_register_flag_t flags)
                                  MCA_BASE_VAR_SCOPE_READONLY,
                                  &mca_memheap_base_key_exchange);
 
-    (void) mca_base_var_register("oshmem",
-                                 "memheap",
-                                 "base",
-                                 "include",
-                                 "Specify a specific MEMHEAP implementation to use",
-                                 MCA_BASE_VAR_TYPE_STRING,
-                                 NULL,
-                                 0,
-                                 MCA_BASE_VAR_FLAG_SETTABLE,
-                                 OPAL_INFO_LVL_9,
-                                 MCA_BASE_VAR_SCOPE_READONLY,
-                                 &mca_memheap_base_include);
-
-    if (NULL == mca_memheap_base_include) {
-        mca_memheap_base_include = getenv(SHMEM_HEAP_TYPE);
-        if (NULL == mca_memheap_base_include)
-            mca_memheap_base_include = strdup("");
-        else
-            mca_memheap_base_include = strdup(mca_memheap_base_include);
-    }
-
-    (void) mca_base_var_register("oshmem",
-                                 "memheap",
-                                 "base",
-                                 "exclude",
-                                 "Specify excluded MEMHEAP implementations",
-                                 MCA_BASE_VAR_TYPE_STRING,
-                                 NULL,
-                                 0,
-                                 MCA_BASE_VAR_FLAG_SETTABLE,
-                                 OPAL_INFO_LVL_9,
-                                 MCA_BASE_VAR_SCOPE_READONLY,
-                                 &mca_memheap_base_exclude);
 
     return OSHMEM_SUCCESS;
 }
diff --git a/oshmem/mca/memheap/base/memheap_base_select.c b/oshmem/mca/memheap/base/memheap_base_select.c
@@ -45,133 +45,43 @@ static memheap_context_t* _memheap_create(void);
  */
 int mca_memheap_base_select()
 {
-    int priority = 0;
-    int max_priority = 0;
-    mca_base_component_list_item_t *cli, *next;
-    mca_memheap_base_component_t *component = NULL;
-    mca_memheap_base_component_t *max_priority_component = NULL;
-    mca_memheap_base_module_t *module = NULL;
-    memheap_context_t *context = NULL;
-
-    char** include = opal_argv_split(mca_memheap_base_include, ',');
-    char** exclude = opal_argv_split(mca_memheap_base_exclude, ',');
-
-    context = _memheap_create();
-    if (!context) {
-        opal_argv_free(include);
-        opal_argv_free(exclude);
+    int best_priority;
+    memheap_context_t *context;
+    mca_memheap_base_component_t *best_component = NULL;
+    mca_memheap_base_module_t *best_module = NULL;
+
+    if( OPAL_SUCCESS != mca_base_select("memheap", oshmem_memheap_base_framework.framework_output,
+                                        &oshmem_memheap_base_framework.framework_components,
+                                        (mca_base_module_t **) &best_module,
+                                        (mca_base_component_t **) &best_component,
+                                        &best_priority) ) {
         return OSHMEM_ERROR;
     }
 
-    OPAL_LIST_FOREACH_SAFE(cli, next, &oshmem_memheap_base_framework.framework_components, mca_base_component_list_item_t) {
-        component = (mca_memheap_base_component_t *) cli->cli_component;
-
-        /* Verify if the component is in the include or the exclude list. */
-        /* If there is an include list - item must be in the list to be included */
-        if (NULL != include) {
-            char** argv = include;
-            bool found = false;
-            while (argv && *argv) {
-                if (strcmp(component->memheap_version.mca_component_name, *argv)
-                        == 0) {
-                    found = true;
-                    break;
-                }
-                argv++;
-            }
-            /* If not in the list do not choose this component */
-            if (found == false) {
-                continue;
-            }
-
-            /* Otherwise - check the exclude list to see if this item has been specifically excluded */
-        } else if (NULL != exclude) {
-            char** argv = exclude;
-            bool found = false;
-            while (argv && *argv) {
-                if (strcmp(component->memheap_version.mca_component_name, *argv)
-                        == 0) {
-                    found = true;
-                    break;
-                }
-                argv++;
-            }
-            if (found == true) {
-                continue;
-            }
-        }
-
-        /* Verify that the component has an init function */
-        if (NULL == component->memheap_init) {
-            MEMHEAP_VERBOSE(10,
-                            "select: no init function; for component %s. No component selected",
-                            component->memheap_version.mca_component_name);
-        } else {
-
-            MEMHEAP_VERBOSE(5,
-                            "select: component %s size : user %d private: %d",
-                            component->memheap_version.mca_component_name, (int)context->user_size, (int)context->private_size);
-
-            /* Init the component in order to get its priority */
-            module = component->memheap_init(context, &priority);
-
-            /* If the component didn't initialize, remove it from the opened                                                                                                list, remove it from the component repository and return an error */
-            if (NULL == module) {
-                MEMHEAP_VERBOSE(10,
-                                "select: init of component %s returned failure",
-                                component->memheap_version.mca_component_name);
-
-                opal_list_remove_item(&oshmem_memheap_base_framework.framework_components, &cli->super);
-                mca_base_component_close((mca_base_component_t *) component,
-                                         oshmem_memheap_base_framework.framework_output);
-            } else {
-                /* Calculate memheap size in case it was not set during component initialization */
-                module->memheap_size = context->user_size;
-            }
-        }
-
-        /* Init max priority component */
-        if (NULL == max_priority_component) {
-            max_priority_component = component;
-            mca_memheap_base_module_initialized = module;
-            max_priority = priority;
-        }
-
-        /* Update max priority component if current component has greater priority */
-        if (priority > max_priority) {
-            max_priority = priority;
-            max_priority_component = component;
-            mca_memheap_base_module_initialized = module;
-        }
-    }
-
-    opal_argv_free(include);
-    opal_argv_free(exclude);
-
-    /* Verify that a component was selected */
-    if (NULL == max_priority_component) {
-        MEMHEAP_VERBOSE(10, "select: no component selected");
+    context = _memheap_create();
+    if (NULL == context) {
         return OSHMEM_ERROR;
     }
 
-    /* Verify that some module was initialized */
-    if (NULL == mca_memheap_base_module_initialized) {
+    if (OSHMEM_SUCCESS != best_component->memheap_init(context)) {
         opal_show_help("help-oshmem-memheap.txt",
                        "find-available:none-found",
                        true,
                        "memheap");
-        orte_errmgr.abort(1, NULL );
+        return OSHMEM_ERROR;
     }
 
-    MEMHEAP_VERBOSE(10,
-                    "SELECTED %s component %s",
-                    max_priority_component->memheap_version.mca_type_name, max_priority_component->memheap_version.mca_component_name);
-
+    /* Calculate memheap size in case it was not set during component initialization */
+    best_module->memheap_size = context->user_size;
     setenv(SHMEM_HEAP_TYPE,
-           max_priority_component->memheap_version.mca_component_name,
-           1);
+           best_component->memheap_version.mca_component_name, 1);
 
-    mca_memheap = *mca_memheap_base_module_initialized;
+    mca_memheap = *best_module;
+
+    MEMHEAP_VERBOSE(10,
+                    "SELECTED %s component %s",
+                    best_component->memheap_version.mca_type_name, 
+                    best_component->memheap_version.mca_component_name);
 
     return OSHMEM_SUCCESS;
 }
diff --git a/oshmem/mca/memheap/buddy/memheap_buddy_component.c b/oshmem/mca/memheap/buddy/memheap_buddy_component.c
@@ -18,8 +18,7 @@
 #include "memheap_buddy_component.h"
 
 static int mca_memheap_buddy_component_close(void);
-static mca_memheap_base_module_t* mca_memheap_buddy_component_init(memheap_context_t *,
-                                                                   int *);
+static int mca_memheap_buddy_component_query(mca_base_module_t **module, int *priority);
 
 static int _basic_open(void);
 
@@ -33,12 +32,13 @@ mca_memheap_base_component_t mca_memheap_buddy_component = {
 
         .mca_open_component = _basic_open,
         .mca_close_component = mca_memheap_buddy_component_close,
+        .mca_query_component = mca_memheap_buddy_component_query,
     },
     .memheap_data = {
         /* The component is checkpoint ready */
         MCA_BASE_METADATA_PARAM_CHECKPOINT
     },
-    .memheap_init = mca_memheap_buddy_component_init,
+    .memheap_init = mca_memheap_buddy_module_init
 };
 
 /* Open component */
@@ -47,19 +47,13 @@ static int _basic_open(void)
     return OSHMEM_SUCCESS;
 }
 
-/* Initialize component */
-mca_memheap_base_module_t* mca_memheap_buddy_component_init(memheap_context_t *context,
-                                                            int *priority)
+/* query component */
+static int
+mca_memheap_buddy_component_query(mca_base_module_t **module, int *priority)
 {
-    int rc;
-
     *priority = memheap_buddy.priority;
-    rc = mca_memheap_buddy_module_init(context);
-    if (OSHMEM_SUCCESS != rc) {
-        return NULL ;
-    }
-
-    return &(memheap_buddy.super);
+    *module = (mca_base_module_t *)&memheap_buddy.super;
+    return OSHMEM_SUCCESS;
 }
 
 /*
diff --git a/oshmem/mca/memheap/memheap.h b/oshmem/mca/memheap/memheap.h
@@ -35,8 +35,7 @@ typedef struct memheap_context
 /**
  * Component initialize
  */
-typedef struct mca_memheap_base_module_t* (*mca_memheap_base_component_init_fn_t)(memheap_context_t *,
-                                                                                  int *priority);
+typedef int (*mca_memheap_base_component_init_fn_t)(memheap_context_t *);
 
 /*
  * Symmetric heap allocation. Malloc like interface
diff --git a/oshmem/mca/memheap/ptmalloc/memheap_ptmalloc_component.c b/oshmem/mca/memheap/ptmalloc/memheap_ptmalloc_component.c
@@ -18,8 +18,8 @@
 #include "memheap_ptmalloc_component.h"
 
 static int mca_memheap_ptmalloc_component_close(void);
-static mca_memheap_base_module_t* mca_memheap_ptmalloc_component_init(memheap_context_t *,
-                                                                      int *);
+static int mca_memheap_ptmalloc_component_query(mca_base_module_t **module, 
+                                                int *priority);
 
 static int _basic_open(void);
 
@@ -33,12 +33,13 @@ mca_memheap_base_component_t mca_memheap_ptmalloc_component = {
 
         .mca_open_component = _basic_open,
         .mca_close_component = mca_memheap_ptmalloc_component_close,
+        .mca_query_component = mca_memheap_ptmalloc_component_query,
     },
     .memheap_data = {
         /* The component is checkpoint ready */
         MCA_BASE_METADATA_PARAM_CHECKPOINT
     },
-    .memheap_init = mca_memheap_ptmalloc_component_init,
+    .memheap_init = mca_memheap_ptmalloc_module_init,
 };
 
 /* Open component */
@@ -47,19 +48,13 @@ static int _basic_open(void)
     return OSHMEM_SUCCESS;
 }
 
-/* Initialize component */
-mca_memheap_base_module_t* mca_memheap_ptmalloc_component_init(memheap_context_t *context,
-                                                               int *priority)
+/* query component */
+static int
+mca_memheap_ptmalloc_component_query(mca_base_module_t **module, int *priority)
 {
-    int rc;
-
     *priority = memheap_ptmalloc.priority;
-    rc = mca_memheap_ptmalloc_module_init(context);
-    if (OSHMEM_SUCCESS != rc) {
-        return NULL ;
-    }
-
-    return &(memheap_ptmalloc.super);
+    *module = (mca_base_module_t *)&memheap_ptmalloc.super;
+    return OSHMEM_SUCCESS;
 }
 
 /*
