diff --git a/README.md b/README.md index 923db6f2cb..b31e349f0f 100644 --- a/README.md +++ b/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 diff --git a/examples/dram_and_fsdax/dram_and_fsdax.c b/examples/dram_and_fsdax/dram_and_fsdax.c index bc985692f4..ac7b0434c2 100644 --- a/examples/dram_and_fsdax/dram_and_fsdax.c +++ b/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(), ¶ms_fsdax, &provider_fsdax); diff --git a/include/umf/memory_provider.h b/include/umf/memory_provider.h index 073b04efb9..cff6f9eecb 100644 --- a/include/umf/memory_provider.h +++ b/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 diff --git a/src/provider/provider_devdax_memory.c b/src/provider/provider_devdax_memory.c index bae968a1d5..c7f644db35 100644 --- a/src/provider/provider_devdax_memory.c +++ b/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) - devdax_provider->base = utils_mmap_file(NULL, devdax_provider->size, - devdax_provider->protection, - map_sync_flag, fd, 0 /* offset */); + // mmap /dev/dax with the MAP_SYNC + devdax_provider->base = utils_mmap_file( + 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,25 +461,35 @@ 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); + 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, @@ -478,8 +497,6 @@ static umf_result_t devdax_open_ipc_handle(void *provider, *ptr = addr; - (void)utils_close_fd(fd); - return UMF_RESULT_SUCCESS; } diff --git a/src/provider/provider_file_memory.c b/src/provider/provider_file_memory.c index 7086fe45cf..ec3cd25cdb 100644 --- a/src/provider/provider_file_memory.c +++ b/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; } @@ -293,8 +292,8 @@ static umf_result_t file_mmap_aligned(file_memory_provider_t *file_provider, ASSERT_IS_ALIGNED(extended_size, page_size); ASSERT_IS_ALIGNED(aligned_offset_fd, page_size); - void *ptr = - utils_mmap_file(NULL, extended_size, prot, flag, fd, aligned_offset_fd); + void *ptr = utils_mmap_file(NULL, extended_size, prot, flag, fd, + aligned_offset_fd, NULL); if (ptr == NULL) { LOG_PERR("memory mapping failed"); return UMF_RESULT_ERROR_MEMORY_PROVIDER_SPECIFIC; @@ -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; } @@ -683,7 +678,7 @@ static umf_result_t file_open_ipc_handle(void *provider, void *providerIpcData, char *addr = utils_mmap_file( NULL, file_ipc_data->size, file_ipc_data->protection, - file_ipc_data->visibility, fd, file_ipc_data->offset_fd); + file_ipc_data->visibility, fd, file_ipc_data->offset_fd, NULL); (void)utils_close_fd(fd); if (addr == NULL) { file_store_last_native_error(UMF_FILE_RESULT_ERROR_ALLOC_FAILED, errno); @@ -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; } diff --git a/src/utils/utils_common.h b/src/utils/utils_common.h index eebc461f62..c25fda2ab0 100644 --- a/src/utils/utils_common.h +++ b/src/utils/utils_common.h @@ -11,6 +11,7 @@ #define UMF_COMMON_H 1 #include +#include #include #include @@ -136,7 +137,7 @@ void *utils_mmap(void *hint_addr, size_t length, int prot, int flag, int fd, size_t fd_offset); void *utils_mmap_file(void *hint_addr, size_t length, int prot, int flags, - int fd, size_t fd_offset); + int fd, size_t fd_offset, bool *map_sync); int utils_munmap(void *addr, size_t length); diff --git a/src/utils/utils_linux_common.c b/src/utils/utils_linux_common.c index 1a44a2b64c..b97880ffd0 100644 --- a/src/utils/utils_linux_common.c +++ b/src/utils/utils_linux_common.c @@ -9,6 +9,7 @@ #include #include +#include #include #include #include @@ -31,78 +32,78 @@ 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; } /* * Map given file into memory. - * If (flags & MAP_PRIVATE) it uses just mmap. Otherwise, if (flags & MAP_SYNC) - * it tries to mmap with (flags | MAP_SHARED_VALIDATE | MAP_SYNC) - * which allows flushing from the user-space. If MAP_SYNC fails and the user - * did not specify it by himself it tries to mmap with (flags | MAP_SHARED). + * If (flags & MAP_PRIVATE) it uses just mmap. Otherwise, it tries to mmap + * with (flags | MAP_SHARED_VALIDATE | MAP_SYNC) which allows flushing + * from the user-space. If MAP_SYNC fails and if the user did not specify + * this flag by himself, it falls back to the mmap with (flags | MAP_SHARED). */ void *utils_mmap_file(void *hint_addr, size_t length, int prot, int flags, - int fd, size_t fd_offset) { + int fd, size_t fd_offset, bool *map_sync) { void *addr; + if (map_sync) { + *map_sync = false; + } + /* * MAP_PRIVATE and MAP_SHARED are mutually exclusive, * therefore mmap with MAP_PRIVATE is executed separately. */ if (flags & MAP_PRIVATE) { addr = utils_mmap(hint_addr, length, prot, flags, fd, fd_offset); - if (addr == MAP_FAILED) { + if (addr == NULL) { LOG_PERR("mapping file with the MAP_PRIVATE flag failed (fd=%i, " - "offset=%zu, length=%zu)", - fd, fd_offset, length); + "offset=%zu, length=%zu, flags=%i)", + fd, fd_offset, length, flags); return NULL; } LOG_DEBUG("file mapped with the MAP_PRIVATE flag (fd=%i, offset=%zu, " - "length=%zu)", - fd, fd_offset, length); + "length=%zu, flags=%i)", + fd, fd_offset, length, flags); return addr; } errno = 0; - if (flags & MAP_SYNC) { - /* try to mmap with MAP_SYNC flag */ - const int sync_flags = MAP_SHARED_VALIDATE | MAP_SYNC; - addr = utils_mmap(hint_addr, length, prot, flags | sync_flags, fd, - fd_offset); - if (addr) { - LOG_DEBUG("file mapped with the MAP_SYNC flag (fd=%i, offset=%zu, " - "length=%zu)", - fd, fd_offset, length); - return addr; + /* try to mmap with MAP_SYNC flag */ + const int sync_flags = flags | MAP_SHARED_VALIDATE | MAP_SYNC; + addr = utils_mmap(hint_addr, length, prot, sync_flags, fd, fd_offset); + if (addr) { + LOG_DEBUG("file mapped with the MAP_SYNC flag (fd=%i, offset=%zu, " + "length=%zu, flags=%i)", + fd, fd_offset, length, sync_flags); + if (map_sync) { + *map_sync = true; } - - LOG_PERR("mapping file with the MAP_SYNC flag failed (fd=%i, " - "offset=%zu, length=%zu)", - fd, fd_offset, length); + return addr; } - if ((!(flags & MAP_SYNC)) || errno == EINVAL || errno == ENOTSUP || - errno == EOPNOTSUPP) { - /* try to mmap with MAP_SHARED flag (without MAP_SYNC) */ - const int shared_flags = (flags & (~MAP_SYNC)) | MAP_SHARED; + LOG_PERR("mapping file with the MAP_SYNC flag failed (fd=%i, offset=%zu, " + "length=%zu, flags=%i)", + fd, fd_offset, length, sync_flags); + + /* try to mmap with MAP_SHARED flag (without MAP_SYNC) */ + if (errno == EINVAL || errno == ENOTSUP || errno == EOPNOTSUPP) { + 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, " - "offset=%zu, length=%zu)", - fd, fd_offset, length); + "offset=%zu, length=%zu, flags=%i)", + fd, fd_offset, length, shared_flags); return addr; } LOG_PERR("mapping file with the MAP_SHARED flag failed (fd=%i, " - "offset=%zu, length=%zu)", - fd, fd_offset, length); + "offset=%zu, length=%zu, flags=%i)", + fd, fd_offset, length, shared_flags); } return NULL; diff --git a/src/utils/utils_macosx_common.c b/src/utils/utils_macosx_common.c index 1356a2ef96..ad1de12fdb 100644 --- a/src/utils/utils_macosx_common.c +++ b/src/utils/utils_macosx_common.c @@ -12,6 +12,7 @@ #include #include +#include "utils_common.h" #include "utils_log.h" umf_result_t @@ -23,20 +24,19 @@ 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; } void *utils_mmap_file(void *hint_addr, size_t length, int prot, int flags, - int fd, size_t fd_offset) { + int fd, size_t fd_offset, bool *map_sync) { (void)hint_addr; // unused (void)length; // unused (void)prot; // unused (void)flags; // unused (void)fd; // unused (void)fd_offset; // unused + (void)map_sync; // unused return NULL; // not supported } diff --git a/src/utils/utils_windows_common.c b/src/utils/utils_windows_common.c index 4646b6f55a..50a7b6ed53 100644 --- a/src/utils/utils_windows_common.c +++ b/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; } @@ -148,13 +146,14 @@ void *utils_mmap(void *hint_addr, size_t length, int prot, int flag, int fd, } void *utils_mmap_file(void *hint_addr, size_t length, int prot, int flags, - int fd, size_t fd_offset) { + int fd, size_t fd_offset, bool *map_sync) { (void)hint_addr; // unused (void)length; // unused (void)prot; // unused (void)flags; // unused (void)fd; // unused (void)fd_offset; // unused + (void)map_sync; // unused return NULL; // not supported } diff --git a/test/ipc_file_prov_consumer.c b/test/ipc_file_prov_consumer.c index 6c53ad320f..6333552a2d 100644 --- a/test/ipc_file_prov_consumer.c +++ b/test/ipc_file_prov_consumer.c @@ -18,30 +18,16 @@ int main(int argc, char *argv[]) { if (argc < 3) { - fprintf(stderr, "usage: %s \n", argv[0]); - fprintf(stderr, " should be \"FSDAX\" or \"fsdax\" if " - " is located on FSDAX \n"); + fprintf(stderr, "usage: %s \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; diff --git a/test/ipc_file_prov_fsdax.sh b/test/ipc_file_prov_fsdax.sh index 6f8d75541b..4e908869b7 100755 --- a/test/ipc_file_prov_fsdax.sh +++ b/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} diff --git a/test/ipc_file_prov_producer.c b/test/ipc_file_prov_producer.c index ee9d96f1e1..efcbdd3bf5 100644 --- a/test/ipc_file_prov_producer.c +++ b/test/ipc_file_prov_producer.c @@ -18,30 +18,16 @@ int main(int argc, char *argv[]) { if (argc < 3) { - fprintf(stderr, "usage: %s \n", argv[0]); - fprintf(stderr, " should be \"FSDAX\" or \"fsdax\" if " - " is located on FSDAX \n"); + fprintf(stderr, "usage: %s \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; diff --git a/test/provider_file_memory.cpp b/test/provider_file_memory.cpp index b2c71635d2..eba7e82056 100644 --- a/test/provider_file_memory.cpp +++ b/test/provider_file_memory.cpp @@ -137,7 +137,7 @@ TEST_F(test, test_if_mapped_with_MAP_SYNC) { } auto params = umfFileMemoryProviderParamsDefault(path); - params.visibility = UMF_MEM_MAP_SYNC; + params.visibility = UMF_MEM_MAP_SHARED; umf_result = umfMemoryProviderCreate(umfFileMemoryProviderOps(), ¶ms, &hProvider);