hwloc/base: fix opal proc locality wrt to NUMA nodes on hwloc 2.0

Both opal_hwloc_base_get_relative_locality() and _get_locality_string()
iterate over hwloc levels to build the proc locality information.
Unfortunately, NUMA nodes are not in those normal levels anymore since 2.0.
We have to explicitly look a the special NUMA level to get that locality info.

I am factorizing the core of the iterations inside dedicated "_by_depth"
functions and calling them again for the NUMA level at the end of the loops.

Thanks to Hatem Elshazly for reporting the NUMA communicator split failure
at https://www.mail-archive.com/[email protected]/msg33589.html

It looks like only the opal_hwloc_base_get_locality_string() part is needed
to fix that split, but there's no reason not to fix get_relative_locality()
as well.

Signed-off-by: Brice Goglin <[email protected]>

Author: bgoglin

opal/mca/hwloc/base/hwloc_base_util.c

@@ -20,6 +20,7 @@
  *                         All rights reserved.
  * Copyright (c) 2018      Amazon.com, Inc. or its affiliates.  All Rights reserved.
  * Copyright (c) 2019 IBM Corporation. All rights reserved.
+ * Copyright (c) 2019      Inria.  All rights reserved.
  * $COPYRIGHT$
  *
  * Additional copyrights may follow
@@ -1240,16 +1241,84 @@ int opal_hwloc_base_cpu_list_parse(const char *slot_str,
     return OPAL_SUCCESS;
 }
 
+static void opal_hwloc_base_get_relative_locality_by_depth(hwloc_topology_t topo, unsigned d,
+                                                           hwloc_cpuset_t loc1, hwloc_cpuset_t loc2,
+                                                           opal_hwloc_locality_t *locality, bool *shared)
+{
+    unsigned width, w;
+    hwloc_obj_t obj;
+    int sect1, sect2;
+
+    /* get the width of the topology at this depth */
+    width = hwloc_get_nbobjs_by_depth(topo, d);
+
+    /* scan all objects at this depth to see if
+     * our locations overlap with them
+     */
+    for (w=0; w < width; w++) {
+        /* get the object at this depth/index */
+        obj = hwloc_get_obj_by_depth(topo, d, w);
+        /* see if our locations intersect with the cpuset for this obj */
+        sect1 = hwloc_bitmap_intersects(obj->cpuset, loc1);
+        sect2 = hwloc_bitmap_intersects(obj->cpuset, loc2);
+        /* if both intersect, then we share this level */
+        if (sect1 && sect2) {
+            *shared = true;
+            switch(obj->type) {
+            case HWLOC_OBJ_NODE:
+                *locality |= OPAL_PROC_ON_NUMA;
+                break;
+            case HWLOC_OBJ_SOCKET:
+                *locality |= OPAL_PROC_ON_SOCKET;
+                break;
+#if HWLOC_API_VERSION < 0x20000
+            case HWLOC_OBJ_CACHE:
+                if (3 == obj->attr->cache.depth) {
+                    *locality |= OPAL_PROC_ON_L3CACHE;
+                } else if (2 == obj->attr->cache.depth) {
+                    *locality |= OPAL_PROC_ON_L2CACHE;
+                } else {
+                    *locality |= OPAL_PROC_ON_L1CACHE;
+                }
+                break;
+#else
+            case HWLOC_OBJ_L3CACHE:
+                *locality |= OPAL_PROC_ON_L3CACHE;
+                break;
+            case HWLOC_OBJ_L2CACHE:
+                *locality |= OPAL_PROC_ON_L2CACHE;
+                break;
+            case HWLOC_OBJ_L1CACHE:
+                *locality |= OPAL_PROC_ON_L1CACHE;
+                break;
+#endif
+            case HWLOC_OBJ_CORE:
+                *locality |= OPAL_PROC_ON_CORE;
+                break;
+            case HWLOC_OBJ_PU:
+                *locality |= OPAL_PROC_ON_HWTHREAD;
+                break;
+            default:
+                /* just ignore it */
+                break;
+            }
+            break;
+        }
+        /* otherwise, we don't share this
+         * object - but we still might share another object
+         * on this level, so we have to keep searching
+         */
+    }
+}
+
 opal_hwloc_locality_t opal_hwloc_base_get_relative_locality(hwloc_topology_t topo,
                                                             char *cpuset1, char *cpuset2)
 {
     opal_hwloc_locality_t locality;
-    hwloc_obj_t obj;
-    unsigned depth, d, width, w;
+    hwloc_cpuset_t loc1, loc2;
+    unsigned depth, d;
     bool shared;
     hwloc_obj_type_t type;
-    int sect1, sect2;
-    hwloc_cpuset_t loc1, loc2;
 
     /* start with what we know - they share a node on a cluster
      * NOTE: we may alter that latter part as hwloc's ability to
@@ -1290,66 +1359,8 @@ opal_hwloc_locality_t opal_hwloc_base_get_relative_locality(hwloc_topology_t top
             HWLOC_OBJ_PU != type) {
             continue;
         }
-        /* get the width of the topology at this depth */
-        width = hwloc_get_nbobjs_by_depth(topo, d);
+        opal_hwloc_base_get_relative_locality_by_depth(topo, d, loc1, loc2, &locality, &shared);
 
-        /* scan all objects at this depth to see if
-         * our locations overlap with them
-         */
-        for (w=0; w < width; w++) {
-            /* get the object at this depth/index */
-            obj = hwloc_get_obj_by_depth(topo, d, w);
-            /* see if our locations intersect with the cpuset for this obj */
-            sect1 = hwloc_bitmap_intersects(obj->cpuset, loc1);
-            sect2 = hwloc_bitmap_intersects(obj->cpuset, loc2);
-            /* if both intersect, then we share this level */
-            if (sect1 && sect2) {
-                shared = true;
-                switch(obj->type) {
-                case HWLOC_OBJ_NODE:
-                    locality |= OPAL_PROC_ON_NUMA;
-                    break;
-                case HWLOC_OBJ_SOCKET:
-                    locality |= OPAL_PROC_ON_SOCKET;
-                    break;
-#if HWLOC_API_VERSION < 0x20000
-                case HWLOC_OBJ_CACHE:
-                    if (3 == obj->attr->cache.depth) {
-                        locality |= OPAL_PROC_ON_L3CACHE;
-                    } else if (2 == obj->attr->cache.depth) {
-                        locality |= OPAL_PROC_ON_L2CACHE;
-                    } else {
-                        locality |= OPAL_PROC_ON_L1CACHE;
-                    }
-                    break;
-#else
-                case HWLOC_OBJ_L3CACHE:
-                    locality |= OPAL_PROC_ON_L3CACHE;
-                    break;
-                case HWLOC_OBJ_L2CACHE:
-                    locality |= OPAL_PROC_ON_L2CACHE;
-                    break;
-                case HWLOC_OBJ_L1CACHE:
-                    locality |= OPAL_PROC_ON_L1CACHE;
-                    break;
-#endif
-                case HWLOC_OBJ_CORE:
-                    locality |= OPAL_PROC_ON_CORE;
-                    break;
-                case HWLOC_OBJ_PU:
-                    locality |= OPAL_PROC_ON_HWTHREAD;
-                    break;
-                default:
-                    /* just ignore it */
-                    break;
-                }
-                break;
-            }
-            /* otherwise, we don't share this
-             * object - but we still might share another object
-             * on this level, so we have to keep searching
-             */
-        }
         /* if we spanned the entire width without finding
          * a point of intersection, then no need to go
          * deeper
@@ -1358,6 +1369,9 @@ opal_hwloc_locality_t opal_hwloc_base_get_relative_locality(hwloc_topology_t top
             break;
         }
     }
+#if HWLOC_API_VERSION >= 0x20000
+    opal_hwloc_base_get_relative_locality_by_depth(topo, HWLOC_TYPE_DEPTH_NUMANODE, loc1, loc2, &locality, &shared);
+#endif
 
     opal_output_verbose(5, opal_hwloc_base_framework.framework_output,
                         "locality: %s",
@@ -2089,12 +2103,40 @@ char* opal_hwloc_base_get_topo_signature(hwloc_topology_t topo)
     return sig;
 }
 
+static int opal_hwloc_base_get_locality_string_by_depth(hwloc_topology_t topo,
+                                                        int d,
+                                                        hwloc_cpuset_t cpuset,
+                                                        hwloc_cpuset_t result)
+{
+    hwloc_obj_t obj;
+    unsigned width, w;
+
+    /* get the width of the topology at this depth */
+    width = hwloc_get_nbobjs_by_depth(topo, d);
+    if (0 == width) {
+        return -1;
+    }
+
+    /* scan all objects at this depth to see if
+     * the location overlaps with them
+     */
+    for (w=0; w < width; w++) {
+        /* get the object at this depth/index */
+        obj = hwloc_get_obj_by_depth(topo, d, w);
+        /* see if the location intersects with it */
+        if (hwloc_bitmap_intersects(obj->cpuset, cpuset)) {
+            hwloc_bitmap_set(result, w);
+        }
+    }
+
+    return 0;
+}
+
 char* opal_hwloc_base_get_locality_string(hwloc_topology_t topo,
                                           char *bitmap)
 {
-    hwloc_obj_t obj;
     char *locality=NULL, *tmp, *t2;
-    unsigned depth, d, width, w;
+    unsigned depth, d;
     hwloc_cpuset_t cpuset, result;
     hwloc_obj_type_t type;
 
@@ -2137,28 +2179,15 @@ char* opal_hwloc_base_get_locality_string(hwloc_topology_t topo,
             continue;
         }
 
-        /* get the width of the topology at this depth */
-        width = hwloc_get_nbobjs_by_depth(topo, d);
-        if (0 == width) {
+        if (opal_hwloc_base_get_locality_string_by_depth(topo, d, cpuset, result) < 0) {
             continue;
         }
 
-        /* scan all objects at this depth to see if
-         * the location overlaps with them
-         */
-        for (w=0; w < width; w++) {
-            /* get the object at this depth/index */
-            obj = hwloc_get_obj_by_depth(topo, d, w);
-            /* see if the location intersects with it */
-            if (hwloc_bitmap_intersects(obj->cpuset, cpuset)) {
-                hwloc_bitmap_set(result, w);
-            }
-        }
         /* it should be impossible, but allow for the possibility
          * that we came up empty at this depth */
         if (!hwloc_bitmap_iszero(result)) {
             hwloc_bitmap_list_asprintf(&tmp, result);
-            switch(obj->type) {
+            switch(type) {
                 case HWLOC_OBJ_NODE:
                     opal_asprintf(&t2, "%sNM%s:", (NULL == locality) ? "" : locality, tmp);
                     if (NULL != locality) {
@@ -2243,6 +2272,24 @@ char* opal_hwloc_base_get_locality_string(hwloc_topology_t topo,
         }
         hwloc_bitmap_zero(result);
     }
+
+#if HWLOC_API_VERSION >= 0x20000
+    if (opal_hwloc_base_get_locality_string_by_depth(topo, HWLOC_TYPE_DEPTH_NUMANODE, cpuset, result) == 0) {
+        /* it should be impossible, but allow for the possibility
+         * that we came up empty at this depth */
+        if (!hwloc_bitmap_iszero(result)) {
+            hwloc_bitmap_list_asprintf(&tmp, result);
+            opal_asprintf(&t2, "%sNM%s:", (NULL == locality) ? "" : locality, tmp);
+            if (NULL != locality) {
+                free(locality);
+            }
+            locality = t2;
+            free(tmp);
+        }
+        hwloc_bitmap_zero(result);
+    }
+#endif
+
     hwloc_bitmap_free(result);
     hwloc_bitmap_free(cpuset);