diff --git a/README.md b/README.md index 81a82bfab1..0c569c1b3c 100644 --- a/README.md +++ b/README.md @@ -209,12 +209,6 @@ Additionally, required for tests: A memory provider that provides memory from a device DAX (a character device file /dev/daxX.Y). It can be used when large memory mappings are needed. -The DevDax memory provider does not support the free operation -(`umfMemoryProviderFree()` always returns `UMF_RESULT_ERROR_NOT_SUPPORTED`), -so it should be used with a pool manager that will take over -the managing of the provided memory - for example the jemalloc pool -with the `disable_provider_free` parameter set to true. - ##### Requirements 1) Linux OS @@ -224,12 +218,6 @@ with the `disable_provider_free` parameter set to true. A memory provider that provides memory by mapping a regular, extendable file. -The file memory provider does not support the free operation -(`umfMemoryProviderFree()` always returns `UMF_RESULT_ERROR_NOT_SUPPORTED`), -so it should be used with a pool manager that will take over -the managing of the provided memory - for example the jemalloc pool -with the `disable_provider_free` parameter set to true. - IPC API requires the `UMF_MEM_MAP_SHARED` memory `visibility` mode (`UMF_RESULT_ERROR_INVALID_ARGUMENT` is returned otherwise). diff --git a/examples/dram_and_fsdax/dram_and_fsdax.c b/examples/dram_and_fsdax/dram_and_fsdax.c index 26f4517281..970242e109 100644 --- a/examples/dram_and_fsdax/dram_and_fsdax.c +++ b/examples/dram_and_fsdax/dram_and_fsdax.c @@ -78,41 +78,14 @@ static umf_memory_pool_handle_t create_fsdax_pool(const char *path) { } // Create an FSDAX memory pool - // - // The file memory provider does not support the free operation - // (`umfMemoryProviderFree()` always returns `UMF_RESULT_ERROR_NOT_SUPPORTED`), - // so it should be used with a pool manager that will take over - // the managing of the provided memory - for example the jemalloc pool - // with the `disable_provider_free` parameter set to true. - umf_jemalloc_pool_params_handle_t pool_params; - umf_result = umfJemallocPoolParamsCreate(&pool_params); - if (umf_result != UMF_RESULT_SUCCESS) { - fprintf(stderr, "Failed to create jemalloc params!\n"); - umfMemoryProviderDestroy(provider_fsdax); - return NULL; - } - umf_result = umfJemallocPoolParamsSetKeepAllMemory(pool_params, true); - if (umf_result != UMF_RESULT_SUCCESS) { - fprintf(stderr, "Failed to set KeepAllMemory!\n"); - umfMemoryProviderDestroy(provider_fsdax); - return NULL; - } - - // Create an FSDAX memory pool - umf_result = - umfPoolCreate(umfJemallocPoolOps(), provider_fsdax, pool_params, - UMF_POOL_CREATE_FLAG_OWN_PROVIDER, &pool_fsdax); + umf_result = umfPoolCreate(umfJemallocPoolOps(), provider_fsdax, NULL, + UMF_POOL_CREATE_FLAG_OWN_PROVIDER, &pool_fsdax); if (umf_result != UMF_RESULT_SUCCESS) { fprintf(stderr, "Failed to create an FSDAX memory pool!\n"); umfMemoryProviderDestroy(provider_fsdax); return NULL; } - umf_result = umfJemallocPoolParamsDestroy(pool_params); - if (umf_result != UMF_RESULT_SUCCESS) { - fprintf(stderr, "Failed to destroy jemalloc params!\n"); - } - return pool_fsdax; } diff --git a/include/umf/pools/pool_jemalloc.h b/include/umf/pools/pool_jemalloc.h index 0cbecd38f7..c30df65092 100644 --- a/include/umf/pools/pool_jemalloc.h +++ b/include/umf/pools/pool_jemalloc.h @@ -14,35 +14,8 @@ extern "C" { #endif -#include #include -struct umf_jemalloc_pool_params_t; - -/// @brief handle to the parameters of the jemalloc pool. -typedef struct umf_jemalloc_pool_params_t *umf_jemalloc_pool_params_handle_t; - -/// @brief Create a struct to store parameters of jemalloc pool. -/// @param hParams [out] handle to the newly created parameters struct. -/// @return UMF_RESULT_SUCCESS on success or appropriate error code on failure. -umf_result_t -umfJemallocPoolParamsCreate(umf_jemalloc_pool_params_handle_t *hParams); - -/// @brief Destroy parameters struct. -/// @param hParams handle to the parameters of the jemalloc pool. -/// @return UMF_RESULT_SUCCESS on success or appropriate error code on failure. -umf_result_t -umfJemallocPoolParamsDestroy(umf_jemalloc_pool_params_handle_t hParams); - -/// @brief Set if \p umfMemoryProviderFree() should never be called. -/// @param hParams handle to the parameters of the jemalloc pool. -/// @param keepAllMemory \p true if the jemalloc pool should not call -/// \p umfMemoryProviderFree, \p false otherwise. -/// @return UMF_RESULT_SUCCESS on success or appropriate error code on failure. -umf_result_t -umfJemallocPoolParamsSetKeepAllMemory(umf_jemalloc_pool_params_handle_t hParams, - bool keepAllMemory); - umf_memory_pool_ops_t *umfJemallocPoolOps(void); #ifdef __cplusplus diff --git a/src/libumf.def b/src/libumf.def index f2b24be6c2..3c0b63ce52 100644 --- a/src/libumf.def +++ b/src/libumf.def @@ -37,9 +37,6 @@ EXPORTS umfGetIPCHandle umfGetLastFailedMemoryProvider umfJemallocPoolOps - umfJemallocPoolParamsCreate - umfJemallocPoolParamsDestroy - umfJemallocPoolParamsSetKeepAllMemory umfLevelZeroMemoryProviderOps umfLevelZeroMemoryProviderParamsCreate umfLevelZeroMemoryProviderParamsDestroy diff --git a/src/libumf.map b/src/libumf.map index 067ec88386..85a9042203 100644 --- a/src/libumf.map +++ b/src/libumf.map @@ -31,9 +31,6 @@ UMF_1.0 { umfGetIPCHandle; umfGetLastFailedMemoryProvider; umfJemallocPoolOps; - umfJemallocPoolParamsCreate; - umfJemallocPoolParamsDestroy; - umfJemallocPoolParamsSetKeepAllMemory; umfLevelZeroMemoryProviderOps; umfLevelZeroMemoryProviderParamsCreate; umfLevelZeroMemoryProviderParamsDestroy; diff --git a/src/pool/pool_jemalloc.c b/src/pool/pool_jemalloc.c index 47bc6497fc..f88d5ce9d7 100644 --- a/src/pool/pool_jemalloc.c +++ b/src/pool/pool_jemalloc.c @@ -24,26 +24,6 @@ umf_memory_pool_ops_t *umfJemallocPoolOps(void) { return NULL; } -umf_result_t -umfJemallocPoolParamsCreate(umf_jemalloc_pool_params_handle_t *hParams) { - (void)hParams; // unused - return UMF_RESULT_ERROR_NOT_SUPPORTED; -} - -umf_result_t -umfJemallocPoolParamsDestroy(umf_jemalloc_pool_params_handle_t hParams) { - (void)hParams; // unused - return UMF_RESULT_ERROR_NOT_SUPPORTED; -} - -umf_result_t -umfJemallocPoolParamsSetKeepAllMemory(umf_jemalloc_pool_params_handle_t hParams, - bool keepAllMemory) { - (void)hParams; // unused - (void)keepAllMemory; // unused - return UMF_RESULT_ERROR_NOT_SUPPORTED; -} - #else #include @@ -53,16 +33,8 @@ umfJemallocPoolParamsSetKeepAllMemory(umf_jemalloc_pool_params_handle_t hParams, typedef struct jemalloc_memory_pool_t { umf_memory_provider_handle_t provider; unsigned int arena_index; // index of jemalloc arena - // set to true if umfMemoryProviderFree() should never be called - bool disable_provider_free; } jemalloc_memory_pool_t; -// Configuration of Jemalloc Pool -typedef struct umf_jemalloc_pool_params_t { - /// Set to true if umfMemoryProviderFree() should never be called. - bool disable_provider_free; -} umf_jemalloc_pool_params_t; - static __TLS umf_result_t TLS_last_allocation_error; static jemalloc_memory_pool_t *pool_by_arena_index[MALLCTL_ARENAS_ALL]; @@ -75,52 +47,6 @@ static jemalloc_memory_pool_t *get_pool_by_arena_index(unsigned arena_ind) { return pool_by_arena_index[arena_ind]; } -umf_result_t -umfJemallocPoolParamsCreate(umf_jemalloc_pool_params_handle_t *hParams) { - if (!hParams) { - LOG_ERR("jemalloc pool params handle is NULL"); - return UMF_RESULT_ERROR_INVALID_ARGUMENT; - } - - umf_jemalloc_pool_params_t *params_data = - umf_ba_global_alloc(sizeof(*params_data)); - if (!params_data) { - LOG_ERR("cannot allocate memory for jemalloc poolparams"); - return UMF_RESULT_ERROR_OUT_OF_HOST_MEMORY; - } - - params_data->disable_provider_free = false; - - *hParams = (umf_jemalloc_pool_params_handle_t)params_data; - - return UMF_RESULT_SUCCESS; -} - -umf_result_t -umfJemallocPoolParamsDestroy(umf_jemalloc_pool_params_handle_t hParams) { - if (!hParams) { - LOG_ERR("jemalloc pool params handle is NULL"); - return UMF_RESULT_ERROR_INVALID_ARGUMENT; - } - - umf_ba_global_free(hParams); - - return UMF_RESULT_SUCCESS; -} - -umf_result_t -umfJemallocPoolParamsSetKeepAllMemory(umf_jemalloc_pool_params_handle_t hParams, - bool keepAllMemory) { - if (!hParams) { - LOG_ERR("jemalloc pool params handle is NULL"); - return UMF_RESULT_ERROR_INVALID_ARGUMENT; - } - - hParams->disable_provider_free = keepAllMemory; - - return UMF_RESULT_SUCCESS; -} - // arena_extent_alloc - an extent allocation function conforms to the extent_alloc_t type and upon // success returns a pointer to size bytes of mapped memory on behalf of arena arena_ind such that // the extent's base address is a multiple of alignment, as well as setting *zero to indicate @@ -150,9 +76,7 @@ static void *arena_extent_alloc(extent_hooks_t *extent_hooks, void *new_addr, } if (new_addr != NULL && ptr != new_addr) { - if (!pool->disable_provider_free) { - umfMemoryProviderFree(pool->provider, ptr, size); - } + umfMemoryProviderFree(pool->provider, ptr, size); return NULL; } @@ -186,10 +110,6 @@ static void arena_extent_destroy(extent_hooks_t *extent_hooks, void *addr, jemalloc_memory_pool_t *pool = get_pool_by_arena_index(arena_ind); - if (pool->disable_provider_free) { - return; - } - umf_result_t ret; ret = umfMemoryProviderFree(pool->provider, addr, size); if (ret != UMF_RESULT_SUCCESS) { @@ -212,10 +132,6 @@ static bool arena_extent_dalloc(extent_hooks_t *extent_hooks, void *addr, jemalloc_memory_pool_t *pool = get_pool_by_arena_index(arena_ind); - if (pool->disable_provider_free) { - return true; // opt-out from deallocation - } - umf_result_t ret; ret = umfMemoryProviderFree(pool->provider, addr, size); if (ret != UMF_RESULT_SUCCESS) { @@ -466,12 +382,10 @@ static void *op_aligned_alloc(void *pool, size_t size, size_t alignment) { static umf_result_t op_initialize(umf_memory_provider_handle_t provider, void *params, void **out_pool) { + (void)params; // unused assert(provider); assert(out_pool); - umf_jemalloc_pool_params_handle_t je_params = - (umf_jemalloc_pool_params_handle_t)params; - extent_hooks_t *pHooks = &arena_extent_hooks; size_t unsigned_size = sizeof(unsigned); int err; @@ -484,12 +398,6 @@ static umf_result_t op_initialize(umf_memory_provider_handle_t provider, pool->provider = provider; - if (je_params) { - pool->disable_provider_free = je_params->disable_provider_free; - } else { - pool->disable_provider_free = false; - } - unsigned arena_index; err = je_mallctl("arenas.create", (void *)&arena_index, &unsigned_size, NULL, 0); diff --git a/test/pools/jemalloc_pool.cpp b/test/pools/jemalloc_pool.cpp index 4dddbcd32b..96c3868959 100644 --- a/test/pools/jemalloc_pool.cpp +++ b/test/pools/jemalloc_pool.cpp @@ -62,122 +62,3 @@ TEST_F(test, metadataNotAllocatedUsingProvider) { [pool = pool.get()](void *ptr) { umfPoolFree(pool, ptr); }); } } - -using jemallocPoolParams = bool; -struct umfJemallocPoolParamsTest - : umf_test::test, - ::testing::WithParamInterface { - - struct validation_params_t { - bool keep_all_memory; - }; - - struct provider_validator : public umf_test::provider_ba_global { - using base_provider = umf_test::provider_ba_global; - - umf_result_t initialize(validation_params_t *params) { - EXPECT_NE(params, nullptr); - expected_params = params; - return UMF_RESULT_SUCCESS; - } - umf_result_t free(void *ptr, size_t size) { - EXPECT_EQ(expected_params->keep_all_memory, false); - return base_provider::free(ptr, size); - } - - validation_params_t *expected_params; - }; - - static constexpr umf_memory_provider_ops_t VALIDATOR_PROVIDER_OPS = - umf::providerMakeCOps(); - - umfJemallocPoolParamsTest() : expected_params{false}, params(nullptr) {} - void SetUp() override { - test::SetUp(); - expected_params.keep_all_memory = this->GetParam(); - umf_result_t ret = umfJemallocPoolParamsCreate(¶ms); - ASSERT_EQ(ret, UMF_RESULT_SUCCESS); - ret = umfJemallocPoolParamsSetKeepAllMemory( - params, expected_params.keep_all_memory); - ASSERT_EQ(ret, UMF_RESULT_SUCCESS); - } - - void TearDown() override { - umfJemallocPoolParamsDestroy(params); - test::TearDown(); - } - - umf::pool_unique_handle_t makePool() { - umf_memory_provider_handle_t hProvider = nullptr; - umf_memory_pool_handle_t hPool = nullptr; - - auto ret = umfMemoryProviderCreate(&VALIDATOR_PROVIDER_OPS, - &expected_params, &hProvider); - EXPECT_EQ(ret, UMF_RESULT_SUCCESS); - - ret = umfPoolCreate(umfJemallocPoolOps(), hProvider, params, - UMF_POOL_CREATE_FLAG_OWN_PROVIDER, &hPool); - EXPECT_EQ(ret, UMF_RESULT_SUCCESS); - - return umf::pool_unique_handle_t(hPool, &umfPoolDestroy); - } - - void allocFreeFlow() { - static const size_t ALLOC_SIZE = 128; - static const size_t NUM_ALLOCATIONS = 100; - std::vector ptrs; - - auto pool = makePool(); - ASSERT_NE(pool, nullptr); - - for (size_t i = 0; i < NUM_ALLOCATIONS; ++i) { - auto *ptr = umfPoolMalloc(pool.get(), ALLOC_SIZE); - ASSERT_NE(ptr, nullptr); - ptrs.push_back(ptr); - } - - for (size_t i = 0; i < NUM_ALLOCATIONS; ++i) { - auto ret = umfPoolFree(pool.get(), ptrs[i]); - ASSERT_EQ(ret, UMF_RESULT_SUCCESS); - } - - // Now pool can call free during pool destruction - expected_params.keep_all_memory = false; - } - - validation_params_t expected_params; - umf_jemalloc_pool_params_handle_t params; -}; - -TEST_P(umfJemallocPoolParamsTest, allocFree) { allocFreeFlow(); } - -TEST_P(umfJemallocPoolParamsTest, updateParams) { - expected_params.keep_all_memory = !expected_params.keep_all_memory; - umf_result_t ret = umfJemallocPoolParamsSetKeepAllMemory( - params, expected_params.keep_all_memory); - ASSERT_EQ(ret, UMF_RESULT_SUCCESS); - - allocFreeFlow(); -} - -TEST_P(umfJemallocPoolParamsTest, invalidParams) { - umf_result_t ret = umfJemallocPoolParamsCreate(nullptr); - ASSERT_EQ(ret, UMF_RESULT_ERROR_INVALID_ARGUMENT); - - ret = umfJemallocPoolParamsSetKeepAllMemory(nullptr, true); - ASSERT_EQ(ret, UMF_RESULT_ERROR_INVALID_ARGUMENT); - - ret = umfJemallocPoolParamsSetKeepAllMemory(nullptr, false); - ASSERT_EQ(ret, UMF_RESULT_ERROR_INVALID_ARGUMENT); - - ret = umfJemallocPoolParamsDestroy(nullptr); - ASSERT_EQ(ret, UMF_RESULT_ERROR_INVALID_ARGUMENT); -} - -GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(umfJemallocPoolParamsTest); - -/* TODO: enable this test after the issue #903 is fixed. -(https://github.com/oneapi-src/unified-memory-framework/issues/903) -INSTANTIATE_TEST_SUITE_P(jemallocPoolTest, umfJemallocPoolParamsTest, - testing::Values(false, true)); -*/