From 73d012e4d626c08b739d952710f0287ceb0b62ba Mon Sep 17 00:00:00 2001 From: Lukasz Dorau Date: Thu, 21 Nov 2024 08:38:47 +0100 Subject: [PATCH 1/5] Use libcoarse in the file provider Signed-off-by: Lukasz Dorau --- src/provider/provider_file_memory.c | 55 ++++++++++++++++++++++++++--- 1 file changed, 51 insertions(+), 4 deletions(-) diff --git a/src/provider/provider_file_memory.c b/src/provider/provider_file_memory.c index 32383a5ecc..9d332fd467 100644 --- a/src/provider/provider_file_memory.c +++ b/src/provider/provider_file_memory.c @@ -63,6 +63,7 @@ umf_result_t umfFileMemoryProviderParamsSetVisibility( #else // !defined(_WIN32) && !defined(UMF_NO_HWLOC) #include "base_alloc_global.h" +#include "coarse.h" #include "critnib.h" #include "libumf.h" #include "utils_common.h" @@ -101,6 +102,8 @@ typedef struct file_memory_provider_t { // It is needed mainly in the get_ipc_handle and open_ipc_handle hooks // to mmap a specific part of a file. critnib *fd_offset_map; + + coarse_t *coarse; // coarse library handle } file_memory_provider_t; // File Memory Provider settings struct @@ -166,6 +169,14 @@ file_translate_params(umf_file_memory_provider_params_t *in_params, return UMF_RESULT_SUCCESS; } +static umf_result_t file_alloc_cb(void *provider, size_t size, size_t alignment, + void **resultPtr); +static umf_result_t file_allocation_split_cb(void *provider, void *ptr, + size_t totalSize, + size_t firstSize); +static umf_result_t file_allocation_merge_cb(void *provider, void *lowPtr, + void *highPtr, size_t totalSize); + static umf_result_t file_initialize(void *params, void **provider) { umf_result_t ret; @@ -233,10 +244,27 @@ static umf_result_t file_initialize(void *params, void **provider) { file_provider->page_size = utils_get_page_size(); } + coarse_params_t coarse_params = {0}; + coarse_params.provider = file_provider; + coarse_params.page_size = file_provider->page_size; + coarse_params.cb.alloc = file_alloc_cb; + coarse_params.cb.free = NULL; // not available for the file provider + coarse_params.cb.split = file_allocation_split_cb; + coarse_params.cb.merge = file_allocation_merge_cb; + + coarse_t *coarse = NULL; + ret = coarse_new(&coarse_params, &coarse); + if (ret != UMF_RESULT_SUCCESS) { + LOG_ERR("coarse_new() failed"); + goto err_close_fd; + } + + file_provider->coarse = coarse; + if (utils_mutex_init(&file_provider->lock) == NULL) { LOG_ERR("lock init failed"); ret = UMF_RESULT_ERROR_UNKNOWN; - goto err_close_fd; + goto err_coarse_delete; } file_provider->fd_offset_map = critnib_new(); @@ -261,6 +289,8 @@ static umf_result_t file_initialize(void *params, void **provider) { critnib_delete(file_provider->fd_offset_map); err_mutex_destroy_not_free: utils_mutex_destroy_not_free(&file_provider->lock); +err_coarse_delete: + coarse_delete(file_provider->coarse); err_close_fd: utils_close_fd(file_provider->fd); err_free_file_provider: @@ -285,6 +315,7 @@ static void file_finalize(void *provider) { utils_close_fd(file_provider->fd); critnib_delete(file_provider->fd_offset_map); critnib_delete(file_provider->mmaps); + coarse_delete(file_provider->coarse); umf_ba_global_free(file_provider); } @@ -443,6 +474,12 @@ static umf_result_t file_alloc_aligned(file_memory_provider_t *file_provider, static umf_result_t file_alloc(void *provider, size_t size, size_t alignment, void **resultPtr) { + file_memory_provider_t *file_provider = (file_memory_provider_t *)provider; + return coarse_alloc(file_provider->coarse, size, alignment, resultPtr); +} + +static umf_result_t file_alloc_cb(void *provider, size_t size, size_t alignment, + void **resultPtr) { umf_result_t umf_result; int ret; @@ -568,10 +605,15 @@ static const char *file_get_name(void *provider) { return "FILE"; } -// This function is supposed to be thread-safe, so it should NOT be called concurrently -// with file_allocation_merge() with the same pointer. static umf_result_t file_allocation_split(void *provider, void *ptr, size_t totalSize, size_t firstSize) { + file_memory_provider_t *file_provider = (file_memory_provider_t *)provider; + return coarse_split(file_provider->coarse, ptr, totalSize, firstSize); +} + +static umf_result_t file_allocation_split_cb(void *provider, void *ptr, + size_t totalSize, + size_t firstSize) { (void)totalSize; file_memory_provider_t *file_provider = (file_memory_provider_t *)provider; @@ -601,9 +643,14 @@ static umf_result_t file_allocation_split(void *provider, void *ptr, return UMF_RESULT_SUCCESS; } -// It should NOT be called concurrently with file_allocation_split() with the same pointer. static umf_result_t file_allocation_merge(void *provider, void *lowPtr, void *highPtr, size_t totalSize) { + file_memory_provider_t *file_provider = (file_memory_provider_t *)provider; + return coarse_merge(file_provider->coarse, lowPtr, highPtr, totalSize); +} + +static umf_result_t file_allocation_merge_cb(void *provider, void *lowPtr, + void *highPtr, size_t totalSize) { (void)lowPtr; (void)totalSize; From f81c64702db468a9bde7d13d2d2df039fe04e7e4 Mon Sep 17 00:00:00 2001 From: Lukasz Dorau Date: Mon, 25 Nov 2024 10:05:23 +0100 Subject: [PATCH 2/5] Add free() op to the file provider Signed-off-by: Lukasz Dorau --- src/provider/provider_file_memory.c | 6 ++++++ test/provider_file_memory.cpp | 18 +++++++++--------- test/provider_file_memory_ipc.cpp | 2 +- 3 files changed, 16 insertions(+), 10 deletions(-) diff --git a/src/provider/provider_file_memory.c b/src/provider/provider_file_memory.c index 9d332fd467..558b1062a5 100644 --- a/src/provider/provider_file_memory.c +++ b/src/provider/provider_file_memory.c @@ -815,6 +815,11 @@ static umf_result_t file_close_ipc_handle(void *provider, void *ptr, return UMF_RESULT_SUCCESS; } +static umf_result_t file_free(void *provider, void *ptr, size_t size) { + file_memory_provider_t *file_provider = (file_memory_provider_t *)provider; + return coarse_free(file_provider->coarse, ptr, size); +} + static umf_memory_provider_ops_t UMF_FILE_MEMORY_PROVIDER_OPS = { .version = UMF_VERSION_CURRENT, .initialize = file_initialize, @@ -824,6 +829,7 @@ static umf_memory_provider_ops_t UMF_FILE_MEMORY_PROVIDER_OPS = { .get_recommended_page_size = file_get_recommended_page_size, .get_min_page_size = file_get_min_page_size, .get_name = file_get_name, + .ext.free = file_free, .ext.purge_lazy = file_purge_lazy, .ext.purge_force = file_purge_force, .ext.allocation_merge = file_allocation_merge, diff --git a/test/provider_file_memory.cpp b/test/provider_file_memory.cpp index d3124aa11f..0d54c287cb 100644 --- a/test/provider_file_memory.cpp +++ b/test/provider_file_memory.cpp @@ -98,7 +98,7 @@ static void test_alloc_free_success(umf_memory_provider_handle_t provider, } umf_result = umfMemoryProviderFree(provider, ptr, size); - ASSERT_EQ(umf_result, UMF_RESULT_ERROR_NOT_SUPPORTED); + ASSERT_EQ(umf_result, UMF_RESULT_SUCCESS); } static void verify_last_native_error(umf_memory_provider_handle_t provider, @@ -159,7 +159,7 @@ TEST_F(test, test_if_mapped_with_MAP_SYNC) { bool flag_found = is_mapped_with_MAP_SYNC(path, buf, size); umf_result = umfMemoryProviderFree(hProvider, buf, size); - ASSERT_EQ(umf_result, UMF_RESULT_ERROR_NOT_SUPPORTED); + ASSERT_EQ(umf_result, UMF_RESULT_SUCCESS); umfMemoryProviderDestroy(hProvider); @@ -244,10 +244,10 @@ TEST_P(FileProviderParamsDefault, two_allocations) { memset(ptr2, 0x22, size); umf_result = umfMemoryProviderFree(provider.get(), ptr1, size); - ASSERT_EQ(umf_result, UMF_RESULT_ERROR_NOT_SUPPORTED); + ASSERT_EQ(umf_result, UMF_RESULT_SUCCESS); umf_result = umfMemoryProviderFree(provider.get(), ptr2, size); - ASSERT_EQ(umf_result, UMF_RESULT_ERROR_NOT_SUPPORTED); + ASSERT_EQ(umf_result, UMF_RESULT_SUCCESS); } TEST_P(FileProviderParamsDefault, alloc_page64_align_0) { @@ -366,12 +366,12 @@ TEST_P(FileProviderParamsDefault, get_name) { TEST_P(FileProviderParamsDefault, free_size_0_ptr_not_null) { umf_result_t umf_result = umfMemoryProviderFree(provider.get(), INVALID_PTR, 0); - ASSERT_EQ(umf_result, UMF_RESULT_ERROR_NOT_SUPPORTED); + ASSERT_EQ(umf_result, UMF_RESULT_ERROR_INVALID_ARGUMENT); } TEST_P(FileProviderParamsDefault, free_NULL) { umf_result_t umf_result = umfMemoryProviderFree(provider.get(), nullptr, 0); - ASSERT_EQ(umf_result, UMF_RESULT_ERROR_NOT_SUPPORTED); + ASSERT_EQ(umf_result, UMF_RESULT_SUCCESS); } // other negative tests @@ -449,7 +449,7 @@ TEST_F(test, set_null_path) { TEST_P(FileProviderParamsDefault, free_INVALID_POINTER_SIZE_GT_0) { umf_result_t umf_result = umfMemoryProviderFree(provider.get(), INVALID_PTR, page_plus_64); - ASSERT_EQ(umf_result, UMF_RESULT_ERROR_NOT_SUPPORTED); + ASSERT_EQ(umf_result, UMF_RESULT_ERROR_INVALID_ARGUMENT); } TEST_P(FileProviderParamsDefault, purge_lazy_INVALID_POINTER) { @@ -512,7 +512,7 @@ TEST_P(FileProviderParamsShared, IPC_base_success_test) { ASSERT_EQ(ret, 0); umf_result = umfMemoryProviderFree(provider.get(), ptr, size); - ASSERT_EQ(umf_result, UMF_RESULT_ERROR_NOT_SUPPORTED); + ASSERT_EQ(umf_result, UMF_RESULT_SUCCESS); } TEST_P(FileProviderParamsShared, IPC_file_not_exist) { @@ -552,5 +552,5 @@ TEST_P(FileProviderParamsShared, IPC_file_not_exist) { ASSERT_EQ(new_ptr, nullptr); umf_result = umfMemoryProviderFree(provider.get(), ptr, size); - ASSERT_EQ(umf_result, UMF_RESULT_ERROR_NOT_SUPPORTED); + ASSERT_EQ(umf_result, UMF_RESULT_SUCCESS); } diff --git a/test/provider_file_memory_ipc.cpp b/test/provider_file_memory_ipc.cpp index ee7ab6c8ff..115322a476 100644 --- a/test/provider_file_memory_ipc.cpp +++ b/test/provider_file_memory_ipc.cpp @@ -73,7 +73,7 @@ HostMemoryAccessor hostAccessor; static std::vector ipcManyPoolsTestParamsList = { // TODO: enable it when sizes of allocations in ipcFixtures.hpp are fixed // {umfProxyPoolOps(), nullptr, umfFileMemoryProviderOps(), -// file_params_shared.get(), &hostAccessor, true}, +// file_params_shared.get(), &hostAccessor, false}, #ifdef UMF_POOL_JEMALLOC_ENABLED {umfJemallocPoolOps(), nullptr, umfFileMemoryProviderOps(), file_params_shared.get(), &hostAccessor, false}, From a27c45fa76ef8d0528e98b818b32e87a8a52efad Mon Sep 17 00:00:00 2001 From: Lukasz Dorau Date: Tue, 3 Dec 2024 11:46:55 +0100 Subject: [PATCH 3/5] Use libcoarse in the devdax provider Signed-off-by: Lukasz Dorau --- src/provider/provider_devdax_memory.c | 141 ++++++++++++-------------- test/provider_devdax_memory.cpp | 11 +- 2 files changed, 70 insertions(+), 82 deletions(-) diff --git a/src/provider/provider_devdax_memory.c b/src/provider/provider_devdax_memory.c index 32407acbba..c013c7ffba 100644 --- a/src/provider/provider_devdax_memory.c +++ b/src/provider/provider_devdax_memory.c @@ -58,6 +58,7 @@ umf_result_t umfDevDaxMemoryProviderParamsSetProtection( #else // !defined(_WIN32) && !defined(UMF_NO_HWLOC) #include "base_alloc_global.h" +#include "coarse.h" #include "libumf.h" #include "utils_common.h" #include "utils_concurrency.h" @@ -74,6 +75,7 @@ typedef struct devdax_memory_provider_t { size_t offset; // offset in the file used for memory mapping utils_mutex_t lock; // lock of ptr and offset unsigned protection; // combination of OS-specific protection flags + coarse_t *coarse; // coarse library handle } devdax_memory_provider_t; // DevDax Memory provider settings struct @@ -133,6 +135,12 @@ devdax_translate_params(umf_devdax_memory_provider_params_t *in_params, return UMF_RESULT_SUCCESS; } +static umf_result_t devdax_allocation_split_cb(void *provider, void *ptr, + size_t totalSize, + size_t firstSize); +static umf_result_t devdax_allocation_merge_cb(void *provider, void *lowPtr, + void *highPtr, size_t totalSize); + static umf_result_t devdax_initialize(void *params, void **provider) { umf_result_t ret; @@ -161,21 +169,42 @@ static umf_result_t devdax_initialize(void *params, void **provider) { memset(devdax_provider, 0, sizeof(*devdax_provider)); - ret = devdax_translate_params(in_params, devdax_provider); + coarse_params_t coarse_params = {0}; + coarse_params.provider = devdax_provider; + coarse_params.page_size = DEVDAX_PAGE_SIZE_2MB; + // The alloc callback is not available in case of the devdax provider + // because it is a fixed-size memory provider + // and the entire devdax memory is added as a single block + // to the coarse library. + coarse_params.cb.alloc = NULL; + coarse_params.cb.free = NULL; // not available for the devdax provider + coarse_params.cb.split = devdax_allocation_split_cb; + coarse_params.cb.merge = devdax_allocation_merge_cb; + + coarse_t *coarse = NULL; + ret = coarse_new(&coarse_params, &coarse); if (ret != UMF_RESULT_SUCCESS) { + LOG_ERR("coarse_new() failed"); goto err_free_devdax_provider; } + devdax_provider->coarse = coarse; + + ret = devdax_translate_params(in_params, devdax_provider); + if (ret != UMF_RESULT_SUCCESS) { + goto err_coarse_delete; + } + devdax_provider->size = in_params->size; if (utils_copy_path(in_params->path, devdax_provider->path, PATH_MAX)) { - goto err_free_devdax_provider; + goto err_coarse_delete; } int fd = utils_devdax_open(in_params->path); if (fd == -1) { LOG_ERR("cannot open the device DAX: %s", in_params->path); ret = UMF_RESULT_ERROR_INVALID_ARGUMENT; - goto err_free_devdax_provider; + goto err_coarse_delete; } bool is_dax = false; @@ -189,23 +218,26 @@ static umf_result_t devdax_initialize(void *params, void **provider) { LOG_PDEBUG("mapping the devdax failed (path=%s, size=%zu)", in_params->path, devdax_provider->size); ret = UMF_RESULT_ERROR_UNKNOWN; - goto err_free_devdax_provider; + goto err_coarse_delete; } if (!is_dax) { LOG_ERR("mapping the devdax with MAP_SYNC failed: %s", in_params->path); ret = UMF_RESULT_ERROR_UNKNOWN; - - if (devdax_provider->base) { - utils_munmap(devdax_provider->base, devdax_provider->size); - } - - goto err_free_devdax_provider; + goto err_unmap_devdax; } LOG_DEBUG("devdax memory mapped (path=%s, size=%zu, addr=%p)", in_params->path, devdax_provider->size, devdax_provider->base); + // add the entire devdax memory as a single block + ret = coarse_add_memory_fixed(coarse, devdax_provider->base, + devdax_provider->size); + if (ret != UMF_RESULT_SUCCESS) { + LOG_ERR("adding memory block failed"); + goto err_unmap_devdax; + } + if (utils_mutex_init(&devdax_provider->lock) == NULL) { LOG_ERR("lock init failed"); ret = UMF_RESULT_ERROR_UNKNOWN; @@ -217,7 +249,11 @@ static umf_result_t devdax_initialize(void *params, void **provider) { return UMF_RESULT_SUCCESS; err_unmap_devdax: - utils_munmap(devdax_provider->base, devdax_provider->size); + if (devdax_provider->base) { + utils_munmap(devdax_provider->base, devdax_provider->size); + } +err_coarse_delete: + coarse_delete(devdax_provider->coarse); err_free_devdax_provider: umf_ba_global_free(devdax_provider); return ret; @@ -227,78 +263,15 @@ static void devdax_finalize(void *provider) { devdax_memory_provider_t *devdax_provider = provider; utils_mutex_destroy_not_free(&devdax_provider->lock); utils_munmap(devdax_provider->base, devdax_provider->size); + coarse_delete(devdax_provider->coarse); umf_ba_global_free(devdax_provider); } -static int devdax_alloc_aligned(size_t length, size_t alignment, void *base, - size_t size, utils_mutex_t *lock, - void **out_addr, size_t *offset) { - assert(out_addr); - - if (utils_mutex_lock(lock)) { - LOG_ERR("locking file offset failed"); - return -1; - } - - uintptr_t ptr = (uintptr_t)base + *offset; - uintptr_t rest_of_div = alignment ? (ptr % alignment) : 0; - - if (alignment > 0 && rest_of_div > 0) { - ptr += alignment - rest_of_div; - } - - size_t new_offset = ptr - (uintptr_t)base + length; - - if (new_offset > size) { - utils_mutex_unlock(lock); - LOG_ERR("cannot allocate more memory than the device DAX size: %zu", - size); - return -1; - } - - *offset = new_offset; - *out_addr = (void *)ptr; - - utils_mutex_unlock(lock); - - return 0; -} - static umf_result_t devdax_alloc(void *provider, size_t size, size_t alignment, void **resultPtr) { - int ret; - - // alignment must be a power of two and a multiple or a divider of the page size - if (alignment && ((alignment & (alignment - 1)) || - ((alignment % DEVDAX_PAGE_SIZE_2MB) && - (DEVDAX_PAGE_SIZE_2MB % alignment)))) { - LOG_ERR("wrong alignment: %zu (not a power of 2 or a multiple or a " - "divider of the page size (%zu))", - alignment, DEVDAX_PAGE_SIZE_2MB); - return UMF_RESULT_ERROR_INVALID_ARGUMENT; - } - - if (IS_NOT_ALIGNED(alignment, DEVDAX_PAGE_SIZE_2MB)) { - alignment = ALIGN_UP(alignment, DEVDAX_PAGE_SIZE_2MB); - } - devdax_memory_provider_t *devdax_provider = (devdax_memory_provider_t *)provider; - - void *addr = NULL; - errno = 0; - ret = devdax_alloc_aligned(size, alignment, devdax_provider->base, - devdax_provider->size, &devdax_provider->lock, - &addr, &devdax_provider->offset); - if (ret) { - devdax_store_last_native_error(UMF_DEVDAX_RESULT_ERROR_ALLOC_FAILED, 0); - LOG_ERR("memory allocation failed"); - return UMF_RESULT_ERROR_MEMORY_PROVIDER_SPECIFIC; - } - - *resultPtr = addr; - - return UMF_RESULT_SUCCESS; + return coarse_alloc(devdax_provider->coarse, size, alignment, resultPtr); } static void devdax_get_last_native_error(void *provider, const char **ppMessage, @@ -384,6 +357,14 @@ static const char *devdax_get_name(void *provider) { static umf_result_t devdax_allocation_split(void *provider, void *ptr, size_t totalSize, size_t firstSize) { + devdax_memory_provider_t *devdax_provider = + (devdax_memory_provider_t *)provider; + return coarse_split(devdax_provider->coarse, ptr, totalSize, firstSize); +} + +static umf_result_t devdax_allocation_split_cb(void *provider, void *ptr, + size_t totalSize, + size_t firstSize) { (void)provider; (void)ptr; (void)totalSize; @@ -393,6 +374,14 @@ static umf_result_t devdax_allocation_split(void *provider, void *ptr, static umf_result_t devdax_allocation_merge(void *provider, void *lowPtr, void *highPtr, size_t totalSize) { + devdax_memory_provider_t *devdax_provider = + (devdax_memory_provider_t *)provider; + return coarse_merge(devdax_provider->coarse, lowPtr, highPtr, totalSize); +} + +static umf_result_t devdax_allocation_merge_cb(void *provider, void *lowPtr, + void *highPtr, + size_t totalSize) { (void)provider; (void)lowPtr; (void)highPtr; diff --git a/test/provider_devdax_memory.cpp b/test/provider_devdax_memory.cpp index 0fd0705da4..bb00f205a6 100644 --- a/test/provider_devdax_memory.cpp +++ b/test/provider_devdax_memory.cpp @@ -237,30 +237,29 @@ TEST_P(umfProviderTest, purge_force) { TEST_P(umfProviderTest, alloc_page64_align_page_minus_1_WRONG_ALIGNMENT_1) { test_alloc_failure(provider.get(), page_plus_64, page_size - 1, - UMF_RESULT_ERROR_INVALID_ARGUMENT, 0); + UMF_RESULT_ERROR_INVALID_ALIGNMENT, 0); } TEST_P(umfProviderTest, alloc_page64_align_one_half_pages_WRONG_ALIGNMENT_2) { test_alloc_failure(provider.get(), page_plus_64, page_size + (page_size / 2), - UMF_RESULT_ERROR_INVALID_ARGUMENT, 0); + UMF_RESULT_ERROR_INVALID_ALIGNMENT, 0); } TEST_P(umfProviderTest, alloc_page64_WRONG_ALIGNMENT_3_pages) { test_alloc_failure(provider.get(), page_plus_64, 3 * page_size, - UMF_RESULT_ERROR_INVALID_ARGUMENT, 0); + UMF_RESULT_ERROR_INVALID_ALIGNMENT, 0); } TEST_P(umfProviderTest, alloc_3_pages_WRONG_ALIGNMENT_3_pages) { test_alloc_failure(provider.get(), 3 * page_size, 3 * page_size, - UMF_RESULT_ERROR_INVALID_ARGUMENT, 0); + UMF_RESULT_ERROR_INVALID_ALIGNMENT, 0); } TEST_P(umfProviderTest, alloc_WRONG_SIZE) { size_t size = (size_t)(-1) & ~(page_size - 1); test_alloc_failure(provider.get(), size, 0, - UMF_RESULT_ERROR_MEMORY_PROVIDER_SPECIFIC, - UMF_DEVDAX_RESULT_ERROR_ALLOC_FAILED); + UMF_RESULT_ERROR_OUT_OF_HOST_MEMORY, 0); } // other positive tests From 96605c4a3a962d698c5871cf40ae861585abfc7e Mon Sep 17 00:00:00 2001 From: Lukasz Dorau Date: Mon, 25 Nov 2024 10:15:34 +0100 Subject: [PATCH 4/5] Add free() to the devdax provider --- src/provider/provider_devdax_memory.c | 7 +++++++ test/provider_devdax_memory.cpp | 10 +++++----- test/provider_devdax_memory_ipc.cpp | 2 +- 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/src/provider/provider_devdax_memory.c b/src/provider/provider_devdax_memory.c index c013c7ffba..463b796ec0 100644 --- a/src/provider/provider_devdax_memory.c +++ b/src/provider/provider_devdax_memory.c @@ -516,6 +516,12 @@ static umf_result_t devdax_close_ipc_handle(void *provider, void *ptr, return UMF_RESULT_SUCCESS; } +static umf_result_t devdax_free(void *provider, void *ptr, size_t size) { + devdax_memory_provider_t *devdax_provider = + (devdax_memory_provider_t *)provider; + return coarse_free(devdax_provider->coarse, ptr, size); +} + static umf_memory_provider_ops_t UMF_DEVDAX_MEMORY_PROVIDER_OPS = { .version = UMF_VERSION_CURRENT, .initialize = devdax_initialize, @@ -525,6 +531,7 @@ static umf_memory_provider_ops_t UMF_DEVDAX_MEMORY_PROVIDER_OPS = { .get_recommended_page_size = devdax_get_recommended_page_size, .get_min_page_size = devdax_get_min_page_size, .get_name = devdax_get_name, + .ext.free = devdax_free, .ext.purge_lazy = devdax_purge_lazy, .ext.purge_force = devdax_purge_force, .ext.allocation_merge = devdax_allocation_merge, diff --git a/test/provider_devdax_memory.cpp b/test/provider_devdax_memory.cpp index bb00f205a6..afff1de4f7 100644 --- a/test/provider_devdax_memory.cpp +++ b/test/provider_devdax_memory.cpp @@ -100,7 +100,7 @@ static void test_alloc_free_success(umf_memory_provider_handle_t provider, } umf_result = umfMemoryProviderFree(provider, ptr, size); - ASSERT_EQ(umf_result, UMF_RESULT_ERROR_NOT_SUPPORTED); + ASSERT_EQ(umf_result, UMF_RESULT_SUCCESS); } static void verify_last_native_error(umf_memory_provider_handle_t provider, @@ -162,7 +162,7 @@ TEST_F(test, test_if_mapped_with_MAP_SYNC) { bool flag_found = is_mapped_with_MAP_SYNC(path, buf, size); umf_result = umfMemoryProviderFree(hProvider, buf, size); - ASSERT_EQ(umf_result, UMF_RESULT_ERROR_NOT_SUPPORTED); + ASSERT_EQ(umf_result, UMF_RESULT_SUCCESS); umfMemoryProviderDestroy(hProvider); @@ -294,12 +294,12 @@ TEST_P(umfProviderTest, get_name) { TEST_P(umfProviderTest, free_size_0_ptr_not_null) { umf_result_t umf_result = umfMemoryProviderFree(provider.get(), INVALID_PTR, 0); - ASSERT_EQ(umf_result, UMF_RESULT_ERROR_NOT_SUPPORTED); + ASSERT_EQ(umf_result, UMF_RESULT_ERROR_INVALID_ARGUMENT); } TEST_P(umfProviderTest, free_NULL) { umf_result_t umf_result = umfMemoryProviderFree(provider.get(), nullptr, 0); - ASSERT_EQ(umf_result, UMF_RESULT_ERROR_NOT_SUPPORTED); + ASSERT_EQ(umf_result, UMF_RESULT_SUCCESS); } // other negative tests @@ -307,7 +307,7 @@ TEST_P(umfProviderTest, free_NULL) { TEST_P(umfProviderTest, free_INVALID_POINTER_SIZE_GT_0) { umf_result_t umf_result = umfMemoryProviderFree(provider.get(), INVALID_PTR, page_plus_64); - ASSERT_EQ(umf_result, UMF_RESULT_ERROR_NOT_SUPPORTED); + ASSERT_EQ(umf_result, UMF_RESULT_ERROR_INVALID_ARGUMENT); } TEST_P(umfProviderTest, purge_lazy_INVALID_POINTER) { diff --git a/test/provider_devdax_memory_ipc.cpp b/test/provider_devdax_memory_ipc.cpp index 3941f66e90..921347f400 100644 --- a/test/provider_devdax_memory_ipc.cpp +++ b/test/provider_devdax_memory_ipc.cpp @@ -53,7 +53,7 @@ static std::vector getIpcProxyPoolTestParamsList(void) { ipcProxyPoolTestParamsList = { {umfProxyPoolOps(), nullptr, umfDevDaxMemoryProviderOps(), - defaultDevDaxParams.get(), &hostAccessor, true}, + defaultDevDaxParams.get(), &hostAccessor, false}, #ifdef UMF_POOL_JEMALLOC_ENABLED {umfJemallocPoolOps(), nullptr, umfDevDaxMemoryProviderOps(), defaultDevDaxParams.get(), &hostAccessor, false}, From d9f1feec2d314784b7bd2da0281fd4411544cf9d Mon Sep 17 00:00:00 2001 From: Lukasz Dorau Date: Tue, 3 Dec 2024 11:47:23 +0100 Subject: [PATCH 5/5] Remove free_not_supp from the ipcTestParams tuple Remove free_not_supp from the ipcTestParams tuple. It is not needed any more. Signed-off-by: Lukasz Dorau --- test/ipcAPI.cpp | 2 +- test/ipcFixtures.hpp | 44 +++++++------------------- test/provider_devdax_memory_ipc.cpp | 6 ++-- test/provider_file_memory_ipc.cpp | 12 +++---- test/provider_os_memory.cpp | 4 +-- test/providers/provider_level_zero.cpp | 10 +++--- 6 files changed, 29 insertions(+), 49 deletions(-) diff --git a/test/ipcAPI.cpp b/test/ipcAPI.cpp index 4df32a1c9b..aa22f353dc 100644 --- a/test/ipcAPI.cpp +++ b/test/ipcAPI.cpp @@ -116,4 +116,4 @@ HostMemoryAccessor hostMemoryAccessor; INSTANTIATE_TEST_SUITE_P(umfIpcTestSuite, umfIpcTest, ::testing::Values(ipcTestParams{ umfProxyPoolOps(), nullptr, &IPC_MOCK_PROVIDER_OPS, - nullptr, &hostMemoryAccessor, false})); + nullptr, &hostMemoryAccessor})); diff --git a/test/ipcFixtures.hpp b/test/ipcFixtures.hpp index 161a84844c..6fbacfa222 100644 --- a/test/ipcFixtures.hpp +++ b/test/ipcFixtures.hpp @@ -47,25 +47,23 @@ class HostMemoryAccessor : public MemoryAccessor { }; // ipcTestParams: -// pool_ops, pool_params, provider_ops, provider_params, memoryAccessor, free_not_supp -// free_not_supp (bool) - provider does not support the free() op +// pool_ops, pool_params, provider_ops, provider_params, memoryAccessor using ipcTestParams = std::tuple; + void *, MemoryAccessor *>; struct umfIpcTest : umf_test::test, ::testing::WithParamInterface { umfIpcTest() {} void SetUp() override { test::SetUp(); - auto [pool_ops, pool_params, provider_ops, provider_params, accessor, - free_not_supp] = this->GetParam(); + auto [pool_ops, pool_params, provider_ops, provider_params, accessor] = + this->GetParam(); poolOps = pool_ops; poolParams = pool_params; providerOps = provider_ops; providerParams = provider_params; memAccessor = accessor; - freeNotSupported = free_not_supp; } void TearDown() override { test::TearDown(); } @@ -124,18 +122,8 @@ struct umfIpcTest : umf_test::test, void *poolParams = nullptr; umf_memory_provider_ops_t *providerOps = nullptr; void *providerParams = nullptr; - bool freeNotSupported = false; }; -static inline umf_result_t -get_umf_result_of_free(bool freeNotSupported, umf_result_t expected_result) { - if (freeNotSupported) { - return UMF_RESULT_ERROR_NOT_SUPPORTED; - } - - return expected_result; -} - TEST_P(umfIpcTest, GetIPCHandleSize) { size_t size = 0; umf::pool_unique_handle_t pool = makePool(); @@ -177,8 +165,7 @@ TEST_P(umfIpcTest, GetIPCHandleInvalidArgs) { EXPECT_EQ(ret, UMF_RESULT_ERROR_INVALID_ARGUMENT); ret = umfFree(ptr); - EXPECT_EQ(ret, - get_umf_result_of_free(freeNotSupported, UMF_RESULT_SUCCESS)); + EXPECT_EQ(ret, UMF_RESULT_SUCCESS); } TEST_P(umfIpcTest, CloseIPCHandleInvalidPtr) { @@ -239,8 +226,7 @@ TEST_P(umfIpcTest, BasicFlow) { EXPECT_EQ(ret, UMF_RESULT_SUCCESS); ret = umfPoolFree(pool.get(), ptr); - EXPECT_EQ(ret, - get_umf_result_of_free(freeNotSupported, UMF_RESULT_SUCCESS)); + EXPECT_EQ(ret, UMF_RESULT_SUCCESS); pool.reset(nullptr); EXPECT_EQ(stat.getCount, 1); @@ -303,8 +289,7 @@ TEST_P(umfIpcTest, GetPoolByOpenedHandle) { for (size_t i = 0; i < NUM_ALLOCS; ++i) { umf_result_t ret = umfFree(ptrs[i]); - EXPECT_EQ(ret, - get_umf_result_of_free(freeNotSupported, UMF_RESULT_SUCCESS)); + EXPECT_EQ(ret, UMF_RESULT_SUCCESS); } } @@ -330,8 +315,7 @@ TEST_P(umfIpcTest, AllocFreeAllocTest) { EXPECT_EQ(ret, UMF_RESULT_SUCCESS); ret = umfPoolFree(pool.get(), ptr); - EXPECT_EQ(ret, - get_umf_result_of_free(freeNotSupported, UMF_RESULT_SUCCESS)); + EXPECT_EQ(ret, UMF_RESULT_SUCCESS); ptr = umfPoolMalloc(pool.get(), SIZE); ASSERT_NE(ptr, nullptr); @@ -353,8 +337,7 @@ TEST_P(umfIpcTest, AllocFreeAllocTest) { EXPECT_EQ(ret, UMF_RESULT_SUCCESS); ret = umfPoolFree(pool.get(), ptr); - EXPECT_EQ(ret, - get_umf_result_of_free(freeNotSupported, UMF_RESULT_SUCCESS)); + EXPECT_EQ(ret, UMF_RESULT_SUCCESS); pool.reset(nullptr); EXPECT_EQ(stat.getCount, stat.putCount); @@ -405,8 +388,7 @@ TEST_P(umfIpcTest, openInTwoPools) { EXPECT_EQ(ret, UMF_RESULT_SUCCESS); ret = umfPoolFree(pool1.get(), ptr); - EXPECT_EQ(ret, - get_umf_result_of_free(freeNotSupported, UMF_RESULT_SUCCESS)); + EXPECT_EQ(ret, UMF_RESULT_SUCCESS); pool1.reset(nullptr); pool2.reset(nullptr); @@ -457,8 +439,7 @@ TEST_P(umfIpcTest, ConcurrentGetPutHandles) { for (void *ptr : ptrs) { umf_result_t ret = umfPoolFree(pool.get(), ptr); - EXPECT_EQ(ret, - get_umf_result_of_free(freeNotSupported, UMF_RESULT_SUCCESS)); + EXPECT_EQ(ret, UMF_RESULT_SUCCESS); } pool.reset(nullptr); @@ -520,8 +501,7 @@ TEST_P(umfIpcTest, ConcurrentOpenCloseHandles) { for (void *ptr : ptrs) { umf_result_t ret = umfPoolFree(pool.get(), ptr); - EXPECT_EQ(ret, - get_umf_result_of_free(freeNotSupported, UMF_RESULT_SUCCESS)); + EXPECT_EQ(ret, UMF_RESULT_SUCCESS); } pool.reset(nullptr); diff --git a/test/provider_devdax_memory_ipc.cpp b/test/provider_devdax_memory_ipc.cpp index 921347f400..ed4f1a5f8d 100644 --- a/test/provider_devdax_memory_ipc.cpp +++ b/test/provider_devdax_memory_ipc.cpp @@ -53,14 +53,14 @@ static std::vector getIpcProxyPoolTestParamsList(void) { ipcProxyPoolTestParamsList = { {umfProxyPoolOps(), nullptr, umfDevDaxMemoryProviderOps(), - defaultDevDaxParams.get(), &hostAccessor, false}, + defaultDevDaxParams.get(), &hostAccessor}, #ifdef UMF_POOL_JEMALLOC_ENABLED {umfJemallocPoolOps(), nullptr, umfDevDaxMemoryProviderOps(), - defaultDevDaxParams.get(), &hostAccessor, false}, + defaultDevDaxParams.get(), &hostAccessor}, #endif #ifdef UMF_POOL_SCALABLE_ENABLED {umfScalablePoolOps(), nullptr, umfDevDaxMemoryProviderOps(), - defaultDevDaxParams.get(), &hostAccessor, false}, + defaultDevDaxParams.get(), &hostAccessor}, #endif }; diff --git a/test/provider_file_memory_ipc.cpp b/test/provider_file_memory_ipc.cpp index 115322a476..70c1acd8f5 100644 --- a/test/provider_file_memory_ipc.cpp +++ b/test/provider_file_memory_ipc.cpp @@ -73,14 +73,14 @@ HostMemoryAccessor hostAccessor; static std::vector ipcManyPoolsTestParamsList = { // TODO: enable it when sizes of allocations in ipcFixtures.hpp are fixed // {umfProxyPoolOps(), nullptr, umfFileMemoryProviderOps(), -// file_params_shared.get(), &hostAccessor, false}, +// file_params_shared.get(), &hostAccessor}, #ifdef UMF_POOL_JEMALLOC_ENABLED {umfJemallocPoolOps(), nullptr, umfFileMemoryProviderOps(), - file_params_shared.get(), &hostAccessor, false}, + file_params_shared.get(), &hostAccessor}, #endif #ifdef UMF_POOL_SCALABLE_ENABLED {umfScalablePoolOps(), nullptr, umfFileMemoryProviderOps(), - file_params_shared.get(), &hostAccessor, false}, + file_params_shared.get(), &hostAccessor}, #endif }; @@ -96,14 +96,14 @@ static std::vector getIpcFsDaxTestParamsList(void) { ipcFsDaxTestParamsList = { // TODO: enable it when sizes of allocations in ipcFixtures.hpp are fixed // {umfProxyPoolOps(), nullptr, umfFileMemoryProviderOps(), -// file_params_fsdax.get(), &hostAccessor, true}, +// file_params_fsdax.get(), &hostAccessor}, #ifdef UMF_POOL_JEMALLOC_ENABLED {umfJemallocPoolOps(), nullptr, umfFileMemoryProviderOps(), - file_params_fsdax.get(), &hostAccessor, false}, + file_params_fsdax.get(), &hostAccessor}, #endif #ifdef UMF_POOL_SCALABLE_ENABLED {umfScalablePoolOps(), nullptr, umfFileMemoryProviderOps(), - file_params_fsdax.get(), &hostAccessor, false}, + file_params_fsdax.get(), &hostAccessor}, #endif }; diff --git a/test/provider_os_memory.cpp b/test/provider_os_memory.cpp index 57bce46d24..687db08053 100644 --- a/test/provider_os_memory.cpp +++ b/test/provider_os_memory.cpp @@ -465,11 +465,11 @@ disjoint_params_unique_handle_t disjointParams = disjointPoolParams(); static std::vector ipcTestParamsList = { #if (defined UMF_POOL_DISJOINT_ENABLED) {umfDisjointPoolOps(), disjointParams.get(), umfOsMemoryProviderOps(), - os_params.get(), &hostAccessor, false}, + os_params.get(), &hostAccessor}, #endif #ifdef UMF_POOL_JEMALLOC_ENABLED {umfJemallocPoolOps(), nullptr, umfOsMemoryProviderOps(), os_params.get(), - &hostAccessor, false}, + &hostAccessor}, #endif }; diff --git a/test/providers/provider_level_zero.cpp b/test/providers/provider_level_zero.cpp index d0584777be..78b5e4847a 100644 --- a/test/providers/provider_level_zero.cpp +++ b/test/providers/provider_level_zero.cpp @@ -454,9 +454,9 @@ INSTANTIATE_TEST_SUITE_P( #ifdef _WIN32 GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(umfIpcTest); #else -INSTANTIATE_TEST_SUITE_P( - umfLevelZeroProviderTestSuite, umfIpcTest, - ::testing::Values(ipcTestParams{ - umfProxyPoolOps(), nullptr, umfLevelZeroMemoryProviderOps(), - l0Params_device_memory.get(), &l0Accessor, false})); +INSTANTIATE_TEST_SUITE_P(umfLevelZeroProviderTestSuite, umfIpcTest, + ::testing::Values(ipcTestParams{ + umfProxyPoolOps(), nullptr, + umfLevelZeroMemoryProviderOps(), + l0Params_device_memory.get(), &l0Accessor})); #endif