From 084362b85d645fd89b18823e650aaf2fda288fe1 Mon Sep 17 00:00:00 2001 From: Keita Iwabuchi Date: Fri, 4 Oct 2024 22:26:01 -0700 Subject: [PATCH] Bugfix: alignment alloc --- include/metall/basic_manager.hpp | 7 ++++--- include/metall/detail/mmap.hpp | 2 +- include/metall/kernel/manager_kernel_impl.ipp | 1 + include/metall/kernel/segment_allocator.hpp | 12 ++++++++---- include/metall/kernel/segment_storage.hpp | 12 +++++++++--- test/kernel/manager_test.cpp | 5 +++-- 6 files changed, 26 insertions(+), 13 deletions(-) diff --git a/include/metall/basic_manager.hpp b/include/metall/basic_manager.hpp index 10ef5129..0cddc10c 100644 --- a/include/metall/basic_manager.hpp +++ b/include/metall/basic_manager.hpp @@ -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; diff --git a/include/metall/detail/mmap.hpp b/include/metall/detail/mmap.hpp index 3408b1f1..3089f72f 100644 --- a/include/metall/detail/mmap.hpp +++ b/include/metall/detail/mmap.hpp @@ -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; } diff --git a/include/metall/kernel/manager_kernel_impl.ipp b/include/metall/kernel/manager_kernel_impl.ipp index 259cb81d..1cdb0a0c 100644 --- a/include/metall/kernel/manager_kernel_impl.ipp +++ b/include/metall/kernel/manager_kernel_impl.ipp @@ -112,6 +112,7 @@ void *manager_kernel::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; diff --git a/include/metall/kernel/segment_allocator.hpp b/include/metall/kernel/segment_allocator.hpp index 3ef3c1b9..775ecce0 100644 --- a/include/metall/kernel/segment_allocator.hpp +++ b/include/metall/kernel/segment_allocator.hpp @@ -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 @@ -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); diff --git a/include/metall/kernel/segment_storage.hpp b/include/metall/kernel/segment_storage.hpp index bacad41c..e5ca3c36 100644 --- a/include/metall/kernel/segment_storage.hpp +++ b/include/metall/kernel/segment_storage.hpp @@ -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; @@ -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; diff --git a/test/kernel/manager_test.cpp b/test/kernel/manager_test.cpp index 579b0207..8e16a048 100644 --- a/test/kernel/manager_test.cpp +++ b/test/kernel/manager_test.cpp @@ -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(manager.allocate_aligned(sz, alignment));