Skip to content

enable tracker in ProxyLib by default #761

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

Merged
merged 5 commits into from
Nov 12, 2024
Merged

enable tracker in ProxyLib by default #761

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0d64d11
enable tracker in ProxyLib by default
bratpiorka Oct 2, 2024
57b96f9
add IPC test with proxy lib
bratpiorka Oct 2, 2024
62d7b06
enable Proxy with static HWLOC
bratpiorka Oct 19, 2024
05ddc12
disable Proxy Lib when UMF is build as static lib
bratpiorka Oct 28, 2024
09ee70e
fix devdax_allocation_merge arg checks
bratpiorka Nov 8, 2024
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
6 changes: 3 additions & 3 deletions .github/workflows/reusable_basic.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ jobs:
--build-type ${{matrix.build_type}}
--disjoint-pool
--jemalloc-pool
${{ matrix.install_tbb == 'ON' && matrix.disable_hwloc != 'ON' && matrix.link_hwloc_statically != 'ON' && '--proxy' || '' }}
${{ matrix.install_tbb == 'ON' && matrix.disable_hwloc != 'ON' && matrix.shared_library == 'ON' && '--proxy' || '' }}
--umf-version ${{env.UMF_VERSION}}
${{ matrix.shared_library == 'ON' && '--shared-library' || '' }}

Expand Down Expand Up @@ -300,7 +300,7 @@ jobs:
--build-type ${{matrix.build_type}}
--disjoint-pool
--jemalloc-pool
--proxy
${{matrix.shared_library == 'ON' && '--proxy' || '' }}
--umf-version ${{env.UMF_VERSION}}
${{ matrix.shared_library == 'ON' && '--shared-library' || ''}}

Expand All @@ -310,7 +310,7 @@ jobs:
shell: pwsh

- name: check /DEPENDENTLOADFLAG in umf_proxy.dll
if: ${{matrix.compiler.cxx == 'cl'}}
if: ${{matrix.shared_library == 'ON' && matrix.compiler.cxx == 'cl'}}
run: ${{github.workspace}}/.github/scripts/check_dll_flags.ps1 ${{env.BUILD_DIR}}/src/proxy_lib/${{matrix.build_type}}/umf_proxy.dll
shell: pwsh

Expand Down
11 changes: 10 additions & 1 deletion CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -432,8 +432,17 @@ if(WINDOWS)
)
endif()
endif()

# set UMF_PROXY_LIB_ENABLED
if(UMF_PROXY_LIB_BASED_ON_POOL STREQUAL SCALABLE)
if(UMF_DISABLE_HWLOC)
message(STATUS "Disabling the proxy library, because HWLOC is disabled")
elseif(NOT UMF_BUILD_SHARED_LIBRARY)
# TODO enable this scenario
message(
STATUS
"Disabling the proxy library, because UMF is built as static library"
)
elseif(UMF_PROXY_LIB_BASED_ON_POOL STREQUAL SCALABLE)
if(UMF_POOL_SCALABLE_ENABLED)
set(UMF_PROXY_LIB_ENABLED ON)
set(PROXY_LIB_USES_SCALABLE_POOL ON)
Expand Down
4 changes: 1 addition & 3 deletions src/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,8 +195,6 @@ install(TARGETS umf EXPORT ${PROJECT_NAME}-targets)

add_subdirectory(pool)

if(UMF_PROXY_LIB_ENABLED
AND NOT UMF_LINK_HWLOC_STATICALLY
AND NOT UMF_DISABLE_HWLOC)
if(UMF_PROXY_LIB_ENABLED)
Copy link
Contributor

Choose a reason for hiding this comment

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

AND UMF_BUILD_SHARED_LIBRARY?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

UMF_PROXY_LIB_ENABLED is set only in shared library mode, see https://github.com/oneapi-src/unified-memory-framework/pull/761/files#diff-1e7de1ae2d059d21e1dd75d5812d5a34b0222cef273b7c3a2af62eb747f9d20aR439

Copy link
Contributor

Choose a reason for hiding this comment

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

Verified.

add_subdirectory(proxy_lib)
endif()
10 changes: 3 additions & 7 deletions src/provider/provider_devdax_memory.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -360,13 +360,9 @@ static umf_result_t devdax_allocation_split(void *provider, void *ptr,

static umf_result_t devdax_allocation_merge(void *provider, void *lowPtr,
void *highPtr, size_t totalSize) {
(void)provider;

if ((uintptr_t)highPtr <= (uintptr_t)lowPtr) {
return UMF_RESULT_ERROR_INVALID_ARGUMENT;
}

if ((uintptr_t)highPtr - (uintptr_t)lowPtr <= totalSize) {
if (provider == NULL || lowPtr == NULL || highPtr == NULL ||
((uintptr_t)highPtr <= (uintptr_t)lowPtr) ||
((uintptr_t)highPtr - (uintptr_t)lowPtr >= totalSize)) {
return UMF_RESULT_ERROR_INVALID_ARGUMENT;
}

Expand Down
17 changes: 11 additions & 6 deletions src/provider/provider_tracking.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,13 +48,14 @@ static umf_result_t umfMemoryTrackerAdd(umf_memory_tracker_handle_t hTracker,
int ret = critnib_insert(hTracker->map, (uintptr_t)ptr, value, 0);

if (ret == 0) {
LOG_DEBUG("memory region is added, tracker=%p, ptr=%p, size=%zu",
(void *)hTracker, ptr, size);
LOG_DEBUG(
"memory region is added, tracker=%p, ptr=%p, pool=%p, size=%zu",
(void *)hTracker, ptr, (void *)pool, size);
return UMF_RESULT_SUCCESS;
}

LOG_ERR("failed to insert tracker value, ret=%d, ptr=%p, size=%zu", ret,
ptr, size);
LOG_ERR("failed to insert tracker value, ret=%d, ptr=%p, pool=%p, size=%zu",
ret, ptr, (void *)pool, size);

umf_ba_free(hTracker->tracker_allocator, value);

Expand Down Expand Up @@ -303,8 +304,8 @@ static umf_result_t trackingAllocationMerge(void *hProvider, void *lowPtr,
ret = umfMemoryProviderAllocationMerge(provider->hUpstream, lowPtr, highPtr,
totalSize);
if (ret != UMF_RESULT_SUCCESS) {
LOG_ERR("upstream provider failed to merge regions");
goto err;
LOG_WARN("upstream provider failed to merge regions");
goto not_merged;
}

// We'll have a duplicate entry for the range [highPtr, highValue->size] but this is fine,
Expand All @@ -329,7 +330,11 @@ static umf_result_t trackingAllocationMerge(void *hProvider, void *lowPtr,
return UMF_RESULT_SUCCESS;

err:
assert(0);

not_merged:
utils_mutex_unlock(&provider->hTracker->splitMergeMutex);

err_lock:
umf_ba_free(provider->hTracker->tracker_allocator, mergedValue);
return ret;
Expand Down
7 changes: 2 additions & 5 deletions src/proxy_lib/proxy_lib.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,8 +124,6 @@ void proxy_lib_create_common(void) {

} else if (utils_env_var_has_str("UMF_PROXY",
"page.disposition=shared-shm")) {
LOG_DEBUG("proxy_lib: using the MAP_SHARED visibility mode with the "
"named shared memory");
os_params.visibility = UMF_MEM_MAP_SHARED;

memset(shm_name, 0, NAME_MAX);
Expand All @@ -145,9 +143,8 @@ void proxy_lib_create_common(void) {
exit(-1);
}

umf_result =
umfPoolCreate(umfPoolManagerOps(), OS_memory_provider, NULL,
UMF_POOL_CREATE_FLAG_DISABLE_TRACKING, &Proxy_pool);
umf_result = umfPoolCreate(umfPoolManagerOps(), OS_memory_provider, NULL, 0,
&Proxy_pool);
if (umf_result != UMF_RESULT_SUCCESS) {
LOG_ERR("creating UMF pool manager failed");
exit(-1);
Expand Down
17 changes: 13 additions & 4 deletions test/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -408,10 +408,7 @@ add_umf_test(
LIBS ${UMF_UTILS_FOR_TEST})

# tests for the proxy library
if(UMF_PROXY_LIB_ENABLED
AND UMF_BUILD_SHARED_LIBRARY
AND NOT UMF_DISABLE_HWLOC
AND NOT UMF_LINK_HWLOC_STATICALLY)
if(UMF_PROXY_LIB_ENABLED AND UMF_BUILD_SHARED_LIBRARY)
add_umf_test(
NAME proxy_lib_basic
SRCS ${BA_SOURCES_FOR_TEST} test_proxy_lib.cpp
Expand Down Expand Up @@ -486,6 +483,18 @@ if(LINUX)
add_umf_ipc_test(TEST ipc_os_prov_anon_fd)
add_umf_ipc_test(TEST ipc_os_prov_shm)

if(UMF_PROXY_LIB_ENABLED AND UMF_BUILD_SHARED_LIBRARY)
build_umf_test(
NAME
ipc_os_prov_proxy
SRCS
ipc_os_prov_proxy.c
common/ipc_common.c
LIBS
${UMF_UTILS_FOR_TEST})
add_umf_ipc_test(TEST ipc_os_prov_proxy)
endif()

build_umf_test(
NAME
ipc_devdax_prov_consumer
Expand Down
Loading
Loading