diff --git a/examples/custom_file_provider/custom_file_provider.c b/examples/custom_file_provider/custom_file_provider.c index 38247acf0..b0f1bc062 100644 --- a/examples/custom_file_provider/custom_file_provider.c +++ b/examples/custom_file_provider/custom_file_provider.c @@ -251,11 +251,11 @@ static umf_memory_provider_ops_t file_ops = { .initialize = file_init, .finalize = file_deinit, .alloc = file_alloc, - .free = file_free, .get_name = file_get_name, .get_last_native_error = file_get_last_native_error, .get_recommended_page_size = file_get_recommended_page_size, .get_min_page_size = file_get_min_page_size, + .ext.free = file_free, }; // Main function diff --git a/include/umf/memory_provider_ops.h b/include/umf/memory_provider_ops.h index a61e0aad0..0b9c7cfce 100644 --- a/include/umf/memory_provider_ops.h +++ b/include/umf/memory_provider_ops.h @@ -22,6 +22,15 @@ extern "C" { /// can keep them NULL. /// typedef struct umf_memory_provider_ext_ops_t { + /// + /// @brief Frees the memory space pointed by \p ptr from the memory \p provider + /// @param provider pointer to the memory provider + /// @param ptr pointer to the allocated memory to free + /// @param size size of the allocation + /// @return UMF_RESULT_SUCCESS on success or appropriate error code on failure + /// + umf_result_t (*free)(void *provider, void *ptr, size_t size); + /// /// @brief Discard physical pages within the virtual memory mapping associated at the given addr /// and \p size. This call is asynchronous and may delay purging the pages indefinitely. @@ -172,15 +181,6 @@ typedef struct umf_memory_provider_ops_t { umf_result_t (*alloc)(void *provider, size_t size, size_t alignment, void **ptr); - /// - /// @brief Frees the memory space pointed by \p ptr from the memory \p provider - /// @param provider pointer to the memory provider - /// @param ptr pointer to the allocated memory to free - /// @param size size of the allocation - /// @return UMF_RESULT_SUCCESS on success or appropriate error code on failure - /// - umf_result_t (*free)(void *provider, void *ptr, size_t size); - /// /// @brief Retrieve string representation of the underlying provider specific /// result reported by the last API that returned diff --git a/src/cpp_helpers.hpp b/src/cpp_helpers.hpp index 86204a20e..aafc7c4db 100644 --- a/src/cpp_helpers.hpp +++ b/src/cpp_helpers.hpp @@ -84,7 +84,7 @@ template umf_memory_provider_ops_t providerOpsBase() { ops.version = UMF_VERSION_CURRENT; ops.finalize = [](void *obj) { delete reinterpret_cast(obj); }; UMF_ASSIGN_OP(ops, T, alloc, UMF_RESULT_ERROR_UNKNOWN); - UMF_ASSIGN_OP(ops, T, free, UMF_RESULT_ERROR_UNKNOWN); + UMF_ASSIGN_OP(ops.ext, T, free, UMF_RESULT_ERROR_UNKNOWN); UMF_ASSIGN_OP_NORETURN(ops, T, get_last_native_error); UMF_ASSIGN_OP(ops, T, get_recommended_page_size, UMF_RESULT_ERROR_UNKNOWN); UMF_ASSIGN_OP(ops, T, get_min_page_size, UMF_RESULT_ERROR_UNKNOWN); diff --git a/src/memory_pool.c b/src/memory_pool.c index f6ae8841f..c45ddafe7 100644 --- a/src/memory_pool.c +++ b/src/memory_pool.c @@ -43,8 +43,7 @@ static umf_result_t umfPoolCreateInternal(const umf_memory_pool_ops_t *ops, if (!(flags & UMF_POOL_CREATE_FLAG_DISABLE_TRACKING)) { // Wrap provider with memory tracking provider. // Check if the provider supports the free() operation. - bool upstreamDoesNotFree = (umfMemoryProviderFree(provider, NULL, 0) == - UMF_RESULT_ERROR_NOT_SUPPORTED); + bool upstreamDoesNotFree = umfIsFreeOpDefault(provider); ret = umfTrackingMemoryProviderCreate(provider, pool, &pool->provider, upstreamDoesNotFree); if (ret != UMF_RESULT_SUCCESS) { diff --git a/src/memory_provider.c b/src/memory_provider.c index f6e07af62..1e47a248a 100644 --- a/src/memory_provider.c +++ b/src/memory_provider.c @@ -25,6 +25,13 @@ typedef struct umf_memory_provider_t { void *provider_priv; } umf_memory_provider_t; +static umf_result_t umfDefaultFree(void *provider, void *ptr, size_t size) { + (void)provider; + (void)ptr; + (void)size; + return UMF_RESULT_ERROR_NOT_SUPPORTED; +} + static umf_result_t umfDefaultPurgeLazy(void *provider, void *ptr, size_t size) { (void)provider; @@ -99,6 +106,9 @@ static umf_result_t umfDefaultCloseIPCHandle(void *provider, void *ptr, } void assignOpsExtDefaults(umf_memory_provider_ops_t *ops) { + if (!ops->ext.free) { + ops->ext.free = umfDefaultFree; + } if (!ops->ext.purge_lazy) { ops->ext.purge_lazy = umfDefaultPurgeLazy; } @@ -133,7 +143,7 @@ void assignOpsIpcDefaults(umf_memory_provider_ops_t *ops) { static bool validateOpsMandatory(const umf_memory_provider_ops_t *ops) { // Mandatory ops should be non-NULL - return ops->alloc && ops->free && ops->get_recommended_page_size && + return ops->alloc && ops->get_recommended_page_size && ops->get_min_page_size && ops->initialize && ops->finalize && ops->get_last_native_error && ops->get_name; } @@ -159,6 +169,10 @@ static bool validateOps(const umf_memory_provider_ops_t *ops) { validateOpsIpc(&(ops->ipc)); } +bool umfIsFreeOpDefault(umf_memory_provider_handle_t hProvider) { + return (hProvider->ops.ext.free == umfDefaultFree); +} + umf_result_t umfMemoryProviderCreate(const umf_memory_provider_ops_t *ops, void *params, umf_memory_provider_handle_t *hProvider) { @@ -219,7 +233,8 @@ umf_result_t umfMemoryProviderAlloc(umf_memory_provider_handle_t hProvider, umf_result_t umfMemoryProviderFree(umf_memory_provider_handle_t hProvider, void *ptr, size_t size) { UMF_CHECK((hProvider != NULL), UMF_RESULT_ERROR_INVALID_ARGUMENT); - umf_result_t res = hProvider->ops.free(hProvider->provider_priv, ptr, size); + umf_result_t res = + hProvider->ops.ext.free(hProvider->provider_priv, ptr, size); checkErrorAndSetLastProvider(res, hProvider); return res; } diff --git a/src/memory_provider_internal.h b/src/memory_provider_internal.h index 4e858992d..49b2f2e53 100644 --- a/src/memory_provider_internal.h +++ b/src/memory_provider_internal.h @@ -10,6 +10,8 @@ #ifndef UMF_MEMORY_PROVIDER_INTERNAL_H #define UMF_MEMORY_PROVIDER_INTERNAL_H 1 +#include + #include #ifdef __cplusplus @@ -18,6 +20,7 @@ extern "C" { void *umfMemoryProviderGetPriv(umf_memory_provider_handle_t hProvider); umf_memory_provider_handle_t *umfGetLastFailedMemoryProviderPtr(void); +bool umfIsFreeOpDefault(umf_memory_provider_handle_t hProvider); #ifdef __cplusplus } diff --git a/src/provider/provider_cuda.c b/src/provider/provider_cuda.c index b7c4a308d..22dfc49f1 100644 --- a/src/provider/provider_cuda.c +++ b/src/provider/provider_cuda.c @@ -348,11 +348,11 @@ static struct umf_memory_provider_ops_t UMF_CUDA_MEMORY_PROVIDER_OPS = { .initialize = cu_memory_provider_initialize, .finalize = cu_memory_provider_finalize, .alloc = cu_memory_provider_alloc, - .free = cu_memory_provider_free, .get_last_native_error = cu_memory_provider_get_last_native_error, .get_recommended_page_size = cu_memory_provider_get_recommended_page_size, .get_min_page_size = cu_memory_provider_get_min_page_size, .get_name = cu_memory_provider_get_name, + .ext.free = cu_memory_provider_free, // TODO /* .ext.purge_lazy = cu_memory_provider_purge_lazy, diff --git a/src/provider/provider_devdax_memory.c b/src/provider/provider_devdax_memory.c index 45fc725cc..0127f75ae 100644 --- a/src/provider/provider_devdax_memory.c +++ b/src/provider/provider_devdax_memory.c @@ -247,15 +247,6 @@ static umf_result_t devdax_alloc(void *provider, size_t size, size_t alignment, return UMF_RESULT_SUCCESS; } -// free() is not supported -static umf_result_t devdax_free(void *provider, void *ptr, size_t size) { - (void)provider; // unused - (void)ptr; // unused - (void)size; // unused - - return UMF_RESULT_ERROR_NOT_SUPPORTED; -} - static void devdax_get_last_native_error(void *provider, const char **ppMessage, int32_t *pError) { (void)provider; // unused @@ -520,7 +511,6 @@ static umf_memory_provider_ops_t UMF_DEVDAX_MEMORY_PROVIDER_OPS = { .initialize = devdax_initialize, .finalize = devdax_finalize, .alloc = devdax_alloc, - .free = devdax_free, .get_last_native_error = devdax_get_last_native_error, .get_recommended_page_size = devdax_get_recommended_page_size, .get_min_page_size = devdax_get_min_page_size, diff --git a/src/provider/provider_file_memory.c b/src/provider/provider_file_memory.c index ec967f3df..b89e6bd86 100644 --- a/src/provider/provider_file_memory.c +++ b/src/provider/provider_file_memory.c @@ -392,15 +392,6 @@ static umf_result_t file_alloc(void *provider, size_t size, size_t alignment, return UMF_RESULT_SUCCESS; } -// free() is not supported -static umf_result_t file_free(void *provider, void *ptr, size_t size) { - (void)provider; // unused - (void)ptr; // unused - (void)size; // unused - - return UMF_RESULT_ERROR_NOT_SUPPORTED; -} - static void file_get_last_native_error(void *provider, const char **ppMessage, int32_t *pError) { (void)provider; // unused @@ -688,7 +679,6 @@ static umf_memory_provider_ops_t UMF_FILE_MEMORY_PROVIDER_OPS = { .initialize = file_initialize, .finalize = file_finalize, .alloc = file_alloc, - .free = file_free, .get_last_native_error = file_get_last_native_error, .get_recommended_page_size = file_get_recommended_page_size, .get_min_page_size = file_get_min_page_size, diff --git a/src/provider/provider_level_zero.c b/src/provider/provider_level_zero.c index d940d8ded..6b6cc2e2c 100644 --- a/src/provider/provider_level_zero.c +++ b/src/provider/provider_level_zero.c @@ -545,11 +545,11 @@ static struct umf_memory_provider_ops_t UMF_LEVEL_ZERO_MEMORY_PROVIDER_OPS = { .initialize = ze_memory_provider_initialize, .finalize = ze_memory_provider_finalize, .alloc = ze_memory_provider_alloc, - .free = ze_memory_provider_free, .get_last_native_error = ze_memory_provider_get_last_native_error, .get_recommended_page_size = ze_memory_provider_get_recommended_page_size, .get_min_page_size = ze_memory_provider_get_min_page_size, .get_name = ze_memory_provider_get_name, + .ext.free = ze_memory_provider_free, .ext.purge_lazy = ze_memory_provider_purge_lazy, .ext.purge_force = ze_memory_provider_purge_force, .ext.allocation_merge = ze_memory_provider_allocation_merge, diff --git a/src/provider/provider_os_memory.c b/src/provider/provider_os_memory.c index 3b8bbbe91..fe4ca0460 100644 --- a/src/provider/provider_os_memory.c +++ b/src/provider/provider_os_memory.c @@ -1311,11 +1311,11 @@ static umf_memory_provider_ops_t UMF_OS_MEMORY_PROVIDER_OPS = { .initialize = os_initialize, .finalize = os_finalize, .alloc = os_alloc, - .free = os_free, .get_last_native_error = os_get_last_native_error, .get_recommended_page_size = os_get_recommended_page_size, .get_min_page_size = os_get_min_page_size, .get_name = os_get_name, + .ext.free = os_free, .ext.purge_lazy = os_purge_lazy, .ext.purge_force = os_purge_force, .ext.allocation_merge = os_allocation_merge, diff --git a/src/provider/provider_tracking.c b/src/provider/provider_tracking.c index 83aa5c335..cbe028e2f 100644 --- a/src/provider/provider_tracking.c +++ b/src/provider/provider_tracking.c @@ -665,11 +665,11 @@ umf_memory_provider_ops_t UMF_TRACKING_MEMORY_PROVIDER_OPS = { .initialize = trackingInitialize, .finalize = trackingFinalize, .alloc = trackingAlloc, - .free = trackingFree, .get_last_native_error = trackingGetLastError, .get_min_page_size = trackingGetMinPageSize, .get_recommended_page_size = trackingGetRecommendedPageSize, .get_name = trackingName, + .ext.free = trackingFree, .ext.purge_force = trackingPurgeForce, .ext.purge_lazy = trackingPurgeLazy, .ext.allocation_split = trackingAllocationSplit, diff --git a/test/common/provider_null.c b/test/common/provider_null.c index e667bfce4..5db389e89 100644 --- a/test/common/provider_null.c +++ b/test/common/provider_null.c @@ -134,11 +134,11 @@ umf_memory_provider_ops_t UMF_NULL_PROVIDER_OPS = { .initialize = nullInitialize, .finalize = nullFinalize, .alloc = nullAlloc, - .free = nullFree, .get_last_native_error = nullGetLastError, .get_recommended_page_size = nullGetRecommendedPageSize, .get_min_page_size = nullGetPageSize, .get_name = nullName, + .ext.free = nullFree, .ext.purge_lazy = nullPurgeLazy, .ext.purge_force = nullPurgeForce, .ext.allocation_merge = nullAllocationMerge, diff --git a/test/common/provider_trace.c b/test/common/provider_trace.c index 9d063b4f5..219dde5cd 100644 --- a/test/common/provider_trace.c +++ b/test/common/provider_trace.c @@ -195,11 +195,11 @@ umf_memory_provider_ops_t UMF_TRACE_PROVIDER_OPS = { .initialize = traceInitialize, .finalize = traceFinalize, .alloc = traceAlloc, - .free = traceFree, .get_last_native_error = traceGetLastError, .get_recommended_page_size = traceGetRecommendedPageSize, .get_min_page_size = traceGetPageSize, .get_name = traceName, + .ext.free = traceFree, .ext.purge_lazy = tracePurgeLazy, .ext.purge_force = tracePurgeForce, .ext.allocation_merge = traceAllocationMerge, diff --git a/test/memoryProviderAPI.cpp b/test/memoryProviderAPI.cpp index 144aa4d55..41196de34 100644 --- a/test/memoryProviderAPI.cpp +++ b/test/memoryProviderAPI.cpp @@ -86,6 +86,19 @@ TEST_F(test, memoryProviderTrace) { ASSERT_EQ(calls.size(), ++call_count); } +TEST_F(test, memoryProviderOpsNullFreeField) { + umf_memory_provider_ops_t provider_ops = UMF_NULL_PROVIDER_OPS; + provider_ops.ext.free = nullptr; + umf_memory_provider_handle_t hProvider; + auto ret = umfMemoryProviderCreate(&provider_ops, nullptr, &hProvider); + ASSERT_EQ(ret, UMF_RESULT_SUCCESS); + + ret = umfMemoryProviderFree(hProvider, nullptr, 0); + ASSERT_EQ(ret, UMF_RESULT_ERROR_NOT_SUPPORTED); + + umfMemoryProviderDestroy(hProvider); +} + TEST_F(test, memoryProviderOpsNullPurgeLazyField) { umf_memory_provider_ops_t provider_ops = UMF_NULL_PROVIDER_OPS; provider_ops.ext.purge_lazy = nullptr; @@ -155,14 +168,6 @@ TEST_F(test, memoryProviderOpsNullAllocField) { ASSERT_EQ(ret, UMF_RESULT_ERROR_INVALID_ARGUMENT); } -TEST_F(test, memoryProviderOpsNullFreeField) { - umf_memory_provider_ops_t provider_ops = UMF_NULL_PROVIDER_OPS; - provider_ops.free = nullptr; - umf_memory_provider_handle_t hProvider; - auto ret = umfMemoryProviderCreate(&provider_ops, nullptr, &hProvider); - ASSERT_EQ(ret, UMF_RESULT_ERROR_INVALID_ARGUMENT); -} - TEST_F(test, memoryProviderOpsNullGetLastNativeErrorField) { umf_memory_provider_ops_t provider_ops = UMF_NULL_PROVIDER_OPS; provider_ops.get_last_native_error = nullptr; diff --git a/test/pools/disjoint_pool.cpp b/test/pools/disjoint_pool.cpp index 2f5d61142..d7612f4d5 100644 --- a/test/pools/disjoint_pool.cpp +++ b/test/pools/disjoint_pool.cpp @@ -82,10 +82,7 @@ TEST_F(test, sharedLimits) { } umf_result_t free(void *ptr, [[maybe_unused]] size_t size) noexcept { ::free(ptr); - // umfMemoryProviderFree(provider, NULL, 0) is called inside umfPoolCreateInternal() - if (ptr != NULL && size != 0) { - numFrees++; - } + numFrees++; return UMF_RESULT_SUCCESS; } };