Merge pull request #883 from ldorau/Add_threshold_to_proxy_lib_to_call_system_allocator

Add threshold to proxy lib to call system allocator

Author: lukaszstolarczuk

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

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=<value>` 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:

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

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

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;

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;

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