Skip to content

Remove umf_mem_visibility_t #236

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion benchmark/ubench.c
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,6 @@ UBENCH_EX(simple, glibc_malloc) {

static umf_os_memory_provider_params_t UMF_OS_MEMORY_PROVIDER_PARAMS = {
/* .protection = */ UMF_PROTECTION_READ | UMF_PROTECTION_WRITE,
/* .visibility = */ UMF_VISIBILITY_PRIVATE,

// NUMA config
/* .nodemask = */ NULL,
Expand Down
11 changes: 0 additions & 11 deletions include/umf/providers/provider_os_memory.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,6 @@ typedef enum umf_mem_protection_flags_t {
UMF_PROTECTION_MAX // must be the last one
} umf_mem_protection_flags_t;

/// @brief Visibility of the memory allocations
typedef enum umf_mem_visibility_t {
UMF_VISIBILITY_SHARED, ///< Updates to the memory allocated using OS provider are visible to other processes.
/// TODO: need to expose functionality to share open the mapping in other process and explicit sync?
UMF_VISIBILITY_PRIVATE, ///< Updates to the memory allocated using OS provider are not visible to other processes.
} umf_mem_visibility_t;

/// @brief Memory binding mode
///
/// Specifies how memory is bound to NUMA nodes on systems that support NUMA.
Expand All @@ -57,9 +50,6 @@ typedef struct umf_os_memory_provider_params_t {
/// combination of 'umf_mem_protection_flags_t' flags
unsigned protection;

/// shared or private visibility of memory mapped by a provider
umf_mem_visibility_t visibility;

// NUMA config
/// points to a bit mask of nodes containing up to maxnode bits, depending on
/// selected numa_mode newly allocated memory will be bound to those nodes
Expand Down Expand Up @@ -92,7 +82,6 @@ static inline umf_os_memory_provider_params_t
umfOsMemoryProviderParamsDefault(void) {
umf_os_memory_provider_params_t params = {
UMF_PROTECTION_READ | UMF_PROTECTION_WRITE, /* protection */
UMF_VISIBILITY_PRIVATE, /* visibility */
NULL, /* nodemask */
0, /* maxnode */
UMF_NUMA_MODE_DEFAULT, /* numa_mode */
Expand Down
29 changes: 2 additions & 27 deletions src/provider/provider_os_memory.c
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@

typedef struct umf_os_memory_provider_config_t {
unsigned protection; // combination of OS-specific protection flags
unsigned visibility;

// NUMA config
unsigned long *nodemask;
Expand All @@ -35,7 +34,6 @@ typedef struct umf_os_memory_provider_config_t {

typedef struct os_memory_provider_t {
unsigned protection; // combination of OS-specific protection flags
unsigned visibility;

// NUMA config
hwloc_bitmap_t nodeset;
Expand Down Expand Up @@ -204,16 +202,6 @@ static umf_result_t translate_params(umf_os_memory_provider_params_t *in_params,
}
provider->protection = ret;

ret = os_translate_mem_visibility(in_params->visibility);
if (ret < 0) {
if (in_params->traces) {
fprintf(stderr, "error: incorrect memory visibility mode: %u\n",
in_params->visibility);
}
return UMF_RESULT_ERROR_INVALID_ARGUMENT;
}
provider->visibility = ret;

// NUMA config
int emptyNodeset = (!in_params->maxnode || !in_params->nodemask);
ret = translate_numa_mode(in_params->numa_mode, emptyNodeset);
Expand Down Expand Up @@ -241,18 +229,6 @@ static umf_result_t os_initialize(void *params, void **provider) {
umf_os_memory_provider_params_t *in_params =
(umf_os_memory_provider_params_t *)params;

if (in_params->visibility == UMF_VISIBILITY_SHARED &&
in_params->numa_mode != UMF_NUMA_MODE_DEFAULT) {
// TODO: add support for that
if (in_params->traces) {
fprintf(stderr,
"NUMA binding mode (%i) not supported for "
"UMF_VISIBILITY_SHARED\n",
in_params->numa_mode);
}
return UMF_RESULT_ERROR_INVALID_ARGUMENT;
}

umf_ba_pool_t *base_allocator =
umf_ba_get_pool(sizeof(os_memory_provider_t));
if (!base_allocator) {
Expand Down Expand Up @@ -387,13 +363,12 @@ static umf_result_t os_alloc(void *provider, size_t size, size_t alignment,
return UMF_RESULT_ERROR_INVALID_ARGUMENT;
}

int flags = os_provider->visibility;
int protection = os_provider->protection;

void *addr = NULL;
errno = 0;
ret = os_mmap_aligned(NULL, size, alignment, page_size, protection, flags,
-1, 0, &addr);
ret = os_mmap_aligned(NULL, size, alignment, page_size, protection, -1, 0,
&addr);
if (ret) {
os_store_last_native_error(UMF_OS_RESULT_ERROR_ALLOC_FAILED, errno);
if (os_provider->traces) {
Expand Down
4 changes: 1 addition & 3 deletions src/provider/provider_os_memory_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,8 @@ int os_translate_flags(unsigned in_flags, unsigned max,

int os_translate_mem_protection_flags(unsigned protection);

int os_translate_mem_visibility(umf_mem_visibility_t visibility);

int os_mmap_aligned(void *hint_addr, size_t length, size_t alignment,
size_t page_size, int prot, int flags, int fd, long offset,
size_t page_size, int prot, int fd, long offset,
void **out_addr);

int os_munmap(void *addr, size_t length);
Expand Down
17 changes: 3 additions & 14 deletions src/provider/provider_os_memory_linux.c
Original file line number Diff line number Diff line change
Expand Up @@ -34,17 +34,6 @@ int os_translate_mem_protection_flags(unsigned protection_flags) {
os_translate_mem_protection_one_flag);
}

int os_translate_mem_visibility(umf_mem_visibility_t visibility) {
switch (visibility) {
case UMF_VISIBILITY_SHARED:
return MAP_SHARED;
case UMF_VISIBILITY_PRIVATE:
return MAP_PRIVATE;
}
assert(0);
return -1;
}

static int os_translate_purge_advise(umf_purge_advise_t advise) {
switch (advise) {
case UMF_PURGE_LAZY:
Expand All @@ -63,7 +52,7 @@ static inline void assert_is_page_aligned(uintptr_t ptr, size_t page_size) {
}

int os_mmap_aligned(void *hint_addr, size_t length, size_t alignment,
size_t page_size, int prot, int flags, int fd, long offset,
size_t page_size, int prot, int fd, long offset,
void **out_addr) {
assert(out_addr);

Expand All @@ -78,8 +67,8 @@ int os_mmap_aligned(void *hint_addr, size_t length, size_t alignment,
}

// MAP_ANONYMOUS - the mapping is not backed by any file
void *ptr = mmap(hint_addr, extended_length, prot, MAP_ANONYMOUS | flags,
fd, offset);
void *ptr = mmap(hint_addr, extended_length, prot,
MAP_ANONYMOUS | MAP_PRIVATE, fd, offset);
if (ptr == MAP_FAILED) {
return -1;
}
Expand Down
42 changes: 0 additions & 42 deletions test/provider_os_memory_config.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -188,45 +188,3 @@ TEST_P(providerConfigTestNumaMode, numa_modes) {
}
}
}

struct providerConfigTestVisibility
: providerConfigTest,
testing::WithParamInterface<umf_mem_visibility_t> {
std::string primary_str = "primary";
std::string new_str = "new";
std::string expected_str;
umf_os_memory_provider_params_t params = umfOsMemoryProviderParamsDefault();

void SetUp() override {
providerConfigTest::SetUp();
params.visibility = GetParam();
if (params.visibility == UMF_VISIBILITY_PRIVATE) {
expected_str = primary_str;
} else if (params.visibility == UMF_VISIBILITY_SHARED) {
expected_str = new_str;
}
}
};

INSTANTIATE_TEST_SUITE_P(visibility, providerConfigTestVisibility,
testing::Values(UMF_VISIBILITY_PRIVATE,
UMF_VISIBILITY_SHARED));

TEST_P(providerConfigTestVisibility, visibility) {
create_provider(&params);
allocate_memory();
write_memory(primary_str);

int pid = fork();
if (pid == 0) {
// child process
write_memory(new_str);
} else if (pid > 0) {
// main process
int status;
while (-1 == waitpid(0, &status, 0)) {
// wait for the child process to complete
}
EXPECT_EQ(static_cast<char *>(ptr), expected_str);
}
}