Remove the UMF_MEM_MAP_SYNC flag

Remove the UMF_MEM_MAP_SYNC flag,
UMF_MEM_MAP_SHARED should be used instead.

Verify if /dev/dax was mapped with MAP_SYNC.

Signed-off-by: Lukasz Dorau <lukasz.dorau@intel.com>

Author: ldorau

README.md

@@ -207,10 +207,10 @@ so it should be used with a pool manager that will take over
 the managing of the provided memory - for example the jemalloc pool
 with the `disable_provider_free` parameter set to true.
 
-IPC API requires the `UMF_MEM_MAP_SHARED` or `UMF_MEM_MAP_SYNC` memory `visibility` mode
+IPC API requires the `UMF_MEM_MAP_SHARED` memory `visibility` mode
 (`UMF_RESULT_ERROR_INVALID_ARGUMENT` is returned otherwise).
 
-The memory visibility mode parameter must be set to `UMF_MEM_MAP_SYNC` in case of FSDAX.
+The memory visibility mode parameter must be set to `UMF_MEM_MAP_SHARED` in case of FSDAX.
 
 ##### Requirements
 

examples/dram_and_fsdax/dram_and_fsdax.c

@@ -50,8 +50,8 @@ static umf_memory_pool_handle_t create_fsdax_pool(const char *path) {
 
     umf_file_memory_provider_params_t params_fsdax =
         umfFileMemoryProviderParamsDefault(path);
-    // FSDAX requires mapping the UMF_MEM_MAP_SYNC flag
-    params_fsdax.visibility = UMF_MEM_MAP_SYNC;
+    // FSDAX requires mapping the UMF_MEM_MAP_SHARED flag
+    params_fsdax.visibility = UMF_MEM_MAP_SHARED;
 
     umf_result = umfMemoryProviderCreate(umfFileMemoryProviderOps(),
                                          &params_fsdax, &provider_fsdax);

include/umf/memory_provider.h

@@ -21,7 +21,6 @@ extern "C" {
 typedef enum umf_memory_visibility_t {
     UMF_MEM_MAP_PRIVATE = 1, ///< private memory mapping
     UMF_MEM_MAP_SHARED,      ///< shared memory mapping (Linux only)
-    UMF_MEM_MAP_SYNC, ///< direct mapping of persistent memory (supported only for files supporting DAX, Linux only)
 } umf_memory_visibility_t;
 
 /// @brief Protection of the memory allocations

src/provider/provider_devdax_memory.c

@@ -139,21 +139,31 @@ static umf_result_t devdax_initialize(void *params, void **provider) {
         goto err_free_devdax_provider;
     }
 
-    unsigned map_sync_flag = 0;
-    utils_translate_mem_visibility_flag(UMF_MEM_MAP_SYNC, &map_sync_flag);
+    bool is_dax = false;
 
-    // mmap /dev/dax with the MAP_SYNC xor MAP_SHARED flag (if MAP_SYNC fails)
+    // mmap /dev/dax with the MAP_SYNC
     devdax_provider->base = utils_mmap_file(
-        NULL, devdax_provider->size, devdax_provider->protection, map_sync_flag,
-        fd, 0 /* offset */, NULL);
+        NULL, devdax_provider->size, devdax_provider->protection, 0 /* flags */,
+        fd, 0 /* offset */, &is_dax);
     utils_close_fd(fd);
     if (devdax_provider->base == NULL) {
-        LOG_PDEBUG("devdax memory mapping failed (path=%s, size=%zu)",
+        LOG_PDEBUG("mapping the devdax failed (path=%s, size=%zu)",
                    in_params->path, devdax_provider->size);
         ret = UMF_RESULT_ERROR_UNKNOWN;
         goto err_free_devdax_provider;
     }
 
+    if (!is_dax) {
+        LOG_ERR("mapping the devdax with MAP_SYNC failed: %s", in_params->path);
+        ret = UMF_RESULT_ERROR_UNKNOWN;
+
+        if (devdax_provider->base) {
+            utils_munmap(devdax_provider->base, devdax_provider->size);
+        }
+
+        goto err_free_devdax_provider;
+    }
+
     LOG_DEBUG("devdax memory mapped (path=%s, size=%zu, addr=%p)",
               in_params->path, devdax_provider->size, devdax_provider->base);
 
@@ -433,6 +443,8 @@ static umf_result_t devdax_open_ipc_handle(void *provider,
         return UMF_RESULT_ERROR_INVALID_ARGUMENT;
     }
 
+    *ptr = NULL;
+
     devdax_ipc_data_t *devdax_ipc_data = (devdax_ipc_data_t *)providerIpcData;
 
     int fd = utils_devdax_open(devdax_ipc_data->path);
@@ -441,9 +453,6 @@ static umf_result_t devdax_open_ipc_handle(void *provider,
         return UMF_RESULT_ERROR_INVALID_ARGUMENT;
     }
 
-    unsigned map_sync_flag = 0;
-    utils_translate_mem_visibility_flag(UMF_MEM_MAP_SYNC, &map_sync_flag);
-
     // It is just a workaround for case when
     // devdax_alloc() was called with the size argument
     // that is not a multiplier of DEVDAX_PAGE_SIZE_2MB.
@@ -452,34 +461,42 @@ static umf_result_t devdax_open_ipc_handle(void *provider,
     utils_align_ptr_down_size_up((void **)&offset_aligned, &length_aligned,
                                  DEVDAX_PAGE_SIZE_2MB);
 
-    // mmap /dev/dax with the MAP_SYNC xor MAP_SHARED flag (if MAP_SYNC fails)
+    bool is_dax = false;
+
+    // mmap /dev/dax with the MAP_SYNC
     char *addr =
         utils_mmap_file(NULL, length_aligned, devdax_ipc_data->protection,
-                        map_sync_flag, fd, offset_aligned, NULL);
+                        0 /* flags */, fd, offset_aligned, &is_dax);
+    (void)utils_close_fd(fd);
     if (addr == NULL) {
-        devdax_store_last_native_error(UMF_DEVDAX_RESULT_ERROR_ALLOC_FAILED,
-                                       errno);
-
         LOG_PERR("devdax mapping failed (path: %s, size: %zu, protection: %i, "
                  "fd: %i, offset: %zu)",
                  devdax_ipc_data->path, length_aligned,
                  devdax_ipc_data->protection, fd, offset_aligned);
 
-        *ptr = NULL;
-        (void)utils_close_fd(fd);
-
+        devdax_store_last_native_error(UMF_DEVDAX_RESULT_ERROR_ALLOC_FAILED,
+                                       errno);
         return UMF_RESULT_ERROR_MEMORY_PROVIDER_SPECIFIC;
     }
 
+    if (!is_dax) {
+        LOG_ERR("mapping the devdax with MAP_SYNC failed: %s",
+                devdax_ipc_data->path);
+
+        if (addr) {
+            utils_munmap(addr, length_aligned);
+        }
+
+        return UMF_RESULT_ERROR_UNKNOWN;
+    }
+
     LOG_DEBUG("devdax mapped (path: %s, size: %zu, protection: %i, fd: %i, "
               "offset: %zu) to address %p",
               devdax_ipc_data->path, length_aligned,
               devdax_ipc_data->protection, fd, offset_aligned, addr);
 
     *ptr = addr;
 
-    (void)utils_close_fd(fd);
-
     return UMF_RESULT_SUCCESS;
 }
 

src/provider/provider_file_memory.c

@@ -51,7 +51,7 @@ typedef struct file_memory_provider_t {
     unsigned visibility; // memory visibility mode
     size_t page_size;    // minimum page size
 
-    // IPC is enabled only for UMF_MEM_MAP_SHARED or UMF_MEM_MAP_SYNC visibility
+    // IPC is enabled only for the UMF_MEM_MAP_SHARED visibility
     bool IPC_enabled;
 
     critnib *mmaps; // a critnib map storing mmap mappings (addr, size)
@@ -114,9 +114,8 @@ file_translate_params(umf_file_memory_provider_params_t *in_params,
         return result;
     }
 
-    // IPC is enabled only for UMF_MEM_MAP_SHARED or UMF_MEM_MAP_SYNC visibility
-    provider->IPC_enabled = (in_params->visibility == UMF_MEM_MAP_SHARED ||
-                             in_params->visibility == UMF_MEM_MAP_SYNC);
+    // IPC is enabled only for the UMF_MEM_MAP_SHARED visibility
+    provider->IPC_enabled = (in_params->visibility == UMF_MEM_MAP_SHARED);
 
     return UMF_RESULT_SUCCESS;
 }
@@ -594,8 +593,7 @@ static umf_result_t file_get_ipc_handle_size(void *provider, size_t *size) {
 
     file_memory_provider_t *file_provider = (file_memory_provider_t *)provider;
     if (!file_provider->IPC_enabled) {
-        LOG_ERR("memory visibility mode is not UMF_MEM_MAP_SHARED nor "
-                "UMF_MEM_MAP_SYNC")
+        LOG_ERR("memory visibility mode is not UMF_MEM_MAP_SHARED")
         return UMF_RESULT_ERROR_INVALID_ARGUMENT;
     }
 
@@ -612,8 +610,7 @@ static umf_result_t file_get_ipc_handle(void *provider, const void *ptr,
 
     file_memory_provider_t *file_provider = (file_memory_provider_t *)provider;
     if (!file_provider->IPC_enabled) {
-        LOG_ERR("memory visibility mode is not UMF_MEM_MAP_SHARED nor "
-                "UMF_MEM_MAP_SYNC")
+        LOG_ERR("memory visibility mode is not UMF_MEM_MAP_SHARED")
         return UMF_RESULT_ERROR_INVALID_ARGUMENT;
     }
 
@@ -643,8 +640,7 @@ static umf_result_t file_put_ipc_handle(void *provider, void *providerIpcData) {
 
     file_memory_provider_t *file_provider = (file_memory_provider_t *)provider;
     if (!file_provider->IPC_enabled) {
-        LOG_ERR("memory visibility mode is not UMF_MEM_MAP_SHARED nor "
-                "UMF_MEM_MAP_SYNC")
+        LOG_ERR("memory visibility mode is not UMF_MEM_MAP_SHARED")
         return UMF_RESULT_ERROR_INVALID_ARGUMENT;
     }
 
@@ -665,8 +661,7 @@ static umf_result_t file_open_ipc_handle(void *provider, void *providerIpcData,
 
     file_memory_provider_t *file_provider = (file_memory_provider_t *)provider;
     if (!file_provider->IPC_enabled) {
-        LOG_ERR("memory visibility mode is not UMF_MEM_MAP_SHARED nor "
-                "UMF_MEM_MAP_SYNC")
+        LOG_ERR("memory visibility mode is not UMF_MEM_MAP_SHARED")
         return UMF_RESULT_ERROR_INVALID_ARGUMENT;
     }
 
@@ -712,8 +707,7 @@ static umf_result_t file_close_ipc_handle(void *provider, void *ptr,
 
     file_memory_provider_t *file_provider = (file_memory_provider_t *)provider;
     if (!file_provider->IPC_enabled) {
-        LOG_ERR("memory visibility mode is not UMF_MEM_MAP_SHARED nor "
-                "UMF_MEM_MAP_SYNC")
+        LOG_ERR("memory visibility mode is not UMF_MEM_MAP_SHARED")
         return UMF_RESULT_ERROR_INVALID_ARGUMENT;
     }
 

src/utils/utils_linux_common.c

@@ -32,9 +32,6 @@ utils_translate_mem_visibility_flag(umf_memory_visibility_t in_flag,
     case UMF_MEM_MAP_SHARED:
         *out_flag = MAP_SHARED;
         return UMF_RESULT_SUCCESS;
-    case UMF_MEM_MAP_SYNC:
-        *out_flag = MAP_SYNC;
-        return UMF_RESULT_SUCCESS;
     }
     return UMF_RESULT_ERROR_INVALID_ARGUMENT;
 }
@@ -95,7 +92,7 @@ void *utils_mmap_file(void *hint_addr, size_t length, int prot, int flags,
 
     /* try to mmap with MAP_SHARED flag (without MAP_SYNC) */
     if (errno == EINVAL || errno == ENOTSUP || errno == EOPNOTSUPP) {
-        const int shared_flags = (flags & (~MAP_SYNC)) | MAP_SHARED;
+        const int shared_flags = flags | MAP_SHARED;
         addr = utils_mmap(hint_addr, length, prot, shared_flags, fd, fd_offset);
         if (addr) {
             LOG_DEBUG("file mapped with the MAP_SHARED flag (fd=%i, "

src/utils/utils_macosx_common.c

@@ -24,8 +24,6 @@ utils_translate_mem_visibility_flag(umf_memory_visibility_t in_flag,
         return UMF_RESULT_SUCCESS;
     case UMF_MEM_MAP_SHARED:
         return UMF_RESULT_ERROR_NOT_SUPPORTED; // not supported on MacOSX
-    case UMF_MEM_MAP_SYNC:
-        return UMF_RESULT_ERROR_NOT_SUPPORTED; // not supported on MacOSX
     }
     return UMF_RESULT_ERROR_INVALID_ARGUMENT;
 }

src/utils/utils_windows_common.c

@@ -96,8 +96,6 @@ utils_translate_mem_visibility_flag(umf_memory_visibility_t in_flag,
         return UMF_RESULT_SUCCESS;
     case UMF_MEM_MAP_SHARED:
         return UMF_RESULT_ERROR_NOT_SUPPORTED; // not supported on Windows yet
-    case UMF_MEM_MAP_SYNC:
-        return UMF_RESULT_ERROR_NOT_SUPPORTED; // not supported on Windows yet
     }
     return UMF_RESULT_ERROR_INVALID_ARGUMENT;
 }

test/ipc_file_prov_consumer.c

@@ -18,30 +18,16 @@
 
 int main(int argc, char *argv[]) {
     if (argc < 3) {
-        fprintf(stderr, "usage: %s <port> <file_name> <fsdax>\n", argv[0]);
-        fprintf(stderr, "       <fsdax> should be \"FSDAX\" or \"fsdax\" if "
-                        "<file_name> is located on FSDAX \n");
+        fprintf(stderr, "usage: %s <port> <file_name>\n", argv[0]);
         return -1;
     }
 
     int port = atoi(argv[1]);
     char *file_name = argv[2];
-    bool is_fsdax = false;
-
-    if (argc >= 4) {
-        if (strncasecmp(argv[3], "FSDAX", strlen("FSDAX")) == 0) {
-            is_fsdax = true;
-        }
-    }
 
     umf_file_memory_provider_params_t file_params;
-
     file_params = umfFileMemoryProviderParamsDefault(file_name);
-    if (is_fsdax) {
-        file_params.visibility = UMF_MEM_MAP_SYNC;
-    } else {
-        file_params.visibility = UMF_MEM_MAP_SHARED;
-    }
+    file_params.visibility = UMF_MEM_MAP_SHARED;
 
     void *pool_params = NULL;
 

test/ipc_file_prov_fsdax.sh

@@ -31,13 +31,13 @@ UMF_LOG_VAL="level:debug;flush:debug;output:stderr;pid:yes"
 rm -f ${FILE_NAME}
 
 echo "Starting ipc_file_prov_fsdax CONSUMER on port $PORT ..."
-UMF_LOG=$UMF_LOG_VAL ./umf_test-ipc_file_prov_consumer $PORT $FILE_NAME "FSDAX" &
+UMF_LOG=$UMF_LOG_VAL ./umf_test-ipc_file_prov_consumer $PORT $FILE_NAME &
 
 echo "Waiting 1 sec ..."
 sleep 1
 
 echo "Starting ipc_file_prov_fsdax PRODUCER on port $PORT ..."
-UMF_LOG=$UMF_LOG_VAL ./umf_test-ipc_file_prov_producer $PORT $FILE_NAME_2 "FSDAX"
+UMF_LOG=$UMF_LOG_VAL ./umf_test-ipc_file_prov_producer $PORT $FILE_NAME_2
 
 # remove the SHM file
 rm -f ${FILE_NAME}