Merge pull request #236 from igchor/os_provider_vis

Remove umf_mem_visibility_t

Author: bratpiorka

benchmark/ubench.c

@@ -99,7 +99,6 @@ UBENCH_EX(simple, glibc_malloc) {
 
 static umf_os_memory_provider_params_t UMF_OS_MEMORY_PROVIDER_PARAMS = {
     /* .protection = */ UMF_PROTECTION_READ | UMF_PROTECTION_WRITE,
-    /* .visibility = */ UMF_VISIBILITY_PRIVATE,
 
     // NUMA config
     /* .nodemask = */ NULL,

include/umf/providers/provider_os_memory.h

@@ -26,13 +26,6 @@ typedef enum umf_mem_protection_flags_t {
     UMF_PROTECTION_MAX // must be the last one
 } umf_mem_protection_flags_t;
 
-/// @brief Visibility of the memory allocations
-typedef enum umf_mem_visibility_t {
-    UMF_VISIBILITY_SHARED, ///< Updates to the memory allocated using OS provider are visible to other processes.
-    /// TODO: need to expose functionality to share open the mapping in other process and explicit sync?
-    UMF_VISIBILITY_PRIVATE, ///<  Updates to the memory allocated using OS provider are not visible to other processes.
-} umf_mem_visibility_t;
-
 /// @brief Memory binding mode
 ///
 /// Specifies how memory is bound to NUMA nodes on systems that support NUMA.
@@ -57,9 +50,6 @@ typedef struct umf_os_memory_provider_params_t {
     /// combination of 'umf_mem_protection_flags_t' flags
     unsigned protection;
 
-    /// shared or private visibility of memory mapped by a provider
-    umf_mem_visibility_t visibility;
-
     // NUMA config
     /// points to a bit mask of nodes containing up to maxnode bits, depending on
     /// selected numa_mode newly allocated memory will be bound to those nodes
@@ -92,7 +82,6 @@ static inline umf_os_memory_provider_params_t
 umfOsMemoryProviderParamsDefault(void) {
     umf_os_memory_provider_params_t params = {
         UMF_PROTECTION_READ | UMF_PROTECTION_WRITE, /* protection */
-        UMF_VISIBILITY_PRIVATE,                     /* visibility */
         NULL,                                       /* nodemask */
         0,                                          /* maxnode */
         UMF_NUMA_MODE_DEFAULT,                      /* numa_mode */

src/provider/provider_os_memory.c

@@ -22,7 +22,6 @@
 
 typedef struct umf_os_memory_provider_config_t {
     unsigned protection; // combination of OS-specific protection flags
-    unsigned visibility;
 
     // NUMA config
     unsigned long *nodemask;
@@ -35,7 +34,6 @@ typedef struct umf_os_memory_provider_config_t {
 
 typedef struct os_memory_provider_t {
     unsigned protection; // combination of OS-specific protection flags
-    unsigned visibility;
 
     // NUMA config
     hwloc_bitmap_t nodeset;
@@ -204,16 +202,6 @@ static umf_result_t translate_params(umf_os_memory_provider_params_t *in_params,
     }
     provider->protection = ret;
 
-    ret = os_translate_mem_visibility(in_params->visibility);
-    if (ret < 0) {
-        if (in_params->traces) {
-            fprintf(stderr, "error: incorrect memory visibility mode: %u\n",
-                    in_params->visibility);
-        }
-        return UMF_RESULT_ERROR_INVALID_ARGUMENT;
-    }
-    provider->visibility = ret;
-
     // NUMA config
     int emptyNodeset = (!in_params->maxnode || !in_params->nodemask);
     ret = translate_numa_mode(in_params->numa_mode, emptyNodeset);
@@ -241,18 +229,6 @@ static umf_result_t os_initialize(void *params, void **provider) {
     umf_os_memory_provider_params_t *in_params =
         (umf_os_memory_provider_params_t *)params;
 
-    if (in_params->visibility == UMF_VISIBILITY_SHARED &&
-        in_params->numa_mode != UMF_NUMA_MODE_DEFAULT) {
-        // TODO: add support for that
-        if (in_params->traces) {
-            fprintf(stderr,
-                    "NUMA binding mode (%i) not supported for "
-                    "UMF_VISIBILITY_SHARED\n",
-                    in_params->numa_mode);
-        }
-        return UMF_RESULT_ERROR_INVALID_ARGUMENT;
-    }
-
     umf_ba_pool_t *base_allocator =
         umf_ba_get_pool(sizeof(os_memory_provider_t));
     if (!base_allocator) {
@@ -387,13 +363,12 @@ static umf_result_t os_alloc(void *provider, size_t size, size_t alignment,
         return UMF_RESULT_ERROR_INVALID_ARGUMENT;
     }
 
-    int flags = os_provider->visibility;
     int protection = os_provider->protection;
 
     void *addr = NULL;
     errno = 0;
-    ret = os_mmap_aligned(NULL, size, alignment, page_size, protection, flags,
-                          -1, 0, &addr);
+    ret = os_mmap_aligned(NULL, size, alignment, page_size, protection, -1, 0,
+                          &addr);
     if (ret) {
         os_store_last_native_error(UMF_OS_RESULT_ERROR_ALLOC_FAILED, errno);
         if (os_provider->traces) {

src/provider/provider_os_memory_internal.h

@@ -25,10 +25,8 @@ int os_translate_flags(unsigned in_flags, unsigned max,
 
 int os_translate_mem_protection_flags(unsigned protection);
 
-int os_translate_mem_visibility(umf_mem_visibility_t visibility);
-
 int os_mmap_aligned(void *hint_addr, size_t length, size_t alignment,
-                    size_t page_size, int prot, int flags, int fd, long offset,
+                    size_t page_size, int prot, int fd, long offset,
                     void **out_addr);
 
 int os_munmap(void *addr, size_t length);

src/provider/provider_os_memory_linux.c

@@ -34,17 +34,6 @@ int os_translate_mem_protection_flags(unsigned protection_flags) {
                               os_translate_mem_protection_one_flag);
 }
 
-int os_translate_mem_visibility(umf_mem_visibility_t visibility) {
-    switch (visibility) {
-    case UMF_VISIBILITY_SHARED:
-        return MAP_SHARED;
-    case UMF_VISIBILITY_PRIVATE:
-        return MAP_PRIVATE;
-    }
-    assert(0);
-    return -1;
-}
-
 static int os_translate_purge_advise(umf_purge_advise_t advise) {
     switch (advise) {
     case UMF_PURGE_LAZY:
@@ -63,7 +52,7 @@ static inline void assert_is_page_aligned(uintptr_t ptr, size_t page_size) {
 }
 
 int os_mmap_aligned(void *hint_addr, size_t length, size_t alignment,
-                    size_t page_size, int prot, int flags, int fd, long offset,
+                    size_t page_size, int prot, int fd, long offset,
                     void **out_addr) {
     assert(out_addr);
 
@@ -78,8 +67,8 @@ int os_mmap_aligned(void *hint_addr, size_t length, size_t alignment,
     }
 
     // MAP_ANONYMOUS - the mapping is not backed by any file
-    void *ptr = mmap(hint_addr, extended_length, prot, MAP_ANONYMOUS | flags,
-                     fd, offset);
+    void *ptr = mmap(hint_addr, extended_length, prot,
+                     MAP_ANONYMOUS | MAP_PRIVATE, fd, offset);
     if (ptr == MAP_FAILED) {
         return -1;
     }

test/provider_os_memory_config.cpp

@@ -188,45 +188,3 @@ TEST_P(providerConfigTestNumaMode, numa_modes) {
         }
     }
 }
-
-struct providerConfigTestVisibility
-    : providerConfigTest,
-      testing::WithParamInterface<umf_mem_visibility_t> {
-    std::string primary_str = "primary";
-    std::string new_str = "new";
-    std::string expected_str;
-    umf_os_memory_provider_params_t params = umfOsMemoryProviderParamsDefault();
-
-    void SetUp() override {
-        providerConfigTest::SetUp();
-        params.visibility = GetParam();
-        if (params.visibility == UMF_VISIBILITY_PRIVATE) {
-            expected_str = primary_str;
-        } else if (params.visibility == UMF_VISIBILITY_SHARED) {
-            expected_str = new_str;
-        }
-    }
-};
-
-INSTANTIATE_TEST_SUITE_P(visibility, providerConfigTestVisibility,
-                         testing::Values(UMF_VISIBILITY_PRIVATE,
-                                         UMF_VISIBILITY_SHARED));
-
-TEST_P(providerConfigTestVisibility, visibility) {
-    create_provider(&params);
-    allocate_memory();
-    write_memory(primary_str);
-
-    int pid = fork();
-    if (pid == 0) {
-        // child process
-        write_memory(new_str);
-    } else if (pid > 0) {
-        // main process
-        int status;
-        while (-1 == waitpid(0, &status, 0)) {
-            // wait for the child process to complete
-        }
-        EXPECT_EQ(static_cast<char *>(ptr), expected_str);
-    }
-}