Merge pull request #878 from ldorau/Enable_IPC_API_for_FSDAX

ldorau · web-flow · commit 64456b7b97ec · 2024-11-14T16:03:34.000+01:00
Enable IPC API for FSDAX
diff --git a/src/provider/provider_devdax_memory.c b/src/provider/provider_devdax_memory.c
@@ -493,7 +493,7 @@ static umf_result_t devdax_open_ipc_handle(void *provider,
     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);
+              devdax_ipc_data->protection, fd, offset_aligned, (void *)addr);
 
     *ptr = addr;
 
diff --git a/src/provider/provider_file_memory.c b/src/provider/provider_file_memory.c
@@ -33,12 +33,15 @@ umf_memory_provider_ops_t *umfFileMemoryProviderOps(void) {
 #include "utils_concurrency.h"
 #include "utils_log.h"
 
+#define FSDAX_PAGE_SIZE_2MB ((size_t)(2 * 1024 * 1024)) // == 2 MB
+
 #define TLS_MSG_BUF_LEN 1024
 
 typedef struct file_memory_provider_t {
     utils_mutex_t lock; // lock for file parameters (size and offsets)
 
     char path[PATH_MAX]; // a path to the file
+    bool is_fsdax;       // true if file is located on FSDAX
     int fd;              // file descriptor for memory mapping
     size_t size_fd;      // size of the file used for memory mappings
     size_t offset_fd;    // offset in the file used for memory mappings
@@ -130,8 +133,6 @@ static umf_result_t file_initialize(void *params, void **provider) {
     umf_file_memory_provider_params_t *in_params =
         (umf_file_memory_provider_params_t *)params;
 
-    size_t page_size = utils_get_page_size();
-
     if (in_params->path == NULL) {
         LOG_ERR("file path is missing");
         return UMF_RESULT_ERROR_INVALID_ARGUMENT;
@@ -145,8 +146,6 @@ static umf_result_t file_initialize(void *params, void **provider) {
 
     memset(file_provider, 0, sizeof(*file_provider));
 
-    file_provider->page_size = page_size;
-
     ret = file_translate_params(in_params, file_provider);
     if (ret != UMF_RESULT_SUCCESS) {
         goto err_free_file_provider;
@@ -163,17 +162,34 @@ static umf_result_t file_initialize(void *params, void **provider) {
         goto err_free_file_provider;
     }
 
-    if (utils_set_file_size(file_provider->fd, page_size)) {
+    if (utils_set_file_size(file_provider->fd, FSDAX_PAGE_SIZE_2MB)) {
         LOG_ERR("cannot set size of the file: %s", in_params->path);
         ret = UMF_RESULT_ERROR_UNKNOWN;
         goto err_close_fd;
     }
 
-    file_provider->size_fd = page_size;
+    file_provider->size_fd = FSDAX_PAGE_SIZE_2MB;
 
     LOG_DEBUG("size of the file %s is: %zu", in_params->path,
               file_provider->size_fd);
 
+    if (!(in_params->visibility & UMF_MEM_MAP_PRIVATE)) {
+        // check if file is located on FSDAX
+        void *addr = utils_mmap_file(
+            NULL, file_provider->size_fd, file_provider->protection,
+            file_provider->visibility, file_provider->fd, 0,
+            &file_provider->is_fsdax);
+        if (addr) {
+            utils_munmap(addr, file_provider->size_fd);
+        }
+    }
+
+    if (file_provider->is_fsdax) {
+        file_provider->page_size = FSDAX_PAGE_SIZE_2MB;
+    } else {
+        file_provider->page_size = utils_get_page_size();
+    }
+
     if (utils_mutex_init(&file_provider->lock) == NULL) {
         LOG_ERR("lock init failed");
         ret = UMF_RESULT_ERROR_UNKNOWN;
@@ -480,7 +496,8 @@ static umf_result_t file_get_recommended_page_size(void *provider, size_t size,
         return UMF_RESULT_ERROR_INVALID_ARGUMENT;
     }
 
-    *page_size = utils_get_page_size();
+    file_memory_provider_t *file_provider = (file_memory_provider_t *)provider;
+    *page_size = file_provider->page_size;
 
     return UMF_RESULT_SUCCESS;
 }
@@ -669,30 +686,41 @@ static umf_result_t file_open_ipc_handle(void *provider, void *providerIpcData,
     umf_result_t ret = UMF_RESULT_SUCCESS;
     int fd;
 
+    size_t offset_aligned = file_ipc_data->offset_fd;
+    size_t size_aligned = file_ipc_data->size;
+
+    if (file_provider->is_fsdax) {
+        // It is just a workaround for case when
+        // file_alloc() was called with the size argument
+        // that is not a multiplier of FSDAX_PAGE_SIZE_2MB.
+        utils_align_ptr_down_size_up((void **)&offset_aligned, &size_aligned,
+                                     FSDAX_PAGE_SIZE_2MB);
+    }
+
     fd = utils_file_open(file_ipc_data->path);
     if (fd == -1) {
         LOG_PERR("opening the file to be mapped (%s) failed",
                  file_ipc_data->path);
         return UMF_RESULT_ERROR_INVALID_ARGUMENT;
     }
 
-    char *addr = utils_mmap_file(
-        NULL, file_ipc_data->size, file_ipc_data->protection,
-        file_ipc_data->visibility, fd, file_ipc_data->offset_fd, NULL);
+    char *addr =
+        utils_mmap_file(NULL, size_aligned, file_ipc_data->protection,
+                        file_ipc_data->visibility, fd, offset_aligned, NULL);
     (void)utils_close_fd(fd);
     if (addr == NULL) {
         file_store_last_native_error(UMF_FILE_RESULT_ERROR_ALLOC_FAILED, errno);
-        LOG_PERR("file mapping failed (path: %s, size: %zu, protection: %i, "
-                 "fd: %i, offset: %zu)",
-                 file_ipc_data->path, file_ipc_data->size,
-                 file_ipc_data->protection, fd, file_ipc_data->offset_fd);
+        LOG_PERR("file mapping failed (path: %s, size: %zu, protection: %u, "
+                 "visibility: %u, fd: %i, offset: %zu)",
+                 file_ipc_data->path, size_aligned, file_ipc_data->protection,
+                 file_ipc_data->visibility, fd, offset_aligned);
         return UMF_RESULT_ERROR_MEMORY_PROVIDER_SPECIFIC;
     }
 
-    LOG_DEBUG("file mapped (path: %s, size: %zu, protection: %i, fd: %i, "
-              "offset: %zu) at address %p",
-              file_ipc_data->path, file_ipc_data->size,
-              file_ipc_data->protection, fd, file_ipc_data->offset_fd, addr);
+    LOG_DEBUG("file mapped (path: %s, size: %zu, protection: %u, visibility: "
+              "%u, fd: %i, offset: %zu) at address %p",
+              file_ipc_data->path, size_aligned, file_ipc_data->protection,
+              file_ipc_data->visibility, fd, offset_aligned, (void *)addr);
 
     *ptr = addr;
 
@@ -711,6 +739,13 @@ static umf_result_t file_close_ipc_handle(void *provider, void *ptr,
         return UMF_RESULT_ERROR_INVALID_ARGUMENT;
     }
 
+    if (file_provider->is_fsdax) {
+        // It is just a workaround for case when
+        // file_alloc() was called with the size argument
+        // that is not a multiplier of FSDAX_PAGE_SIZE_2MB.
+        utils_align_ptr_down_size_up(&ptr, &size, FSDAX_PAGE_SIZE_2MB);
+    }
+
     errno = 0;
     int ret = utils_munmap(ptr, size);
     // ignore error when size == 0
diff --git a/test/provider_file_memory_ipc.cpp b/test/provider_file_memory_ipc.cpp
@@ -27,6 +27,16 @@ umf_file_memory_provider_params_t get_file_params_shared(char *path) {
 umf_file_memory_provider_params_t file_params_shared =
     get_file_params_shared(FILE_PATH);
 
+umf_file_memory_provider_params_t get_file_params_fsdax(char *path) {
+    umf_file_memory_provider_params_t file_params =
+        umfFileMemoryProviderParamsDefault(path);
+    file_params.visibility = UMF_MEM_MAP_SHARED;
+    return file_params;
+}
+
+umf_file_memory_provider_params_t file_params_fsdax =
+    get_file_params_fsdax(getenv("UMF_TESTS_FSDAX_PATH"));
+
 HostMemoryAccessor hostAccessor;
 
 static std::vector<ipcTestParams> ipcManyPoolsTestParamsList = {
@@ -43,7 +53,36 @@ static std::vector<ipcTestParams> ipcManyPoolsTestParamsList = {
 #endif
 };
 
+static std::vector<ipcTestParams> getIpcFsDaxTestParamsList(void) {
+    std::vector<ipcTestParams> ipcFsDaxTestParamsList = {};
+
+    char *path = getenv("UMF_TESTS_FSDAX_PATH");
+    if (path == nullptr || path[0] == 0) {
+        // skipping the test, UMF_TESTS_FSDAX_PATH is not set
+        return ipcFsDaxTestParamsList;
+    }
+
+    ipcFsDaxTestParamsList = {
+// TODO: enable it when sizes of allocations in ipcFixtures.hpp are fixed
+//        {umfProxyPoolOps(), nullptr, umfFileMemoryProviderOps(),
+//         &file_params_fsdax, &hostAccessor, true},
+#ifdef UMF_POOL_JEMALLOC_ENABLED
+        {umfJemallocPoolOps(), nullptr, umfFileMemoryProviderOps(),
+         &file_params_fsdax, &hostAccessor, false},
+#endif
+#ifdef UMF_POOL_SCALABLE_ENABLED
+        {umfScalablePoolOps(), nullptr, umfFileMemoryProviderOps(),
+         &file_params_fsdax, &hostAccessor, false},
+#endif
+    };
+
+    return ipcFsDaxTestParamsList;
+}
+
 GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(umfIpcTest);
 
 INSTANTIATE_TEST_SUITE_P(FileProviderDifferentPoolsTest, umfIpcTest,
                          ::testing::ValuesIn(ipcManyPoolsTestParamsList));
+
+INSTANTIATE_TEST_SUITE_P(FileProviderDifferentPoolsFSDAXTest, umfIpcTest,
+                         ::testing::ValuesIn(getIpcFsDaxTestParamsList()));
diff --git a/test/supp/helgrind-umf_test-ipc.supp b/test/supp/helgrind-umf_test-ipc.supp
@@ -0,0 +1,16 @@
+{
+   False-positive race in critnib_insert (lack of instrumentation)
+   Helgrind:Race
+   fun:store
+   fun:critnib_insert
+   ...
+}
+
+{
+   False-positive race in critnib_find (lack of instrumentation)
+   Helgrind:Race
+   fun:find_predecessor
+   fun:find_le
+   fun:critnib_find
+   ...
+}
diff --git a/test/supp/helgrind-umf_test-provider_file_memory_ipc.supp b/test/supp/helgrind-umf_test-provider_file_memory_ipc.supp
@@ -6,3 +6,20 @@
    fun:umfOpenIPCHandle
    ...
 }
+
+{
+   False-positive race in critnib_insert (lack of instrumentation)
+   Helgrind:Race
+   fun:store
+   fun:critnib_insert
+   ...
+}
+
+{
+   False-positive race in critnib_find (lack of instrumentation)
+   Helgrind:Race
+   fun:find_predecessor
+   fun:find_le
+   fun:critnib_find
+   ...
+}
