Merge pull request #620 from bgoglin/master

add hwloc_topology_free_group() and fix/document when groups are freed

Author: bgoglin

doc/Makefile.am

@@ -466,6 +466,7 @@ man3_tinker_DATA = \
         $(DOX_MAN_DIR)/man3/hwloc_topology_allow.3 \
         $(DOX_MAN_DIR)/man3/hwloc_topology_insert_misc_object.3 \
         $(DOX_MAN_DIR)/man3/hwloc_topology_alloc_group_object.3 \
+        $(DOX_MAN_DIR)/man3/hwloc_topology_free_group_object.3 \
         $(DOX_MAN_DIR)/man3/hwloc_topology_insert_group_object.3 \
         $(DOX_MAN_DIR)/man3/hwloc_obj_add_other_obj_sets.3 \
         $(DOX_MAN_DIR)/man3/hwloc_topology_refresh.3

hwloc/topology.c

@@ -2031,6 +2031,22 @@ hwloc_topology_alloc_group_object(struct hwloc_topology *topology)
   return hwloc_alloc_setup_object(topology, HWLOC_OBJ_GROUP, HWLOC_UNKNOWN_INDEX);
 }
 
+int
+hwloc_topology_free_group_object(struct hwloc_topology *topology, hwloc_obj_t obj)
+{
+  if (!(topology->state & HWLOC_TOPOLOGY_STATE_IS_LOADED)) {
+    /* this could actually work when IS_LOADING, see insert() below */
+    errno = EINVAL;
+    return -1;
+  }
+  if (topology->adopted_shmem_addr) {
+    errno = EPERM;
+    return -1;
+  }
+  hwloc_free_unlinked_object(obj);
+  return 0;
+}
+
 static void hwloc_propagate_symmetric_subtree(hwloc_topology_t topology, hwloc_obj_t root);
 static void propagate_total_memory(hwloc_obj_t obj);
 static void hwloc_set_group_depth(hwloc_topology_t topology);
@@ -2051,6 +2067,7 @@ hwloc_topology_insert_group_object(struct hwloc_topology *topology, hwloc_obj_t
     return NULL;
   }
   if (topology->adopted_shmem_addr) {
+    hwloc_free_unlinked_object(obj);
     errno = EPERM;
     return NULL;
   }
@@ -2104,6 +2121,7 @@ hwloc_topology_insert_group_object(struct hwloc_topology *topology, hwloc_obj_t
     res = hwloc__insert_object_by_cpuset(topology, NULL, obj, NULL /* do not show errors on stdout */);
   } else {
     /* just merge root */
+    hwloc_free_unlinked_object(obj);
     res = root;
   }
 

include/hwloc.h

@@ -2793,9 +2793,27 @@ HWLOC_DECLSPEC hwloc_obj_t hwloc_topology_insert_misc_object(hwloc_topology_t to
  *
  * \return The allocated object on success.
  * \return \c NULL on error.
- */
+ *
+ * \note The only way to free this object is to pass it to hwloc_topology_insert_group_object().
+ * If properly inserted, it will be freed when the entire topology is freed.
+ * If insertion failed (e.g. \c NULL or empty CPU and node-sets),
+ * it is freed before returning the error.
+  */
 HWLOC_DECLSPEC hwloc_obj_t hwloc_topology_alloc_group_object(hwloc_topology_t topology);
 
+/** \brief Free a group object allocated with hwloc_topology_alloc_group_object().
+ *
+ * This function is only useful if the group object was not given
+ * to hwloc_topology_insert_group_object() as planned.
+ *
+ * \note \p topology must be the same as the one previously passed
+ * to hwloc_topology_alloc_group_object().
+ *
+ * \return \c 0 on success.
+ * \return \c -1 on error, for instance if an invalid topology is given.
+ */
+HWLOC_DECLSPEC int hwloc_topology_free_group_object(hwloc_topology_t topology, hwloc_obj_t group);
+
 /** \brief Add more structure to the topology by adding an intermediate Group
  *
  * The caller should first allocate a new Group object with hwloc_topology_alloc_group_object().
@@ -2833,6 +2851,11 @@ HWLOC_DECLSPEC hwloc_obj_t hwloc_topology_alloc_group_object(hwloc_topology_t to
  * hence the existing objects may get reordered (including PUs and NUMA nodes),
  * and their logical indexes may change.
  *
+ * \note If the insertion fails, the input group object is freed.
+ *
+ * \note \p topology must be the same as the one previously passed
+ * to hwloc_topology_alloc_group_object().
+ *
  * \return The inserted object if it was properly inserted.
  *
  * \return An existing object if the Group was merged or discarded

include/hwloc/rename.h

@@ -178,6 +178,7 @@ extern "C" {
 
 #define hwloc_topology_insert_misc_object HWLOC_NAME(topology_insert_misc_object)
 #define hwloc_topology_alloc_group_object HWLOC_NAME(topology_alloc_group_object)
+#define hwloc_topology_free_group_object HWLOC_NAME(topology_free_group_object)
 #define hwloc_topology_insert_group_object HWLOC_NAME(topology_insert_group_object)
 #define hwloc_obj_add_other_obj_sets HWLOC_NAME(obj_add_other_obj_sets)
 #define hwloc_topology_refresh HWLOC_NAME(topology_refresh)

tests/hwloc/hwloc_groups.c

@@ -1,5 +1,5 @@
 /*
- * Copyright © 2011-2021 Inria.  All rights reserved.
+ * Copyright © 2011-2023 Inria.  All rights reserved.
  * See COPYING in top-level directory.
  */
 
@@ -31,6 +31,17 @@ int main(void)
   hwloc_topology_load(topology);
   root = hwloc_get_root_obj(topology);
   assert(hwloc_topology_get_depth(topology) == 3);
+  /* free instead of inserting */
+  group = hwloc_topology_alloc_group_object(topology);
+  assert(group);
+  group->cpuset = hwloc_bitmap_dup(root->cpuset);
+  err = hwloc_topology_free_group_object(topology, group);
+  assert(err == 0);
+  /* insert without sets, fails */
+  group = hwloc_topology_alloc_group_object(topology);
+  assert(group);
+  res = hwloc_topology_insert_group_object(topology, group);
+  assert(res == NULL);
   /* insert a group identical to root, will be merged */
   group = hwloc_topology_alloc_group_object(topology);
   assert(group);