Skip to content

Cleanup OS provider config API #209

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 3 commits into from
Feb 13, 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 @@ -104,7 +104,6 @@ static umf_os_memory_provider_params_t UMF_OS_MEMORY_PROVIDER_PARAMS = {
/* .nodemask = */ NULL,
/* .maxnode = */ 0,
/* .numa_mode = */ UMF_NUMA_MODE_DEFAULT,
/* .numa_flags = */ 0,

// others
/* .traces = */ OS_MEMORY_PROVIDER_TRACE,
Expand Down
14 changes: 0 additions & 14 deletions include/umf/providers/provider_os_memory.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,21 +34,10 @@ typedef enum umf_numa_mode_t {
UMF_NUMA_MODE_DEFAULT,
UMF_NUMA_MODE_BIND,
UMF_NUMA_MODE_INTERLEAVE,
/* TODO: consider removing UMF_NUMA_MODE_PREFERRED and rely only on combination of BIND mode and STRICT flag like hwloc */
UMF_NUMA_MODE_PREFERRED,
UMF_NUMA_MODE_LOCAL,
UMF_NUMA_MODE_STATIC_NODES,
UMF_NUMA_MODE_RELATIVE_NODES,
} umf_numa_mode_t;

typedef enum umf_numa_flags_t {
UMF_NUMA_FLAGS_STRICT = (1 << 0),
UMF_NUMA_FLAGS_MOVE = (1 << 1),
UMF_NUMA_FLAGS_MOVE_ALL = (1 << 2),

UMF_NUMA_FLAGS_MAX // must be the last one
} umf_numa_flags_t;

typedef enum umf_purge_advise_t {
UMF_PURGE_LAZY,
UMF_PURGE_FORCE,
Expand All @@ -69,8 +58,6 @@ typedef struct umf_os_memory_provider_params_t {
unsigned long maxnode;
/// flag that relates to one of the MPOL_* flags used in internal mbind() calls
umf_numa_mode_t numa_mode;
/// combination of 'umf_numa_flags_t' flags
unsigned numa_flags;

// others
/// log level of debug traces
Expand Down Expand Up @@ -98,7 +85,6 @@ umfOsMemoryProviderParamsDefault(void) {
NULL, /* nodemask */
0, /* maxnode */
UMF_NUMA_MODE_DEFAULT, /* numa_mode */
0, /* numa_flags */
0 /* traces */
};

Expand Down
1 change: 0 additions & 1 deletion src/memory_targets/memory_target_numa.c
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,6 @@ static enum umf_result_t numa_memory_provider_create_from_memspace(
params.nodemask = nodemask;
params.maxnode = maxnode;
params.numa_mode = UMF_NUMA_MODE_BIND;
params.numa_flags = UMF_NUMA_FLAGS_STRICT;

umf_memory_provider_handle_t numaProvider = NULL;
ret = umfMemoryProviderCreate(umfOsMemoryProviderOps(), &params,
Expand Down
40 changes: 5 additions & 35 deletions src/provider/provider_os_memory.c
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ typedef struct umf_os_memory_provider_config_t {
unsigned long *nodemask;
unsigned long maxnode;
unsigned numa_mode;
unsigned numa_flags; // combination of OS-specific NUMA flags

// others
int traces; // log level of debug traces
Expand Down Expand Up @@ -158,38 +157,18 @@ static hwloc_membind_policy_t translate_numa_mode(umf_numa_mode_t mode,
return -1;
}
return HWLOC_MEMBIND_BIND;
case UMF_NUMA_MODE_STATIC_NODES: // unsupported
// MPOL_F_STATIC_NODES is undefined
return -1;
case UMF_NUMA_MODE_RELATIVE_NODES: // unsupported
// MPOL_F_RELATIVE_NODES is undefined
return -1;
}
return -1;
}

static int translate_one_numa_flag(unsigned numa_flag) {
switch (numa_flag) {
case UMF_NUMA_FLAGS_STRICT:
return HWLOC_MEMBIND_STRICT;
case UMF_NUMA_FLAGS_MOVE:
return HWLOC_MEMBIND_MIGRATE;
case UMF_NUMA_FLAGS_MOVE_ALL:
return -1; /* unsupported */
}
return -1;
}

static int translate_numa_flags(unsigned numa_flags, umf_numa_mode_t mode) {
// translate numa_flags - combination of 'umf_numa_flags_t' flags
int flags = os_translate_flags(numa_flags, UMF_NUMA_FLAGS_MAX,
translate_one_numa_flag);
static int getHwlocMembindFlags(umf_numa_mode_t mode) {
/* UMF always operates on NUMA nodes */
int flags = HWLOC_MEMBIND_BYNODESET;
if (mode == UMF_NUMA_MODE_BIND) {
/* HWLOC uses MPOL_PREFERRED[_MANY] unless HWLOC_MEMBIND_STRICT is specified */
flags |= HWLOC_MEMBIND_STRICT;
}
/* UMF always operates on NUMA nodes */
return flags | HWLOC_MEMBIND_BYNODESET;
return flags;
}

static umf_result_t translate_params(umf_os_memory_provider_params_t *in_params,
Expand Down Expand Up @@ -230,16 +209,7 @@ static umf_result_t translate_params(umf_os_memory_provider_params_t *in_params,
return UMF_RESULT_ERROR_INVALID_ARGUMENT;
}
provider->numa_policy = ret;

ret = translate_numa_flags(in_params->numa_flags, in_params->numa_mode);
if (ret < 0) {
if (in_params->traces) {
fprintf(stderr, "error: incorrect NUMA flags: %u\n",
in_params->numa_flags);
}
return UMF_RESULT_ERROR_INVALID_ARGUMENT;
}
provider->numa_flags = ret;
provider->numa_flags = getHwlocMembindFlags(in_params->numa_mode);

return nodemask_to_hwloc_nodeset(in_params->nodemask, in_params->maxnode,
&provider->nodeset);
Expand Down
16 changes: 0 additions & 16 deletions test/provider_os_memory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -143,22 +143,6 @@ TEST_F(test, create_WRONG_NUMA_MODE) {
ASSERT_EQ(os_memory_provider, nullptr);
}

TEST_F(test, create_WRONG_NUMA_FLAGS) {
umf_result_t umf_result;
umf_memory_provider_handle_t os_memory_provider = nullptr;
umf_os_memory_provider_params_t os_memory_provider_params =
umfOsMemoryProviderParamsDefault();

// wrong NUMA flags
os_memory_provider_params.numa_flags = (unsigned int)-1;

umf_result = umfMemoryProviderCreate(umfOsMemoryProviderOps(),
&os_memory_provider_params,
&os_memory_provider);
ASSERT_EQ(umf_result, UMF_RESULT_ERROR_INVALID_ARGUMENT);
ASSERT_EQ(os_memory_provider, nullptr);
}

// positive tests using test_alloc_free_success

auto defaultParams = umfOsMemoryProviderParamsDefault();
Expand Down
1 change: 0 additions & 1 deletion test/provider_os_memory_config.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,6 @@ INSTANTIATE_TEST_SUITE_P(numa_modes, providerConfigTestNumaMode,
testing::Values(UMF_NUMA_MODE_DEFAULT,
UMF_NUMA_MODE_BIND,
UMF_NUMA_MODE_INTERLEAVE,
UMF_NUMA_MODE_PREFERRED,
UMF_NUMA_MODE_LOCAL));
#ifndef MPOL_LOCAL
#define MPOL_LOCAL 4
Expand Down