Revert moving free() to optional (ext) provider ops

This reverts commit b0bfbb7.

Remove umfDefaultFree() and umfIsFreeOpDefault().

Remove the `disable_upstream_provider_free` parameter
of the Coarse provider.

Remove the `upstreamDoesNotFree` argument of
the `umfTrackingMemoryProviderCreate()` function.

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

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

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

src/cpp_helpers.hpp

@@ -84,7 +84,7 @@ template <typename T> constexpr umf_memory_provider_ops_t providerOpsBase() {
     ops.version = UMF_VERSION_CURRENT;
     ops.finalize = [](void *obj) { delete reinterpret_cast<T *>(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);

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

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

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
 }

src/provider/provider_coarse.c

@@ -59,10 +59,6 @@ typedef struct coarse_memory_provider_t {
     // "coarse (<name_of_upstream_provider>)"
     // 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,

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,

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,

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,

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,

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,

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");

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,

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,

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,