Merge pull request #761 from bratpiorka/rrudnick_proxy_tracking_flag

enable tracker in ProxyLib by default

Author: ldorau

.github/workflows/reusable_basic.yml

@@ -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' || '' }}
 
@@ -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' || ''}}
         
@@ -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
 

CMakeLists.txt

@@ -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)

src/CMakeLists.txt

@@ -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)
     add_subdirectory(proxy_lib)
 endif()

src/provider/provider_devdax_memory.c

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

src/provider/provider_tracking.c

@@ -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);
 
@@ -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,
@@ -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;

src/proxy_lib/proxy_lib.c

@@ -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);
@@ -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);

test/CMakeLists.txt

@@ -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
@@ -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