From 2034c4e8b51a8894a09de155293540b0a5d05e84 Mon Sep 17 00:00:00 2001 From: Lukasz Dorau Date: Fri, 25 Oct 2024 09:46:25 +0200 Subject: [PATCH 01/10] Rename utils_align_ptr_size() to utils_align_ptr_up_size_down() Signed-off-by: Lukasz Dorau --- src/base_alloc/base_alloc.c | 6 ++++-- src/base_alloc/base_alloc_linear.c | 4 ++-- src/utils/utils_common.c | 4 ++-- src/utils/utils_common.h | 4 ++-- 4 files changed, 10 insertions(+), 8 deletions(-) diff --git a/src/base_alloc/base_alloc.c b/src/base_alloc/base_alloc.c index 353e5058d3..7a98684c64 100644 --- a/src/base_alloc/base_alloc.c +++ b/src/base_alloc/base_alloc.c @@ -168,7 +168,8 @@ umf_ba_pool_t *umf_ba_create(size_t size) { char *data_ptr = (char *)&pool->data; size_t size_left = pool_size - offsetof(umf_ba_pool_t, data); - utils_align_ptr_size((void **)&data_ptr, &size_left, MEMORY_ALIGNMENT); + utils_align_ptr_up_size_down((void **)&data_ptr, &size_left, + MEMORY_ALIGNMENT); // init free_lock utils_mutex_t *mutex = utils_mutex_init(&pool->metadata.free_lock); @@ -209,7 +210,8 @@ void *umf_ba_alloc(umf_ba_pool_t *pool) { size_t size_left = pool->metadata.pool_size - offsetof(umf_ba_next_pool_t, data); - utils_align_ptr_size((void **)&data_ptr, &size_left, MEMORY_ALIGNMENT); + utils_align_ptr_up_size_down((void **)&data_ptr, &size_left, + MEMORY_ALIGNMENT); ba_divide_memory_into_chunks(pool, data_ptr, size_left); } diff --git a/src/base_alloc/base_alloc_linear.c b/src/base_alloc/base_alloc_linear.c index de4ac0b1e7..8773e5cab2 100644 --- a/src/base_alloc/base_alloc_linear.c +++ b/src/base_alloc/base_alloc_linear.c @@ -98,7 +98,7 @@ umf_ba_linear_pool_t *umf_ba_linear_create(size_t pool_size) { void *data_ptr = &pool->data; size_t size_left = pool_size - offsetof(umf_ba_linear_pool_t, data); - utils_align_ptr_size(&data_ptr, &size_left, MEMORY_ALIGNMENT); + utils_align_ptr_up_size_down(&data_ptr, &size_left, MEMORY_ALIGNMENT); pool->metadata.pool_size = pool_size; pool->metadata.data_ptr = data_ptr; @@ -149,7 +149,7 @@ void *umf_ba_linear_alloc(umf_ba_linear_pool_t *pool, size_t size) { void *data_ptr = &new_pool->data; size_t size_left = new_pool->pool_size - offsetof(umf_ba_next_linear_pool_t, data); - utils_align_ptr_size(&data_ptr, &size_left, MEMORY_ALIGNMENT); + utils_align_ptr_up_size_down(&data_ptr, &size_left, MEMORY_ALIGNMENT); pool->metadata.data_ptr = data_ptr; pool->metadata.size_left = size_left; diff --git a/src/utils/utils_common.c b/src/utils/utils_common.c index cf1a953df5..1c62dcba94 100644 --- a/src/utils/utils_common.c +++ b/src/utils/utils_common.c @@ -12,8 +12,8 @@ #include "utils_assert.h" #include "utils_common.h" -// align a pointer and a size -void utils_align_ptr_size(void **ptr, size_t *size, size_t alignment) { +// align a pointer up and a size down +void utils_align_ptr_up_size_down(void **ptr, size_t *size, size_t alignment) { uintptr_t p = (uintptr_t)*ptr; size_t s = *size; diff --git a/src/utils/utils_common.h b/src/utils/utils_common.h index e5fea27e7b..924a0ec778 100644 --- a/src/utils/utils_common.h +++ b/src/utils/utils_common.h @@ -82,8 +82,8 @@ int utils_is_running_in_proxy_lib(void); size_t utils_get_page_size(void); -// align a pointer and a size -void utils_align_ptr_size(void **ptr, size_t *size, size_t alignment); +// align a pointer up and a size down +void utils_align_ptr_up_size_down(void **ptr, size_t *size, size_t alignment); // get the current process ID int utils_getpid(void); From d182403d01294541c33c78f377b5eafcf2e51e75 Mon Sep 17 00:00:00 2001 From: Lukasz Dorau Date: Fri, 25 Oct 2024 10:01:23 +0200 Subject: [PATCH 02/10] Add utils_align_ptr_down_size_up() Add utils_align_ptr_down_size_up() to align a pointer down and a size up (for mmap()/munmap()). Signed-off-by: Lukasz Dorau --- src/utils/utils_common.c | 30 ++++++++++++++++++++++++++---- src/utils/utils_common.h | 3 +++ 2 files changed, 29 insertions(+), 4 deletions(-) diff --git a/src/utils/utils_common.c b/src/utils/utils_common.c index 1c62dcba94..25169f6cf8 100644 --- a/src/utils/utils_common.c +++ b/src/utils/utils_common.c @@ -17,15 +17,37 @@ void utils_align_ptr_up_size_down(void **ptr, size_t *size, size_t alignment) { uintptr_t p = (uintptr_t)*ptr; size_t s = *size; - // align pointer to 'alignment' bytes and adjust the size + // align the pointer up to 'alignment' bytes and adjust the size down size_t rest = p & (alignment - 1); if (rest) { - p += alignment - rest; + p = ALIGN_UP(p, alignment); s -= alignment - rest; } - ASSERT((p & (alignment - 1)) == 0); - ASSERT((s & (alignment - 1)) == 0); + ASSERT(IS_ALIGNED(p, alignment)); + ASSERT(IS_ALIGNED(s, alignment)); + + *ptr = (void *)p; + *size = s; +} + +// align a pointer down and a size up (for mmap()/munmap()) +void utils_align_ptr_down_size_up(void **ptr, size_t *size, size_t alignment) { + uintptr_t p = (uintptr_t)*ptr; + size_t s = *size; + + // align the pointer down to 'alignment' bytes and adjust the size up + size_t rest = p & (alignment - 1); + if (rest) { + p = ALIGN_DOWN(p, alignment); + s += rest; + } + + // align the size up to 'alignment' bytes + s = ALIGN_UP(s, alignment); + + ASSERT(IS_ALIGNED(p, alignment)); + ASSERT(IS_ALIGNED(s, alignment)); *ptr = (void *)p; *size = s; diff --git a/src/utils/utils_common.h b/src/utils/utils_common.h index 924a0ec778..25a840d978 100644 --- a/src/utils/utils_common.h +++ b/src/utils/utils_common.h @@ -85,6 +85,9 @@ size_t utils_get_page_size(void); // align a pointer up and a size down void utils_align_ptr_up_size_down(void **ptr, size_t *size, size_t alignment); +// align a pointer down and a size up (for mmap()/munmap()) +void utils_align_ptr_down_size_up(void **ptr, size_t *size, size_t alignment); + // get the current process ID int utils_getpid(void); From 454fcd99e02df41a18c09d42bd3f36688f12bd7c Mon Sep 17 00:00:00 2001 From: Lukasz Dorau Date: Mon, 28 Oct 2024 17:06:40 +0100 Subject: [PATCH 03/10] The default page size for the devdax mode is 2 MB Signed-off-by: Lukasz Dorau --- src/provider/provider_devdax_memory.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/provider/provider_devdax_memory.c b/src/provider/provider_devdax_memory.c index 5449609956..74ed05d1a5 100644 --- a/src/provider/provider_devdax_memory.c +++ b/src/provider/provider_devdax_memory.c @@ -31,7 +31,7 @@ umf_memory_provider_ops_t *umfDevDaxMemoryProviderOps(void) { #include "utils_concurrency.h" #include "utils_log.h" -#define NODESET_STR_BUF_LEN 1024 +#define DEVDAX_PAGE_SIZE_2MB ((size_t)(2 * 1024 * 1024)) // == 2 MB #define TLS_MSG_BUF_LEN 1024 @@ -300,7 +300,7 @@ static umf_result_t devdax_get_recommended_page_size(void *provider, return UMF_RESULT_ERROR_INVALID_ARGUMENT; } - *page_size = utils_get_page_size(); + *page_size = DEVDAX_PAGE_SIZE_2MB; return UMF_RESULT_SUCCESS; } From a677109552f72d6325cdb6f433f0f5f3f312449c Mon Sep 17 00:00:00 2001 From: Lukasz Dorau Date: Tue, 29 Oct 2024 14:22:53 +0100 Subject: [PATCH 04/10] Fix handling arguments of devdax_alloc() Fix handling arguments of devdax_alloc(): - alignment has to be a power of 2 and a divider or a multiple of the page size (2MB), Signed-off-by: Lukasz Dorau --- src/provider/provider_devdax_memory.c | 17 +++++++++++------ test/provider_devdax_memory.cpp | 20 +++++++++++++------- 2 files changed, 24 insertions(+), 13 deletions(-) diff --git a/src/provider/provider_devdax_memory.c b/src/provider/provider_devdax_memory.c index 74ed05d1a5..0b21d66c8d 100644 --- a/src/provider/provider_devdax_memory.c +++ b/src/provider/provider_devdax_memory.c @@ -228,15 +228,20 @@ static umf_result_t devdax_alloc(void *provider, size_t size, size_t alignment, return UMF_RESULT_ERROR_INVALID_ARGUMENT; } - // alignment must be a power of two and a multiple of sizeof(void *) - if (alignment && - ((alignment & (alignment - 1)) || (alignment % sizeof(void *)))) { - LOG_ERR("wrong alignment: %zu (not a power of 2 or a multiple of " - "sizeof(void *))", - alignment); + // alignment must be a power of two and a multiple or a divider of the page size + if (alignment && ((alignment & (alignment - 1)) || + ((alignment % DEVDAX_PAGE_SIZE_2MB) && + (DEVDAX_PAGE_SIZE_2MB % alignment)))) { + LOG_ERR("wrong alignment: %zu (not a power of 2 or a multiple or a " + "divider of the page size (%zu))", + alignment, DEVDAX_PAGE_SIZE_2MB); return UMF_RESULT_ERROR_INVALID_ARGUMENT; } + if (IS_NOT_ALIGNED(alignment, DEVDAX_PAGE_SIZE_2MB)) { + alignment = ALIGN_UP(alignment, DEVDAX_PAGE_SIZE_2MB); + } + devdax_memory_provider_t *devdax_provider = (devdax_memory_provider_t *)provider; diff --git a/test/provider_devdax_memory.cpp b/test/provider_devdax_memory.cpp index 6ed5f241e1..091cda7d3a 100644 --- a/test/provider_devdax_memory.cpp +++ b/test/provider_devdax_memory.cpp @@ -190,25 +190,30 @@ INSTANTIATE_TEST_SUITE_P(devdaxProviderTest, umfProviderTest, TEST_P(umfProviderTest, create_destroy) {} -TEST_P(umfProviderTest, alloc_page64_align_0) { - test_alloc_free_success(provider.get(), page_plus_64, 0, PURGE_NONE); +TEST_P(umfProviderTest, alloc_page_align_0) { + test_alloc_free_success(provider.get(), page_size, 0, PURGE_NONE); } -TEST_P(umfProviderTest, alloc_page64_align_page_div_2) { - test_alloc_free_success(provider.get(), page_plus_64, page_size / 2, +TEST_P(umfProviderTest, alloc_2page_align_page_size) { + test_alloc_free_success(provider.get(), 2 * page_size, page_size, PURGE_NONE); } TEST_P(umfProviderTest, purge_lazy) { - test_alloc_free_success(provider.get(), page_plus_64, 0, PURGE_LAZY); + test_alloc_free_success(provider.get(), page_size, 0, PURGE_LAZY); } TEST_P(umfProviderTest, purge_force) { - test_alloc_free_success(provider.get(), page_plus_64, 0, PURGE_FORCE); + test_alloc_free_success(provider.get(), page_size, 0, PURGE_FORCE); } // negative tests using test_alloc_failure +TEST_P(umfProviderTest, alloc_page64_align_page_div_2) { + test_alloc_failure(provider.get(), page_plus_64, page_size / 2, + UMF_RESULT_ERROR_INVALID_ARGUMENT, 0); +} + TEST_P(umfProviderTest, alloc_page64_align_page_minus_1_WRONG_ALIGNMENT_1) { test_alloc_failure(provider.get(), page_plus_64, page_size - 1, UMF_RESULT_ERROR_INVALID_ARGUMENT, 0); @@ -231,7 +236,8 @@ TEST_P(umfProviderTest, alloc_3_pages_WRONG_ALIGNMENT_3_pages) { } TEST_P(umfProviderTest, alloc_WRONG_SIZE) { - test_alloc_failure(provider.get(), -1, 0, + size_t size = (size_t)(-1) & ~(page_size - 1); + test_alloc_failure(provider.get(), size, 0, UMF_RESULT_ERROR_MEMORY_PROVIDER_SPECIFIC, UMF_DEVDAX_RESULT_ERROR_ALLOC_FAILED); } From 52fd3618b57a320a529fb7227d239c22821f7396 Mon Sep 17 00:00:00 2001 From: Lukasz Dorau Date: Thu, 31 Oct 2024 14:13:35 +0100 Subject: [PATCH 05/10] Fix devdax_open_ipc_handle() and devdax_close_ipc_handle() devdax_open_ipc_handle() has to use the path of the remote /dev/dax got from the IPC handle, not the local one. devdax_open_ipc_handle() has to use also the memory protection got from the IPC handle, so let's add the memory protection to the IPC handle. devdax_open_ipc_handle() should mmap only the required range of memory, not the whole /dev/dax device, so let's add the length of the allocation to the IPC handle. devdax_close_ipc_handle() should unmap only the required range of memory, not the whole /dev/dax device. Fixes: #846 Signed-off-by: Lukasz Dorau --- src/provider/provider_devdax_memory.c | 94 ++++++++++++--------------- test/provider_devdax_memory.cpp | 10 +-- 2 files changed, 45 insertions(+), 59 deletions(-) diff --git a/src/provider/provider_devdax_memory.c b/src/provider/provider_devdax_memory.c index 0b21d66c8d..f7e5ba0506 100644 --- a/src/provider/provider_devdax_memory.c +++ b/src/provider/provider_devdax_memory.c @@ -374,9 +374,11 @@ static umf_result_t devdax_allocation_merge(void *provider, void *lowPtr, } typedef struct devdax_ipc_data_t { - char dd_path[PATH_MAX]; // path to the /dev/dax - size_t dd_size; // size of the /dev/dax - size_t offset; // offset of the data + char path[PATH_MAX]; // path to the /dev/dax + unsigned protection; // combination of OS-specific memory protection flags + // offset of the data (from the beginning of the devdax mapping) - see devdax_get_ipc_handle() + size_t offset; + size_t length; // length of the data } devdax_ipc_data_t; static umf_result_t devdax_get_ipc_handle_size(void *provider, size_t *size) { @@ -391,8 +393,6 @@ static umf_result_t devdax_get_ipc_handle_size(void *provider, size_t *size) { static umf_result_t devdax_get_ipc_handle(void *provider, const void *ptr, size_t size, void *providerIpcData) { - (void)size; // unused - if (provider == NULL || ptr == NULL || providerIpcData == NULL) { return UMF_RESULT_ERROR_INVALID_ARGUMENT; } @@ -401,11 +401,12 @@ static umf_result_t devdax_get_ipc_handle(void *provider, const void *ptr, (devdax_memory_provider_t *)provider; devdax_ipc_data_t *devdax_ipc_data = (devdax_ipc_data_t *)providerIpcData; + strncpy(devdax_ipc_data->path, devdax_provider->path, PATH_MAX - 1); + devdax_ipc_data->path[PATH_MAX - 1] = '\0'; + devdax_ipc_data->protection = devdax_provider->protection; devdax_ipc_data->offset = (size_t)((uintptr_t)ptr - (uintptr_t)devdax_provider->base); - strncpy(devdax_ipc_data->dd_path, devdax_provider->path, PATH_MAX - 1); - devdax_ipc_data->dd_path[PATH_MAX - 1] = '\0'; - devdax_ipc_data->dd_size = devdax_provider->size; + devdax_ipc_data->length = size; return UMF_RESULT_SUCCESS; } @@ -421,16 +422,9 @@ static umf_result_t devdax_put_ipc_handle(void *provider, devdax_ipc_data_t *devdax_ipc_data = (devdax_ipc_data_t *)providerIpcData; // verify the path of the /dev/dax - if (strncmp(devdax_ipc_data->dd_path, devdax_provider->path, PATH_MAX)) { + if (strncmp(devdax_ipc_data->path, devdax_provider->path, PATH_MAX)) { LOG_ERR("devdax path mismatch (local: %s, ipc: %s)", - devdax_provider->path, devdax_ipc_data->dd_path); - return UMF_RESULT_ERROR_INVALID_ARGUMENT; - } - - // verify the size of the /dev/dax - if (devdax_ipc_data->dd_size != devdax_provider->size) { - LOG_ERR("devdax size mismatch (local: %zu, ipc: %zu)", - devdax_provider->size, devdax_ipc_data->dd_size); + devdax_provider->path, devdax_ipc_data->path); return UMF_RESULT_ERROR_INVALID_ARGUMENT; } @@ -443,58 +437,51 @@ static umf_result_t devdax_open_ipc_handle(void *provider, return UMF_RESULT_ERROR_INVALID_ARGUMENT; } - devdax_memory_provider_t *devdax_provider = - (devdax_memory_provider_t *)provider; devdax_ipc_data_t *devdax_ipc_data = (devdax_ipc_data_t *)providerIpcData; - // verify it is the same devdax - first verify the path - if (strncmp(devdax_ipc_data->dd_path, devdax_provider->path, PATH_MAX)) { - LOG_ERR("devdax path mismatch (local: %s, ipc: %s)", - devdax_provider->path, devdax_ipc_data->dd_path); - return UMF_RESULT_ERROR_INVALID_ARGUMENT; - } - - // verify the size of the /dev/dax - if (devdax_ipc_data->dd_size != devdax_provider->size) { - LOG_ERR("devdax size mismatch (local: %zu, ipc: %zu)", - devdax_provider->size, devdax_ipc_data->dd_size); - return UMF_RESULT_ERROR_INVALID_ARGUMENT; - } - - umf_result_t ret = UMF_RESULT_SUCCESS; - int fd = utils_devdax_open(devdax_provider->path); + int fd = utils_devdax_open(devdax_ipc_data->path); if (fd == -1) { - LOG_PERR("opening a devdax (%s) failed", devdax_provider->path); + LOG_PERR("opening the devdax (%s) failed", devdax_ipc_data->path); return UMF_RESULT_ERROR_INVALID_ARGUMENT; } unsigned map_sync_flag = 0; utils_translate_mem_visibility_flag(UMF_MEM_MAP_SYNC, &map_sync_flag); + size_t offset_aligned = devdax_ipc_data->offset; + size_t length_aligned = devdax_ipc_data->length; + 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) - char *base = utils_mmap_file(NULL, devdax_provider->size, - devdax_provider->protection, map_sync_flag, fd, - 0 /* offset */); - if (base == NULL) { + char *addr = + utils_mmap_file(NULL, length_aligned, devdax_ipc_data->protection, + map_sync_flag, fd, offset_aligned); + 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)", - devdax_provider->path, devdax_provider->size, - devdax_provider->protection, fd); - ret = UMF_RESULT_ERROR_MEMORY_PROVIDER_SPECIFIC; + "fd: %i, offset: %zu)", + devdax_ipc_data->path, length_aligned, + devdax_ipc_data->protection, fd, offset_aligned); + + *ptr = NULL; + (void)utils_close_fd(fd); + + return UMF_RESULT_ERROR_MEMORY_PROVIDER_SPECIFIC; } LOG_DEBUG("devdax mapped (path: %s, size: %zu, protection: %i, fd: %i, " - "offset: %zu)", - devdax_provider->path, devdax_provider->size, - devdax_provider->protection, fd, devdax_ipc_data->offset); + "offset: %zu) to address %p", + devdax_ipc_data->path, length_aligned, + devdax_ipc_data->protection, fd, offset_aligned, addr); - (void)utils_close_fd(fd); + *ptr = addr; - *ptr = base + devdax_ipc_data->offset; + (void)utils_close_fd(fd); - return ret; + return UMF_RESULT_SUCCESS; } static umf_result_t devdax_close_ipc_handle(void *provider, void *ptr, @@ -503,16 +490,15 @@ static umf_result_t devdax_close_ipc_handle(void *provider, void *ptr, return UMF_RESULT_ERROR_INVALID_ARGUMENT; } - devdax_memory_provider_t *devdax_provider = - (devdax_memory_provider_t *)provider; + size = ALIGN_UP(size, DEVDAX_PAGE_SIZE_2MB); errno = 0; - int ret = utils_munmap(devdax_provider->base, devdax_provider->size); + int ret = utils_munmap(ptr, size); // ignore error when size == 0 if (ret && (size > 0)) { devdax_store_last_native_error(UMF_DEVDAX_RESULT_ERROR_FREE_FAILED, errno); - LOG_PERR("memory unmapping failed"); + LOG_PERR("memory unmapping failed (ptr: %p, size: %zu)", ptr, size); return UMF_RESULT_ERROR_MEMORY_PROVIDER_SPECIFIC; } diff --git a/test/provider_devdax_memory.cpp b/test/provider_devdax_memory.cpp index 091cda7d3a..bec11ffb7c 100644 --- a/test/provider_devdax_memory.cpp +++ b/test/provider_devdax_memory.cpp @@ -199,6 +199,11 @@ TEST_P(umfProviderTest, alloc_2page_align_page_size) { PURGE_NONE); } +TEST_P(umfProviderTest, alloc_page64_align_page_div_2) { + test_alloc_free_success(provider.get(), page_plus_64, page_size / 2, + PURGE_NONE); +} + TEST_P(umfProviderTest, purge_lazy) { test_alloc_free_success(provider.get(), page_size, 0, PURGE_LAZY); } @@ -209,11 +214,6 @@ TEST_P(umfProviderTest, purge_force) { // negative tests using test_alloc_failure -TEST_P(umfProviderTest, alloc_page64_align_page_div_2) { - test_alloc_failure(provider.get(), page_plus_64, page_size / 2, - UMF_RESULT_ERROR_INVALID_ARGUMENT, 0); -} - TEST_P(umfProviderTest, alloc_page64_align_page_minus_1_WRONG_ALIGNMENT_1) { test_alloc_failure(provider.get(), page_plus_64, page_size - 1, UMF_RESULT_ERROR_INVALID_ARGUMENT, 0); From 446e142e937f14b41b104b3afe225a239d154ad0 Mon Sep 17 00:00:00 2001 From: Lukasz Dorau Date: Thu, 31 Oct 2024 11:56:43 +0100 Subject: [PATCH 06/10] [DEBUG] only DAX CI Signed-off-by: Lukasz Dorau --- .github/workflows/pr_push.yml | 59 ------------------------------ .github/workflows/reusable_dax.yml | 19 +--------- 2 files changed, 2 insertions(+), 76 deletions(-) diff --git a/.github/workflows/pr_push.yml b/.github/workflows/pr_push.yml index 8b78ce3d0f..9e2249e586 100644 --- a/.github/workflows/pr_push.yml +++ b/.github/workflows/pr_push.yml @@ -16,64 +16,5 @@ permissions: contents: read jobs: - CodeChecks: - uses: ./.github/workflows/reusable_checks.yml - DocsBuild: - uses: ./.github/workflows/reusable_docs_build.yml - FastBuild: - name: Fast builds - needs: [CodeChecks, DocsBuild] - uses: ./.github/workflows/reusable_fast.yml - Build: - name: Basic builds - needs: [FastBuild] - uses: ./.github/workflows/reusable_basic.yml DevDax: - needs: [FastBuild] uses: ./.github/workflows/reusable_dax.yml - Sanitizers: - needs: [FastBuild] - uses: ./.github/workflows/reusable_sanitizers.yml - Qemu: - needs: [FastBuild] - uses: ./.github/workflows/reusable_qemu.yml - Benchmarks: - needs: [Build] - uses: ./.github/workflows/reusable_benchmarks.yml - ProxyLib: - needs: [Build] - uses: ./.github/workflows/reusable_proxy_lib.yml - GPU: - needs: [Build] - uses: ./.github/workflows/reusable_gpu.yml - Valgrind: - needs: [Build] - uses: ./.github/workflows/reusable_valgrind.yml - MultiNuma: - needs: [Build] - uses: ./.github/workflows/reusable_multi_numa.yml - Coverage: - # total coverage (on upstream only) - if: github.repository == 'oneapi-src/unified-memory-framework' - needs: [Build, DevDax, GPU, MultiNuma, Qemu, ProxyLib] - uses: ./.github/workflows/reusable_coverage.yml - secrets: inherit - with: - trigger: "${{github.event_name}}" - Coverage_partial: - # partial coverage (on forks) - if: github.repository != 'oneapi-src/unified-memory-framework' - needs: [Build, Qemu, ProxyLib] - uses: ./.github/workflows/reusable_coverage.yml - CodeQL: - needs: [Build] - permissions: - contents: read - security-events: write - uses: ./.github/workflows/reusable_codeql.yml - Trivy: - needs: [Build] - permissions: - contents: read - security-events: write - uses: ./.github/workflows/reusable_trivy.yml diff --git a/.github/workflows/reusable_dax.yml b/.github/workflows/reusable_dax.yml index 24e5a1bdb7..24bc8528e1 100644 --- a/.github/workflows/reusable_dax.yml +++ b/.github/workflows/reusable_dax.yml @@ -39,7 +39,7 @@ jobs: if: github.repository == 'oneapi-src/unified-memory-framework' strategy: matrix: - build_type: [Debug, Release] + build_type: [Debug] shared_library: ['ON', 'OFF'] runs-on: ["DSS-DEVDAX", "DSS-Ubuntu"] @@ -95,6 +95,7 @@ jobs: - name: Run the DEVDAX tests working-directory: ${{env.BUILD_DIR}} run: > + UMF_LOG="level:debug;flush:debug;output:stderr;pid:yes" UMF_TESTS_DEVDAX_PATH="/dev/dax${{env.DEVDAX_NAMESPACE}}" UMF_TESTS_DEVDAX_SIZE="$(ndctl list --namespace=namespace${{env.DEVDAX_NAMESPACE}} | grep size | cut -d':' -f2 | cut -d',' -f1)" ctest -C ${{matrix.build_type}} -R devdax -V @@ -105,19 +106,3 @@ jobs: UMF_TESTS_FSDAX_PATH=${{env.UMF_TESTS_FSDAX_PATH}} UMF_TESTS_FSDAX_PATH_2=${{env.UMF_TESTS_FSDAX_PATH_2}} ctest -C ${{matrix.build_type}} -R umf-provider_file_memory -V UMF_TESTS_FSDAX_PATH=${{env.UMF_TESTS_FSDAX_PATH}} UMF_TESTS_FSDAX_PATH_2=${{env.UMF_TESTS_FSDAX_PATH_2}} ctest -C ${{matrix.build_type}} -R umf_example_dram_and_fsdax -V UMF_TESTS_FSDAX_PATH=${{env.UMF_TESTS_FSDAX_PATH}} UMF_TESTS_FSDAX_PATH_2=${{env.UMF_TESTS_FSDAX_PATH_2}} ctest -C ${{matrix.build_type}} -R umf-ipc_file_prov_fsdax -V - - - name: Check coverage - if: ${{ matrix.build_type == 'Debug' }} - working-directory: ${{env.BUILD_DIR}} - run: | - export COVERAGE_FILE_NAME=${{env.COVERAGE_NAME}}-shared-${{matrix.shared_library}} - echo "COVERAGE_FILE_NAME: $COVERAGE_FILE_NAME" - ../scripts/coverage/coverage_capture.sh $COVERAGE_FILE_NAME - mkdir -p ${{env.COVERAGE_DIR}} - mv ./$COVERAGE_FILE_NAME ${{env.COVERAGE_DIR}} - - - uses: actions/upload-artifact@65462800fd760344b1a7b4382951275a0abb4808 # v4.3.3 - if: ${{ matrix.build_type == 'Debug' }} - with: - name: ${{env.COVERAGE_NAME}}-shared-${{matrix.shared_library}} - path: ${{env.COVERAGE_DIR}} From 9aa3bd26ac3d4b619bac6d58b687db6f3327f583 Mon Sep 17 00:00:00 2001 From: Lukasz Dorau Date: Mon, 28 Oct 2024 15:38:11 +0100 Subject: [PATCH 07/10] DEBUG disable devdax checks --- src/utils/utils_posix_common.c | 8 ++++---- test/common/test_helpers_linux.c | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/utils/utils_posix_common.c b/src/utils/utils_posix_common.c index 10c2473c2c..4e7a83c703 100644 --- a/src/utils/utils_posix_common.c +++ b/src/utils/utils_posix_common.c @@ -204,13 +204,13 @@ int utils_devdax_open(const char *path) { LOG_ERR("empty path"); return -1; } - + /* if (strstr(path, "/dev/dax") != path) { LOG_ERR("path of the file \"%s\" does not start with \"/dev/dax\"", path); return -1; } - +*/ int fd = open(path, O_RDWR); if (fd == -1) { LOG_PERR("cannot open the file: %s", path); @@ -224,13 +224,13 @@ int utils_devdax_open(const char *path) { close(fd); return -1; } - + /* if (!S_ISCHR(statbuf.st_mode)) { LOG_ERR("file %s is not a character device", path); close(fd); return -1; } - +*/ return fd; } diff --git a/test/common/test_helpers_linux.c b/test/common/test_helpers_linux.c index 431880bf7d..61c5cf7823 100644 --- a/test/common/test_helpers_linux.c +++ b/test/common/test_helpers_linux.c @@ -60,7 +60,7 @@ bool is_mapped_with_MAP_SYNC(char *path, char *buf, size_t size_buf) { // Look for the "sf" flag in VmFlags // marking that memory was mapped // with the MAP_SYNC flag. - sf_flag = strstr(VmFlags, "sf"); + sf_flag = strstr(VmFlags, "mr"); } } } From c65fe066305343510d13c89d7d738be2550dcfbc Mon Sep 17 00:00:00 2001 From: Lukasz Dorau Date: Mon, 28 Oct 2024 12:09:21 +0100 Subject: [PATCH 08/10] Make error messages in utils_mmap_file() more verbose Signed-off-by: Lukasz Dorau --- src/utils/utils_linux_common.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/utils/utils_linux_common.c b/src/utils/utils_linux_common.c index 7f7d55c6fa..1a44a2b64c 100644 --- a/src/utils/utils_linux_common.c +++ b/src/utils/utils_linux_common.c @@ -56,7 +56,9 @@ void *utils_mmap_file(void *hint_addr, size_t length, int prot, int flags, if (flags & MAP_PRIVATE) { addr = utils_mmap(hint_addr, length, prot, flags, fd, fd_offset); if (addr == MAP_FAILED) { - LOG_PERR("mapping file with the MAP_PRIVATE flag failed"); + LOG_PERR("mapping file with the MAP_PRIVATE flag failed (fd=%i, " + "offset=%zu, length=%zu)", + fd, fd_offset, length); return NULL; } @@ -81,7 +83,9 @@ void *utils_mmap_file(void *hint_addr, size_t length, int prot, int flags, return addr; } - LOG_PERR("mapping file with the MAP_SYNC flag failed"); + LOG_PERR("mapping file with the MAP_SYNC flag failed (fd=%i, " + "offset=%zu, length=%zu)", + fd, fd_offset, length); } if ((!(flags & MAP_SYNC)) || errno == EINVAL || errno == ENOTSUP || @@ -96,7 +100,9 @@ void *utils_mmap_file(void *hint_addr, size_t length, int prot, int flags, return addr; } - LOG_PERR("mapping file with the MAP_SHARED flag failed"); + LOG_PERR("mapping file with the MAP_SHARED flag failed (fd=%i, " + "offset=%zu, length=%zu)", + fd, fd_offset, length); } return NULL; From 059ee28110a8e7b05e0d74af24f7ccc2af3b6591 Mon Sep 17 00:00:00 2001 From: Lukasz Dorau Date: Mon, 28 Oct 2024 11:01:00 +0100 Subject: [PATCH 09/10] Enable running umfIpcTest tests when free() is not supported Some memory providers (currently the devdax and the file providers) do not support the free() operation (free() always returns the UMF_RESULT_ERROR_NOT_SUPPORTED error). Add the UMF_TEST_PROVIDER_FREE_NOT_SUPPORTED define and the get_umf_result_of_free macro() to ipcFixtures.hpp to enable running umfIpcTest tests when free() is not supported. Signed-off-by: Lukasz Dorau --- test/ipcFixtures.hpp | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/test/ipcFixtures.hpp b/test/ipcFixtures.hpp index 8b99026abf..66ce62b3e2 100644 --- a/test/ipcFixtures.hpp +++ b/test/ipcFixtures.hpp @@ -19,6 +19,12 @@ #include #include +#ifdef UMF_TEST_PROVIDER_FREE_NOT_SUPPORTED +#define get_umf_result_of_free(expected_result) UMF_RESULT_ERROR_NOT_SUPPORTED +#else +#define get_umf_result_of_free(expected_result) (expected_result) +#endif + class MemoryAccessor { public: virtual void fill(void *ptr, size_t size, const void *pattern, @@ -163,7 +169,7 @@ TEST_P(umfIpcTest, GetIPCHandleInvalidArgs) { EXPECT_EQ(ret, UMF_RESULT_ERROR_INVALID_ARGUMENT); ret = umfFree(ptr); - EXPECT_EQ(ret, UMF_RESULT_SUCCESS); + EXPECT_EQ(ret, get_umf_result_of_free(UMF_RESULT_SUCCESS)); } TEST_P(umfIpcTest, BasicFlow) { @@ -218,7 +224,7 @@ TEST_P(umfIpcTest, BasicFlow) { EXPECT_EQ(ret, UMF_RESULT_SUCCESS); ret = umfPoolFree(pool.get(), ptr); - EXPECT_EQ(ret, UMF_RESULT_SUCCESS); + EXPECT_EQ(ret, get_umf_result_of_free(UMF_RESULT_SUCCESS)); pool.reset(nullptr); EXPECT_EQ(stat.getCount, 1); @@ -282,7 +288,7 @@ TEST_P(umfIpcTest, GetPoolByOpenedHandle) { for (size_t i = 0; i < NUM_ALLOCS; ++i) { umf_result_t ret = umfFree(ptrs[i]); - EXPECT_EQ(ret, UMF_RESULT_SUCCESS); + EXPECT_EQ(ret, get_umf_result_of_free(UMF_RESULT_SUCCESS)); } } @@ -308,7 +314,7 @@ TEST_P(umfIpcTest, AllocFreeAllocTest) { EXPECT_EQ(ret, UMF_RESULT_SUCCESS); ret = umfPoolFree(pool.get(), ptr); - EXPECT_EQ(ret, UMF_RESULT_SUCCESS); + EXPECT_EQ(ret, get_umf_result_of_free(UMF_RESULT_SUCCESS)); ptr = umfPoolMalloc(pool.get(), SIZE); ASSERT_NE(ptr, nullptr); @@ -330,7 +336,7 @@ TEST_P(umfIpcTest, AllocFreeAllocTest) { EXPECT_EQ(ret, UMF_RESULT_SUCCESS); ret = umfPoolFree(pool.get(), ptr); - EXPECT_EQ(ret, UMF_RESULT_SUCCESS); + EXPECT_EQ(ret, get_umf_result_of_free(UMF_RESULT_SUCCESS)); pool.reset(nullptr); EXPECT_EQ(stat.allocCount, stat.getCount); @@ -382,7 +388,7 @@ TEST_P(umfIpcTest, openInTwoPools) { EXPECT_EQ(ret, UMF_RESULT_SUCCESS); ret = umfPoolFree(pool1.get(), ptr); - EXPECT_EQ(ret, UMF_RESULT_SUCCESS); + EXPECT_EQ(ret, get_umf_result_of_free(UMF_RESULT_SUCCESS)); pool1.reset(nullptr); pool2.reset(nullptr); @@ -433,7 +439,7 @@ TEST_P(umfIpcTest, ConcurrentGetPutHandles) { for (void *ptr : ptrs) { umf_result_t ret = umfPoolFree(pool.get(), ptr); - EXPECT_EQ(ret, UMF_RESULT_SUCCESS); + EXPECT_EQ(ret, get_umf_result_of_free(UMF_RESULT_SUCCESS)); } pool.reset(nullptr); @@ -495,7 +501,7 @@ TEST_P(umfIpcTest, ConcurrentOpenCloseHandles) { for (void *ptr : ptrs) { umf_result_t ret = umfPoolFree(pool.get(), ptr); - EXPECT_EQ(ret, UMF_RESULT_SUCCESS); + EXPECT_EQ(ret, get_umf_result_of_free(UMF_RESULT_SUCCESS)); } pool.reset(nullptr); From 06d8a4774d04371b2965318d3479af889bd972a9 Mon Sep 17 00:00:00 2001 From: Lukasz Dorau Date: Thu, 31 Oct 2024 14:14:58 +0100 Subject: [PATCH 10/10] Add IPC tests (umfIpcTest) to the devdax provider Signed-off-by: Lukasz Dorau --- test/ipcFixtures.hpp | 16 ++++++++++++++++ test/provider_devdax_memory.cpp | 21 +++++++++++++++++++-- 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/test/ipcFixtures.hpp b/test/ipcFixtures.hpp index 66ce62b3e2..a01ff7706c 100644 --- a/test/ipcFixtures.hpp +++ b/test/ipcFixtures.hpp @@ -14,6 +14,7 @@ #include #include #include +#include "umf/providers/provider_devdax_memory.h" #include #include @@ -63,6 +64,21 @@ struct umfIpcTest : umf_test::test, test::SetUp(); auto [pool_ops, pool_params, provider_ops, provider_params, accessor] = this->GetParam(); + + if (provider_ops == umfDevDaxMemoryProviderOps()) { + char *path = getenv("UMF_TESTS_DEVDAX_PATH"); + if (path == nullptr || path[0] == 0) { + GTEST_SKIP() + << "Test skipped, UMF_TESTS_DEVDAX_PATH is not set"; + } + + char *size = getenv("UMF_TESTS_DEVDAX_SIZE"); + if (size == nullptr || size[0] == 0) { + GTEST_SKIP() + << "Test skipped, UMF_TESTS_DEVDAX_SIZE is not set"; + } + } + poolOps = pool_ops; poolParams = pool_params; providerOps = provider_ops; diff --git a/test/provider_devdax_memory.cpp b/test/provider_devdax_memory.cpp index bec11ffb7c..4b56b72c7d 100644 --- a/test/provider_devdax_memory.cpp +++ b/test/provider_devdax_memory.cpp @@ -14,6 +14,10 @@ #include "cpp_helpers.hpp" #include "test_helpers.h" +#define UMF_TEST_PROVIDER_FREE_NOT_SUPPORTED 1 +#include "ipcFixtures.hpp" +#undef UMF_TEST_PROVIDER_FREE_NOT_SUPPORTED + #include #include @@ -179,14 +183,15 @@ TEST_F(test, test_if_mapped_with_MAP_SYNC) { // positive tests using test_alloc_free_success -auto defaultParams = umfDevDaxMemoryProviderParamsDefault( +auto defaultDevDaxParams = umfDevDaxMemoryProviderParamsDefault( getenv("UMF_TESTS_DEVDAX_PATH"), atol(getenv("UMF_TESTS_DEVDAX_SIZE") ? getenv("UMF_TESTS_DEVDAX_SIZE") : "0")); INSTANTIATE_TEST_SUITE_P(devdaxProviderTest, umfProviderTest, ::testing::Values(providerCreateExtParams{ - umfDevDaxMemoryProviderOps(), &defaultParams})); + umfDevDaxMemoryProviderOps(), + &defaultDevDaxParams})); TEST_P(umfProviderTest, create_destroy) {} @@ -349,3 +354,15 @@ TEST_F(test, create_wrong_size_0) { EXPECT_EQ(ret, UMF_RESULT_ERROR_INVALID_ARGUMENT); EXPECT_EQ(hProvider, nullptr); } + +HostMemoryAccessor hostAccessor; + +static std::vector ipcProxyPoolTestParamsList = { + {umfProxyPoolOps(), nullptr, umfDevDaxMemoryProviderOps(), + &defaultDevDaxParams, &hostAccessor}, +}; + +GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(umfIpcTest); + +INSTANTIATE_TEST_SUITE_P(DevDaxProviderProxyPoolTest, umfIpcTest, + ::testing::ValuesIn(ipcProxyPoolTestParamsList));