Merge pull request #390 from igchor/memspace_highest_capacity_alloc

Fix highest capacity memspace with OS provider integration.

Author: lukaszstolarczuk

.github/workflows/qemu.yml

@@ -73,11 +73,11 @@ jobs:
         -machine q35,usb=off,hmat=on \
         -enable-kvm \
         -net nic -net user,hostfwd=tcp::2222-:22 \
-        -m 3G \
+        -m 3500M \
         -smp 4 \
-        -object memory-backend-ram,size=1G,id=ram0 \
-        -object memory-backend-ram,size=1G,id=ram1 \
-        -object memory-backend-ram,size=1G,id=ram2 \
+        -object memory-backend-ram,size=1100M,id=ram0 \
+        -object memory-backend-ram,size=1200M,id=ram1 \
+        -object memory-backend-ram,size=1200M,id=ram2 \
         -numa node,nodeid=0,memdev=ram0,cpus=0-1 \
         -numa node,nodeid=1,memdev=ram1,cpus=2-3 \
         -numa node,nodeid=2,memdev=ram2,initiator=0 \

scripts/qemu/run-build.sh

@@ -36,3 +36,6 @@ make -j $(nproc)
 
 ctest --output-on-failure
 
+# run tests bound to a numa node
+numactl -N 0 ctest --output-on-failure
+numactl -N 1 ctest --output-on-failure

src/memory_targets/memory_target_numa.c

@@ -21,7 +21,7 @@
 #include "topology.h"
 
 struct numa_memory_target_t {
-    size_t id;
+    size_t physical_id;
 };
 
 static umf_result_t numa_initialize(void *params, void **memTarget) {
@@ -38,7 +38,7 @@ static umf_result_t numa_initialize(void *params, void **memTarget) {
         return UMF_RESULT_ERROR_OUT_OF_HOST_MEMORY;
     }
 
-    numaTarget->id = config->id;
+    numaTarget->physical_id = config->physical_id;
     *memTarget = numaTarget;
     return UMF_RESULT_SUCCESS;
 }
@@ -60,7 +60,7 @@ numa_targets_create_nodemask(struct numa_memory_target_t **targets,
     }
 
     for (size_t i = 0; i < numTargets; i++) {
-        if (hwloc_bitmap_set(bitmap, targets[i]->id)) {
+        if (hwloc_bitmap_set(bitmap, targets[i]->physical_id)) {
             hwloc_bitmap_free(bitmap);
             return UMF_RESULT_ERROR_OUT_OF_HOST_MEMORY;
         }
@@ -106,6 +106,7 @@ static enum umf_result_t numa_memory_provider_create_from_memspace(
     umf_memspace_policy_handle_t policy,
     umf_memory_provider_handle_t *provider) {
     (void)memspace;
+    (void)numTargets;
     // TODO: apply policy
     (void)policy;
 
@@ -115,9 +116,18 @@ static enum umf_result_t numa_memory_provider_create_from_memspace(
     unsigned long *nodemask;
     unsigned maxnode;
     size_t nodemask_size;
+    size_t numNodesProvider;
+
+    if (memspace == umfMemspaceHighestCapacityGet()) {
+        // Pass only a single node to provider for now.
+        // TODO: change this once we implement memspace policies
+        numNodesProvider = 1;
+    } else {
+        numNodesProvider = numTargets;
+    }
 
     umf_result_t ret = numa_targets_create_nodemask(
-        numaTargets, numTargets, &nodemask, &maxnode, &nodemask_size);
+        numaTargets, numNodesProvider, &nodemask, &maxnode, &nodemask_size);
     if (ret != UMF_RESULT_SUCCESS) {
         return ret;
     }
@@ -160,7 +170,7 @@ static umf_result_t numa_clone(void *memTarget, void **outMemTarget) {
         return UMF_RESULT_ERROR_OUT_OF_HOST_MEMORY;
     }
 
-    newNumaTarget->id = numaTarget->id;
+    newNumaTarget->physical_id = numaTarget->physical_id;
     *outMemTarget = newNumaTarget;
     return UMF_RESULT_SUCCESS;
 }
@@ -171,9 +181,8 @@ static umf_result_t numa_get_capacity(void *memTarget, size_t *capacity) {
         return UMF_RESULT_ERROR_NOT_SUPPORTED;
     }
 
-    hwloc_obj_t numaNode =
-        hwloc_get_obj_by_type(topology, HWLOC_OBJ_NUMANODE,
-                              ((struct numa_memory_target_t *)memTarget)->id);
+    hwloc_obj_t numaNode = hwloc_get_numanode_obj_by_os_index(
+        topology, ((struct numa_memory_target_t *)memTarget)->physical_id);
     if (!numaNode) {
         return UMF_RESULT_ERROR_INVALID_ARGUMENT;
     }

src/memory_targets/memory_target_numa.h

@@ -21,7 +21,7 @@ extern "C" {
 #endif
 
 struct umf_numa_memory_target_config_t {
-    size_t id;
+    size_t physical_id;
 };
 
 extern struct umf_memory_target_ops_t UMF_MEMORY_TARGET_NUMA_OPS;

test/common/numa_helpers.h

@@ -0,0 +1,32 @@
+// Copyright (C) 2024 Intel Corporation
+// Under the Apache License v2.0 with LLVM Exceptions. See LICENSE.TXT.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+
+#ifndef UMF_NUMA_HELPERS_H
+#define UMF_NUMA_HELPERS_H 1
+
+#include <numa.h>
+#include <numaif.h>
+#include <stdint.h>
+#include <stdio.h>
+
+#include "test_helpers.h"
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+// returns the node where page starting at 'ptr' resides
+int getNumaNodeByPtr(void *ptr) {
+    int nodeId;
+    int retm =
+        get_mempolicy(&nodeId, nullptr, 0, ptr, MPOL_F_ADDR | MPOL_F_NODE);
+    UT_ASSERTeq(retm, 0);
+    return nodeId;
+}
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* UMF_NUMA_HELPERS_H */

test/memspaces/memspace_highest_capacity.cpp

@@ -5,6 +5,7 @@
 #include "memory_target_numa.h"
 #include "memspace_helpers.hpp"
 #include "memspace_internal.h"
+#include "numa_helpers.h"
 #include "test_helpers.h"
 
 #include <numa.h>
@@ -39,28 +40,29 @@ TEST_F(memspaceHighestCapacityProviderTest, highestCapacityVerify) {
     static constexpr size_t alloc_size = 1024;
 
     long long maxCapacity = 0;
-    int maxCapacityNode = -1;
+    std::vector<int> maxCapacityNodes{};
     for (auto nodeId : nodeIds) {
         if (numa_node_size64(nodeId, nullptr) > maxCapacity) {
-            maxCapacityNode = nodeId;
             maxCapacity = numa_node_size64(nodeId, nullptr);
         }
     }
 
+    for (auto nodeId : nodeIds) {
+        if (numa_node_size64(nodeId, nullptr) == maxCapacity) {
+            maxCapacityNodes.push_back(nodeId);
+        }
+    }
+
     // Confirm that the HighestCapacity memspace indeed has highest capacity
     void *ptr;
     auto ret = umfMemoryProviderAlloc(hProvider, alloc_size, 0, &ptr);
+    memset(ptr, 0, alloc_size);
     ASSERT_EQ(ret, UMF_RESULT_SUCCESS);
 
-    struct bitmask *nodemask = numa_allocate_nodemask();
-    UT_ASSERTne(nodemask, nullptr);
-    int retm = get_mempolicy(nullptr, nodemask->maskp, nodemask->size, ptr,
-                             MPOL_F_ADDR);
-    UT_ASSERTeq(retm, 0);
-    UT_ASSERT(numa_bitmask_isbitset(nodemask, maxCapacityNode));
+    auto nodeId = getNumaNodeByPtr(ptr);
+    ASSERT_TRUE(std::any_of(maxCapacityNodes.begin(), maxCapacityNodes.end(),
+                            [nodeId](int node) { return nodeId == node; }));
 
     ret = umfMemoryProviderFree(hProvider, ptr, alloc_size);
     ASSERT_EQ(ret, UMF_RESULT_SUCCESS);
-
-    numa_bitmask_free(nodemask);
 }

test/memspaces/memspace_host_all.cpp

@@ -5,6 +5,7 @@
 #include "memory_target_numa.h"
 #include "memspace_helpers.hpp"
 #include "memspace_internal.h"
+#include "numa_helpers.h"
 #include "test_helpers.h"
 #include "utils_sanitizers.h"
 
@@ -27,7 +28,7 @@ TEST_F(numaNodesTest, memspaceGet) {
         struct umf_numa_memory_target_config_t *numaTargetCfg =
             (struct umf_numa_memory_target_config_t *)hMemspace->nodes[i]->priv;
         UT_ASSERT(std::find(nodeIds.begin(), nodeIds.end(),
-                            numaTargetCfg->id) != nodeIds.end());
+                            numaTargetCfg->physical_id) != nodeIds.end());
     }
 }
 
@@ -91,9 +92,7 @@ static void getAllocationPolicy(void *ptr, unsigned long maxNodeId, int &mode,
     }
 
     // Get the node that allocated the memory at 'ptr'.
-    int nodeId = -1;
-    ret = get_mempolicy(&nodeId, nullptr, 0, ptr, MPOL_F_ADDR | MPOL_F_NODE);
-    UT_ASSERTeq(ret, 0);
+    int nodeId = getNumaNodeByPtr(ptr);
     allocNodeId = static_cast<size_t>(nodeId);
 }
 

test/provider_os_memory_multiple_numa_nodes.cpp

@@ -3,6 +3,7 @@
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 
 #include "base.hpp"
+#include "numa_helpers.h"
 
 #include <numa.h>
 #include <numaif.h>
@@ -52,14 +53,6 @@ struct testNumaNodes : public testing::TestWithParam<int> {
         ASSERT_NE(os_memory_provider, nullptr);
     }
 
-    int retrieve_numa_node_number(void *addr) {
-        int numa_node;
-        int ret = get_mempolicy(&numa_node, nullptr, 0, addr,
-                                MPOL_F_NODE | MPOL_F_ADDR);
-        EXPECT_EQ(ret, 0);
-        return numa_node;
-    }
-
     void TearDown() override {
         umf_result_t umf_result;
         if (ptr) {
@@ -108,6 +101,6 @@ TEST_P(testNumaNodes, checkNumaNodesAllocations) {
     // This pointer must point to an initialized value before retrieving a number of
     // the numa node that the pointer was allocated on (calling get_mempolicy).
     memset(ptr, 0xFF, alloc_size);
-    int retrieved_numa_node_number = retrieve_numa_node_number(ptr);
+    int retrieved_numa_node_number = getNumaNodeByPtr(ptr);
     ASSERT_EQ(retrieved_numa_node_number, numa_node_number);
 }