restoring more hwloc --cpu-set behavior from OMPI 3.x

My rough understanding of where some important masks come from is:
    rdata->available = OMPI's allowed PUs, possibly from --cpu-set cmdline
    hwloc_topology_get_allowed_cpuset() = hwloc maybe removing offline cpus
                                          and/or cpus not in our cgroup
the old hwloc had fields
    obj->online_cpuset
    obj->allowed_cpuset
that I'm guessing is similar to hwloc_topology_get_allowed_cpuset()

Here's an example of the behavior this checkin changes:
Suppose a machine's hardware has
  [ 0 1 2 3 / 4 5 6 7 / 8 9 10 11 / 12 13 14 15 ]
and we run with
  --cpu-set 4,5,8,9,12,13,14,15
  --map-by ppr:2:node:pe=3
   -bind-to hwthread:overload-allowed
The allowed hardware threads would be
   [..../xx../xx../xxxx]
and the 3.x code would place the ranks on
  MCW 0 : [..../BB../B.../....]
  MCW 1 : [..../..../.B../BB..]
which at least is using the allowed resources.  The 4.x code doesn't pay as
much attention to rdata->available and was placing the ranks on
  MCW 0 : [..../BBB./..../....]
  MCW 1 : [..../..../BBB./....]

This part of the old code's behavior came from bind_downward() which would
cycle through a sequence of potential target objects checking
opal_hwloc_base_get_npus() under each potential object, and also using
opal_hwloc_base_get_available_cpus() for a selected object. Those
functions used to take into account rdata->available.

The changes I've made are to maintain old behavior in
    opal_hwloc_base_get_npus(topo, obj)
and to add back in the opal_hwloc_base_get_available_cpus() function
that had been removed.

The old behavior of opal_hwloc_base_get_npus() had two paths:

1. df_search_cores which counts how many cores exist under obj that
   intersect the hwloc_topology_get_allowed_cpuset()
   (I think it likely should also intersect with rdata->available, but
   it didn't before and I didn't change that aspect).

   Here the OMPI 4.x code just used hwloc_get_nbobjs_inside_cpuset_by_type(,CORE)
   so I restored the df_search_cores.  It could likely be replaced by
   intersecting with the available/allowed masks and then using the
   get_nbobjs_inside.. call, but I went with the old code

2. opal_hwloc_base_get_available_cpus(topo,obj) that intersected the
   allowed_cpuset as well as the rdata->available.

   Here the OMPI 4.x code just used cpuset = obj->cpuset, so I restored
   the call to intersect it with the allowed_cpuset from hwloc and the
   rdata->available from ompi.

For the use-case at the top of this PR this is all the changes needed.
I did look a little further to see where else OMPI 3.x used
opal_hwloc_base_get_available_cpus(), as I believe every place that
removed that function is a place that used to account for rdata->available
that now doesn't.

So I also added opal_hwloc_base_get_available_cpus back into bind_in_place
even though I wasn't tracing that codepath.

I didn't restore all the uses I found in 3.x of the previously removed
opal_hwloc_base_get_available_cpus() function, instead only touching
code that seemed similar/related to code I was already touching.

Author: markalle

opal/mca/hwloc/base/base.h

@@ -171,6 +171,8 @@ OPAL_DECLSPEC int opal_hwloc_base_filter_cpus(hwloc_topology_t topo);
  * Free the hwloc topology.
  */
 OPAL_DECLSPEC void opal_hwloc_base_free_topology(hwloc_topology_t topo);
+OPAL_DECLSPEC hwloc_cpuset_t opal_hwloc_base_get_available_cpus(hwloc_topology_t topo,
+                                                                hwloc_obj_t obj);
 OPAL_DECLSPEC unsigned int opal_hwloc_base_get_nbobjs_by_type(hwloc_topology_t topo,
                                                               hwloc_obj_type_t target,
                                                               unsigned cache_level,

opal/mca/hwloc/base/hwloc_base_frame.c

@@ -423,14 +423,21 @@ char* opal_hwloc_base_print_locality(opal_hwloc_locality_t locality)
 
 static void obj_data_const(opal_hwloc_obj_data_t *ptr)
 {
+    ptr->available = NULL;
     ptr->npus_calculated = false;
     ptr->npus = 0;
     ptr->idx = UINT_MAX;
     ptr->num_bound = 0;
 }
+static void obj_data_dest(opal_hwloc_obj_data_t *ptr)
+{
+    if (NULL != ptr->available) {
+        hwloc_bitmap_free(ptr->available);
+    }
+}
 OBJ_CLASS_INSTANCE(opal_hwloc_obj_data_t,
                    opal_object_t,
-                   obj_data_const, NULL);
+                   obj_data_const, obj_data_dest);
 
 static void sum_const(opal_hwloc_summary_t *ptr)
 {

opal/mca/hwloc/base/hwloc_base_util.c

@@ -531,6 +531,7 @@ void opal_hwloc_base_free_topology(hwloc_topology_t topo)
 void opal_hwloc_base_get_local_cpuset(void)
 {
     hwloc_obj_t root;
+    hwloc_cpuset_t base_cpus;
 
     if (NULL != opal_hwloc_topology) {
         if (NULL == opal_hwloc_my_cpuset) {
@@ -543,7 +544,8 @@ void opal_hwloc_base_get_local_cpuset(void)
                               HWLOC_CPUBIND_PROCESS) < 0) {
             /* we are not bound - use the root's available cpuset */
             root = hwloc_get_root_obj(opal_hwloc_topology);
-            hwloc_bitmap_copy(opal_hwloc_my_cpuset, root->cpuset);
+            base_cpus = opal_hwloc_base_get_available_cpus(opal_hwloc_topology, root);
+            hwloc_bitmap_copy(opal_hwloc_my_cpuset, base_cpus);
         }
     }
 }
@@ -571,6 +573,77 @@ int opal_hwloc_base_report_bind_failure(const char *file,
     return OPAL_SUCCESS;
 }
 
+hwloc_cpuset_t opal_hwloc_base_get_available_cpus(hwloc_topology_t topo,
+                                                  hwloc_obj_t obj)
+{
+    hwloc_obj_t root;
+    hwloc_cpuset_t avail, specd=NULL;
+    opal_hwloc_topo_data_t *rdata;
+    opal_hwloc_obj_data_t *data;
+
+    OPAL_OUTPUT_VERBOSE((10, opal_hwloc_base_framework.framework_output,
+                         "hwloc:base: get available cpus"));
+
+    /* get the node-level information */
+    root = hwloc_get_root_obj(topo);
+    rdata = (opal_hwloc_topo_data_t*)root->userdata;
+    /* bozo check */
+    if (NULL == rdata) {
+        rdata = OBJ_NEW(opal_hwloc_topo_data_t);
+        root->userdata = (void*)rdata;
+        OPAL_OUTPUT_VERBOSE((5, opal_hwloc_base_framework.framework_output,
+                             "hwloc:base:get_available_cpus first time - filtering cpus"));
+    }
+
+    /* are we asking about the root object? */
+    if (obj == root) {
+        OPAL_OUTPUT_VERBOSE((5, opal_hwloc_base_framework.framework_output,
+                             "hwloc:base:get_available_cpus root object"));
+        return rdata->available;
+     }
+
+    /* some hwloc object types don't have cpus */
+// This used to use online_cpuset:
+    if (NULL == obj->cpuset) {
+        return NULL;
+    }
+    if (!hwloc_bitmap_intersects(obj->cpuset, hwloc_topology_get_allowed_cpuset(topo))) {
+        return NULL;
+    }
+
+    /* see if we already have this info */
+    if (NULL == (data = (opal_hwloc_obj_data_t*)obj->userdata)) {
+        /* nope - create the object */
+        data = OBJ_NEW(opal_hwloc_obj_data_t);
+        obj->userdata = (void*)data;
+    }
+
+    /* do we have the cpuset */
+    if (NULL != data->available) {
+        return data->available;
+    }
+
+    /* find the available processors on this object */
+    avail = hwloc_bitmap_alloc();
+// This used to use online_cpuset:
+    hwloc_bitmap_and(avail, obj->cpuset, hwloc_topology_get_allowed_cpuset(topo));
+
+    /* filter this against the node-available processors */
+    if (NULL == rdata->available) {
+        hwloc_bitmap_free(avail);
+        return NULL;
+    }
+    specd = hwloc_bitmap_alloc();
+    hwloc_bitmap_and(specd, avail, rdata->available);
+
+    /* cache the info */
+    data->available = specd;
+
+    /* cleanup */
+    hwloc_bitmap_free(avail);
+    return specd;
+}
+
 /* determine if there is a single cpu in a bitmap */
 bool opal_hwloc_base_single_cpu(hwloc_cpuset_t cpuset)
 {
@@ -599,6 +672,50 @@ bool opal_hwloc_base_single_cpu(hwloc_cpuset_t cpuset)
     return one;
 }
 
+static void df_search_cores(hwloc_topology_t topo, hwloc_obj_t obj, unsigned int *cnt)
+{
+    unsigned k;
+    opal_hwloc_obj_data_t *data;
+
+// I'm updating df_search_cores a bit to deal with virtual elements in a tree,
+// these are objects that have no arity and aren't really in the tree, but
+// they're associated with a real tree element, their parent.  For the purposes
+// of tree-walking we should use the parent.
+//
+// It's likely we don't even need this routine and should just use a bitwise
+// intersection plus hwloc_get_nbobjs_inside_cpuset_by_type(,,CORE) but for
+// now I was just sticking close to the 3.x code
+    while (obj->depth < 0) {
+        obj = obj->parent;
+    }
+    if (HWLOC_OBJ_CORE == obj->type) {
+        data = (opal_hwloc_obj_data_t*)obj->userdata;
+        if (NULL == data) {
+            data = OBJ_NEW(opal_hwloc_obj_data_t);
+            obj->userdata = (void*)data;
+        }
+        if (NULL == opal_hwloc_base_cpu_list) {
+            if (!hwloc_bitmap_intersects(obj->cpuset, hwloc_topology_get_allowed_cpuset(topo))) {
+
+                /*
+                 * do not count not allowed cores (e.g. cores with zero allowed PU)
+                 * if SMT is enabled, do count cores with at least one allowed hwthread
+                 */
+                return;
+            }
+            data->npus = 1;
+        }
+        *cnt += data->npus;
+        return;
+    }
+
+    for (k=0; k < obj->arity; k++) {
+        df_search_cores(topo, obj->children[k], cnt);
+    }
+    return;
+}
+
+
 /* get the number of pu's under a given hwloc object */
 unsigned int opal_hwloc_base_get_npus(hwloc_topology_t topo,
                                       hwloc_obj_t obj)
@@ -616,15 +733,47 @@ unsigned int opal_hwloc_base_get_npus(hwloc_topology_t topo,
              * one hwthread/core. Instead, find the number of cores
              * in the system
              */
-            cnt = hwloc_get_nbobjs_inside_cpuset_by_type(topo, obj->cpuset, HWLOC_OBJ_CORE);
+// The OMPI 3.x code used df_search_cores() which counts the number
+// of objects of type CORE under obj if the object intersected
+// obj->allowed_cpuset which was a hwloc field that left out things
+// like offline cpus or cpus not in our cgroup.
+//
+// The OMPI 4.x code initially moved to a vanilla
+//     cnt = hwloc_get_nbobjs_inside_cpuset_by_type(topo, obj->cpuset, HWLOC_OBJ_CORE);
+//
+// I'm currently restoring it to df_search_cores() and intersecting
+// hwloc_topology_get_allowed_cpuset(topo) to decide if a CORE obj
+// should be counted.  I think it ought to also be intersecting with
+// rdata->available (eg if the user specified a --cpu-set to constrain
+// the binding to) but for now I'm just restoring the old behavior.
+            if (NULL != hwloc_get_obj_by_type(topo, HWLOC_OBJ_CORE, 0)) {
+                /* starting at the incoming obj, do a down-first search
+                 * and count the number of cores under it
+                 */
+                cnt = 0;
+                df_search_cores(topo, obj, &cnt);
+            }
+
         } else {
             hwloc_cpuset_t cpuset;
 
             /* if we are treating cores as cpus, or the system can't detect
              * "cores", then get the available cpuset for this object - this will
              * create and store the data
              */
-            if (NULL == (cpuset = obj->cpuset)) {
+// The OMPI 3.x code used cpuset = opal_hwloc_base_get_available_cpus() which
+// intersected obj->cpuset with all of obj->online_cpuset, obj->allowed_cpuset
+// (the old hwloc objects that omitted offline out-of-our-cgroup cpus)
+// and rdata->available (eg from mpirun cmdline --cpu-set).
+//
+// The OMPI 4.x code initially moved to a vanilla
+//     cpuset = obj->cpuset
+//
+// I'm restoring it to opal_hwloc_base_get_available_cpus() and intersecting
+// hwloc_topology_get_allowed_cpuset(topo) to decide if a CORE obj
+// should be counted, and still using the rdata->available there too.
+
+            if (NULL == (cpuset = opal_hwloc_base_get_available_cpus(topo, obj))) {
                 return 0;
             }
             /* count the number of bits that are set - there is

opal/mca/hwloc/hwloc-internal.h

@@ -139,6 +139,7 @@ typedef uint8_t opal_hwloc_resource_type_t;
 /* structs for storing info on objects */
 typedef struct {
     opal_object_t super;
+    hwloc_cpuset_t available;
     bool npus_calculated;
     unsigned int npus;
     unsigned int idx;

orte/mca/rmaps/base/rmaps_base_binding.c

@@ -130,6 +130,7 @@ static int bind_generic(orte_job_t *jdata,
     orte_job_map_t *map;
     orte_proc_t *proc;
     hwloc_obj_t trg_obj, tmp_obj, nxt_obj;
+    hwloc_cpuset_t cpus;
     unsigned int ncpus;
     opal_hwloc_obj_data_t *data;
     int total_cpus;
@@ -248,7 +249,9 @@ static int bind_generic(orte_job_t *jdata,
                 }
             }
             /* bind the proc here */
-            hwloc_bitmap_or(totalcpuset, totalcpuset, trg_obj->cpuset);
+            cpus = opal_hwloc_base_get_available_cpus(node->topology->topo, trg_obj);
+            hwloc_bitmap_or(totalcpuset, totalcpuset, cpus);
+
             /* track total #cpus */
             total_cpus += ncpus;
             /* move to the next location, in case we need it */
@@ -301,6 +304,7 @@ static int bind_in_place(orte_job_t *jdata,
     orte_job_map_t *map;
     orte_node_t *node;
     orte_proc_t *proc;
+    hwloc_cpuset_t cpus;
     unsigned int idx, ncpus;
     struct hwloc_topology_support *support;
     opal_hwloc_obj_data_t *data;
@@ -487,7 +491,8 @@ static int bind_in_place(orte_job_t *jdata,
                                 ORTE_NAME_PRINT(&proc->name),
                                 hwloc_obj_type_string(locale->type), idx);
             /* bind the proc here */
-            hwloc_bitmap_list_asprintf(&cpu_bitmap, locale->cpuset);
+            cpus = opal_hwloc_base_get_available_cpus(node->topology->topo, locale);
+            hwloc_bitmap_list_asprintf(&cpu_bitmap, cpus);
             orte_set_attribute(&proc->attributes, ORTE_PROC_CPU_BITMAP, ORTE_ATTR_GLOBAL, cpu_bitmap, OPAL_STRING);
             /* update the location, in case it changed */
             orte_set_attribute(&proc->attributes, ORTE_PROC_HWLOC_BOUND, ORTE_ATTR_LOCAL, locale, OPAL_PTR);

orte/mca/rmaps/ppr/rmaps_ppr.c

@@ -443,7 +443,7 @@ static void prune(orte_jobid_t jobid,
     hwloc_obj_type_t lvl;
     unsigned cache_level = 0, k;
     int nprocs;
-    hwloc_cpuset_t avail;
+    hwloc_cpuset_t avail, cpus, childcpus;
     int n, limit, nmax, nunder, idx, idxmax = 0;
     orte_proc_t *proc, *pptr, *procmax;
     opal_hwloc_level_t ll;
@@ -494,7 +494,7 @@ static void prune(orte_jobid_t jobid,
                                               lvl, cache_level,
                                               i, OPAL_HWLOC_AVAILABLE);
         /* get the available cpuset */
-        avail = obj->cpuset;
+        avail = opal_hwloc_base_get_available_cpus(node->topology->topo, obj);
 
         /* look at the intersection of this object's cpuset and that
          * of each proc in the job/app - if they intersect, then count this proc
@@ -514,7 +514,8 @@ static void prune(orte_jobid_t jobid,
                 ORTE_ERROR_LOG(ORTE_ERR_NOT_FOUND);
                 return;
             }
-            if (hwloc_bitmap_intersects(avail, locale->cpuset)) {
+            cpus = opal_hwloc_base_get_available_cpus(node->topology->topo, locale);
+            if (hwloc_bitmap_intersects(avail, cpus)) {
                 nprocs++;
             }
         }
@@ -551,6 +552,7 @@ static void prune(orte_jobid_t jobid,
             /* find the child with the most procs underneath it */
             for (k=0; k < top->arity && limit < nprocs; k++) {
                 /* get this object's available cpuset */
+                childcpus = opal_hwloc_base_get_available_cpus(node->topology->topo, top->children[k]);
                 nunder = 0;
                 pptr = NULL;
                 for (n=0; n < node->procs->size; n++) {
@@ -566,7 +568,8 @@ static void prune(orte_jobid_t jobid,
                         ORTE_ERROR_LOG(ORTE_ERR_NOT_FOUND);
                         return;
                     }
-                    if (hwloc_bitmap_intersects(top->children[k]->cpuset, locale->cpuset)) {
+                    cpus = opal_hwloc_base_get_available_cpus(node->topology->topo, locale);
+                    if (hwloc_bitmap_intersects(childcpus, cpus)) {
                         nunder++;
                         if (NULL == pptr) {
                             /* save the location of the first proc under this object */