Skip to content

Commit c5735e4

Browse files
bratpiorkabratpiorka
authored
Merge pull request #533 from bratpiorka/rrudnick_default_numa
Handle default kernel behavior for CPU access to all NUMA nodes
2 parents 670398b + 13b0b68 commit c5735e4

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,6 +2,13 @@
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_fixtures.hpp"
714
#include "memspace_helpers.hpp"
@@ -10,11 +17,6 @@
1017
#include "test_helpers.h"
1118
#include "utils_sanitizers.h"
1219

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

2022
struct memspaceHostAllTest : ::numaNodesTest {
@@ -88,6 +90,69 @@ TEST_F(memspaceHostAllProviderTest, allocFree) {
8890
UT_ASSERTeq(ret, UMF_RESULT_SUCCESS);
8991
}
9092

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

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

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

0 commit comments

Comments
 (0)