From 2fca364ae3d116395d813380c23157837d3e57f6 Mon Sep 17 00:00:00 2001 From: Lukasz Dorau Date: Fri, 6 Dec 2024 17:11:14 +0100 Subject: [PATCH 1/4] Fix: remove incorrect assert in utils_align_ptr_up_size_down() Remove incorrect assert in utils_align_ptr_up_size_down(). A pointer is aligned, but a size is only adjusted to the new pointer. The size does not have to be aligned. Signed-off-by: Lukasz Dorau --- src/utils/utils_common.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/utils/utils_common.c b/src/utils/utils_common.c index bffc9f3559..eaf5420fc6 100644 --- a/src/utils/utils_common.c +++ b/src/utils/utils_common.c @@ -25,7 +25,6 @@ void utils_align_ptr_up_size_down(void **ptr, size_t *size, size_t alignment) { } ASSERT(IS_ALIGNED(p, alignment)); - ASSERT(IS_ALIGNED(s, alignment)); *ptr = (void *)p; *size = s; From 4b09af0ba6b9b55f3b8bcc9fd10d2626825424f4 Mon Sep 17 00:00:00 2001 From: Lukasz Dorau Date: Mon, 9 Dec 2024 11:25:24 +0100 Subject: [PATCH 2/4] Make coarse_alloc() return aligned address when alignment==0 Make coarse_alloc() always return address aligned to coarse->page_size when alignment==0. Signed-off-by: Lukasz Dorau --- src/coarse/coarse.c | 36 ++++++++++++++++++++++--------- test/coarse_lib.cpp | 52 ++++++++++++++++++++++++++++++++++++--------- 2 files changed, 68 insertions(+), 20 deletions(-) diff --git a/src/coarse/coarse.c b/src/coarse/coarse.c index 7294801544..0ce4ded3d3 100644 --- a/src/coarse/coarse.c +++ b/src/coarse/coarse.c @@ -744,12 +744,18 @@ static block_t *find_free_block(struct ravl *free_blocks, size_t size, size_t alignment, coarse_strategy_t allocation_strategy) { block_t *block; + size_t new_size = size + alignment; switch (allocation_strategy) { case UMF_COARSE_MEMORY_STRATEGY_FASTEST: // Always allocate a free block of the (size + alignment) size // and later cut out the properly aligned part leaving two remaining parts. - return free_blocks_rm_ge(free_blocks, size + alignment, 0, + if (new_size < size) { + LOG_ERR("arithmetic overflow (size + alignment)"); + return NULL; + } + + return free_blocks_rm_ge(free_blocks, new_size, 0, CHECK_ONLY_THE_FIRST_BLOCK); case UMF_COARSE_MEMORY_STRATEGY_FASTEST_BUT_ONE: @@ -760,8 +766,13 @@ static block_t *find_free_block(struct ravl *free_blocks, size_t size, return block; } + if (new_size < size) { + LOG_ERR("arithmetic overflow (size + alignment)"); + return NULL; + } + // If not, use the `UMF_COARSE_MEMORY_STRATEGY_FASTEST` strategy. - return free_blocks_rm_ge(free_blocks, size + alignment, 0, + return free_blocks_rm_ge(free_blocks, new_size, 0, CHECK_ONLY_THE_FIRST_BLOCK); case UMF_COARSE_MEMORY_STRATEGY_CHECK_ALL_SIZE: @@ -773,9 +784,14 @@ static block_t *find_free_block(struct ravl *free_blocks, size_t size, return block; } + if (new_size < size) { + LOG_ERR("arithmetic overflow (size + alignment)"); + return NULL; + } + // If none of them had the correct alignment, // use the `UMF_COARSE_MEMORY_STRATEGY_FASTEST` strategy. - return free_blocks_rm_ge(free_blocks, size + alignment, 0, + return free_blocks_rm_ge(free_blocks, new_size, 0, CHECK_ONLY_THE_FIRST_BLOCK); } @@ -1017,17 +1033,17 @@ umf_result_t coarse_alloc(coarse_t *coarse, size_t size, size_t alignment, } // alignment must be a power of two and a multiple or a divider of the page size - if (alignment && - ((alignment & (alignment - 1)) || ((alignment % coarse->page_size) && - (coarse->page_size % alignment)))) { + if (alignment == 0) { + alignment = coarse->page_size; + } else if ((alignment & (alignment - 1)) || + ((alignment % coarse->page_size) && + (coarse->page_size % alignment))) { LOG_ERR("wrong alignment: %zu (not a power of 2 or a multiple or a " "divider of the page size (%zu))", alignment, coarse->page_size); return UMF_RESULT_ERROR_INVALID_ALIGNMENT; - } - - if (IS_NOT_ALIGNED(alignment, coarse->page_size)) { - alignment = ALIGN_UP(alignment, coarse->page_size); + } else if (IS_NOT_ALIGNED(alignment, coarse->page_size)) { + alignment = ALIGN_UP_SAFE(alignment, coarse->page_size); } if (utils_mutex_lock(&coarse->lock) != 0) { diff --git a/test/coarse_lib.cpp b/test/coarse_lib.cpp index 6a3d9637ec..c5e30ee8fc 100644 --- a/test/coarse_lib.cpp +++ b/test/coarse_lib.cpp @@ -166,9 +166,10 @@ TEST_P(CoarseWithMemoryStrategyTest, coarseTest_basic_provider) { TEST_P(CoarseWithMemoryStrategyTest, coarseTest_basic_fixed_memory) { // preallocate some memory and initialize the vector with zeros - const size_t buff_size = 20 * MB; + const size_t buff_size = 20 * MB + coarse_params.page_size; std::vector buffer(buff_size, 0); - void *buf = (void *)buffer.data(); + void *buf = (void *)ALIGN_UP_SAFE((uintptr_t)buffer.data(), + coarse_params.page_size); ASSERT_NE(buf, nullptr); coarse_params.cb.alloc = NULL; @@ -206,9 +207,10 @@ TEST_P(CoarseWithMemoryStrategyTest, coarseTest_basic_fixed_memory) { TEST_P(CoarseWithMemoryStrategyTest, coarseTest_fixed_memory_various) { // preallocate some memory and initialize the vector with zeros - const size_t buff_size = 20 * MB; + const size_t buff_size = 20 * MB + coarse_params.page_size; std::vector buffer(buff_size, 0); - void *buf = (void *)buffer.data(); + void *buf = (void *)ALIGN_UP_SAFE((uintptr_t)buffer.data(), + coarse_params.page_size); ASSERT_NE(buf, nullptr); coarse_params.cb.alloc = NULL; @@ -627,6 +629,15 @@ TEST_P(CoarseWithMemoryStrategyTest, coarseTest_basic_free_cb_fails) { } TEST_P(CoarseWithMemoryStrategyTest, coarseTest_split_cb_fails) { + if (coarse_params.allocation_strategy == + UMF_COARSE_MEMORY_STRATEGY_FASTEST) { + // This test is designed for the UMF_COARSE_MEMORY_STRATEGY_FASTEST_BUT_ONE + // and UMF_COARSE_MEMORY_STRATEGY_CHECK_ALL_SIZE strategies, + // because the UMF_COARSE_MEMORY_STRATEGY_FASTEST strategy + // looks always for a block of size greater by the page size. + return; + } + umf_memory_provider_handle_t malloc_memory_provider; umf_result = umfMemoryProviderCreate(&UMF_MALLOC_MEMORY_PROVIDER_OPS, NULL, &malloc_memory_provider); @@ -702,9 +713,10 @@ TEST_P(CoarseWithMemoryStrategyTest, coarseTest_split_cb_fails) { TEST_P(CoarseWithMemoryStrategyTest, coarseTest_merge_cb_fails) { // preallocate some memory and initialize the vector with zeros - const size_t buff_size = 10 * MB; + const size_t buff_size = 10 * MB + coarse_params.page_size; std::vector buffer(buff_size, 0); - void *buf = (void *)buffer.data(); + void *buf = (void *)ALIGN_UP_SAFE((uintptr_t)buffer.data(), + coarse_params.page_size); ASSERT_NE(buf, nullptr); coarse_params.cb.alloc = NULL; @@ -901,6 +913,15 @@ TEST_P(CoarseWithMemoryStrategyTest, coarseTest_provider_alloc_not_set) { } TEST_P(CoarseWithMemoryStrategyTest, coarseTest_basic) { + if (coarse_params.allocation_strategy == + UMF_COARSE_MEMORY_STRATEGY_FASTEST) { + // This test is designed for the UMF_COARSE_MEMORY_STRATEGY_FASTEST_BUT_ONE + // and UMF_COARSE_MEMORY_STRATEGY_CHECK_ALL_SIZE strategies, + // because the UMF_COARSE_MEMORY_STRATEGY_FASTEST strategy + // looks always for a block of size greater by the page size. + return; + } + umf_memory_provider_handle_t malloc_memory_provider; umf_result = umfMemoryProviderCreate(&UMF_MALLOC_MEMORY_PROVIDER_OPS, NULL, &malloc_memory_provider); @@ -1065,6 +1086,15 @@ TEST_P(CoarseWithMemoryStrategyTest, coarseTest_basic) { } TEST_P(CoarseWithMemoryStrategyTest, coarseTest_simple1) { + if (coarse_params.allocation_strategy == + UMF_COARSE_MEMORY_STRATEGY_FASTEST) { + // This test is designed for the UMF_COARSE_MEMORY_STRATEGY_FASTEST_BUT_ONE + // and UMF_COARSE_MEMORY_STRATEGY_CHECK_ALL_SIZE strategies, + // because the UMF_COARSE_MEMORY_STRATEGY_FASTEST strategy + // looks always for a block of size greater by the page size. + return; + } + umf_memory_provider_handle_t malloc_memory_provider; umf_result = umfMemoryProviderCreate(&UMF_MALLOC_MEMORY_PROVIDER_OPS, NULL, &malloc_memory_provider); @@ -1106,8 +1136,9 @@ TEST_P(CoarseWithMemoryStrategyTest, coarseTest_simple1) { ASSERT_NE(t[i], nullptr); } - if (max_alloc_size == 0) { - max_alloc_size = coarse_get_stats(ch).alloc_size; + size_t alloc_size = coarse_get_stats(ch).alloc_size; + if (alloc_size > max_alloc_size) { + max_alloc_size = alloc_size; } for (int i = 0; i < nptrs; i++) { @@ -1253,9 +1284,10 @@ TEST_P(CoarseWithMemoryStrategyTest, coarseTest_alignment_provider) { TEST_P(CoarseWithMemoryStrategyTest, coarseTest_alignment_fixed_memory) { // preallocate some memory and initialize the vector with zeros - const size_t alloc_size = 40 * MB; + const size_t alloc_size = 40 * MB + coarse_params.page_size; std::vector buffer(alloc_size, 0); - void *buf = (void *)buffer.data(); + void *buf = (void *)ALIGN_UP_SAFE((uintptr_t)buffer.data(), + coarse_params.page_size); ASSERT_NE(buf, nullptr); coarse_params.cb.alloc = NULL; From d2ecbe9c44da0939a9f97edeabb82e0ff1a31c27 Mon Sep 17 00:00:00 2001 From: Lukasz Dorau Date: Mon, 9 Dec 2024 12:39:15 +0100 Subject: [PATCH 3/4] Make UMF_COARSE_MEMORY_STRATEGY_FASTEST_BUT_ONE the default strategy Make UMF_COARSE_MEMORY_STRATEGY_FASTEST_BUT_ONE the default strategy, because the alignment never equals 0 now. Signed-off-by: Lukasz Dorau --- src/coarse/coarse.h | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/coarse/coarse.h b/src/coarse/coarse.h index cd151ca27a..93ec990027 100644 --- a/src/coarse/coarse.h +++ b/src/coarse/coarse.h @@ -34,16 +34,16 @@ typedef struct coarse_callbacks_t { // coarse library allocation strategy typedef enum coarse_strategy_t { + // Check if the first free block of the 'size' size has the correct alignment. + // If not, use the `UMF_COARSE_MEMORY_STRATEGY_FASTEST` strategy. + UMF_COARSE_MEMORY_STRATEGY_FASTEST_BUT_ONE = 0, + // Always allocate a free block of the (size + alignment) size // and cut out the properly aligned part leaving two remaining parts. // It is the fastest strategy but causes memory fragmentation // when alignment is greater than 0. // It is the best strategy when alignment always equals 0. - UMF_COARSE_MEMORY_STRATEGY_FASTEST = 0, - - // Check if the first free block of the 'size' size has the correct alignment. - // If not, use the `UMF_COARSE_MEMORY_STRATEGY_FASTEST` strategy. - UMF_COARSE_MEMORY_STRATEGY_FASTEST_BUT_ONE, + UMF_COARSE_MEMORY_STRATEGY_FASTEST, // Look through all free blocks of the 'size' size // and choose the first one with the correct alignment. From 62496ed59ffe9321b1cd46db3b9c81412739607c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Plewa?= Date: Thu, 5 Dec 2024 18:27:01 +0100 Subject: [PATCH 4/4] add extra test for dax provider --- test/provider_devdax_memory.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/test/provider_devdax_memory.cpp b/test/provider_devdax_memory.cpp index afff1de4f7..1feaaaaa63 100644 --- a/test/provider_devdax_memory.cpp +++ b/test/provider_devdax_memory.cpp @@ -233,6 +233,13 @@ TEST_P(umfProviderTest, purge_force) { test_alloc_free_success(provider.get(), page_size, 0, PURGE_FORCE); } +TEST_P(umfProviderTest, purge_force_unalligned_alloc) { + void *ptr; + auto ret = umfMemoryProviderAlloc(provider.get(), page_plus_64, 0, &ptr); + ASSERT_EQ(ret, UMF_RESULT_SUCCESS); + test_alloc_free_success(provider.get(), page_size, 0, PURGE_FORCE); + umfMemoryProviderFree(provider.get(), ptr, page_plus_64); +} // negative tests using test_alloc_failure TEST_P(umfProviderTest, alloc_page64_align_page_minus_1_WRONG_ALIGNMENT_1) {