Remove the disable_provider_free parameter of jemalloc pool

Remove the disable_provider_free parameter of jemalloc pool.
and the umfJemallocPoolParamsSetKeepAllMemory() API.

Fixes: #904

Signed-off-by: Lukasz Dorau <[email protected]>

Author: ldorau

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).
 

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;
 }
 

include/umf/pools/pool_jemalloc.h

@@ -34,15 +34,6 @@ umfJemallocPoolParamsCreate(umf_jemalloc_pool_params_handle_t *hParams);
 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

src/libumf.def

@@ -39,7 +39,6 @@ EXPORTS
     umfJemallocPoolOps
     umfJemallocPoolParamsCreate
     umfJemallocPoolParamsDestroy
-    umfJemallocPoolParamsSetKeepAllMemory
     umfLevelZeroMemoryProviderOps
     umfLevelZeroMemoryProviderParamsCreate
     umfLevelZeroMemoryProviderParamsDestroy

src/libumf.map

@@ -33,7 +33,6 @@ UMF_1.0 {
         umfJemallocPoolOps;
         umfJemallocPoolParamsCreate;
         umfJemallocPoolParamsDestroy;
-        umfJemallocPoolParamsSetKeepAllMemory;
         umfLevelZeroMemoryProviderOps;
         umfLevelZeroMemoryProviderParamsCreate;
         umfLevelZeroMemoryProviderParamsDestroy;

src/pool/pool_jemalloc.c

@@ -36,14 +36,6 @@ umfJemallocPoolParamsDestroy(umf_jemalloc_pool_params_handle_t hParams) {
     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 <jemalloc/jemalloc.h>
@@ -53,14 +45,11 @@ 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;
+    int placeholder; // just a placeholder
 } umf_jemalloc_pool_params_t;
 
 static __TLS umf_result_t TLS_last_allocation_error;
@@ -89,7 +78,7 @@ umfJemallocPoolParamsCreate(umf_jemalloc_pool_params_handle_t *hParams) {
         return UMF_RESULT_ERROR_OUT_OF_HOST_MEMORY;
     }
 
-    params_data->disable_provider_free = false;
+    params_data->placeholder = 1;
 
     *hParams = (umf_jemalloc_pool_params_handle_t)params_data;
 
@@ -108,19 +97,6 @@ umfJemallocPoolParamsDestroy(umf_jemalloc_pool_params_handle_t 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 +126,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 +160,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 +182,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 +432,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 +448,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);

test/pools/jemalloc_pool.cpp

@@ -69,7 +69,7 @@ struct umfJemallocPoolParamsTest
       ::testing::WithParamInterface<jemallocPoolParams> {
 
     struct validation_params_t {
-        bool keep_all_memory;
+        bool placeholder;
     };
 
     struct provider_validator : public umf_test::provider_ba_global {
@@ -81,7 +81,7 @@ struct umfJemallocPoolParamsTest
             return UMF_RESULT_SUCCESS;
         }
         umf_result_t free(void *ptr, size_t size) {
-            EXPECT_EQ(expected_params->keep_all_memory, false);
+            EXPECT_EQ(expected_params->placeholder, false);
             return base_provider::free(ptr, size);
         }
 
@@ -94,12 +94,9 @@ struct umfJemallocPoolParamsTest
     umfJemallocPoolParamsTest() : expected_params{false}, params(nullptr) {}
     void SetUp() override {
         test::SetUp();
-        expected_params.keep_all_memory = this->GetParam();
+        expected_params.placeholder = this->GetParam();
         umf_result_t ret = umfJemallocPoolParamsCreate(&params);
         ASSERT_EQ(ret, UMF_RESULT_SUCCESS);
-        ret = umfJemallocPoolParamsSetKeepAllMemory(
-            params, expected_params.keep_all_memory);
-        ASSERT_EQ(ret, UMF_RESULT_SUCCESS);
     }
 
     void TearDown() override {
@@ -142,7 +139,7 @@ struct umfJemallocPoolParamsTest
         }
 
         // Now pool can call free during pool destruction
-        expected_params.keep_all_memory = false;
+        expected_params.placeholder = false;
     }
 
     validation_params_t expected_params;
@@ -152,24 +149,14 @@ struct umfJemallocPoolParamsTest
 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);
-
+    expected_params.placeholder = !expected_params.placeholder;
     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);
 }