Skip to content

Commit 13b0b68

Browse files
bratpiorkabratpiorka
committed
do not bind NUMA nodes for HostAll memspace
1 parent 32543b1 commit 13b0b68

File tree

3 files changed

+129
-54
lines changed

3 files changed

+129
-54
lines changed

‎src/memory_targets/memory_target_numa.c

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -87,22 +87,34 @@ static umf_result_t numa_memory_provider_create_from_memspace(
8787
return UMF_RESULT_ERROR_INVALID_ARGUMENT;
8888
}
8989
} else {
90-
params.numa_mode = UMF_NUMA_MODE_BIND;
90+
if (memspace == umfMemspaceHostAllGet()) {
91+
// For the default memspace, we use the default mode without any
92+
// call to mbind
93+
params.numa_mode = UMF_NUMA_MODE_DEFAULT;
94+
} else {
95+
params.numa_mode = UMF_NUMA_MODE_BIND;
96+
}
9197
}
9298

93-
params.numa_list =
94-
umf_ba_global_alloc(sizeof(*params.numa_list) * numNodesProvider);
99+
if (memspace == umfMemspaceHostAllGet() && policy == NULL) {
100+
// For default memspace with default policy we use all numa nodes so
101+
// simply left numa list empty
102+
params.numa_list_len = 0;
103+
params.numa_list = NULL;
104+
} else {
105+
params.numa_list =
106+
umf_ba_global_alloc(sizeof(*params.numa_list) * numNodesProvider);
95107

96-
if (!params.numa_list) {
97-
return UMF_RESULT_ERROR_OUT_OF_HOST_MEMORY;
98-
}
108+
if (!params.numa_list) {
109+
return UMF_RESULT_ERROR_OUT_OF_HOST_MEMORY;
110+
}
99111

100-
for (size_t i = 0; i < numNodesProvider; i++) {
101-
params.numa_list[i] = numaTargets[i]->physical_id;
112+
for (size_t i = 0; i < numNodesProvider; i++) {
113+
params.numa_list[i] = numaTargets[i]->physical_id;
114+
}
115+
params.numa_list_len = numNodesProvider;
102116
}
103117

104-
params.numa_list_len = numNodesProvider;
105-
106118
umf_memory_provider_handle_t numaProvider = NULL;
107119
int ret = umfMemoryProviderCreate(umfOsMemoryProviderOps(), &params,
108120
&numaProvider);

‎src/provider/provider_os_memory.c

Lines changed: 32 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -567,35 +567,39 @@ static umf_result_t os_alloc(void *provider, size_t size, size_t alignment,
567567
goto err_unmap;
568568
}
569569

570-
errno = 0;
571-
unsigned membind = get_membind(os_provider, ALIGN_UP(size, page_size));
572-
size_t bind_size = os_provider->nodeset_len == 1
573-
? size
574-
: ALIGN_UP(os_provider->part_size, page_size);
575-
char *ptr_iter = addr;
576-
577-
do {
578-
size_t s = bind_size < size ? bind_size : size;
579-
ret = hwloc_set_area_membind(
580-
os_provider->topo, ptr_iter, s, os_provider->nodeset[membind++],
581-
os_provider->numa_policy, os_provider->numa_flags);
582-
583-
size -= s;
584-
ptr_iter += s;
585-
membind %= os_provider->nodeset_len;
586-
if (ret) {
587-
os_store_last_native_error(UMF_OS_RESULT_ERROR_BIND_FAILED, errno);
588-
LOG_PERR("binding memory to NUMA node failed");
589-
// TODO: (errno == 0) when hwloc_set_area_membind() fails on Windows,
590-
// ignore this temporarily
591-
if (errno != ENOSYS &&
592-
errno != 0) { // ENOSYS - Function not implemented
593-
// Do not error out if memory binding is not implemented at all
594-
// (like in case of WSL on Windows).
595-
goto err_unmap;
570+
// Bind memory to NUMA nodes if numa_policy is other than DEFAULT
571+
if (os_provider->numa_policy != HWLOC_MEMBIND_DEFAULT) {
572+
errno = 0;
573+
unsigned membind = get_membind(os_provider, ALIGN_UP(size, page_size));
574+
size_t bind_size = os_provider->nodeset_len == 1
575+
? size
576+
: ALIGN_UP(os_provider->part_size, page_size);
577+
char *ptr_iter = addr;
578+
579+
do {
580+
size_t s = bind_size < size ? bind_size : size;
581+
ret = hwloc_set_area_membind(
582+
os_provider->topo, ptr_iter, s, os_provider->nodeset[membind++],
583+
os_provider->numa_policy, os_provider->numa_flags);
584+
585+
size -= s;
586+
ptr_iter += s;
587+
membind %= os_provider->nodeset_len;
588+
if (ret) {
589+
os_store_last_native_error(UMF_OS_RESULT_ERROR_BIND_FAILED,
590+
errno);
591+
LOG_PERR("binding memory to NUMA node failed");
592+
// TODO: (errno == 0) when hwloc_set_area_membind() fails on Windows,
593+
// ignore this temporarily
594+
if (errno != ENOSYS &&
595+
errno != 0) { // ENOSYS - Function not implemented
596+
// Do not error out if memory binding is not implemented at all
597+
// (like in case of WSL on Windows).
598+
goto err_unmap;
599+
}
596600
}
597-
}
598-
} while (size > 0);
601+
} while (size > 0);
602+
}
599603

600604
if (os_provider->fd > 0) {
601605
// store (fd_offset + 1) to be able to store fd_offset == 0

‎test/memspaces/memspace_host_all.cpp

Lines changed: 75 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2,18 +2,20 @@
22
// Under the Apache License v2.0 with LLVM Exceptions. See LICENSE.TXT.
33
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
44

5+
#include <numa.h>
6+
#include <numaif.h>
7+
#include <sys/mman.h>
8+
#include <unordered_set>
9+
10+
#include <umf/memspace.h>
11+
512
#include "memory_target_numa.h"
613
#include "memspace_helpers.hpp"
714
#include "memspace_internal.h"
815
#include "numa_helpers.h"
916
#include "test_helpers.h"
1017
#include "utils_sanitizers.h"
1118

12-
#include <numa.h>
13-
#include <numaif.h>
14-
#include <umf/memspace.h>
15-
#include <unordered_set>
16-
1719
using umf_test::test;
1820

1921
struct memspaceHostAllTest : ::numaNodesTest {
@@ -87,6 +89,69 @@ TEST_F(memspaceHostAllProviderTest, allocFree) {
8789
UT_ASSERTeq(ret, UMF_RESULT_SUCCESS);
8890
}
8991

92+
TEST_F(memspaceHostAllProviderTest, hostAllDefaults) {
93+
// This testcase checks if the allocations made using the provider with
94+
// default parameters based on default memspace (HostAll) uses the fast,
95+
// default kernel path (no mbind).
96+
97+
umf_memspace_handle_t hMemspace = umfMemspaceHostAllGet();
98+
UT_ASSERTne(hMemspace, nullptr);
99+
100+
umf_memory_provider_handle_t hProvider = nullptr;
101+
umf_result_t ret = umfMemoryProviderCreateFromMemspace(
102+
umfMemspaceHostAllGet(), NULL, &hProvider);
103+
ASSERT_EQ(ret, UMF_RESULT_SUCCESS);
104+
ASSERT_NE(hProvider, nullptr);
105+
106+
// Create single allocation using the provider.
107+
void *ptr1 = nullptr;
108+
size_t size = SIZE_4K;
109+
size_t alignment = 0;
110+
111+
ret = umfMemoryProviderAlloc(hProvider, size, alignment, &ptr1);
112+
UT_ASSERTeq(ret, UMF_RESULT_SUCCESS);
113+
UT_ASSERTne(ptr1, nullptr);
114+
memset(ptr1, 0xFF, size);
115+
116+
// Create single allocation using mmap
117+
void *ptr2 = mmap(nullptr, size, PROT_READ | PROT_WRITE,
118+
MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
119+
UT_ASSERTne(ptr2, nullptr);
120+
memset(ptr2, 0xFF, size);
121+
122+
// Compare UMF and kernel default allocation policy
123+
struct bitmask *nodemask1 = numa_allocate_nodemask();
124+
struct bitmask *nodemask2 = numa_allocate_nodemask();
125+
int memMode1 = -1, memMode2 = -1;
126+
127+
int ret2 = get_mempolicy(&memMode1, nodemask1->maskp, nodemask1->size, ptr1,
128+
MPOL_F_ADDR);
129+
UT_ASSERTeq(ret2, 0);
130+
ret2 = get_mempolicy(&memMode2, nodemask2->maskp, nodemask2->size, ptr2,
131+
MPOL_F_ADDR);
132+
UT_ASSERTeq(ret2, 0);
133+
UT_ASSERTeq(memMode1, memMode2);
134+
UT_ASSERTeq(nodemask1->size, nodemask2->size);
135+
UT_ASSERTeq(numa_bitmask_equal(nodemask1, nodemask2), 1);
136+
137+
int nodeId1 = -1, nodeId2 = -1;
138+
ret2 = get_mempolicy(&nodeId1, nullptr, 0, ptr1, MPOL_F_ADDR | MPOL_F_NODE);
139+
UT_ASSERTeq(ret2, 0);
140+
ret2 = get_mempolicy(&nodeId2, nullptr, 0, ptr2, MPOL_F_ADDR | MPOL_F_NODE);
141+
UT_ASSERTeq(ret2, 0);
142+
UT_ASSERTeq(nodeId1, nodeId2);
143+
144+
numa_free_nodemask(nodemask2);
145+
numa_free_nodemask(nodemask1);
146+
147+
ret2 = munmap(ptr2, size);
148+
UT_ASSERTeq(ret2, 0);
149+
150+
ret = umfMemoryProviderFree(hProvider, ptr1, size);
151+
UT_ASSERTeq(ret, UMF_RESULT_SUCCESS);
152+
umfMemoryProviderDestroy(hProvider);
153+
}
154+
90155
TEST_F(memspaceHostAllProviderTest, allocsSpreadAcrossAllNumaNodes) {
91156
// This testcase is unsuitable for TSan.
92157
#ifdef __SANITIZE_THREAD__
@@ -140,17 +205,11 @@ TEST_F(memspaceHostAllProviderTest, allocsSpreadAcrossAllNumaNodes) {
140205
size_t allocNodeId = SIZE_MAX;
141206
getAllocationPolicy(ptr, maxNodeId, mode, boundNodeIds, allocNodeId);
142207

143-
// 'BIND' mode specifies that the memory is bound to a set of NUMA nodes.
144-
// In case of 'HOST ALL' memspace, those set of nodes should be all
145-
// available nodes.
146-
UT_ASSERTeq(mode, MPOL_BIND);
147-
148-
// Confirm that the memory is bound to all the nodes from 'HOST ALL'
149-
// memspace.
150-
for (auto &id : nodeIds) {
151-
auto it = std::find(boundNodeIds.begin(), boundNodeIds.end(), id);
152-
UT_ASSERT(it != boundNodeIds.end());
153-
}
208+
// In case of 'HOST ALL' memspace, the default set of nodes (that
209+
// contains all available nodes) is used but get_mempolicy() would
210+
// return an empty set of nodes.
211+
UT_ASSERTeq(mode, MPOL_DEFAULT);
212+
UT_ASSERTeq(boundNodeIds.size(), 0);
154213

155214
// Confirm that the memory is allocated on one of the nodes in
156215
// 'HOST ALL' memspace.

0 commit comments

Comments
 (0)