UCM: added madvise into VM_UNMAP events group

- added gtest for missed madvise event - added test for madvise call
openucx · Jul 9, 2019 · 51a14c3 · 51a14c3
1 parent f489f39
commit 51a14c3
Show file tree

Hide file tree

Showing 3 changed files with 60 additions and 27 deletions.
diff --git a/src/ucm/event/event.h b/src/ucm/event/event.h
@@ -17,7 +17,8 @@
 
 #define UCM_NATIVE_EVENT_VM_UNMAPPED (UCM_EVENT_MMAP   | UCM_EVENT_MUNMAP | \
                                       UCM_EVENT_MREMAP | UCM_EVENT_SHMDT  | \
-                                      UCM_EVENT_SHMAT  | UCM_EVENT_SBRK)
+                                      UCM_EVENT_SHMAT  | UCM_EVENT_SBRK   | \
+                                      UCM_EVENT_MADVISE)
 
 
 typedef struct ucm_event_handler {

diff --git a/src/ucm/mmap/install.c b/src/ucm/mmap/install.c
@@ -160,13 +160,13 @@ ucm_fire_mmap_events_internal(int events, ucm_mmap_test_events_data_t *data)
                        data, (void)sbrk(-sbrk_size));
     }
 
-    if (events & UCM_EVENT_MADVISE) {
+    if (events & (UCM_EVENT_MADVISE|UCM_EVENT_VM_UNMAPPED)) {
         UCM_FIRE_EVENT(events, UCM_EVENT_MMAP|UCM_EVENT_VM_MAPPED, data,
                        p = mmap(NULL, ucm_get_page_size(), PROT_READ|PROT_WRITE,
                                 MAP_PRIVATE|MAP_ANON, -1, 0));
         if (p != MAP_FAILED) {
             UCM_FIRE_EVENT(events, UCM_EVENT_MADVISE, data,
-                           madvise(p, ucm_get_page_size(), MADV_NORMAL));
+                           madvise(p, ucm_get_page_size(), MADV_DONTNEED));
             UCM_FIRE_EVENT(events, UCM_EVENT_MUNMAP|UCM_EVENT_VM_UNMAPPED, data,
                            munmap(p, ucm_get_page_size()));
         } else {

diff --git a/test/gtest/ucm/malloc_hook.cc b/test/gtest/ucm/malloc_hook.cc
@@ -132,6 +132,39 @@ class malloc_hook : public ucs::test {
             UCM_BISTRO_EPILOGUE;
             return res;
         }
+
+        static int madvise(void *addr, size_t length, int advise)
+        {
+            UCM_BISTRO_PROLOGUE;
+            malloc_hook::bistro_call_counter++;
+            if (C) {
+                /* notify aggregate vm_munmap event only */
+                ucm_vm_munmap(addr, length);
+            }
+            int res = (intptr_t)syscall(SYS_madvise, addr, length, advise);
+            UCM_BISTRO_EPILOGUE;
+            return res;
+        }
+    };
+
+    class bistro_patch {
+    public:
+        bistro_patch(const char* symbol, void *hook)
+        {
+            ucs_status_t status;
+
+            status = ucm_bistro_patch(symbol, hook, &m_rp);
+            ASSERT_UCS_OK(status);
+            EXPECT_NE((intptr_t)m_rp, 0);
+        }
+
+        ~bistro_patch()
+        {
+            ucm_bistro_restore(m_rp);
+        }
+
+    protected:
+        ucm_bistro_restore_point_t *m_rp;
     };
 
     void mem_event(ucm_event_type_t event_type, ucm_event_t *event)
@@ -399,6 +432,14 @@ void test_thread::test() {
 
     EXPECT_TRUE(is_ptr_in_range(ptr, shm_seg_size, m_unmap_ranges));
 
+    ptr = mmap(NULL, shm_seg_size, PROT_READ|PROT_WRITE,
+               MAP_PRIVATE|MAP_ANONYMOUS, -1, 0);
+    ASSERT_NE(MAP_FAILED, ptr) << strerror(errno);
+    madvise(ptr, shm_seg_size, MADV_DONTNEED);
+
+    EXPECT_TRUE(is_ptr_in_range(ptr, shm_seg_size, m_unmap_ranges));
+    munmap(ptr, shm_seg_size);
+
     /* Print results */
     pthread_mutex_lock(&lock);
     UCS_TEST_MESSAGE << m_name
@@ -984,42 +1025,37 @@ UCS_TEST_SKIP_COND_F(malloc_hook, test_event_failed,
                      RUNNING_ON_VALGRIND || !skip_on_bistro()) {
     mmap_event<malloc_hook> event(this);
     ucs_status_t status;
-    const char *symbol = "munmap";
-    ucm_bistro_restore_point_t *rp = NULL;
+    const char *symbol_munmap  = "munmap";
+    const char *symbol_madvise = "madvise";
 
     status = event.set(UCM_EVENT_MUNMAP | UCM_EVENT_VM_UNMAPPED);
     ASSERT_UCS_OK(status);
 
-    /* set hook to mmap call */
-    status = ucm_bistro_patch(symbol, (void*)bistro_hook<0>::munmap, &rp);
-    ASSERT_UCS_OK(status);
-    EXPECT_NE((intptr_t)rp, 0);
-
-    status = ucm_test_events(UCM_EVENT_MUNMAP);
-    EXPECT_TRUE(status == UCS_ERR_UNSUPPORTED);
-
-    status = ucm_test_events(UCM_EVENT_VM_UNMAPPED);
-    EXPECT_TRUE(status == UCS_ERR_UNSUPPORTED);
-
-    /* restore original munmap body */
-    status = ucm_bistro_restore(rp);
-    ASSERT_UCS_OK(status);
+    /* set hook to munmap call */
+    {
+        bistro_patch patch(symbol_munmap, (void*)bistro_hook<0>::munmap);
+        EXPECT_TRUE(ucm_test_events(UCM_EVENT_MUNMAP)      == UCS_ERR_UNSUPPORTED);
+        EXPECT_TRUE(ucm_test_events(UCM_EVENT_VM_UNMAPPED) == UCS_ERR_UNSUPPORTED);
+    }
+    /* set hook to madvise call */
+    {
+        bistro_patch patch(symbol_madvise, (void*)bistro_hook<0>::madvise);
+        EXPECT_TRUE(ucm_test_events(UCM_EVENT_MADVISE)     == UCS_ERR_UNSUPPORTED);
+        EXPECT_TRUE(ucm_test_events(UCM_EVENT_VM_UNMAPPED) == UCS_ERR_UNSUPPORTED);
+    }
 }
 
 UCS_TEST_SKIP_COND_F(malloc_hook, test_event_unmap,
                      RUNNING_ON_VALGRIND || !skip_on_bistro()) {
     mmap_event<malloc_hook> event(this);
     ucs_status_t status;
     const char *symbol = "munmap";
-    ucm_bistro_restore_point_t *rp = NULL;
 
     status = event.set(UCM_EVENT_MMAP | UCM_EVENT_MUNMAP | UCM_EVENT_VM_UNMAPPED);
     ASSERT_UCS_OK(status);
 
     /* set hook to mmap call */
-    status = ucm_bistro_patch(symbol, (void*)bistro_hook<1>::munmap, &rp);
-    ASSERT_UCS_OK(status);
-    EXPECT_NE((intptr_t)rp, 0);
+    bistro_patch patch(symbol, (void*)bistro_hook<1>::munmap);
 
     /* munmap should be broken */
     status = ucm_test_events(UCM_EVENT_MUNMAP);
@@ -1032,10 +1068,6 @@ UCS_TEST_SKIP_COND_F(malloc_hook, test_event_unmap,
     /* mmap should still work */
     status = ucm_test_events(UCM_EVENT_MMAP);
     EXPECT_TRUE(status == UCS_OK);
-
-    /* restore original munmap body */
-    status = ucm_bistro_restore(rp);
-    ASSERT_UCS_OK(status);
 }
 
 class malloc_hook_dlopen : public malloc_hook {