diff --git a/examples/custom_file_provider/custom_file_provider.c b/examples/custom_file_provider/custom_file_provider.c index ffa61d63f5..ad897fe5ea 100644 --- a/examples/custom_file_provider/custom_file_provider.c +++ b/examples/custom_file_provider/custom_file_provider.c @@ -237,11 +237,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 0b9c7cfce3..a61e0aad07 100644 --- a/include/umf/memory_provider_ops.h +++ b/include/umf/memory_provider_ops.h @@ -22,15 +22,6 @@ 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. @@ -181,6 +172,15 @@ 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 6316ccbc7e..8789105814 100644 --- a/src/cpp_helpers.hpp +++ b/src/cpp_helpers.hpp @@ -84,7 +84,7 @@ template constexpr 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.ext, T, free, UMF_RESULT_ERROR_UNKNOWN); + UMF_ASSIGN_OP(ops, 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 4a85955efa..cb1d303f5f 100644 --- a/src/memory_pool.c +++ b/src/memory_pool.c @@ -42,10 +42,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 = umfIsFreeOpDefault(provider); - ret = umfTrackingMemoryProviderCreate(provider, pool, &pool->provider, - upstreamDoesNotFree); + ret = umfTrackingMemoryProviderCreate(provider, pool, &pool->provider); if (ret != UMF_RESULT_SUCCESS) { goto err_provider_create; } diff --git a/src/memory_provider.c b/src/memory_provider.c index 883f1be263..59f3f12592 100644 --- a/src/memory_provider.c +++ b/src/memory_provider.c @@ -25,13 +25,6 @@ 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; @@ -106,9 +99,6 @@ 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; } @@ -143,7 +133,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->get_recommended_page_size && + return ops->alloc && ops->free && ops->get_recommended_page_size && ops->get_min_page_size && ops->initialize && ops->finalize && ops->get_last_native_error && ops->get_name; } @@ -169,10 +159,6 @@ 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) { @@ -236,8 +222,7 @@ 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.ext.free(hProvider->provider_priv, ptr, size); + umf_result_t res = hProvider->ops.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 49b2f2e531..60955e0fbd 100644 --- a/src/memory_provider_internal.h +++ b/src/memory_provider_internal.h @@ -20,7 +20,6 @@ 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_coarse.c b/src/provider/provider_coarse.c index c3027b91d7..72985faaf2 100644 --- a/src/provider/provider_coarse.c +++ b/src/provider/provider_coarse.c @@ -59,10 +59,6 @@ typedef struct coarse_memory_provider_t { // "coarse ()" // for example: "coarse (L0)" char *name; - - // Set to true if the free() operation of the upstream memory provider is not supported - // (i.e. if (umfMemoryProviderFree(upstream_memory_provider, NULL, 0) == UMF_RESULT_ERROR_NOT_SUPPORTED) - bool disable_upstream_provider_free; } coarse_memory_provider_t; typedef struct ravl_node ravl_node_t; @@ -918,13 +914,6 @@ static umf_result_t coarse_memory_provider_initialize(void *params, coarse_provider->allocation_strategy = coarse_params->allocation_strategy; coarse_provider->init_buffer = coarse_params->init_buffer; - if (coarse_provider->upstream_memory_provider) { - coarse_provider->disable_upstream_provider_free = - umfIsFreeOpDefault(coarse_provider->upstream_memory_provider); - } else { - coarse_provider->disable_upstream_provider_free = false; - } - umf_result_t umf_result = coarse_memory_provider_set_name(coarse_provider); if (umf_result != UMF_RESULT_SUCCESS) { LOG_ERR("name initialization failed"); @@ -1027,8 +1016,7 @@ static void coarse_ravl_cb_rm_upstream_blocks_node(void *data, void *arg) { block_t *alloc = node_data->value; assert(alloc); - if (coarse_provider->upstream_memory_provider && - !coarse_provider->disable_upstream_provider_free) { + if (coarse_provider->upstream_memory_provider) { // We continue to deallocate alloc blocks even if the upstream provider doesn't return success. umfMemoryProviderFree(coarse_provider->upstream_memory_provider, alloc->data, alloc->size); @@ -1288,10 +1276,8 @@ static umf_result_t coarse_memory_provider_alloc(void *provider, size_t size, umf_result = coarse_add_upstream_block(coarse_provider, *resultPtr, size); if (umf_result != UMF_RESULT_SUCCESS) { - if (!coarse_provider->disable_upstream_provider_free) { - umfMemoryProviderFree(coarse_provider->upstream_memory_provider, - *resultPtr, size); - } + umfMemoryProviderFree(coarse_provider->upstream_memory_provider, + *resultPtr, size); goto err_unlock; } @@ -1657,12 +1643,12 @@ umf_memory_provider_ops_t UMF_COARSE_MEMORY_PROVIDER_OPS = { .initialize = coarse_memory_provider_initialize, .finalize = coarse_memory_provider_finalize, .alloc = coarse_memory_provider_alloc, + .free = coarse_memory_provider_free, .get_last_native_error = coarse_memory_provider_get_last_native_error, .get_recommended_page_size = coarse_memory_provider_get_recommended_page_size, .get_min_page_size = coarse_memory_provider_get_min_page_size, .get_name = coarse_memory_provider_get_name, - .ext.free = coarse_memory_provider_free, .ext.purge_lazy = coarse_memory_provider_purge_lazy, .ext.purge_force = coarse_memory_provider_purge_force, .ext.allocation_merge = coarse_memory_provider_allocation_merge, diff --git a/src/provider/provider_cuda.c b/src/provider/provider_cuda.c index baccbd0235..f46e049723 100644 --- a/src/provider/provider_cuda.c +++ b/src/provider/provider_cuda.c @@ -599,11 +599,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 463b796ec0..79a0662754 100644 --- a/src/provider/provider_devdax_memory.c +++ b/src/provider/provider_devdax_memory.c @@ -527,11 +527,11 @@ 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, .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/src/provider/provider_file_memory.c b/src/provider/provider_file_memory.c index 558b1062a5..edf733180b 100644 --- a/src/provider/provider_file_memory.c +++ b/src/provider/provider_file_memory.c @@ -825,11 +825,11 @@ 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, .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/src/provider/provider_level_zero.c b/src/provider/provider_level_zero.c index f4a3e97c27..70f0acfe54 100644 --- a/src/provider/provider_level_zero.c +++ b/src/provider/provider_level_zero.c @@ -682,11 +682,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 4c19944a96..2cc8e9827c 100644 --- a/src/provider/provider_os_memory.c +++ b/src/provider/provider_os_memory.c @@ -1408,11 +1408,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 e726feefb0..c4fff4133b 100644 --- a/src/provider/provider_tracking.c +++ b/src/provider/provider_tracking.c @@ -154,9 +154,6 @@ typedef struct umf_tracking_memory_provider_t { umf_memory_pool_handle_t pool; critnib *ipcCache; ipc_mapped_handle_cache_handle_t hIpcMappedCache; - - // the upstream provider does not support the free() operation - bool upstreamDoesNotFree; } umf_tracking_memory_provider_t; typedef struct umf_tracking_memory_provider_t umf_tracking_memory_provider_t; @@ -422,8 +419,7 @@ static umf_result_t trackingInitialize(void *params, void **ret) { // TODO clearing the tracker is a temporary solution and should be removed. // The tracker should be cleared using the provider's free() operation. static void clear_tracker_for_the_pool(umf_memory_tracker_handle_t hTracker, - umf_memory_pool_handle_t pool, - bool upstreamDoesNotFree) { + umf_memory_pool_handle_t pool) { uintptr_t rkey; void *rvalue; size_t n_items = 0; @@ -448,7 +444,7 @@ static void clear_tracker_for_the_pool(umf_memory_tracker_handle_t hTracker, #ifndef NDEBUG // print error messages only if provider supports the free() operation - if (n_items && !upstreamDoesNotFree) { + if (n_items) { if (pool) { LOG_ERR( "tracking provider of pool %p is not empty! (%zu items left)", @@ -459,13 +455,12 @@ static void clear_tracker_for_the_pool(umf_memory_tracker_handle_t hTracker, } } #else /* DEBUG */ - (void)upstreamDoesNotFree; // unused in DEBUG build - (void)n_items; // unused in DEBUG build + (void)n_items; // unused in DEBUG build #endif /* DEBUG */ } static void clear_tracker(umf_memory_tracker_handle_t hTracker) { - clear_tracker_for_the_pool(hTracker, NULL, false); + clear_tracker_for_the_pool(hTracker, NULL); } static void trackingFinalize(void *provider) { @@ -480,8 +475,7 @@ static void trackingFinalize(void *provider) { // because it may need those resources till // the very end of exiting the application. if (!utils_is_running_in_proxy_lib()) { - clear_tracker_for_the_pool(p->hTracker, p->pool, - p->upstreamDoesNotFree); + clear_tracker_for_the_pool(p->hTracker, p->pool); } umf_ba_global_free(provider); @@ -760,11 +754,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, @@ -777,11 +771,10 @@ umf_memory_provider_ops_t UMF_TRACKING_MEMORY_PROVIDER_OPS = { umf_result_t umfTrackingMemoryProviderCreate( umf_memory_provider_handle_t hUpstream, umf_memory_pool_handle_t hPool, - umf_memory_provider_handle_t *hTrackingProvider, bool upstreamDoesNotFree) { + umf_memory_provider_handle_t *hTrackingProvider) { umf_tracking_memory_provider_t params; params.hUpstream = hUpstream; - params.upstreamDoesNotFree = upstreamDoesNotFree; params.hTracker = TRACKER; if (!params.hTracker) { LOG_ERR("failed, TRACKER is NULL"); diff --git a/src/provider/provider_tracking.h b/src/provider/provider_tracking.h index 9444ee4757..2abc365051 100644 --- a/src/provider/provider_tracking.h +++ b/src/provider/provider_tracking.h @@ -54,7 +54,7 @@ umf_result_t umfMemoryTrackerGetAllocInfo(const void *ptr, // forwards all requests to hUpstream memory Provider. hUpstream lifetime should be managed by the user of this function. umf_result_t umfTrackingMemoryProviderCreate( umf_memory_provider_handle_t hUpstream, umf_memory_pool_handle_t hPool, - umf_memory_provider_handle_t *hTrackingProvider, bool upstreamDoesNotFree); + umf_memory_provider_handle_t *hTrackingProvider); void umfTrackingMemoryProviderGetUpstreamProvider( umf_memory_provider_handle_t hTrackingProvider, diff --git a/test/common/provider_null.c b/test/common/provider_null.c index 5db389e895..e667bfce40 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 219dde5cd7..9d063b4f56 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 866ae6dae3..2dc7261f06 100644 --- a/test/memoryProviderAPI.cpp +++ b/test/memoryProviderAPI.cpp @@ -89,19 +89,6 @@ 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; @@ -204,6 +191,14 @@ 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;