Skip to content

Commit b7a9b6a

Browse files
ldorauldorau
authored
Merge pull request #953 from ldorau/Revert_Move_free_to_optional_ext_provider_ops
Revert moving `free()` to optional (ext) provider ops
2 parents 7d8d780 + 3112e2b commit b7a9b6a

17 files changed

+41
-86
lines changed

‎examples/custom_file_provider/custom_file_provider.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -237,11 +237,11 @@ static umf_memory_provider_ops_t file_ops = {
237237
.initialize = file_init,
238238
.finalize = file_deinit,
239239
.alloc = file_alloc,
240+
.free = file_free,
240241
.get_name = file_get_name,
241242
.get_last_native_error = file_get_last_native_error,
242243
.get_recommended_page_size = file_get_recommended_page_size,
243244
.get_min_page_size = file_get_min_page_size,
244-
.ext.free = file_free,
245245
};
246246

247247
// Main function

‎include/umf/memory_provider_ops.h

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -22,15 +22,6 @@ extern "C" {
2222
/// can keep them NULL.
2323
///
2424
typedef struct umf_memory_provider_ext_ops_t {
25-
///
26-
/// @brief Frees the memory space pointed by \p ptr from the memory \p provider
27-
/// @param provider pointer to the memory provider
28-
/// @param ptr pointer to the allocated memory to free
29-
/// @param size size of the allocation
30-
/// @return UMF_RESULT_SUCCESS on success or appropriate error code on failure
31-
///
32-
umf_result_t (*free)(void *provider, void *ptr, size_t size);
33-
3425
///
3526
/// @brief Discard physical pages within the virtual memory mapping associated at the given addr
3627
/// 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 {
181172
umf_result_t (*alloc)(void *provider, size_t size, size_t alignment,
182173
void **ptr);
183174

175+
///
176+
/// @brief Frees the memory space pointed by \p ptr from the memory \p provider
177+
/// @param provider pointer to the memory provider
178+
/// @param ptr pointer to the allocated memory to free
179+
/// @param size size of the allocation
180+
/// @return UMF_RESULT_SUCCESS on success or appropriate error code on failure
181+
///
182+
umf_result_t (*free)(void *provider, void *ptr, size_t size);
183+
184184
///
185185
/// @brief Retrieve string representation of the underlying provider specific
186186
/// result reported by the last API that returned

‎src/cpp_helpers.hpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ template <typename T> constexpr umf_memory_provider_ops_t providerOpsBase() {
8484
ops.version = UMF_VERSION_CURRENT;
8585
ops.finalize = [](void *obj) { delete reinterpret_cast<T *>(obj); };
8686
UMF_ASSIGN_OP(ops, T, alloc, UMF_RESULT_ERROR_UNKNOWN);
87-
UMF_ASSIGN_OP(ops.ext, T, free, UMF_RESULT_ERROR_UNKNOWN);
87+
UMF_ASSIGN_OP(ops, T, free, UMF_RESULT_ERROR_UNKNOWN);
8888
UMF_ASSIGN_OP_NORETURN(ops, T, get_last_native_error);
8989
UMF_ASSIGN_OP(ops, T, get_recommended_page_size, UMF_RESULT_ERROR_UNKNOWN);
9090
UMF_ASSIGN_OP(ops, T, get_min_page_size, UMF_RESULT_ERROR_UNKNOWN);

‎src/memory_pool.c

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,10 +42,7 @@ static umf_result_t umfPoolCreateInternal(const umf_memory_pool_ops_t *ops,
4242

4343
if (!(flags & UMF_POOL_CREATE_FLAG_DISABLE_TRACKING)) {
4444
// Wrap provider with memory tracking provider.
45-
// Check if the provider supports the free() operation.
46-
bool upstreamDoesNotFree = umfIsFreeOpDefault(provider);
47-
ret = umfTrackingMemoryProviderCreate(provider, pool, &pool->provider,
48-
upstreamDoesNotFree);
45+
ret = umfTrackingMemoryProviderCreate(provider, pool, &pool->provider);
4946
if (ret != UMF_RESULT_SUCCESS) {
5047
goto err_provider_create;
5148
}

‎src/memory_provider.c

Lines changed: 2 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -25,13 +25,6 @@ typedef struct umf_memory_provider_t {
2525
void *provider_priv;
2626
} umf_memory_provider_t;
2727

28-
static umf_result_t umfDefaultFree(void *provider, void *ptr, size_t size) {
29-
(void)provider;
30-
(void)ptr;
31-
(void)size;
32-
return UMF_RESULT_ERROR_NOT_SUPPORTED;
33-
}
34-
3528
static umf_result_t umfDefaultPurgeLazy(void *provider, void *ptr,
3629
size_t size) {
3730
(void)provider;
@@ -106,9 +99,6 @@ static umf_result_t umfDefaultCloseIPCHandle(void *provider, void *ptr,
10699
}
107100

108101
void assignOpsExtDefaults(umf_memory_provider_ops_t *ops) {
109-
if (!ops->ext.free) {
110-
ops->ext.free = umfDefaultFree;
111-
}
112102
if (!ops->ext.purge_lazy) {
113103
ops->ext.purge_lazy = umfDefaultPurgeLazy;
114104
}
@@ -143,7 +133,7 @@ void assignOpsIpcDefaults(umf_memory_provider_ops_t *ops) {
143133

144134
static bool validateOpsMandatory(const umf_memory_provider_ops_t *ops) {
145135
// Mandatory ops should be non-NULL
146-
return ops->alloc && ops->get_recommended_page_size &&
136+
return ops->alloc && ops->free && ops->get_recommended_page_size &&
147137
ops->get_min_page_size && ops->initialize && ops->finalize &&
148138
ops->get_last_native_error && ops->get_name;
149139
}
@@ -169,10 +159,6 @@ static bool validateOps(const umf_memory_provider_ops_t *ops) {
169159
validateOpsIpc(&(ops->ipc));
170160
}
171161

172-
bool umfIsFreeOpDefault(umf_memory_provider_handle_t hProvider) {
173-
return (hProvider->ops.ext.free == umfDefaultFree);
174-
}
175-
176162
umf_result_t umfMemoryProviderCreate(const umf_memory_provider_ops_t *ops,
177163
void *params,
178164
umf_memory_provider_handle_t *hProvider) {
@@ -236,8 +222,7 @@ umf_result_t umfMemoryProviderAlloc(umf_memory_provider_handle_t hProvider,
236222
umf_result_t umfMemoryProviderFree(umf_memory_provider_handle_t hProvider,
237223
void *ptr, size_t size) {
238224
UMF_CHECK((hProvider != NULL), UMF_RESULT_ERROR_INVALID_ARGUMENT);
239-
umf_result_t res =
240-
hProvider->ops.ext.free(hProvider->provider_priv, ptr, size);
225+
umf_result_t res = hProvider->ops.free(hProvider->provider_priv, ptr, size);
241226
checkErrorAndSetLastProvider(res, hProvider);
242227
return res;
243228
}

‎src/memory_provider_internal.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ extern "C" {
2020

2121
void *umfMemoryProviderGetPriv(umf_memory_provider_handle_t hProvider);
2222
umf_memory_provider_handle_t *umfGetLastFailedMemoryProviderPtr(void);
23-
bool umfIsFreeOpDefault(umf_memory_provider_handle_t hProvider);
2423

2524
#ifdef __cplusplus
2625
}

‎src/provider/provider_coarse.c

Lines changed: 4 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -59,10 +59,6 @@ typedef struct coarse_memory_provider_t {
5959
// "coarse (<name_of_upstream_provider>)"
6060
// for example: "coarse (L0)"
6161
char *name;
62-
63-
// Set to true if the free() operation of the upstream memory provider is not supported
64-
// (i.e. if (umfMemoryProviderFree(upstream_memory_provider, NULL, 0) == UMF_RESULT_ERROR_NOT_SUPPORTED)
65-
bool disable_upstream_provider_free;
6662
} coarse_memory_provider_t;
6763

6864
typedef struct ravl_node ravl_node_t;
@@ -918,13 +914,6 @@ static umf_result_t coarse_memory_provider_initialize(void *params,
918914
coarse_provider->allocation_strategy = coarse_params->allocation_strategy;
919915
coarse_provider->init_buffer = coarse_params->init_buffer;
920916

921-
if (coarse_provider->upstream_memory_provider) {
922-
coarse_provider->disable_upstream_provider_free =
923-
umfIsFreeOpDefault(coarse_provider->upstream_memory_provider);
924-
} else {
925-
coarse_provider->disable_upstream_provider_free = false;
926-
}
927-
928917
umf_result_t umf_result = coarse_memory_provider_set_name(coarse_provider);
929918
if (umf_result != UMF_RESULT_SUCCESS) {
930919
LOG_ERR("name initialization failed");
@@ -1027,8 +1016,7 @@ static void coarse_ravl_cb_rm_upstream_blocks_node(void *data, void *arg) {
10271016
block_t *alloc = node_data->value;
10281017
assert(alloc);
10291018

1030-
if (coarse_provider->upstream_memory_provider &&
1031-
!coarse_provider->disable_upstream_provider_free) {
1019+
if (coarse_provider->upstream_memory_provider) {
10321020
// We continue to deallocate alloc blocks even if the upstream provider doesn't return success.
10331021
umfMemoryProviderFree(coarse_provider->upstream_memory_provider,
10341022
alloc->data, alloc->size);
@@ -1288,10 +1276,8 @@ static umf_result_t coarse_memory_provider_alloc(void *provider, size_t size,
12881276

12891277
umf_result = coarse_add_upstream_block(coarse_provider, *resultPtr, size);
12901278
if (umf_result != UMF_RESULT_SUCCESS) {
1291-
if (!coarse_provider->disable_upstream_provider_free) {
1292-
umfMemoryProviderFree(coarse_provider->upstream_memory_provider,
1293-
*resultPtr, size);
1294-
}
1279+
umfMemoryProviderFree(coarse_provider->upstream_memory_provider,
1280+
*resultPtr, size);
12951281
goto err_unlock;
12961282
}
12971283

@@ -1657,12 +1643,12 @@ umf_memory_provider_ops_t UMF_COARSE_MEMORY_PROVIDER_OPS = {
16571643
.initialize = coarse_memory_provider_initialize,
16581644
.finalize = coarse_memory_provider_finalize,
16591645
.alloc = coarse_memory_provider_alloc,
1646+
.free = coarse_memory_provider_free,
16601647
.get_last_native_error = coarse_memory_provider_get_last_native_error,
16611648
.get_recommended_page_size =
16621649
coarse_memory_provider_get_recommended_page_size,
16631650
.get_min_page_size = coarse_memory_provider_get_min_page_size,
16641651
.get_name = coarse_memory_provider_get_name,
1665-
.ext.free = coarse_memory_provider_free,
16661652
.ext.purge_lazy = coarse_memory_provider_purge_lazy,
16671653
.ext.purge_force = coarse_memory_provider_purge_force,
16681654
.ext.allocation_merge = coarse_memory_provider_allocation_merge,

‎src/provider/provider_cuda.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -599,11 +599,11 @@ static struct umf_memory_provider_ops_t UMF_CUDA_MEMORY_PROVIDER_OPS = {
599599
.initialize = cu_memory_provider_initialize,
600600
.finalize = cu_memory_provider_finalize,
601601
.alloc = cu_memory_provider_alloc,
602+
.free = cu_memory_provider_free,
602603
.get_last_native_error = cu_memory_provider_get_last_native_error,
603604
.get_recommended_page_size = cu_memory_provider_get_recommended_page_size,
604605
.get_min_page_size = cu_memory_provider_get_min_page_size,
605606
.get_name = cu_memory_provider_get_name,
606-
.ext.free = cu_memory_provider_free,
607607
// TODO
608608
/*
609609
.ext.purge_lazy = cu_memory_provider_purge_lazy,

‎src/provider/provider_devdax_memory.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -527,11 +527,11 @@ static umf_memory_provider_ops_t UMF_DEVDAX_MEMORY_PROVIDER_OPS = {
527527
.initialize = devdax_initialize,
528528
.finalize = devdax_finalize,
529529
.alloc = devdax_alloc,
530+
.free = devdax_free,
530531
.get_last_native_error = devdax_get_last_native_error,
531532
.get_recommended_page_size = devdax_get_recommended_page_size,
532533
.get_min_page_size = devdax_get_min_page_size,
533534
.get_name = devdax_get_name,
534-
.ext.free = devdax_free,
535535
.ext.purge_lazy = devdax_purge_lazy,
536536
.ext.purge_force = devdax_purge_force,
537537
.ext.allocation_merge = devdax_allocation_merge,

‎src/provider/provider_file_memory.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -825,11 +825,11 @@ static umf_memory_provider_ops_t UMF_FILE_MEMORY_PROVIDER_OPS = {
825825
.initialize = file_initialize,
826826
.finalize = file_finalize,
827827
.alloc = file_alloc,
828+
.free = file_free,
828829
.get_last_native_error = file_get_last_native_error,
829830
.get_recommended_page_size = file_get_recommended_page_size,
830831
.get_min_page_size = file_get_min_page_size,
831832
.get_name = file_get_name,
832-
.ext.free = file_free,
833833
.ext.purge_lazy = file_purge_lazy,
834834
.ext.purge_force = file_purge_force,
835835
.ext.allocation_merge = file_allocation_merge,

0 commit comments

Comments
 (0)