Checkpoint split semantics rework. (#113)

Checkpoint work toward improving our hardware resource split semantics. Signed-off-by: Samuel K. Gutierrez <[email protected]>
hpc · Apr 22, 2024 · 883c1ff · 883c1ff
1 parent a0a93c3
commit 883c1ff
Show file tree

Hide file tree

Showing 9 changed files with 109 additions and 161 deletions.
diff --git a/src/qvi-hwloc.cc b/src/qvi-hwloc.cc
@@ -59,7 +59,7 @@ static const int QVI_HWLOC_DEVICE_INVISIBLE_ID = -1;
 typedef struct qvi_hwloc_device_s {
     int qvim_rc = QV_ERR_INTERNAL;
     /** Device cpuset */
-    qvi_hwloc_bitmap_t cpuset;
+    qvi_hwloc_bitmap_s cpuset;
     /** Internal object type information */
     qvi_hwloc_objx_t objx = {};
     /** Vendor ID */
@@ -1421,20 +1421,18 @@ split_cpuset_by_range(
     hwloc_const_cpuset_t cpuset,
     int base,
     int extent,
-    hwloc_bitmap_t *result
+    hwloc_bitmap_t result
 ) {
-    // Allocate and zero-out a new bitmap that will encode the split.
-    hwloc_bitmap_t iresult = nullptr;
-    int rc = qvi_hwloc_bitmap_calloc(&iresult);
-    if (rc != QV_SUCCESS) goto out;
+    // Zero-out the result bitmap that will encode the split.
+    hwloc_bitmap_zero(result);
     // We use PUs to split resources. Each set bit represents a PU. The number
     // of bits set represents the number of PUs present on the system. The
     // least-significant (right-most) bit represents logical ID 0.
-    int pu_depth;
-    rc = qvi_hwloc_obj_type_depth(
+    int pu_depth = 0;
+    int rc = qvi_hwloc_obj_type_depth(
         hwl, QV_HW_OBJ_PU, &pu_depth
     );
-    if (rc != QV_SUCCESS) goto out;
+    if (rc != QV_SUCCESS) return rc;
     // Calculate split based on given range.
     for (int i = base; i < base + extent; ++i) {
         hwloc_obj_t dobj;
@@ -1443,17 +1441,12 @@ split_cpuset_by_range(
         );
         if (rc != QV_SUCCESS) break;
 
-        rc = hwloc_bitmap_or(iresult, iresult, dobj->cpuset);
+        rc = hwloc_bitmap_or(result, result, dobj->cpuset);
         if (rc != 0) {
             rc = QV_ERR_HWLOC;
             break;
         }
     }
-out:
-    if (rc != QV_SUCCESS) {
-        qvi_hwloc_bitmap_free(&iresult);
-    }
-    *result = iresult;
     return rc;
 }
 
@@ -1503,36 +1496,29 @@ qvi_hwloc_split_cpuset_by_color(
     hwloc_const_cpuset_t cpuset,
     int ncolors,
     int color,
-    hwloc_cpuset_t *result
+    hwloc_cpuset_t result
 ) {
     int chunk = 0;
     int rc = split_cpuset_chunk_size(
         hwl, cpuset, ncolors, &chunk
     );
-    if (rc != QV_SUCCESS) goto out;
+    if (rc != QV_SUCCESS) return rc;
     // This happens when n > npus. We can't support that split.
     // TODO(skg) Perhaps we can create an empty cpuset that denotes no
     // resources?
     if (chunk == 0) {
-        rc = QV_ERR_SPLIT;
-        goto out;
+        return QV_ERR_SPLIT;
     }
     // Group IDs must be < n: 0, 1, ... , ncolors-1.
     // TODO(skg) We could also allow this. That might mean that this processor
     // should not be considered in the split.
     if (color >= ncolors) {
-        rc = QV_ERR_SPLIT;
-        goto out;
+        return QV_ERR_SPLIT;
     }
 
-    rc = split_cpuset_by_range(
+    return split_cpuset_by_range(
         hwl, cpuset, chunk * color, chunk, result
     );
-out:
-    if (rc != QV_SUCCESS) {
-        qvi_hwloc_bitmap_free(result);
-    }
-    return rc;
 }
 
 int

diff --git a/src/qvi-hwloc.h b/src/qvi-hwloc.h
@@ -27,9 +27,6 @@ extern "C" {
 struct qvi_hwloc_s;
 typedef struct qvi_hwloc_s qvi_hwloc_t;
 
-struct qvi_hwloc_bitmap_s;
-typedef struct qvi_hwloc_bitmap_s qvi_hwloc_bitmap_t;
-
 struct qvi_hwloc_device_s;
 typedef struct qvi_hwloc_device_s qvi_hwloc_device_t;
 
@@ -384,7 +381,7 @@ qvi_hwloc_split_cpuset_by_color(
     hwloc_const_cpuset_t cpuset,
     int ncolors,
     int color,
-    hwloc_cpuset_t *result
+    hwloc_cpuset_t result
 );
 
 /**
@@ -420,11 +417,25 @@ struct qvi_hwloc_bitmap_s {
     int qvim_rc = QV_ERR_INTERNAL;
     /** Internal bitmap */
     hwloc_bitmap_t data = nullptr;
-    /** Constructor */
+    /** Default constructor */
     qvi_hwloc_bitmap_s(void)
     {
         qvim_rc = qvi_hwloc_bitmap_calloc(&data);
     }
+    /** Construct via hwloc_const_bitmap_t */
+    qvi_hwloc_bitmap_s(hwloc_const_bitmap_t bitmap)
+    {
+        qvim_rc = qvi_hwloc_bitmap_calloc(&data);
+        if (qvim_rc != QV_SUCCESS) return;
+        set(bitmap);
+    }
+    /** Copy constructor. */
+    qvi_hwloc_bitmap_s(const qvi_hwloc_bitmap_s &src)
+    {
+        qvim_rc = qvi_hwloc_bitmap_calloc(&data);
+        if (qvim_rc != QV_SUCCESS) return;
+        qvim_rc = qvi_hwloc_bitmap_copy(src.data, data);
+    }
     /** Destructor */
     ~qvi_hwloc_bitmap_s(void)
     {
@@ -440,8 +451,21 @@ struct qvi_hwloc_bitmap_s {
     {
         qvim_rc = qvi_hwloc_bitmap_copy(src.data, data);
     }
+    /**
+     * Sets the object's internal bitmap to match src's.
+     */
+    int
+    set(hwloc_const_bitmap_t src)
+    {
+        return qvi_hwloc_bitmap_copy(src, data);
+    }
 };
 
+/**
+ * Vector of cpuset objects.
+ */
+using qvi_hwloc_cpusets_t = std::vector<qvi_hwloc_bitmap_s>;
+
 #endif
 
 #endif

diff --git a/src/qvi-hwpool.h b/src/qvi-hwpool.h
@@ -65,7 +65,7 @@ struct qvi_hwpool_resource_s {
 struct qvi_hwpool_cpus_s : qvi_hwpool_resource_s {
     int qvim_rc = QV_ERR_INTERNAL;
     /** The cpuset of the maintained CPUs. */
-    qvi_hwloc_bitmap_t cpuset;
+    qvi_hwloc_bitmap_s cpuset;
     /** Constructor */
     qvi_hwpool_cpus_s(void)
     {
@@ -87,7 +87,7 @@ struct qvi_hwpool_devinfo_s {
     /** UUID */
     std::string uuid;
     /** The bitmap encoding CPU affinity. */
-    qvi_hwloc_bitmap_t affinity;
+    qvi_hwloc_bitmap_s affinity;
     /** No default constructor. */
     qvi_hwpool_devinfo_s(void) = delete;
     /** Constructor */

diff --git a/src/qvi-map.cc b/src/qvi-map.cc
@@ -129,7 +129,7 @@ int
 qvi_map_packed(
     qvi_map_t &map,
     uint_t nfids,
-    const qvi_map_cpusets_t &tres
+    const qvi_hwloc_cpusets_t &tres
 ) {
     const uint_t ntres = tres.size();
     // Max consumers per resource.
@@ -160,7 +160,7 @@ int
 qvi_map_spread(
     qvi_map_t &map,
     uint_t nfids,
-    const qvi_map_cpusets_t &tres
+    const qvi_hwloc_cpusets_t &tres
 ) {
     const uint_t ntres = tres.size();
     // Keeps track of the next IDs to map: 'from' and 'to' IDs.
@@ -196,8 +196,8 @@ qvi_map_disjoint_affinity(
 
 int
 qvi_map_calc_shaffinity(
-    const qvi_map_cpusets_t &faffs,
-    const qvi_map_cpusets_t &tores,
+    const qvi_hwloc_cpusets_t &faffs,
+    const qvi_hwloc_cpusets_t &tores,
     qvi_map_shaffinity_t &res_affinity_map
 ) {
     // Number of consumers.
@@ -208,7 +208,7 @@ qvi_map_calc_shaffinity(
     for (uint_t cid = 0; cid < ncon; ++cid) {
         for (uint_t rid = 0; rid < nres; ++rid) {
             const int intersects = hwloc_bitmap_intersects(
-                faffs.at(cid), tores.at(rid)
+                faffs.at(cid).data, tores.at(rid).data
             );
             if (intersects) {
                 res_affinity_map[rid].insert(cid);
@@ -222,11 +222,11 @@ int
 qvi_map_affinity_preserving(
     qvi_map_t &map,
     qvi_map_affinity_preserving_policy_t policy,
-    const qvi_map_cpusets_t &faffs,
-    const qvi_map_cpusets_t &tores
+    const qvi_hwloc_cpusets_t &faffs,
+    const qvi_hwloc_cpusets_t &tores
 ) {
     using map_fn_t = std::function<
-        int(qvi_map_t &map, uint_t nfids, const qvi_map_cpusets_t &tres)
+        int(qvi_map_t &map, uint_t nfids, const qvi_hwloc_cpusets_t &tres)
     >;
 
     int rc = QV_SUCCESS;
@@ -293,10 +293,10 @@ qvi_map_affinity_preserving(
 hwloc_const_cpuset_t
 qvi_map_cpuset_at(
     const qvi_map_t &map,
-    const qvi_map_cpusets_t &cpusets,
+    const qvi_hwloc_cpusets_t &cpusets,
     int fid
 ) {
-    return cpusets.at(map.at(fid));
+    return cpusets.at(map.at(fid)).data;
 }
 
 std::vector<int>

diff --git a/src/qvi-map.h b/src/qvi-map.h
@@ -15,6 +15,7 @@
 #define QVI_MAP_H
 
 #include "qvi-common.h"
+#include "qvi-hwloc.h"
 
 /**
  * Modes used to influence how affinity preserving mapping is done.
@@ -51,12 +52,6 @@ using qvi_map_t = std::map<int, int>;
  */
 using qvi_map_shaffinity_t = std::map<int, std::set<int>>;
 
-/**
- * Vector of cpusets.
- */
-// TODO(skg) Update to use qvi_hwloc_bitmap_t.
-using qvi_map_cpusets_t = std::vector<hwloc_cpuset_t>;
-
 /**
  * Prints debug output.
  */
@@ -90,14 +85,14 @@ int
 qvi_map_packed(
     qvi_map_t &map,
     uint_t nfids,
-    const qvi_map_cpusets_t &tres
+    const qvi_hwloc_cpusets_t &tres
 );
 
 int
 qvi_map_spread(
     qvi_map_t &map,
     uint_t nfids,
-    const qvi_map_cpusets_t &tres
+    const qvi_hwloc_cpusets_t &tres
 );
 
 /**
@@ -106,8 +101,8 @@ qvi_map_spread(
  */
 int
 qvi_map_calc_shaffinity(
-    const qvi_map_cpusets_t &faffs,
-    const qvi_map_cpusets_t &tores,
+    const qvi_hwloc_cpusets_t &faffs,
+    const qvi_hwloc_cpusets_t &tores,
     qvi_map_shaffinity_t &res_affinity_map
 );
 
@@ -124,8 +119,8 @@ int
 qvi_map_affinity_preserving(
     qvi_map_t &map,
     qvi_map_affinity_preserving_policy_t policy,
-    const qvi_map_cpusets_t &faffs,
-    const qvi_map_cpusets_t &tores
+    const qvi_hwloc_cpusets_t &faffs,
+    const qvi_hwloc_cpusets_t &tores
 );
 
 /**
@@ -134,7 +129,7 @@ qvi_map_affinity_preserving(
 hwloc_const_cpuset_t
 qvi_map_cpuset_at(
     const qvi_map_t &map,
-    const qvi_map_cpusets_t &cpusets,
+    const qvi_hwloc_cpusets_t &cpusets,
     int fid
 );
 

diff --git a/src/qvi-rmi.cc b/src/qvi-rmi.cc
@@ -1184,22 +1184,6 @@ qvi_rmi_get_device_in_cpuset(
     return rpcrc;
 }
 
-int
-qvi_rmi_split_cpuset_by_color(
-    qvi_rmi_client_t *client,
-    hwloc_const_cpuset_t cpuset,
-    int ncolors,
-    int color,
-    hwloc_cpuset_t *result
-) {
-    // TODO(skg) At some point we will acquire the resources
-    // for improved splitting and resource distribution.
-    return qvi_hwloc_split_cpuset_by_color(
-        client->config->hwloc, cpuset,
-        ncolors, color, result
-    );
-}
-
 int
 qvi_rmi_get_cpuset_for_nobjs(
     qvi_rmi_client_t *client,

diff --git a/src/qvi-rmi.h b/src/qvi-rmi.h
@@ -167,18 +167,6 @@ qvi_rmi_get_device_in_cpuset(
     char **dev_id
 );
 
-/**
- *
- */
-int
-qvi_rmi_split_cpuset_by_color(
-    qvi_rmi_client_t *client,
-    hwloc_const_cpuset_t cpuset,
-    int ncolors,
-    int color,
-    hwloc_cpuset_t *result
-);
-
 int
 qvi_rmi_get_cpuset_for_nobjs(
     qvi_rmi_client_t *client,