Skip to content

Commit

Permalink
Bugfix: alignment alloc
Browse files Browse the repository at this point in the history
  • Loading branch information
Keita Iwabuchi committed Oct 5, 2024
1 parent 317fad2 commit 084362b
Show file tree
Hide file tree
Showing 6 changed files with 26 additions and 13 deletions.
7 changes: 4 additions & 3 deletions include/metall/basic_manager.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -902,9 +902,10 @@ class basic_manager {
/// \copydoc doc_thread_safe_alloc
///
/// \param nbytes Number of bytes to allocate. Must
/// be a multiple alignment. \param alignment Alignment size. Alignment must
/// be a power of two and satisfy [min allocation size, chunk size]. \return
/// Returns a pointer to the allocated memory.
/// be a multiple alignment.
/// \param alignment Alignment size. Alignment must be a power of two and
/// satisfy [min allocation size, system page size].
/// \return Returns a pointer to the allocated memory.
void *allocate_aligned(size_type nbytes, size_type alignment) noexcept {
if (!check_sanity()) {
return nullptr;
Expand Down
2 changes: 1 addition & 1 deletion include/metall/detail/mmap.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,7 @@ inline void *reserve_aligned_vm_region(const size_t alignment,
if (length % alignment != 0) {
std::stringstream ss;
ss << "length (" << length << ") is not a multiple of alignment ("
<< ::sysconf(_SC_PAGE_SIZE) << ")";
<< alignment << ")";
logger::out(logger::level::error, __FILE__, __LINE__, ss.str().c_str());
return nullptr;
}
Expand Down
1 change: 1 addition & 0 deletions include/metall/kernel/manager_kernel_impl.ipp
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ void *manager_kernel<st, sst, cn, cs>::allocate_aligned(
}
assert(offset >= 0);

assert((std::ptrdiff_t)m_segment_storage.get_segment() % alignment == 0);
auto *addr = priv_to_address(offset);
assert((uint64_t)addr % alignment == 0);
return addr;
Expand Down
12 changes: 8 additions & 4 deletions include/metall/kernel/segment_allocator.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -164,10 +164,10 @@ class segment_allocator {
/// within this segment, i.e., this function does not know the address this
/// segment is mapped to. \param nbytes A size to allocate. Must be a multiple
/// of alignment. \param alignment An alignment requirement. Alignment must be
/// a power of two and satisfy [min allocation size, chunk size]. \return On
/// success, returns the pointer to the beginning of newly allocated memory.
/// Returns k_null_offset, if the given arguments do not satisfy the
/// requirements above.
/// a power of two and satisfy [min allocation size, system page size].
/// \return On success, returns the pointer to the beginning of newly
/// allocated memory. Returns k_null_offset, if the given arguments do not
/// satisfy the requirements above.
difference_type allocate_aligned(const size_type nbytes,
const size_type alignment) {
// This aligned allocation algorithm assumes that all power of 2 numbers
Expand All @@ -194,6 +194,10 @@ class segment_allocator {
return k_null_offset;
}

// Internal allocation size must be a multiple of alignment
assert(bin_no_mngr::to_object_size(bin_no_mngr::to_bin_no(nbytes)) %
alignment == 0);

// As long as the above requirements are satisfied, just calling the normal
// allocate function is enough
const auto offset = allocate(nbytes);
Expand Down
12 changes: 9 additions & 3 deletions include/metall/kernel/segment_storage.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -436,7 +436,10 @@ class segment_storage {
}
}

priv_prepare_header_and_segment(segment_capacity_request);
if (!priv_prepare_header_and_segment(segment_capacity_request)) {
priv_set_broken_status();
return false;
}

m_top_path = top_path;
m_read_only = false;
Expand Down Expand Up @@ -473,8 +476,11 @@ class segment_storage {
logger::out(logger::level::verbose, __FILE__, __LINE__, s.c_str());
}

priv_prepare_header_and_segment(read_only ? priv_get_size(top_path)
: segment_capacity_request);
if (!priv_prepare_header_and_segment(
read_only ? priv_get_size(top_path) : segment_capacity_request)) {
priv_set_broken_status();
return false;
}

m_top_path = top_path;
m_read_only = read_only;
Expand Down
5 changes: 3 additions & 2 deletions test/kernel/manager_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1154,10 +1154,11 @@ TEST(ManagerTest, AlignedAllocation) {
{
manager_type::remove(dir_path());
manager_type manager(metall::create_only, dir_path());
const std::size_t page_size = metall::mtlldetail::get_page_size();

for (std::size_t alignment = k_min_object_size; alignment <= k_chunk_size;
for (std::size_t alignment = k_min_object_size; alignment <= page_size;
alignment *= 2) {
for (std::size_t sz = alignment; sz <= k_chunk_size * 2;
for (std::size_t sz = alignment; sz <= page_size * 2;
sz += alignment) {
auto addr1 =
static_cast<char *>(manager.allocate_aligned(sz, alignment));
Expand Down

0 comments on commit 084362b

Please sign in to comment.