Merge pull request #294 from kswiecicki/mem-policy-fail

Fix OOM test for HOST ALL memspace

Author: bratpiorka

test/CMakeLists.txt

@@ -135,22 +135,22 @@ if(UMF_BUILD_OS_MEMORY_PROVIDER AND LINUX) # OS-specific functions are implement
                  LIBS umf_utils ${LIBNUMA_LIBRARIES})
     add_umf_test(NAME memspace_host_all
                  SRCS memspaces/memspace_host_all.cpp
-                 LIBS ${LIBNUMA_LIBRARIES})
+                 LIBS umf_utils ${LIBNUMA_LIBRARIES})
 endif()
 
 # TODO add support for Windows
 if(UMF_BUILD_LEVEL_ZERO_PROVIDER AND LINUX)
-    # we have two test binaries here that use the same sources, but differ in 
-    # the way they are linked to the Level Zero (statically or at runtime using 
+    # we have two test binaries here that use the same sources, but differ in
+    # the way they are linked to the Level Zero (statically or at runtime using
     # dlopen)
     add_umf_test(NAME provider_level_zero
                  SRCS providers/provider_level_zero.cpp
                  LIBS umf_utils ze_loader)
-                
+
     add_umf_test(NAME provider_level_zero_dlopen
                  SRCS providers/provider_level_zero.cpp
                  LIBS umf_utils)
-    target_compile_definitions(umf_test-provider_level_zero_dlopen 
+    target_compile_definitions(umf_test-provider_level_zero_dlopen
                  PUBLIC USE_DLOPEN=1)
 endif()
 

test/memspaces/memspace_host_all.cpp

@@ -6,6 +6,7 @@
 #include "memspace_helpers.hpp"
 #include "memspace_internal.h"
 #include "test_helpers.h"
+#include "utils_sanitizers.h"
 
 #include <numa.h>
 #include <numaif.h>
@@ -96,39 +97,53 @@ static void getAllocationPolicy(void *ptr, unsigned long maxNodeId, int &mode,
     allocNodeId = static_cast<size_t>(nodeId);
 }
 
-TEST_F(memspaceHostAllProviderTest, memoryPolicyOOM) {
+TEST_F(memspaceHostAllProviderTest, allocsSpreadAcrossAllNumaNodes) {
+    // This testcase is unsuitable for TSan.
+#ifdef __SANITIZE_THREAD__
+    GTEST_SKIP();
+#endif
+
     // Arbitrary allocation size, should be big enough to avoid unnecessarily
     // prolonging the test execution.
-    size_t size = SIZE_4M * 128;
+    size_t size = SIZE_4M;
     size_t alignment = 0;
-    std::vector<void *> allocs;
 
-    enum umf_result_t umf_ret = UMF_RESULT_SUCCESS;
-    // Create allocations until OOM.
-    while (true) {
+    long long numaCombinedFreeSize = 0;
+    // Gather free size of all numa nodes.
+    for (auto &id : nodeIds) {
+        long long numaFreeSize = 0;
+        long long numaSize = numa_node_size64(id, &numaFreeSize);
+        UT_ASSERTne(numaSize, -1);
+        // We need the space for at least two allocations, so that we can
+        // have some space left to avoid OOM killer.
+        UT_ASSERT(numaFreeSize >= (long long)(2 * size));
+
+        numaCombinedFreeSize += numaFreeSize;
+    }
+
+    umf_result_t umf_ret = UMF_RESULT_SUCCESS;
+    // Create allocations until all the NUMA nodes until there's space only for
+    // one allocation.
+    std::vector<void *> allocs;
+    std::unordered_set<size_t> allocNodeIds;
+    while (numaCombinedFreeSize >= (long long)(2 * size)) {
         void *ptr = nullptr;
         umf_ret = umfMemoryProviderAlloc(hProvider, size, alignment, &ptr);
         if (umf_ret != UMF_RESULT_SUCCESS) {
+            UT_ASSERTeq(umf_ret, UMF_RESULT_ERROR_MEMORY_PROVIDER_SPECIFIC);
+            const char *msg = nullptr;
+            int32_t err = 0;
+            umfMemoryProviderGetLastNativeError(hProvider, &msg, &err);
+            // In this scenario, 'UMF_OS_RESULT_ERROR_ALLOC_FAILED' indicates OOM.
+            UT_ASSERTeq(err, UMF_OS_RESULT_ERROR_ALLOC_FAILED);
             break;
         }
 
         UT_ASSERTne(ptr, nullptr);
-        allocs.push_back(ptr);
-    }
+        // Access the allocation, so that all the pages associated with it are
+        // allocated on available NUMA nodes.
+        memset(ptr, 0xFF, size);
 
-    UT_ASSERTeq(umf_ret, UMF_RESULT_ERROR_MEMORY_PROVIDER_SPECIFIC);
-    const char *msg = nullptr;
-    int32_t err = 0;
-    umfMemoryProviderGetLastNativeError(hProvider, &msg, &err);
-    // In this scenario, 'UMF_OS_RESULT_ERROR_ALLOC_FAILED' indicates OOM.
-    UT_ASSERTeq(err, UMF_OS_RESULT_ERROR_ALLOC_FAILED);
-
-    // When allocating until OOM, the allocations should be distributed across
-    // all the NUMA nodes bound to 'HOST ALL' memspace, until each node runs
-    // out of memory.
-    UT_ASSERT(allocs.size() >= nodeIds.size());
-    std::unordered_set<size_t> allocNodeIds;
-    for (auto &ptr : allocs) {
         int mode = -1;
         std::vector<size_t> boundNodeIds;
         size_t allocNodeId = SIZE_MAX;
@@ -151,8 +166,14 @@ TEST_F(memspaceHostAllProviderTest, memoryPolicyOOM) {
         auto it = std::find(nodeIds.begin(), nodeIds.end(), allocNodeId);
         UT_ASSERT(it != nodeIds.end());
 
+        allocs.push_back(ptr);
         allocNodeIds.insert(allocNodeId);
 
+        numaCombinedFreeSize -= size;
+    }
+
+    UT_ASSERT(allocs.size() >= nodeIds.size());
+    for (auto &ptr : allocs) {
         umf_ret = umfMemoryProviderFree(hProvider, ptr, size);
         UT_ASSERTeq(umf_ret, UMF_RESULT_SUCCESS);
     }

test/supp/umf_test-memspace_host_all.supp

@@ -0,0 +1,21 @@
+{
+   4,608 bytes in 2 blocks are possibly lost in loss record 242 of 246
+   Memcheck:Leak
+   match-leak-kinds: possible
+   fun:malloc
+   fun:malloc
+   fun:_dlfo_mappings_segment_allocate
+   fun:_dl_find_object_update_1
+   fun:_dl_find_object_update
+   fun:dl_open_worker_begin
+   fun:_dl_catch_exception
+   fun:dl_open_worker
+   fun:_dl_catch_exception
+   fun:_dl_open
+   fun:dlopen_doit
+   fun:_dl_catch_exception
+   fun:_dl_catch_error
+   fun:_dlerror_run
+   fun:dlopen_implementation
+   fun:dlopen@@GLIBC_*
+}