From 2e504544b7ac31b8a27445693be2349e8b70335b Mon Sep 17 00:00:00 2001 From: Lukasz Dorau Date: Tue, 12 Nov 2024 12:01:49 +0100 Subject: [PATCH 1/8] Fix format string Signed-off-by: Lukasz Dorau --- test/ipc_os_prov_proxy.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/ipc_os_prov_proxy.c b/test/ipc_os_prov_proxy.c index a17518658..0a4b64442 100644 --- a/test/ipc_os_prov_proxy.c +++ b/test/ipc_os_prov_proxy.c @@ -220,7 +220,7 @@ int main(int argc, char *argv[]) { fprintf( stderr, "[producer] ERROR: The consumer did NOT write the correct value " - "(the old one / 2 = %llu) to my shared memory: %llu\n", + "(the old one / 2 = %zu) to my shared memory: %llu\n", expected_val, new_val); } From ee1400d51e75d73b33aa699f7d4a4dc4c0f97fb6 Mon Sep 17 00:00:00 2001 From: Lukasz Dorau Date: Sun, 10 Nov 2024 18:26:07 +0100 Subject: [PATCH 2/8] Do not assert(ptr) in umfMemoryTrackerGetAllocInfo() Do not assert(ptr) in umfMemoryTrackerGetAllocInfo(), return UMF_RESULT_ERROR_INVALID_ARGUMENT instead. Replace LOG_WARN() with LOG_DEBUG(). --- src/provider/provider_tracking.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/provider/provider_tracking.c b/src/provider/provider_tracking.c index 769ed6a94..e726feefb 100644 --- a/src/provider/provider_tracking.c +++ b/src/provider/provider_tracking.c @@ -106,16 +106,19 @@ umf_memory_pool_handle_t umfMemoryTrackerGetPool(const void *ptr) { umf_result_t umfMemoryTrackerGetAllocInfo(const void *ptr, umf_alloc_info_t *pAllocInfo) { - assert(ptr); assert(pAllocInfo); + if (ptr == NULL) { + return UMF_RESULT_ERROR_INVALID_ARGUMENT; + } + if (TRACKER == NULL) { - LOG_ERR("tracker is not created"); + LOG_ERR("tracker does not exist"); return UMF_RESULT_ERROR_NOT_SUPPORTED; } if (TRACKER->map == NULL) { - LOG_ERR("tracker's map is not created"); + LOG_ERR("tracker's map does not exist"); return UMF_RESULT_ERROR_NOT_SUPPORTED; } @@ -124,9 +127,8 @@ umf_result_t umfMemoryTrackerGetAllocInfo(const void *ptr, int found = critnib_find(TRACKER->map, (uintptr_t)ptr, FIND_LE, (void *)&rkey, (void **)&rvalue); if (!found || (uintptr_t)ptr >= rkey + rvalue->size) { - LOG_WARN("pointer %p not found in the " - "tracker, TRACKER=%p", - ptr, (void *)TRACKER); + LOG_DEBUG("pointer %p not found in the tracker, TRACKER=%p", ptr, + (void *)TRACKER); return UMF_RESULT_ERROR_INVALID_ARGUMENT; } From 865ccbf28eeebdc99133a9a280b826435975c52c Mon Sep 17 00:00:00 2001 From: Lukasz Dorau Date: Mon, 11 Nov 2024 10:30:45 +0100 Subject: [PATCH 3/8] Add utils_env_var_get_str() to utils_common Add utils_env_var_get_str() to utils_common. Use utils_env_var_get_str() inside utils_env_var_has_str() and utils_is_running_in_proxy_lib(). Signed-off-by: Lukasz Dorau --- src/utils/utils_common.c | 13 ++----------- src/utils/utils_common.h | 15 +++++++++++---- 2 files changed, 13 insertions(+), 15 deletions(-) diff --git a/src/utils/utils_common.c b/src/utils/utils_common.c index 25169f6cf..bffc9f355 100644 --- a/src/utils/utils_common.c +++ b/src/utils/utils_common.c @@ -53,18 +53,9 @@ void utils_align_ptr_down_size_up(void **ptr, size_t *size, size_t alignment) { *size = s; } -int utils_env_var_has_str(const char *envvar, const char *str) { +char *utils_env_var_get_str(const char *envvar, const char *str) { char *value = getenv(envvar); - if (value && strstr(value, str)) { - return 1; - } - - return 0; -} - -// check if we are running in the proxy library -int utils_is_running_in_proxy_lib(void) { - return utils_env_var_has_str("LD_PRELOAD", "libumf_proxy.so"); + return value ? strstr(value, str) : NULL; } const char *utils_parse_var(const char *var, const char *option, diff --git a/src/utils/utils_common.h b/src/utils/utils_common.h index c25fda2ab..7751a2ac9 100644 --- a/src/utils/utils_common.h +++ b/src/utils/utils_common.h @@ -62,8 +62,18 @@ typedef enum umf_purge_advise_t { #endif /* _WIN32 */ +// get the address of the given string in the environment variable (or NULL) +char *utils_env_var_get_str(const char *envvar, const char *str); + // Check if the environment variable contains the given string. -int utils_env_var_has_str(const char *envvar, const char *str); +static inline int utils_env_var_has_str(const char *envvar, const char *str) { + return utils_env_var_get_str(envvar, str) ? 1 : 0; +} + +// check if we are running in the proxy library +static inline int utils_is_running_in_proxy_lib(void) { + return utils_env_var_get_str("LD_PRELOAD", "libumf_proxy.so") ? 1 : 0; +} // utils_parse_var - Parses var for a prefix, // optionally identifying a following argument. @@ -82,9 +92,6 @@ int utils_env_var_has_str(const char *envvar, const char *str); const char *utils_parse_var(const char *var, const char *option, const char **extraArg); -// check if we are running in the proxy library -int utils_is_running_in_proxy_lib(void); - size_t utils_get_page_size(void); // align a pointer up and a size down From bddbffd8aa12f5a2be131016c383776e94d2e9a3 Mon Sep 17 00:00:00 2001 From: Lukasz Dorau Date: Tue, 12 Nov 2024 09:22:10 +0100 Subject: [PATCH 4/8] Do not destroy tracker in umfTearDown() under the proxy library Signed-off-by: Lukasz Dorau --- src/base_alloc/base_alloc_global.c | 2 ++ src/libumf.c | 24 ++++++++++++++++++++++++ src/utils/utils_common.h | 8 ++++++++ 3 files changed, 34 insertions(+) diff --git a/src/base_alloc/base_alloc_global.c b/src/base_alloc/base_alloc_global.c index 11d88b731..2aca5d29c 100644 --- a/src/base_alloc/base_alloc_global.c +++ b/src/base_alloc/base_alloc_global.c @@ -67,6 +67,8 @@ static void umf_ba_create_global(void) { size_t smallestSize = BASE_ALLOC.ac_sizes[0]; BASE_ALLOC.smallest_ac_size_log2 = log2Utils(smallestSize); + + LOG_DEBUG("UMF base allocator created"); } // returns index of the allocation class for a given size diff --git a/src/libumf.c b/src/libumf.c index 3b69ce396..b89e5c844 100644 --- a/src/libumf.c +++ b/src/libumf.c @@ -13,6 +13,7 @@ #include "ipc_cache.h" #include "memspace_internal.h" #include "provider_tracking.h" +#include "utils_common.h" #include "utils_log.h" #if !defined(UMF_NO_HWLOC) #include "topology.h" @@ -30,11 +31,20 @@ int umfInit(void) { LOG_ERR("Failed to create memory tracker"); return -1; } + + LOG_DEBUG("UMF tracker created"); + umf_result_t umf_result = umfIpcCacheGlobalInit(); if (umf_result != UMF_RESULT_SUCCESS) { LOG_ERR("Failed to initialize IPC cache"); return -1; } + + LOG_DEBUG("UMF IPC cache initialized"); + } + + if (TRACKER) { + LOG_DEBUG("UMF library initialized"); } return 0; @@ -50,12 +60,26 @@ void umfTearDown(void) { umfDestroyTopology(); #endif umfIpcCacheGlobalTearDown(); + + if (utils_is_running_in_proxy_lib_with_size_threshold()) { + // We cannot destroy the TRACKER nor the base allocator + // when we are running in the proxy library with a size threshold, + // because it could result in calling the system free() + // with an invalid pointer and a segfault. + goto fini_umfTearDown; + } + // make sure TRACKER is not used after being destroyed umf_memory_tracker_handle_t t = TRACKER; TRACKER = NULL; umfMemoryTrackerDestroy(t); + LOG_DEBUG("UMF tracker destroyed"); umf_ba_destroy_global(); + LOG_DEBUG("UMF base allocator destroyed"); + + fini_umfTearDown: + LOG_DEBUG("UMF library finalized"); } } diff --git a/src/utils/utils_common.h b/src/utils/utils_common.h index 7751a2ac9..90b95e3ba 100644 --- a/src/utils/utils_common.h +++ b/src/utils/utils_common.h @@ -75,6 +75,14 @@ static inline int utils_is_running_in_proxy_lib(void) { return utils_env_var_get_str("LD_PRELOAD", "libumf_proxy.so") ? 1 : 0; } +// check if we are running in the proxy library with a size threshold +static inline int utils_is_running_in_proxy_lib_with_size_threshold(void) { + return (utils_env_var_get_str("LD_PRELOAD", "libumf_proxy.so") && + utils_env_var_get_str("UMF_PROXY", "size.threshold=")) + ? 1 + : 0; +} + // utils_parse_var - Parses var for a prefix, // optionally identifying a following argument. // Parameters: From 1dd43b2534350e793a0a0e5d7a86e1f6d7664726 Mon Sep 17 00:00:00 2001 From: Lukasz Dorau Date: Mon, 18 Nov 2024 14:23:50 +0100 Subject: [PATCH 5/8] Add size threshold to proxy lib to call system allocator (Linux only) Add a size threshold to proxy lib to call system allocator when the size is less than the given threshold (Linux only yet). Signed-off-by: Lukasz Dorau --- README.md | 7 ++ src/proxy_lib/proxy_lib.c | 159 +++++++++++++++++++++++++++---- src/utils/utils_common.h | 2 + src/utils/utils_posix_common.c | 25 +++++ src/utils/utils_windows_common.c | 7 ++ test/utils/utils_linux.cpp | 14 +++ 6 files changed, 198 insertions(+), 16 deletions(-) diff --git a/README.md b/README.md index f0c98c112..6f1233c63 100644 --- a/README.md +++ b/README.md @@ -317,6 +317,13 @@ The memory used by the proxy memory allocator is mmap'ed: - `page.disposition=shared-shm` - IPC uses the named shared memory. An SHM name is generated using the `umf_proxy_lib_shm_pid_$PID` pattern, where `$PID` is the PID of the process. It creates the `/dev/shm/umf_proxy_lib_shm_pid_$PID` file. - `page.disposition=shared-fd` - IPC uses the file descriptor duplication. It requires using `pidfd_getfd(2)` to obtain a duplicate of another process's file descriptor. Permission to duplicate another process's file descriptor is governed by a ptrace access mode `PTRACE_MODE_ATTACH_REALCREDS` check (see `ptrace(2)`) that can be changed using the `/proc/sys/kernel/yama/ptrace_scope` interface. `pidfd_getfd(2)` is supported since Linux 5.6. +**Size threshold** + +The **size threshold** feature (Linux only) causes that all allocations of size less than the given threshold value go to the default system allocator instead of the proxy library. +It can be enabled by adding the `size.threshold=` string to the `UMF_PROXY` environment variable (with `';'` as a separator), for example: `UMF_PROXY="page.disposition=shared-shm;size.threshold=64"`. + +**Remark:** changing a size of allocation (using `realloc()` ) does not change the allocator (`realloc(malloc(threshold - 1), threshold + 1)` still belongs to the default system allocator and `realloc(malloc(threshold + 1), threshold - 1)` still belongs to the proxy library pool allocator). + #### Windows In case of Windows it requires: diff --git a/src/proxy_lib/proxy_lib.c b/src/proxy_lib/proxy_lib.c index 5a5912b13..d687598bb 100644 --- a/src/proxy_lib/proxy_lib.c +++ b/src/proxy_lib/proxy_lib.c @@ -27,6 +27,12 @@ * - _aligned_offset_recalloc() */ +#ifndef _WIN32 +#define _GNU_SOURCE // for RTLD_NEXT +#include +#undef _GNU_SOURCE +#endif /* _WIN32 */ + #if (defined PROXY_LIB_USES_JEMALLOC_POOL) #include #define umfPoolManagerOps umfJemallocPoolOps @@ -48,6 +54,7 @@ #include "base_alloc_linear.h" #include "proxy_lib.h" #include "utils_common.h" +#include "utils_load_library.h" #include "utils_log.h" #ifdef _WIN32 /* Windows ***************************************/ @@ -94,6 +101,24 @@ void utils_init_once(UTIL_ONCE_FLAG *flag, void (*onceCb)(void)); * of a UMF pool to allocate memory needed by an application. It should be freed * by an application. */ +#ifndef _WIN32 +typedef void *(*system_aligned_alloc_t)(size_t alignment, size_t size); +typedef void *(*system_calloc_t)(size_t nmemb, size_t size); +typedef void (*system_free_t)(void *ptr); +typedef void *(*system_malloc_t)(size_t size); +typedef size_t (*system_malloc_usable_size_t)(void *ptr); +typedef void *(*system_realloc_t)(void *ptr, size_t size); + +// pointers to the default system allocator's API +static system_aligned_alloc_t System_aligned_alloc; +static system_calloc_t System_calloc; +static system_free_t System_free; +static system_malloc_t System_malloc; +static system_malloc_usable_size_t System_malloc_usable_size; +static system_realloc_t System_realloc; + +static size_t Size_threshold_value = 0; +#endif /* _WIN32 */ static UTIL_ONCE_FLAG Base_alloc_leak_initialized = UTIL_ONCE_FLAG_INIT; static umf_ba_linear_pool_t *Base_alloc_leak = NULL; @@ -107,6 +132,48 @@ static __TLS int was_called_from_umfPool = 0; /*** The constructor and destructor of the proxy library *********************/ /*****************************************************************************/ +#ifndef _WIN32 +static size_t get_size_threshold(void) { + char *str_threshold = utils_env_var_get_str("UMF_PROXY", "size.threshold="); + LOG_DEBUG("UMF_PROXY[size.threshold] = %s", str_threshold); + long threshold = utils_get_size_threshold(str_threshold); + if (threshold < 0) { + LOG_ERR("incorrect size threshold: %s", str_threshold); + exit(-1); + } + + return (size_t)threshold; +} + +static int get_system_allocator_symbols(void) { + *((void **)(&System_aligned_alloc)) = + utils_get_symbol_addr(RTLD_NEXT, "aligned_alloc", NULL); + *((void **)(&System_calloc)) = + utils_get_symbol_addr(RTLD_NEXT, "calloc", NULL); + *((void **)(&System_free)) = utils_get_symbol_addr(RTLD_NEXT, "free", NULL); + *((void **)(&System_malloc)) = + utils_get_symbol_addr(RTLD_NEXT, "malloc", NULL); + *((void **)(&System_malloc_usable_size)) = + utils_get_symbol_addr(RTLD_NEXT, "malloc_usable_size", NULL); + *((void **)(&System_realloc)) = + utils_get_symbol_addr(RTLD_NEXT, "realloc", NULL); + + if (System_aligned_alloc && System_calloc && System_free && System_malloc && + System_malloc_usable_size && System_realloc) { + return 0; + } + + *((void **)(&System_aligned_alloc)) = NULL; + *((void **)(&System_calloc)) = NULL; + *((void **)(&System_free)) = NULL; + *((void **)(&System_malloc)) = NULL; + *((void **)(&System_malloc_usable_size)) = NULL; + *((void **)(&System_realloc)) = NULL; + + return -1; +} +#endif /* _WIN32 */ + void proxy_lib_create_common(void) { utils_log_init(); umf_os_memory_provider_params_t os_params = @@ -114,11 +181,21 @@ void proxy_lib_create_common(void) { umf_result_t umf_result; #ifndef _WIN32 - char shm_name[NAME_MAX]; + size_t _threshold = get_size_threshold(); + if (_threshold > 0) { + if (get_system_allocator_symbols()) { + LOG_ERR("initialization of the system allocator failed!"); + exit(-1); + } + + Size_threshold_value = _threshold; + LOG_INFO("system allocator initialized, size threshold value = %zu", + Size_threshold_value); + } if (utils_env_var_has_str("UMF_PROXY", "page.disposition=shared-fd")) { - LOG_DEBUG("proxy_lib: using the MAP_SHARED visibility mode with the " - "file descriptor duplication"); + LOG_INFO("proxy_lib: using the MAP_SHARED visibility mode with the " + "file descriptor duplication"); os_params.visibility = UMF_MEM_MAP_SHARED; os_params.shm_name = NULL; @@ -126,15 +203,16 @@ void proxy_lib_create_common(void) { "page.disposition=shared-shm")) { os_params.visibility = UMF_MEM_MAP_SHARED; + char shm_name[NAME_MAX]; memset(shm_name, 0, NAME_MAX); sprintf(shm_name, "umf_proxy_lib_shm_pid_%i", utils_getpid()); os_params.shm_name = shm_name; - LOG_DEBUG("proxy_lib: using the MAP_SHARED visibility mode with the " - "named shared memory: %s", - os_params.shm_name); + LOG_INFO("proxy_lib: using the MAP_SHARED visibility mode with the " + "named shared memory: %s", + os_params.shm_name); } -#endif +#endif /* _WIN32 */ umf_result = umfMemoryProviderCreate(umfOsMemoryProviderOps(), &os_params, &OS_memory_provider); @@ -149,8 +227,10 @@ void proxy_lib_create_common(void) { LOG_ERR("creating UMF pool manager failed"); exit(-1); } + // The UMF pool has just been created (Proxy_pool != NULL). Stop using // the linear allocator and start using the UMF pool allocator from now on. + LOG_DEBUG("proxy library initialized"); } void proxy_lib_destroy_common(void) { @@ -158,7 +238,7 @@ void proxy_lib_destroy_common(void) { // We cannot destroy 'Base_alloc_leak' nor 'Proxy_pool' nor 'OS_memory_provider', // because it could lead to use-after-free in the program's unloader // (for example _dl_fini() on Linux). - return; + goto fini_proxy_lib_destroy_common; } umf_memory_pool_handle_t pool = Proxy_pool; @@ -168,6 +248,10 @@ void proxy_lib_destroy_common(void) { umf_memory_provider_handle_t provider = OS_memory_provider; OS_memory_provider = NULL; umfMemoryProviderDestroy(provider); + LOG_DEBUG("proxy library destroyed"); + +fini_proxy_lib_destroy_common: + LOG_DEBUG("proxy library finalized"); } /*****************************************************************************/ @@ -246,6 +330,12 @@ static inline size_t ba_leak_pool_contains_pointer(void *ptr) { /*****************************************************************************/ void *malloc(size_t size) { +#ifndef _WIN32 + if (size < Size_threshold_value) { + return System_malloc(size); + } +#endif /* _WIN32 */ + if (!was_called_from_umfPool && Proxy_pool) { was_called_from_umfPool = 1; void *ptr = umfPoolMalloc(Proxy_pool, size); @@ -257,6 +347,12 @@ void *malloc(size_t size) { } void *calloc(size_t nmemb, size_t size) { +#ifndef _WIN32 + if ((nmemb * size) < Size_threshold_value) { + return System_calloc(nmemb, size); + } +#endif /* _WIN32 */ + if (!was_called_from_umfPool && Proxy_pool) { was_called_from_umfPool = 1; void *ptr = umfPoolCalloc(Proxy_pool, nmemb, size); @@ -276,15 +372,22 @@ void free(void *ptr) { return; } - if (Proxy_pool) { + if (Proxy_pool && (umfPoolByPtr(ptr) == Proxy_pool)) { if (umfPoolFree(Proxy_pool, ptr) != UMF_RESULT_SUCCESS) { LOG_ERR("umfPoolFree() failed"); - assert(0); } return; } - assert(0); +#ifndef _WIN32 + if (Size_threshold_value) { + System_free(ptr); + return; + } +#endif /* _WIN32 */ + + LOG_ERR("free() failed: %p", ptr); + return; } @@ -303,18 +406,31 @@ void *realloc(void *ptr, size_t size) { return ba_leak_realloc(ptr, size, leak_pool_contains_pointer); } - if (Proxy_pool) { + if (Proxy_pool && (umfPoolByPtr(ptr) == Proxy_pool)) { was_called_from_umfPool = 1; void *new_ptr = umfPoolRealloc(Proxy_pool, ptr, size); was_called_from_umfPool = 0; return new_ptr; } - assert(0); +#ifndef _WIN32 + if (Size_threshold_value) { + return System_realloc(ptr, size); + } +#endif /* _WIN32 */ + + LOG_ERR("realloc() failed: %p", ptr); + return NULL; } void *aligned_alloc(size_t alignment, size_t size) { +#ifndef _WIN32 + if (size < Size_threshold_value) { + return System_aligned_alloc(alignment, size); + } +#endif /* _WIN32 */ + if (!was_called_from_umfPool && Proxy_pool) { was_called_from_umfPool = 1; void *ptr = umfPoolAlignedMalloc(Proxy_pool, size, alignment); @@ -330,19 +446,30 @@ size_t _msize(void *ptr) { #else size_t malloc_usable_size(void *ptr) { #endif - - // a check to verify we are running the proxy library + // a check to verify if we are running the proxy library if (ptr == (void *)0x01) { return 0xDEADBEEF; } - if (!was_called_from_umfPool && Proxy_pool) { + if (ba_leak_pool_contains_pointer(ptr)) { + return 0; // unsupported in case of the ba_leak allocator + } + + if (Proxy_pool && (umfPoolByPtr(ptr) == Proxy_pool)) { was_called_from_umfPool = 1; size_t size = umfPoolMallocUsableSize(Proxy_pool, ptr); was_called_from_umfPool = 0; return size; } +#ifndef _WIN32 + if (Size_threshold_value) { + return System_malloc_usable_size(ptr); + } +#endif /* _WIN32 */ + + LOG_ERR("malloc_usable_size() failed: %p", ptr); + return 0; // unsupported in this case } diff --git a/src/utils/utils_common.h b/src/utils/utils_common.h index 90b95e3ba..6920d97cf 100644 --- a/src/utils/utils_common.h +++ b/src/utils/utils_common.h @@ -168,6 +168,8 @@ int utils_file_open_or_create(const char *path); int utils_fallocate(int fd, long offset, long len); +long utils_get_size_threshold(char *str_threshold); + #ifdef __cplusplus } #endif diff --git a/src/utils/utils_posix_common.c b/src/utils/utils_posix_common.c index 10c2473c2..b0add36b9 100644 --- a/src/utils/utils_posix_common.c +++ b/src/utils/utils_posix_common.c @@ -7,6 +7,7 @@ * */ +#include #include #include #include @@ -266,3 +267,27 @@ int utils_file_open_or_create(const char *path) { return fd; } + +// Expected input: +// char *str_threshold = utils_env_var_get_str("UMF_PROXY", "size.threshold="); +long utils_get_size_threshold(char *str_threshold) { + if (!str_threshold) { + return 0; + } + + // move to the beginning of the number + str_threshold += strlen("size.threshold="); + + // check if the first character is a digit + if (!isdigit(str_threshold[0])) { + LOG_ERR("incorrect size threshold, expected numerical value >=0: %s", + str_threshold); + return -1; + } + + size_t threshold = atol(str_threshold); + LOG_DEBUG("Size_threshold_value = (char *) %s, (size_t) %zu", str_threshold, + threshold); + + return threshold; +} diff --git a/src/utils/utils_windows_common.c b/src/utils/utils_windows_common.c index 50a7b6ed5..e941b317d 100644 --- a/src/utils/utils_windows_common.c +++ b/src/utils/utils_windows_common.c @@ -215,3 +215,10 @@ int utils_fallocate(int fd, long offset, long len) { return -1; } + +// Expected input: +// char *str_threshold = utils_env_var_get_str("UMF_PROXY", "size.threshold="); +long utils_get_size_threshold(char *str_threshold) { + (void)str_threshold; // unused + return 0; +} diff --git a/test/utils/utils_linux.cpp b/test/utils/utils_linux.cpp index 82061409b..e99bb1161 100644 --- a/test/utils/utils_linux.cpp +++ b/test/utils/utils_linux.cpp @@ -56,3 +56,17 @@ TEST_F(test, utils_shm_create_invalid_args) { ret = utils_shm_create("/abc", -1); EXPECT_EQ(ret, -1); } + +TEST_F(test, utils_get_size_threshold) { + // Expected input to utils_get_size_threshold(): + // char *str_threshold = utils_env_var_get_str("UMF_PROXY", "size.threshold="); + // positive tests + EXPECT_EQ(utils_get_size_threshold((char *)"size.threshold=111"), 111); + EXPECT_EQ(utils_get_size_threshold((char *)"size.threshold=222;abcd"), 222); + EXPECT_EQ(utils_get_size_threshold((char *)"size.threshold=333;var=value"), + 333); + // negative tests + EXPECT_EQ(utils_get_size_threshold(NULL), 0); + EXPECT_EQ(utils_get_size_threshold((char *)"size.threshold=abc"), -1); + EXPECT_EQ(utils_get_size_threshold((char *)"size.threshold=-111"), -1); +} From a4fced635067affb6787da41116ae3ffadfc5597 Mon Sep 17 00:00:00 2001 From: Lukasz Dorau Date: Thu, 14 Nov 2024 22:18:49 +0100 Subject: [PATCH 6/8] Add WA for the issue This WA for the issue: https://github.com/oneapi-src/unified-memory-framework/issues/894 It protects us from a recursion in malloc_usable_size() when the JEMALLOC proxy_lib_pool is used. TODO: remove this WA when the issue is fixed. Signed-off-by: Lukasz Dorau --- src/proxy_lib/proxy_lib.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/proxy_lib/proxy_lib.c b/src/proxy_lib/proxy_lib.c index d687598bb..3320f2898 100644 --- a/src/proxy_lib/proxy_lib.c +++ b/src/proxy_lib/proxy_lib.c @@ -128,6 +128,13 @@ static umf_memory_pool_handle_t Proxy_pool = NULL; // it protects us from recursion in umfPool*() static __TLS int was_called_from_umfPool = 0; +// This WA for the issue: +// https://github.com/oneapi-src/unified-memory-framework/issues/894 +// It protects us from a recursion in malloc_usable_size() +// when the JEMALLOC proxy_lib_pool is used. +// TODO remove this WA when the issue is fixed. +static __TLS int was_called_from_malloc_usable_size = 0; + /*****************************************************************************/ /*** The constructor and destructor of the proxy library *********************/ /*****************************************************************************/ @@ -455,15 +462,18 @@ size_t malloc_usable_size(void *ptr) { return 0; // unsupported in case of the ba_leak allocator } - if (Proxy_pool && (umfPoolByPtr(ptr) == Proxy_pool)) { + if (!was_called_from_malloc_usable_size && Proxy_pool && + (umfPoolByPtr(ptr) == Proxy_pool)) { + was_called_from_malloc_usable_size = 1; was_called_from_umfPool = 1; size_t size = umfPoolMallocUsableSize(Proxy_pool, ptr); was_called_from_umfPool = 0; + was_called_from_malloc_usable_size = 0; return size; } #ifndef _WIN32 - if (Size_threshold_value) { + if (!was_called_from_malloc_usable_size && Size_threshold_value) { return System_malloc_usable_size(ptr); } #endif /* _WIN32 */ From 3966c3a0d5debdfaa824ab3576b1bc54fa8c2d79 Mon Sep 17 00:00:00 2001 From: Lukasz Dorau Date: Mon, 18 Nov 2024 18:24:57 +0100 Subject: [PATCH 7/8] Add tests for the size threshold of the proxy library (Linux only) The proxyLib_size_threshold_* tests test the size threshold of the proxy library (Linux only yet). The size threshold is set to 64 bytes in this test, so all allocations of: 1) size < 64 go through the default system allocator and (umfPoolByPtr(ptr_size < 64) == nullptr) 2) size >= 64 go through the proxy lib allocator and (umfPoolByPtr(ptr_size >= 64) != nullptr). Ref: #894 Signed-off-by: Lukasz Dorau --- .github/workflows/reusable_proxy_lib.yml | 9 ++ test/CMakeLists.txt | 10 ++ test/test_proxy_lib_size_threshold.cpp | 183 +++++++++++++++++++++++ test/utils/utils_linux.cpp | 7 + 4 files changed, 209 insertions(+) create mode 100644 test/test_proxy_lib_size_threshold.cpp diff --git a/.github/workflows/reusable_proxy_lib.yml b/.github/workflows/reusable_proxy_lib.yml index 56211b97d..2a27161b3 100644 --- a/.github/workflows/reusable_proxy_lib.yml +++ b/.github/workflows/reusable_proxy_lib.yml @@ -77,6 +77,15 @@ jobs: working-directory: ${{env.BUILD_DIR}} run: UMF_PROXY="page.disposition=shared-shm" LD_PRELOAD=./lib/libumf_proxy.so /usr/bin/date + # TODO enable the provider_file_memory_ipc test when the IPC tests with the proxy library are fixed + # see the issue: https://github.com/oneapi-src/unified-memory-framework/issues/864 + - name: Run "ctest --output-on-failure" with proxy library and size.threshold=128 + working-directory: ${{env.BUILD_DIR}} + run: > + UMF_PROXY="page.disposition=shared-shm;size.threshold=128" + LD_PRELOAD=./lib/libumf_proxy.so + ctest --output-on-failure -E provider_file_memory_ipc + - name: Check coverage if: ${{ matrix.build_type == 'Debug' }} working-directory: ${{env.BUILD_DIR}} diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index d24244ab0..9899991ce 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -414,6 +414,16 @@ if(UMF_PROXY_LIB_ENABLED AND UMF_BUILD_SHARED_LIBRARY) SRCS ${BA_SOURCES_FOR_TEST} test_proxy_lib.cpp LIBS ${UMF_UTILS_FOR_TEST} umf_proxy) + # TODO enable this test on Windows + if(LINUX) + add_umf_test( + NAME test_proxy_lib_size_threshold + SRCS ${BA_SOURCES_FOR_TEST} test_proxy_lib_size_threshold.cpp + LIBS ${UMF_UTILS_FOR_TEST} umf_proxy) + set_property(TEST umf-test_proxy_lib_size_threshold + PROPERTY ENVIRONMENT UMF_PROXY="size.threshold=64") + endif() + # the memoryPool test run with the proxy library add_umf_test( NAME proxy_lib_memoryPool diff --git a/test/test_proxy_lib_size_threshold.cpp b/test/test_proxy_lib_size_threshold.cpp new file mode 100644 index 000000000..fac1c516b --- /dev/null +++ b/test/test_proxy_lib_size_threshold.cpp @@ -0,0 +1,183 @@ +/* + * Copyright (C) 2024 Intel Corporation + * + * Under the Apache License v2.0 with LLVM Exceptions. See LICENSE.TXT. + * SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +*/ + +#if defined(__APPLE__) +#include +#else +#include +#endif + +#include + +#include "base.hpp" +#include "test_helpers.h" +#include "utils_common.h" + +using umf_test::test; + +// size threshold defined by the env variable UMF_PROXY="size.threshold=64" +#define SIZE_THRESHOLD 64 +#define SIZE_EQ (SIZE_THRESHOLD) +#define SIZE_LT (SIZE_THRESHOLD - 1) + +#define ALIGN_1024 1024 + +TEST_F(test, proxyLib_basic) { + // a check to verify we are running the proxy library + void *ptr = (void *)0x01; + +#ifdef _WIN32 + size_t size = _msize(ptr); +#elif __APPLE__ + size_t size = ::malloc_size(ptr); +#else + size_t size = ::malloc_usable_size(ptr); +#endif + + ASSERT_EQ(size, 0xDEADBEEF); +} + +TEST_F(test, proxyLib_realloc_size0) { + // realloc(ptr, 0) == free(ptr) + // realloc(ptr, 0) returns NULL + ASSERT_EQ(::realloc(::malloc(SIZE_EQ), 0), nullptr); +} + +// The proxyLib_size_threshold_* tests test the size threshold of the proxy library. +// The size threshold is set to SIZE_THRESHOLD bytes in this test, so all allocations of: +// 1) size < SIZE_THRESHOLD go through the default system allocator +// (umfPoolByPtr(ptr_size < SIZE_THRESHOLD) == nullptr) +// 2) size >= SIZE_THRESHOLD go through the proxy library allocator +// (umfPoolByPtr(ptr_size >= SIZE_THRESHOLD) != nullptr) + +TEST_F(test, proxyLib_size_threshold_aligned_alloc) { +#ifdef _WIN32 + void *ptr_LT = _aligned_malloc(SIZE_LT, ALIGN_1024); + void *ptr_EQ = _aligned_malloc(SIZE_EQ, ALIGN_1024); +#else + void *ptr_LT = ::aligned_alloc(ALIGN_1024, SIZE_LT); + void *ptr_EQ = ::aligned_alloc(ALIGN_1024, SIZE_EQ); +#endif + + ASSERT_NE(ptr_LT, nullptr); + ASSERT_NE(ptr_EQ, nullptr); + + // verify alignment + ASSERT_EQ((int)(IS_ALIGNED((uintptr_t)ptr_LT, ALIGN_1024)), 1); + ASSERT_EQ((int)(IS_ALIGNED((uintptr_t)ptr_EQ, ALIGN_1024)), 1); + + ASSERT_EQ(umfPoolByPtr(ptr_LT), nullptr); + ASSERT_NE(umfPoolByPtr(ptr_EQ), nullptr); + +#ifdef _WIN32 + _aligned_free(ptr_LT); + _aligned_free(ptr_EQ); +#else + ::free(ptr_LT); + ::free(ptr_EQ); +#endif +} + +TEST_F(test, proxyLib_size_threshold_malloc) { + void *ptr_LT = malloc(SIZE_LT); + void *ptr_EQ = malloc(SIZE_EQ); + + ASSERT_NE(ptr_LT, nullptr); + ASSERT_NE(ptr_EQ, nullptr); + + ASSERT_EQ(umfPoolByPtr(ptr_LT), nullptr); + ASSERT_NE(umfPoolByPtr(ptr_EQ), nullptr); + + ::free(ptr_LT); + ::free(ptr_EQ); +} + +TEST_F(test, proxyLib_size_threshold_calloc) { + void *ptr_LT = calloc(SIZE_LT, 1); + void *ptr_EQ = calloc(SIZE_EQ, 1); + + ASSERT_NE(ptr_LT, nullptr); + ASSERT_NE(ptr_EQ, nullptr); + + ASSERT_EQ(umfPoolByPtr(ptr_LT), nullptr); + ASSERT_NE(umfPoolByPtr(ptr_EQ), nullptr); + + ::free(ptr_LT); + ::free(ptr_EQ); +} + +TEST_F(test, proxyLib_size_threshold_realloc_up) { + void *ptr_LT = malloc(SIZE_LT); + void *ptr_EQ = malloc(SIZE_EQ); + + ASSERT_NE(ptr_LT, nullptr); + ASSERT_NE(ptr_EQ, nullptr); + + void *ptr_LT_r = realloc(ptr_LT, 2 * SIZE_LT); + void *ptr_EQ_r = realloc(ptr_EQ, 2 * SIZE_EQ); + + ASSERT_NE(ptr_LT_r, nullptr); + ASSERT_NE(ptr_EQ_r, nullptr); + + ASSERT_EQ(umfPoolByPtr(ptr_LT_r), nullptr); + ASSERT_NE(umfPoolByPtr(ptr_EQ_r), nullptr); + + ::free(ptr_LT_r); + ::free(ptr_EQ_r); +} + +TEST_F(test, proxyLib_size_threshold_realloc_down) { + void *ptr_LT = malloc(SIZE_LT); + void *ptr_EQ = malloc(SIZE_EQ); + + ASSERT_NE(ptr_LT, nullptr); + ASSERT_NE(ptr_EQ, nullptr); + + void *ptr_LT_r = realloc(ptr_LT, SIZE_LT / 2); + void *ptr_EQ_r = realloc(ptr_EQ, SIZE_EQ / 2); + + ASSERT_NE(ptr_LT_r, nullptr); + ASSERT_NE(ptr_EQ_r, nullptr); + + ASSERT_EQ(umfPoolByPtr(ptr_LT_r), nullptr); + ASSERT_NE(umfPoolByPtr(ptr_EQ_r), nullptr); + + ::free(ptr_LT_r); + ::free(ptr_EQ_r); +} + +TEST_F(test, proxyLib_size_threshold_malloc_usable_size) { + + void *ptr_LT = ::malloc(SIZE_LT); + void *ptr_EQ = ::malloc(SIZE_EQ); + + ASSERT_NE(ptr_LT, nullptr); + ASSERT_NE(ptr_EQ, nullptr); + + if (ptr_LT == nullptr || ptr_EQ == nullptr) { + // Fix for the following CodeQL's warning on Windows: + // 'ptr' could be '0': this does not adhere to the specification for the function '_msize'. + return; + } + +#ifdef _WIN32 + size_t size_LT = _msize(ptr_LT); + size_t size_EQ = _msize(ptr_EQ); +#elif __APPLE__ + size_t size_LT = ::malloc_size(ptr_LT); + size_t size_EQ = ::malloc_size(ptr_EQ); +#else + size_t size_LT = ::malloc_usable_size(ptr_LT); + size_t size_EQ = ::malloc_usable_size(ptr_EQ); +#endif + + ASSERT_EQ((int)(size_LT == 0 || size_LT >= SIZE_LT), 1); + ASSERT_EQ((int)(size_EQ == 0 || size_EQ >= SIZE_EQ), 1); + + ::free(ptr_LT); + ::free(ptr_EQ); +} diff --git a/test/utils/utils_linux.cpp b/test/utils/utils_linux.cpp index e99bb1161..cbe99ae74 100644 --- a/test/utils/utils_linux.cpp +++ b/test/utils/utils_linux.cpp @@ -60,13 +60,20 @@ TEST_F(test, utils_shm_create_invalid_args) { TEST_F(test, utils_get_size_threshold) { // Expected input to utils_get_size_threshold(): // char *str_threshold = utils_env_var_get_str("UMF_PROXY", "size.threshold="); + // positive tests EXPECT_EQ(utils_get_size_threshold((char *)"size.threshold=111"), 111); EXPECT_EQ(utils_get_size_threshold((char *)"size.threshold=222;abcd"), 222); EXPECT_EQ(utils_get_size_threshold((char *)"size.threshold=333;var=value"), 333); + // LONG_MAX = 9223372036854775807 + EXPECT_EQ(utils_get_size_threshold( + (char *)"size.threshold=9223372036854775807;var=value"), + 9223372036854775807); + // negative tests EXPECT_EQ(utils_get_size_threshold(NULL), 0); + EXPECT_EQ(utils_get_size_threshold((char *)"size.threshold="), -1); EXPECT_EQ(utils_get_size_threshold((char *)"size.threshold=abc"), -1); EXPECT_EQ(utils_get_size_threshold((char *)"size.threshold=-111"), -1); } From 16ca017429b0ca85cb6a7ade574f22869b351135 Mon Sep 17 00:00:00 2001 From: Lukasz Dorau Date: Tue, 19 Nov 2024 11:34:41 +0100 Subject: [PATCH 8/8] Fix warnings about unused parameter Signed-off-by: Lukasz Dorau --- src/provider/provider_cuda.c | 1 + src/provider/provider_devdax_memory.c | 3 +++ 2 files changed, 4 insertions(+) diff --git a/src/provider/provider_cuda.c b/src/provider/provider_cuda.c index db0016c44..68fe0da23 100644 --- a/src/provider/provider_cuda.c +++ b/src/provider/provider_cuda.c @@ -470,6 +470,7 @@ static umf_result_t cu_memory_provider_open_ipc_handle(void *provider, static umf_result_t cu_memory_provider_close_ipc_handle(void *provider, void *ptr, size_t size) { + (void)provider; (void)size; CUresult cu_result; diff --git a/src/provider/provider_devdax_memory.c b/src/provider/provider_devdax_memory.c index d11cfa809..1179ed115 100644 --- a/src/provider/provider_devdax_memory.c +++ b/src/provider/provider_devdax_memory.c @@ -326,6 +326,7 @@ static umf_result_t devdax_purge_lazy(void *provider, void *ptr, size_t size) { } static umf_result_t devdax_purge_force(void *provider, void *ptr, size_t size) { + (void)provider; // unused errno = 0; if (utils_purge(ptr, size, UMF_PURGE_FORCE)) { devdax_store_last_native_error( @@ -410,6 +411,7 @@ static umf_result_t devdax_put_ipc_handle(void *provider, static umf_result_t devdax_open_ipc_handle(void *provider, void *providerIpcData, void **ptr) { + (void)provider; // unused *ptr = NULL; devdax_ipc_data_t *devdax_ipc_data = (devdax_ipc_data_t *)providerIpcData; @@ -469,6 +471,7 @@ static umf_result_t devdax_open_ipc_handle(void *provider, static umf_result_t devdax_close_ipc_handle(void *provider, void *ptr, size_t size) { + (void)provider; // unused size = ALIGN_UP(size, DEVDAX_PAGE_SIZE_2MB); errno = 0;