Skip to content

Commit 1f695ad

Browse files
ldorauldorau
authored
Merge pull request #981 from ldorau/Make_coarse_alloc_return_aligned_address_when_alignment==0
Make coarse_alloc() return aligned address when alignment==0
2 parents 524354b + 62496ed commit 1f695ad

File tree

5 files changed

+80
-26
lines changed

5 files changed

+80
-26
lines changed

‎src/coarse/coarse.c

Lines changed: 26 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -744,12 +744,18 @@ static block_t *find_free_block(struct ravl *free_blocks, size_t size,
744744
size_t alignment,
745745
coarse_strategy_t allocation_strategy) {
746746
block_t *block;
747+
size_t new_size = size + alignment;
747748

748749
switch (allocation_strategy) {
749750
case UMF_COARSE_MEMORY_STRATEGY_FASTEST:
750751
// Always allocate a free block of the (size + alignment) size
751752
// and later cut out the properly aligned part leaving two remaining parts.
752-
return free_blocks_rm_ge(free_blocks, size + alignment, 0,
753+
if (new_size < size) {
754+
LOG_ERR("arithmetic overflow (size + alignment)");
755+
return NULL;
756+
}
757+
758+
return free_blocks_rm_ge(free_blocks, new_size, 0,
753759
CHECK_ONLY_THE_FIRST_BLOCK);
754760

755761
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,
760766
return block;
761767
}
762768

769+
if (new_size < size) {
770+
LOG_ERR("arithmetic overflow (size + alignment)");
771+
return NULL;
772+
}
773+
763774
// If not, use the `UMF_COARSE_MEMORY_STRATEGY_FASTEST` strategy.
764-
return free_blocks_rm_ge(free_blocks, size + alignment, 0,
775+
return free_blocks_rm_ge(free_blocks, new_size, 0,
765776
CHECK_ONLY_THE_FIRST_BLOCK);
766777

767778
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,
773784
return block;
774785
}
775786

787+
if (new_size < size) {
788+
LOG_ERR("arithmetic overflow (size + alignment)");
789+
return NULL;
790+
}
791+
776792
// If none of them had the correct alignment,
777793
// use the `UMF_COARSE_MEMORY_STRATEGY_FASTEST` strategy.
778-
return free_blocks_rm_ge(free_blocks, size + alignment, 0,
794+
return free_blocks_rm_ge(free_blocks, new_size, 0,
779795
CHECK_ONLY_THE_FIRST_BLOCK);
780796
}
781797

@@ -1017,17 +1033,17 @@ umf_result_t coarse_alloc(coarse_t *coarse, size_t size, size_t alignment,
10171033
}
10181034

10191035
// alignment must be a power of two and a multiple or a divider of the page size
1020-
if (alignment &&
1021-
((alignment & (alignment - 1)) || ((alignment % coarse->page_size) &&
1022-
(coarse->page_size % alignment)))) {
1036+
if (alignment == 0) {
1037+
alignment = coarse->page_size;
1038+
} else if ((alignment & (alignment - 1)) ||
1039+
((alignment % coarse->page_size) &&
1040+
(coarse->page_size % alignment))) {
10231041
LOG_ERR("wrong alignment: %zu (not a power of 2 or a multiple or a "
10241042
"divider of the page size (%zu))",
10251043
alignment, coarse->page_size);
10261044
return UMF_RESULT_ERROR_INVALID_ALIGNMENT;
1027-
}
1028-
1029-
if (IS_NOT_ALIGNED(alignment, coarse->page_size)) {
1030-
alignment = ALIGN_UP(alignment, coarse->page_size);
1045+
} else if (IS_NOT_ALIGNED(alignment, coarse->page_size)) {
1046+
alignment = ALIGN_UP_SAFE(alignment, coarse->page_size);
10311047
}
10321048

10331049
if (utils_mutex_lock(&coarse->lock) != 0) {

‎src/coarse/coarse.h

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,16 +34,16 @@ typedef struct coarse_callbacks_t {
3434

3535
// coarse library allocation strategy
3636
typedef enum coarse_strategy_t {
37+
// Check if the first free block of the 'size' size has the correct alignment.
38+
// If not, use the `UMF_COARSE_MEMORY_STRATEGY_FASTEST` strategy.
39+
UMF_COARSE_MEMORY_STRATEGY_FASTEST_BUT_ONE = 0,
40+
3741
// Always allocate a free block of the (size + alignment) size
3842
// and cut out the properly aligned part leaving two remaining parts.
3943
// It is the fastest strategy but causes memory fragmentation
4044
// when alignment is greater than 0.
4145
// It is the best strategy when alignment always equals 0.
42-
UMF_COARSE_MEMORY_STRATEGY_FASTEST = 0,
43-
44-
// Check if the first free block of the 'size' size has the correct alignment.
45-
// If not, use the `UMF_COARSE_MEMORY_STRATEGY_FASTEST` strategy.
46-
UMF_COARSE_MEMORY_STRATEGY_FASTEST_BUT_ONE,
46+
UMF_COARSE_MEMORY_STRATEGY_FASTEST,
4747

4848
// Look through all free blocks of the 'size' size
4949
// and choose the first one with the correct alignment.

‎src/utils/utils_common.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ void utils_align_ptr_up_size_down(void **ptr, size_t *size, size_t alignment) {
2525
}
2626

2727
ASSERT(IS_ALIGNED(p, alignment));
28-
ASSERT(IS_ALIGNED(s, alignment));
2928

3029
*ptr = (void *)p;
3130
*size = s;

‎test/coarse_lib.cpp

Lines changed: 42 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -166,9 +166,10 @@ TEST_P(CoarseWithMemoryStrategyTest, coarseTest_basic_provider) {
166166

167167
TEST_P(CoarseWithMemoryStrategyTest, coarseTest_basic_fixed_memory) {
168168
// preallocate some memory and initialize the vector with zeros
169-
const size_t buff_size = 20 * MB;
169+
const size_t buff_size = 20 * MB + coarse_params.page_size;
170170
std::vector<char> buffer(buff_size, 0);
171-
void *buf = (void *)buffer.data();
171+
void *buf = (void *)ALIGN_UP_SAFE((uintptr_t)buffer.data(),
172+
coarse_params.page_size);
172173
ASSERT_NE(buf, nullptr);
173174

174175
coarse_params.cb.alloc = NULL;
@@ -206,9 +207,10 @@ TEST_P(CoarseWithMemoryStrategyTest, coarseTest_basic_fixed_memory) {
206207

207208
TEST_P(CoarseWithMemoryStrategyTest, coarseTest_fixed_memory_various) {
208209
// preallocate some memory and initialize the vector with zeros
209-
const size_t buff_size = 20 * MB;
210+
const size_t buff_size = 20 * MB + coarse_params.page_size;
210211
std::vector<char> buffer(buff_size, 0);
211-
void *buf = (void *)buffer.data();
212+
void *buf = (void *)ALIGN_UP_SAFE((uintptr_t)buffer.data(),
213+
coarse_params.page_size);
212214
ASSERT_NE(buf, nullptr);
213215

214216
coarse_params.cb.alloc = NULL;
@@ -627,6 +629,15 @@ TEST_P(CoarseWithMemoryStrategyTest, coarseTest_basic_free_cb_fails) {
627629
}
628630

629631
TEST_P(CoarseWithMemoryStrategyTest, coarseTest_split_cb_fails) {
632+
if (coarse_params.allocation_strategy ==
633+
UMF_COARSE_MEMORY_STRATEGY_FASTEST) {
634+
// This test is designed for the UMF_COARSE_MEMORY_STRATEGY_FASTEST_BUT_ONE
635+
// and UMF_COARSE_MEMORY_STRATEGY_CHECK_ALL_SIZE strategies,
636+
// because the UMF_COARSE_MEMORY_STRATEGY_FASTEST strategy
637+
// looks always for a block of size greater by the page size.
638+
return;
639+
}
640+
630641
umf_memory_provider_handle_t malloc_memory_provider;
631642
umf_result = umfMemoryProviderCreate(&UMF_MALLOC_MEMORY_PROVIDER_OPS, NULL,
632643
&malloc_memory_provider);
@@ -702,9 +713,10 @@ TEST_P(CoarseWithMemoryStrategyTest, coarseTest_split_cb_fails) {
702713

703714
TEST_P(CoarseWithMemoryStrategyTest, coarseTest_merge_cb_fails) {
704715
// preallocate some memory and initialize the vector with zeros
705-
const size_t buff_size = 10 * MB;
716+
const size_t buff_size = 10 * MB + coarse_params.page_size;
706717
std::vector<char> buffer(buff_size, 0);
707-
void *buf = (void *)buffer.data();
718+
void *buf = (void *)ALIGN_UP_SAFE((uintptr_t)buffer.data(),
719+
coarse_params.page_size);
708720
ASSERT_NE(buf, nullptr);
709721

710722
coarse_params.cb.alloc = NULL;
@@ -901,6 +913,15 @@ TEST_P(CoarseWithMemoryStrategyTest, coarseTest_provider_alloc_not_set) {
901913
}
902914

903915
TEST_P(CoarseWithMemoryStrategyTest, coarseTest_basic) {
916+
if (coarse_params.allocation_strategy ==
917+
UMF_COARSE_MEMORY_STRATEGY_FASTEST) {
918+
// This test is designed for the UMF_COARSE_MEMORY_STRATEGY_FASTEST_BUT_ONE
919+
// and UMF_COARSE_MEMORY_STRATEGY_CHECK_ALL_SIZE strategies,
920+
// because the UMF_COARSE_MEMORY_STRATEGY_FASTEST strategy
921+
// looks always for a block of size greater by the page size.
922+
return;
923+
}
924+
904925
umf_memory_provider_handle_t malloc_memory_provider;
905926
umf_result = umfMemoryProviderCreate(&UMF_MALLOC_MEMORY_PROVIDER_OPS, NULL,
906927
&malloc_memory_provider);
@@ -1065,6 +1086,15 @@ TEST_P(CoarseWithMemoryStrategyTest, coarseTest_basic) {
10651086
}
10661087

10671088
TEST_P(CoarseWithMemoryStrategyTest, coarseTest_simple1) {
1089+
if (coarse_params.allocation_strategy ==
1090+
UMF_COARSE_MEMORY_STRATEGY_FASTEST) {
1091+
// This test is designed for the UMF_COARSE_MEMORY_STRATEGY_FASTEST_BUT_ONE
1092+
// and UMF_COARSE_MEMORY_STRATEGY_CHECK_ALL_SIZE strategies,
1093+
// because the UMF_COARSE_MEMORY_STRATEGY_FASTEST strategy
1094+
// looks always for a block of size greater by the page size.
1095+
return;
1096+
}
1097+
10681098
umf_memory_provider_handle_t malloc_memory_provider;
10691099
umf_result = umfMemoryProviderCreate(&UMF_MALLOC_MEMORY_PROVIDER_OPS, NULL,
10701100
&malloc_memory_provider);
@@ -1106,8 +1136,9 @@ TEST_P(CoarseWithMemoryStrategyTest, coarseTest_simple1) {
11061136
ASSERT_NE(t[i], nullptr);
11071137
}
11081138

1109-
if (max_alloc_size == 0) {
1110-
max_alloc_size = coarse_get_stats(ch).alloc_size;
1139+
size_t alloc_size = coarse_get_stats(ch).alloc_size;
1140+
if (alloc_size > max_alloc_size) {
1141+
max_alloc_size = alloc_size;
11111142
}
11121143

11131144
for (int i = 0; i < nptrs; i++) {
@@ -1253,9 +1284,10 @@ TEST_P(CoarseWithMemoryStrategyTest, coarseTest_alignment_provider) {
12531284

12541285
TEST_P(CoarseWithMemoryStrategyTest, coarseTest_alignment_fixed_memory) {
12551286
// preallocate some memory and initialize the vector with zeros
1256-
const size_t alloc_size = 40 * MB;
1287+
const size_t alloc_size = 40 * MB + coarse_params.page_size;
12571288
std::vector<char> buffer(alloc_size, 0);
1258-
void *buf = (void *)buffer.data();
1289+
void *buf = (void *)ALIGN_UP_SAFE((uintptr_t)buffer.data(),
1290+
coarse_params.page_size);
12591291
ASSERT_NE(buf, nullptr);
12601292

12611293
coarse_params.cb.alloc = NULL;

‎test/provider_devdax_memory.cpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -233,6 +233,13 @@ TEST_P(umfProviderTest, purge_force) {
233233
test_alloc_free_success(provider.get(), page_size, 0, PURGE_FORCE);
234234
}
235235

236+
TEST_P(umfProviderTest, purge_force_unalligned_alloc) {
237+
void *ptr;
238+
auto ret = umfMemoryProviderAlloc(provider.get(), page_plus_64, 0, &ptr);
239+
ASSERT_EQ(ret, UMF_RESULT_SUCCESS);
240+
test_alloc_free_success(provider.get(), page_size, 0, PURGE_FORCE);
241+
umfMemoryProviderFree(provider.get(), ptr, page_plus_64);
242+
}
236243
// negative tests using test_alloc_failure
237244

238245
TEST_P(umfProviderTest, alloc_page64_align_page_minus_1_WRONG_ALIGNMENT_1) {

0 commit comments

Comments
 (0)