Ensure we clear the usage numbers when binding on multiple nodes so we don't "carry over" info from one node to the next. Use the same tracking mechanism for binding upwards and in-place to avoid doing a bunch of mallocs.

Refs trac:3322

This commit was SVN r27356.

The following Trac tickets were found above:
  Ticket 3322 --> https://svn.open-mpi.org/trac/ompi/ticket/3322

opal/mca/hwloc/base/base.h

@@ -149,6 +149,8 @@ OPAL_DECLSPEC hwloc_obj_t opal_hwloc_base_find_min_bound_target_under_obj(hwloc_
                                                                           hwloc_obj_t obj,
                                                                           hwloc_obj_type_t target,
                                                                           unsigned cache_leve);
+OPAL_DECLSPEC void opal_hwloc_base_clear_usage(hwloc_topology_t topo);
+
 OPAL_DECLSPEC hwloc_obj_t opal_hwloc_base_get_obj_by_type(hwloc_topology_t topo,
                                                           hwloc_obj_type_t target,
                                                           unsigned cache_level,

opal/mca/hwloc/base/hwloc_base_util.c

@@ -889,6 +889,38 @@ hwloc_obj_t opal_hwloc_base_get_obj_by_type(hwloc_topology_t topo,
     return df_search(topo, obj, target, cache_level, instance, rtype, &idx, NULL);
 }
 
+static void df_clear(hwloc_topology_t topo,
+                     hwloc_obj_t start)
+{
+    unsigned k;
+    opal_hwloc_obj_data_t *data;
+
+    /* see how many procs are bound to us */
+    data = (opal_hwloc_obj_data_t*)start->userdata;
+    if (NULL != data) {
+        data->num_bound = 0;
+    }
+
+    for (k=0; k < start->arity; k++) {
+        df_clear(topo, start->children[k]);
+    }
+}
+
+void opal_hwloc_base_clear_usage(hwloc_topology_t topo)
+{
+    hwloc_obj_t root;
+
+    /* bozo check */
+    if (NULL == topo) {
+        OPAL_OUTPUT_VERBOSE((5, opal_hwloc_base_output,
+                             "hwloc:base:clear_usage: NULL topology"));
+        return;
+    }
+
+    root = hwloc_get_root_obj(topo);
+    df_clear(topo, root);
+}
+
 /* The current slot_list notation only goes to the core level - i.e., the location
  * is specified as socket:core. Thus, the code below assumes that all locations
  * are to be parsed under that notation.

orte/mca/rmaps/base/rmaps_base_binding.c

@@ -65,8 +65,9 @@ static int bind_upwards(orte_job_t *jdata,
     orte_proc_t *proc;
     hwloc_obj_t obj;
     hwloc_cpuset_t cpus;
-    unsigned int idx, ncpus, nobjs, nsave = 0, *nbound=NULL;
+    unsigned int idx, ncpus;
     struct hwloc_topology_support *support;
+    opal_hwloc_obj_data_t *data;
 
     opal_output_verbose(5, orte_rmaps_base.rmaps_output,
                         "mca:rmaps: bind upwards for job %s with bindings %s",
@@ -96,9 +97,6 @@ static int bind_upwards(orte_job_t *jdata,
                     continue;
                 }
                 orte_show_help("help-orte-rmaps-base.txt", "rmaps:cpubind-not-supported", true, node->name);
-                if (NULL != nbound) {
-                    free(nbound);
-                }
                 return ORTE_ERR_SILENT;
             }
             /* check if topology supports membind - have to be careful here
@@ -115,30 +113,13 @@ static int bind_upwards(orte_job_t *jdata,
                     membind_warned = true;
                 } else if (OPAL_HWLOC_BASE_MBFA_ERROR == opal_hwloc_base_mbfa) {
                     orte_show_help("help-orte-rmaps-base.txt", "rmaps:membind-not-supported-fatal", true, node->name);
-                    if (NULL != nbound) {
-                        free(nbound);
-                    }
                     return ORTE_ERR_SILENT;
                 }
             }
         }
 
-        /* get the number of objects of this type on this node */
-        nobjs = opal_hwloc_base_get_nbobjs_by_type(node->topology, target,
-                                                   cache_level, OPAL_HWLOC_AVAILABLE);
-        if (0 == nobjs) {
-            orte_show_help("help-orte-rmaps-base.txt", "rmaps:no-bindable-objects", true,
-                           node->name, hwloc_obj_type_string(target));
-            return ORTE_ERR_SILENT;
-        }
-        /* setup the array */
-        if (NULL == nbound) {
-            nbound = (unsigned int*)malloc(nobjs * sizeof(int));
-            nsave = nobjs;
-        } else if (nsave < nobjs) {
-            nbound = (unsigned int*)realloc(nbound, nobjs * sizeof(int));
-        }
-        memset(nbound, 0, nobjs * sizeof(int));
+        /* clear the topology of any prior usage numbers */
+        opal_hwloc_base_clear_usage(node->topology);
 
         /* cycle thru the procs */
         for (j=0; j < node->procs->size; j++) {
@@ -174,24 +155,22 @@ static int bind_upwards(orte_job_t *jdata,
                     }
                     /* get its index */
                     if (UINT_MAX == (idx = opal_hwloc_base_get_obj_idx(node->topology, obj, OPAL_HWLOC_AVAILABLE))) {
-                        free(nbound);
                         return ORTE_ERR_SILENT;
                     }
                     /* track the number bound */
-                    ++nbound[idx];
+                    data = (opal_hwloc_obj_data_t*)obj->userdata;
+                    data->num_bound++;
                     /* get the number of cpus under this location */
                     if (0 == (ncpus = opal_hwloc_base_get_npus(node->topology, obj))) {
                         orte_show_help("help-orte-rmaps-base.txt", "rmaps:no-available-cpus", true, node->name);
-                        free(nbound);
                         return ORTE_ERR_SILENT;
                     }
                     /* error out if adding a proc would cause overload and that wasn't allowed */
-                    if (ncpus < nbound[idx] &&
+                    if (ncpus < data->num_bound &&
                         !OPAL_BIND_OVERLOAD_ALLOWED(jdata->map->binding)) {
                         orte_show_help("help-orte-rmaps-base.txt", "rmaps:binding-overload", true,
                                        opal_hwloc_base_print_binding(map->binding), node->name,
-                                       nbound[idx], ncpus);
-                        free(nbound);
+                                       data->num_bound, ncpus);
                         return ORTE_ERR_SILENT;
                     }
                     /* bind it here */
@@ -214,16 +193,11 @@ static int bind_upwards(orte_job_t *jdata,
                  */
                 orte_show_help("help-orte-rmaps-base.txt", "rmaps:binding-target-not-found", true,
                                opal_hwloc_base_print_binding(map->binding), node->name);
-                free(nbound);
                 return ORTE_ERR_SILENT;
             }
         }
     }
 
-    if (NULL != nbound) {
-        free(nbound);
-    }
-
     return ORTE_SUCCESS;
 }
 
@@ -237,7 +211,7 @@ static int bind_downwards(orte_job_t *jdata,
     orte_proc_t *proc;
     hwloc_obj_t trg_obj;
     hwloc_cpuset_t cpus;
-    unsigned int ncpus;
+    unsigned int ncpus, idx;
     struct hwloc_topology_support *support;
     opal_hwloc_obj_data_t *data;
 
@@ -290,6 +264,9 @@ static int bind_downwards(orte_job_t *jdata,
             }
         }
 
+        /* clear the topology of any prior usage numbers */
+        opal_hwloc_base_clear_usage(node->topology);
+
        /* cycle thru the procs */
         for (j=0; j < node->procs->size; j++) {
             if (NULL == (proc = (orte_proc_t*)opal_pointer_array_get_item(node->procs, j))) {
@@ -317,7 +294,11 @@ static int bind_downwards(orte_job_t *jdata,
                 orte_show_help("help-orte-rmaps-base.txt", "rmaps:no-available-cpus", true, node->name);
                 return ORTE_ERR_SILENT;
             }
-            /* track the number bound */
+            /* get the index of the object */
+            if (UINT_MAX == (idx = opal_hwloc_base_get_obj_idx(node->topology, trg_obj, OPAL_HWLOC_AVAILABLE))) {
+                return ORTE_ERR_SILENT;
+            }
+             /* track the number bound */
             data = (opal_hwloc_obj_data_t*)trg_obj->userdata;
             data->num_bound++;
             opal_output_verbose(5, orte_rmaps_base.rmaps_output,
@@ -342,7 +323,7 @@ static int bind_downwards(orte_job_t *jdata,
                 return ORTE_ERR_SILENT;
             }
             /* bind the proc here */
-            proc->bind_idx = trg_obj->logical_index;
+            proc->bind_idx = idx;
             cpus = opal_hwloc_base_get_available_cpus(node->topology, trg_obj);
             hwloc_bitmap_list_asprintf(&proc->cpu_bitmap, cpus);
             opal_output_verbose(5, orte_rmaps_base.rmaps_output,
@@ -370,8 +351,9 @@ static int bind_in_place(orte_job_t *jdata,
     orte_node_t *node;
     orte_proc_t *proc;
     hwloc_cpuset_t cpus;
-    unsigned int idx, ncpus, nobjs, nsave = 0, *nbound=NULL;
+    unsigned int idx, ncpus;
     struct hwloc_topology_support *support;
+    opal_hwloc_obj_data_t *data;
 
     opal_output_verbose(5, orte_rmaps_base.rmaps_output,
                         "mca:rmaps: bind in place for job %s with bindings %s",
@@ -401,9 +383,6 @@ static int bind_in_place(orte_job_t *jdata,
                     continue;
                 }
                 orte_show_help("help-orte-rmaps-base.txt", "rmaps:cpubind-not-supported", true, node->name);
-                if (NULL != nbound) {
-                    free(nbound);
-                }
                 return ORTE_ERR_SILENT;
             }
             /* check if topology supports membind - have to be careful here
@@ -420,31 +399,15 @@ static int bind_in_place(orte_job_t *jdata,
                     membind_warned = true;
                 } else if (OPAL_HWLOC_BASE_MBFA_ERROR == opal_hwloc_base_mbfa) {
                     orte_show_help("help-orte-rmaps-base.txt", "rmaps:membind-not-supported-fatal", true, node->name);
-                    if (NULL != nbound) {
-                        free(nbound);
-                    }
                     return ORTE_ERR_SILENT;
                 }
             }
         }
 
-        /* get the number of objects of this type on this node */
-        nobjs = opal_hwloc_base_get_nbobjs_by_type(node->topology, target,
-                                                   cache_level, OPAL_HWLOC_AVAILABLE);
-        if (0 == nobjs) {
-            orte_show_help("help-orte-rmaps-base.txt", "rmaps:no-bindable-objects", true,
-                           node->name, hwloc_obj_type_string(target));
-            return ORTE_ERR_SILENT;
-        }
-        /* setup the array */
-        if (NULL == nbound) {
-            nbound = (unsigned int*)malloc(nobjs * sizeof(int));
-            nsave = nobjs;
-        } else if (nsave < nobjs) {
-            nbound = (unsigned int*)realloc(nbound, nobjs * sizeof(int));
-        }
-        memset(nbound, 0, nobjs * sizeof(int));
-        /* cycle thru the procs */
+         /* clear the topology of any prior usage numbers */
+        opal_hwloc_base_clear_usage(node->topology);
+
+       /* cycle thru the procs */
         for (j=0; j < node->procs->size; j++) {
             if (NULL == (proc = (orte_proc_t*)opal_pointer_array_get_item(node->procs, j))) {
                 continue;
@@ -461,28 +424,26 @@ static int bind_in_place(orte_job_t *jdata,
             }
             /* get the index of this location */
             if (UINT_MAX == (idx = opal_hwloc_base_get_obj_idx(node->topology, proc->locale, OPAL_HWLOC_AVAILABLE))) {
-                free(nbound);
                 return ORTE_ERR_SILENT;
             }
-            opal_output_verbose(5, orte_rmaps_base.rmaps_output,
+            /* track the number bound */
+            data = (opal_hwloc_obj_data_t*)proc->locale->userdata;
+            data->num_bound++;
+             opal_output_verbose(5, orte_rmaps_base.rmaps_output,
                                 "BINDING PROC %s TO %s NUMBER %u",
                                 ORTE_NAME_PRINT(&proc->name),
                                 hwloc_obj_type_string(proc->locale->type), idx);
             /* get the number of cpus under this location */
             if (0 == (ncpus = opal_hwloc_base_get_npus(node->topology, proc->locale))) {
                 orte_show_help("help-orte-rmaps-base.txt", "rmaps:no-available-cpus", true, node->name);
-                free(nbound);
                 return ORTE_ERR_SILENT;
             }
-            /* track number bound */
-            ++nbound[idx];
             /* error out if adding a proc would cause overload and that wasn't allowed */
-            if (ncpus < nbound[idx] &&
+            if (ncpus < data->num_bound &&
                 !OPAL_BIND_OVERLOAD_ALLOWED(jdata->map->binding)) {
                 orte_show_help("help-orte-rmaps-base.txt", "rmaps:binding-overload", true,
                                opal_hwloc_base_print_binding(map->binding), node->name,
-                               nbound[idx], ncpus);
-                free(nbound);
+                               data->num_bound, ncpus);
                 return ORTE_ERR_SILENT;
             }
             /* bind the proc here */
@@ -499,9 +460,6 @@ static int bind_in_place(orte_job_t *jdata,
         }
     }
 
-    if (NULL != nbound) {
-        free(nbound);
-    }
     return ORTE_SUCCESS;
 }