Skip to content

add post-initialize function to pools and providers #1500

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+306 −98
Open

add post-initialize function to pools and providers #1500

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 21 additions & 3 deletions .github/workflows/reusable_compatibility.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,16 @@ jobs:
UMF_LOG: level:warning;flush:debug;output:stderr;pid:no
LD_LIBRARY_PATH: ${{github.workspace}}/latest_version/build/lib/
run: |
ctest --verbose -E "test_memoryProvider"
ctest --verbose -E "test_memoryProvider|test_disjoint_pool"

- name: Run disabled tests individually with latest UMF libs (warnings enabled)
working-directory: ${{github.workspace}}/tag_version/build
env:
UMF_LOG: level:warning;flush:debug;output:stderr;pid:no
LD_LIBRARY_PATH: ${{github.workspace}}/latest_version/build/lib/
run: |
test/test_memoryProvider --gtest_filter="-*Trace"
test/test_disjoint_pool --gtest_filter="-test.internals"

# Browse all folders in the examples directory, build them using the
# latest UMF version, and run them, excluding those in the exclude list.
Expand Down Expand Up @@ -228,10 +236,12 @@ jobs:
env:
UMF_LOG: level:warning;flush:debug;output:stderr;pid:no
run: |
$env:UMF_LOG="level:warning;flush:debug;output:stderr;pid:no"
cp ${{github.workspace}}/latest_version/build/bin/Debug/umf.dll ${{github.workspace}}/tag_version/build/bin/Debug/umf.dll
ctest -C Debug --verbose -E "test_memoryProvider"
ctest -C Debug --verbose -E "test_memoryProvider|test_disjoint_pool"
$env:Path = "${{github.workspace}}/tag_version/build/bin/Debug;${{env.VCPKG_BIN_PATH}};$env:Path"
test/Debug/test_memoryProvider.exe --gtest_filter="-*Trace"
test/Debug/test_disjoint_pool.exe --gtest_filter="-test.internals"

# Browse all folders in the examples directory, build them using the
# latest UMF version, and run them, excluding those in the exclude list.
Expand Down Expand Up @@ -373,8 +383,16 @@ jobs:
UMF_LOG: level:warning;flush:debug;output:stderr;pid:no
LD_LIBRARY_PATH: ${{github.workspace}}/latest_version/build/lib/
run: |
ctest --verbose -E "test_memoryProvider"
ctest --verbose -E "test_memoryProvider|test_disjoint_pool"

- name: Run disabled tests individually with latest UMF libs (warnings enabled)
working-directory: ${{github.workspace}}/tag_version/build
env:
UMF_LOG: level:warning;flush:debug;output:stderr;pid:no
LD_LIBRARY_PATH: ${{github.workspace}}/latest_version/build/lib/
run: |
test/test_memoryProvider --gtest_filter="-*Trace"
test/test_disjoint_pool --gtest_filter="-test.internals"

# Browse all folders in the examples directory, build them using the
# latest UMF version, and run them, excluding those in the exclude list.
Expand Down
2 changes: 1 addition & 1 deletion include/umf/memory_pool.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ typedef enum umf_pool_create_flag_t {
/// @brief Type for combinations of pool creation flags
typedef uint32_t umf_pool_create_flags_t;

///
/// @anchor umfPoolCreate
/// @brief Creates new memory pool.
/// @param ops instance of umf_memory_pool_ops_t
/// @param provider memory provider that will be used for coarse-grain allocations.
Expand Down
19 changes: 19 additions & 0 deletions include/umf/memory_pool_ops.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,25 @@ typedef struct umf_memory_pool_ops_t {
/// failure.
///
umf_result_t (*ext_trim_memory)(void *pool, size_t minBytesToKeep);

///
/// @brief Post-initialization hook for deferred setup.
///
/// \details
/// * Called by UMF **after** @ref initialize during umfPoolCreate().
/// * Between @ref initialize and @ref ext_post_initialize there **may** be
/// multiple calls to @ref ext_ctl for early configuration.
Comment on lines +199 to +201
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is unclear how pool/provider developer should decide how initialization steps should be split between initialize and ext_post_initialize

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Honestly, I think it is an error-prone approach

/// * Failure handling: If this function returns an error, the
/// implementation **must** free any resources acquired during
/// initialization and leave no leaks (UMF will not call @ref finalize
/// after a failed post-initialize).
///
/// @param pool Pool handle.
///
/// @return UMF_RESULT_SUCCESS on success; an appropriate error code on failure.
///
umf_result_t (*ext_post_initialize)(void *pool);

} umf_memory_pool_ops_t;

#ifdef __cplusplus
Expand Down
19 changes: 19 additions & 0 deletions include/umf/memory_provider_ops.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,7 @@ typedef struct umf_memory_provider_ops_t {
void *provider, const void *ptr,
umf_memory_property_id_t memory_property_id, void *property_value);

///
/// @brief Retrieve size of the provider-specific properties of the memory
/// allocation.
/// \details
Expand All @@ -322,6 +323,24 @@ typedef struct umf_memory_provider_ops_t {
void *provider, umf_memory_property_id_t memory_property_id,
size_t *size);

///
/// @brief Post-initialization hook for a deferred setup.
///
/// \details
/// * Called by UMF **after** @ref initialize during umfMemoryProviderCreate().
/// * Between @ref initialize and @ref ext_post_initialize there **may** be
/// multiple calls to @ref ext_ctl for early configuration.
/// * Failure handling: If this function returns an error, the
/// implementation **must** free any resources acquired during
/// initialization and leave no leaks (UMF will not call @ref finalize
/// after a failed post-initialize).
///
/// @param provider Provider handle.
///
/// @return UMF_RESULT_SUCCESS on success; an appropriate error code on failure.
///
umf_result_t (*ext_post_initialize)(void *provider);

} umf_memory_provider_ops_t;

#ifdef __cplusplus
Expand Down
22 changes: 18 additions & 4 deletions src/memory_pool.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -463,6 +463,11 @@ static umf_result_t umfDefaultTrimMemory(void *provider,
return UMF_RESULT_ERROR_NOT_SUPPORTED;
}

static umf_result_t umfDefaultPostInitialize(void *pool) {
(void)pool;
return UMF_RESULT_ERROR_NOT_SUPPORTED;
}

// logical sum (OR) of all umf_pool_create_flags_t flags
static const umf_pool_create_flags_t UMF_POOL_CREATE_FLAG_ALL =
UMF_POOL_CREATE_FLAG_OWN_PROVIDER | UMF_POOL_CREATE_FLAG_DISABLE_TRACKING;
Expand Down Expand Up @@ -495,7 +500,6 @@ static umf_result_t umfPoolCreateInternal(const umf_memory_pool_ops_t *ops,
}

umf_result_t ret = UMF_RESULT_SUCCESS;

umf_memory_pool_ops_t compatible_ops;
if (ops->version != UMF_POOL_OPS_VERSION_CURRENT) {
LOG_WARN("Memory Pool ops version \"%d\" is different than the current "
Expand All @@ -504,8 +508,8 @@ static umf_result_t umfPoolCreateInternal(const umf_memory_pool_ops_t *ops,

// Create a new ops compatible structure with the current version
memset(&compatible_ops, 0, sizeof(compatible_ops));
if (UMF_MINOR_VERSION(ops->version) == 0) {
LOG_INFO("Detected 1.0 version of Memory Pool ops, "
if (ops->version < UMF_MAKE_VERSION(1, 1)) {
LOG_INFO("Detected 1.0 version or below of Memory Pool ops, "
"upgrading to current version");
memcpy(&compatible_ops, ops,
offsetof(umf_memory_pool_ops_t, ext_trim_memory));
Expand Down Expand Up @@ -547,13 +551,17 @@ static umf_result_t umfPoolCreateInternal(const umf_memory_pool_ops_t *ops,
pool->ops.ext_trim_memory = umfDefaultTrimMemory;
}

if (NULL == pool->ops.ext_post_initialize) {
pool->ops.ext_post_initialize = umfDefaultPostInitialize;
}

if (NULL == utils_mutex_init(&pool->lock)) {
LOG_ERR("Failed to initialize mutex for pool");
ret = UMF_RESULT_ERROR_UNKNOWN;
goto err_lock_init;
}

ret = ops->initialize(pool->provider, params, &pool->pool_priv);
ret = pool->ops.initialize(pool->provider, params, &pool->pool_priv);
if (ret != UMF_RESULT_SUCCESS) {
goto err_pool_init;
}
Expand All @@ -579,6 +587,12 @@ static umf_result_t umfPoolCreateInternal(const umf_memory_pool_ops_t *ops,
}
}

ret = pool->ops.ext_post_initialize(pool->pool_priv);
if (ret != UMF_RESULT_SUCCESS && ret != UMF_RESULT_ERROR_NOT_SUPPORTED) {
LOG_ERR("Failed to post-initialize pool");
goto err_pool_init;
}

*hPool = pool;
pools_by_name_add(pool);

Expand Down
16 changes: 16 additions & 0 deletions src/memory_provider.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,11 @@ static umf_result_t umfDefaultCloseIPCHandle(void *provider, void *ptr,
return UMF_RESULT_ERROR_NOT_SUPPORTED;
}

static umf_result_t umfDefaultPostInitialize(void *provider) {
(void)provider;
return UMF_RESULT_ERROR_NOT_SUPPORTED;
}

static umf_result_t
umfDefaultCtlHandle(void *provider, umf_ctl_query_source_t operationType,
const char *name, void *arg, size_t size,
Expand Down Expand Up @@ -183,6 +188,10 @@ void assignOpsExtDefaults(umf_memory_provider_ops_t *ops) {
ops->ext_get_allocation_properties_size =
umfDefaultGetAllocationPropertiesSize;
}

if (!ops->ext_post_initialize) {
ops->ext_post_initialize = umfDefaultPostInitialize;
}
}

void assignOpsIpcDefaults(umf_memory_provider_ops_t *ops) {
Expand Down Expand Up @@ -310,6 +319,13 @@ umf_result_t umfMemoryProviderCreate(const umf_memory_provider_ops_t *ops,

provider->provider_priv = provider_priv;

ret = provider->ops.ext_post_initialize(provider_priv);
if (ret != UMF_RESULT_SUCCESS && ret != UMF_RESULT_ERROR_NOT_SUPPORTED) {
LOG_ERR("Failed to post-initialize provider");
umf_ba_global_free(provider);
return ret;
}

*hProvider = provider;

const char *provider_name = NULL;
Expand Down
15 changes: 10 additions & 5 deletions src/pool/pool_disjoint.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -758,6 +758,14 @@ umf_result_t disjoint_pool_initialize(umf_memory_provider_handle_t provider,
disjoint_pool->provider = provider;
disjoint_pool->params = *dp_params;

*ppPool = (void *)disjoint_pool;

return UMF_RESULT_SUCCESS;
}

umf_result_t disjoint_pool_post_initialize(void *ppPool) {
disjoint_pool_t *disjoint_pool = (disjoint_pool_t *)ppPool;

disjoint_pool->known_slabs = critnib_new(free_slab, NULL);
if (disjoint_pool->known_slabs == NULL) {
goto err_free_disjoint_pool;
Expand Down Expand Up @@ -816,13 +824,11 @@ umf_result_t disjoint_pool_initialize(umf_memory_provider_handle_t provider,
}

umf_result_t ret = umfMemoryProviderGetMinPageSize(
provider, NULL, &disjoint_pool->provider_min_page_size);
disjoint_pool->provider, NULL, &disjoint_pool->provider_min_page_size);
if (ret != UMF_RESULT_SUCCESS) {
disjoint_pool->provider_min_page_size = 0;
}

*ppPool = (void *)disjoint_pool;

return UMF_RESULT_SUCCESS;

err_free_buckets:
Expand All @@ -841,7 +847,6 @@ umf_result_t disjoint_pool_initialize(umf_memory_provider_handle_t provider,

err_free_disjoint_pool:
umf_ba_global_free(disjoint_pool);

return UMF_RESULT_ERROR_OUT_OF_HOST_MEMORY;
}

Expand Down Expand Up @@ -1199,7 +1204,7 @@ static umf_memory_pool_ops_t UMF_DISJOINT_POOL_OPS = {
.get_name = disjoint_pool_get_name,
.ext_ctl = disjoint_pool_ctl,
.ext_trim_memory = disjoint_pool_trim_memory,
};
.ext_post_initialize = disjoint_pool_post_initialize};

const umf_memory_pool_ops_t *umfDisjointPoolOps(void) {
return &UMF_DISJOINT_POOL_OPS;
Expand Down
Loading
Loading