From 649f6b98a36453d82fbae342a27d8e8df4c522b4 Mon Sep 17 00:00:00 2001 From: Harihara Kadayam Date: Fri, 10 May 2024 17:15:41 -0700 Subject: [PATCH 1/4] Fixed the number of chunks issue --- conanfile.py | 2 +- src/lib/device/physical_dev.cpp | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/conanfile.py b/conanfile.py index f18a621f6..83f75c3e6 100644 --- a/conanfile.py +++ b/conanfile.py @@ -9,7 +9,7 @@ class HomestoreConan(ConanFile): name = "homestore" - version = "6.4.43" + version = "6.4.44" homepage = "https://github.com/eBay/Homestore" description = "HomeStore Storage Engine" topics = ("ebay", "nublox") diff --git a/src/lib/device/physical_dev.cpp b/src/lib/device/physical_dev.cpp index cfcf9985f..c4f74b576 100644 --- a/src/lib/device/physical_dev.cpp +++ b/src/lib/device/physical_dev.cpp @@ -220,8 +220,10 @@ void PhysicalDev::submit_batch() { m_drive_iface->submit_batch(); } //////////////////////////// Chunk Creation/Load related methods ///////////////////////////////////////// void PhysicalDev::format_chunks() { - m_chunk_info_slots = std::make_unique< sisl::Bitset >(hs_super_blk::chunk_info_bitmap_size(m_dev_info)); + auto const bitmap_size = hs_super_blk::chunk_info_bitmap_size(m_dev_info); + m_chunk_info_slots = std::make_unique< sisl::Bitset >(sisl::Bitset::max_possible_bits(bitmap_size)); auto bitmap_mem = m_chunk_info_slots->serialize(m_pdev_info.dev_attr.align_size); + HS_REL_ASSERT_EQ(bitmap_mem->size(), bitmap_size, "Bitmap size mismatch with expected size"); write_super_block(bitmap_mem->cbytes(), bitmap_mem->size(), hs_super_blk::chunk_sb_offset()); } @@ -235,7 +237,7 @@ std::vector< shared< Chunk > > PhysicalDev::create_chunks(const std::vector< uin try { while (chunks_remaining > 0) { - auto b = m_chunk_info_slots->get_next_contiguous_n_reset_bits(0u, chunks_remaining); + auto b = m_chunk_info_slots->get_next_contiguous_n_reset_bits(0u, std::nullopt, 1u, chunks_remaining); if (b.nbits == 0) { throw std::out_of_range("System has no room for additional chunk"); } buf = hs_utils::iobuf_alloc(chunk_info::size * b.nbits, sisl::buftag::superblk, From e69d4196e714641f231a55ee6b07d21a7ee968e4 Mon Sep 17 00:00:00 2001 From: Harihara Kadayam Date: Tue, 14 May 2024 20:28:44 -0700 Subject: [PATCH 2/4] Addressed review comments --- src/lib/device/physical_dev.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/lib/device/physical_dev.cpp b/src/lib/device/physical_dev.cpp index c4f74b576..90ed02bf5 100644 --- a/src/lib/device/physical_dev.cpp +++ b/src/lib/device/physical_dev.cpp @@ -220,10 +220,10 @@ void PhysicalDev::submit_batch() { m_drive_iface->submit_batch(); } //////////////////////////// Chunk Creation/Load related methods ///////////////////////////////////////// void PhysicalDev::format_chunks() { - auto const bitmap_size = hs_super_blk::chunk_info_bitmap_size(m_dev_info); - m_chunk_info_slots = std::make_unique< sisl::Bitset >(sisl::Bitset::max_possible_bits(bitmap_size)); + m_chunk_info_slots = std::make_unique< sisl::Bitset >(std::max(1u, hs_super_blk::max_chunks_in_pdev(m_dev_info))); auto bitmap_mem = m_chunk_info_slots->serialize(m_pdev_info.dev_attr.align_size); - HS_REL_ASSERT_EQ(bitmap_mem->size(), bitmap_size, "Bitmap size mismatch with expected size"); + HS_REL_ASSERT_LE(bitmap_mem->size(), hs_super_blk::chunk_info_bitmap_size(m_dev_info), + "Chunk info serialized bitmap mismatch with expected size"); write_super_block(bitmap_mem->cbytes(), bitmap_mem->size(), hs_super_blk::chunk_sb_offset()); } From d6f16395102d076ed2b1e706a82ffc8054599872 Mon Sep 17 00:00:00 2001 From: Xiaoxi Chen Date: Wed, 14 Aug 2024 16:16:49 +0800 Subject: [PATCH 3/4] make bitmap_mem align to 4096 Signed-off-by: Xiaoxi Chen --- src/lib/device/hs_super_blk.h | 2 +- src/lib/device/physical_dev.cpp | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/lib/device/hs_super_blk.h b/src/lib/device/hs_super_blk.h index 9f909c5ea..c6b85dbbe 100644 --- a/src/lib/device/hs_super_blk.h +++ b/src/lib/device/hs_super_blk.h @@ -183,7 +183,7 @@ class hs_super_blk { static uint64_t chunk_super_block_size(const dev_info& dinfo); static uint64_t chunk_info_bitmap_size(const dev_info& dinfo) { // Chunk bitmap area has bitmap of max_chunks rounded off to 4k page - return sisl::round_up(std::max(1u, hs_super_blk::max_chunks_in_pdev(dinfo) / 8), 4096); + return sisl::round_up(bitset_serialized::nbytes(hs_super_blk::max_chunks_in_pdev(dinfo)), 4096)); } static uint64_t total_size(const dev_info& dinfo) { return total_used_size(dinfo) + future_padding_size(dinfo); } diff --git a/src/lib/device/physical_dev.cpp b/src/lib/device/physical_dev.cpp index 90ed02bf5..1b6914cf5 100644 --- a/src/lib/device/physical_dev.cpp +++ b/src/lib/device/physical_dev.cpp @@ -220,8 +220,9 @@ void PhysicalDev::submit_batch() { m_drive_iface->submit_batch(); } //////////////////////////// Chunk Creation/Load related methods ///////////////////////////////////////// void PhysicalDev::format_chunks() { - m_chunk_info_slots = std::make_unique< sisl::Bitset >(std::max(1u, hs_super_blk::max_chunks_in_pdev(m_dev_info))); - auto bitmap_mem = m_chunk_info_slots->serialize(m_pdev_info.dev_attr.align_size); + m_chunk_info_slots = std::make_unique< sisl::Bitset >(std::max(1u, hs_super_blk::max_chunks_in_pdev(m_dev_info)), + /* align size */ 4096); + auto bitmap_mem = m_chunk_info_slots->serialize(/* align size */ 4096); HS_REL_ASSERT_LE(bitmap_mem->size(), hs_super_blk::chunk_info_bitmap_size(m_dev_info), "Chunk info serialized bitmap mismatch with expected size"); write_super_block(bitmap_mem->cbytes(), bitmap_mem->size(), hs_super_blk::chunk_sb_offset()); From d1ac41dde0f6c0b204756c83ea4ec4df8e7bf685 Mon Sep 17 00:00:00 2001 From: Xiaoxi Chen Date: Wed, 14 Aug 2024 03:15:17 -0700 Subject: [PATCH 4/4] fix ut Signed-off-by: Xiaoxi Chen --- src/lib/device/hs_super_blk.h | 15 +++++++-------- src/tests/test_pdev.cpp | 4 +++- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/lib/device/hs_super_blk.h b/src/lib/device/hs_super_blk.h index c6b85dbbe..a539c1e56 100644 --- a/src/lib/device/hs_super_blk.h +++ b/src/lib/device/hs_super_blk.h @@ -183,7 +183,9 @@ class hs_super_blk { static uint64_t chunk_super_block_size(const dev_info& dinfo); static uint64_t chunk_info_bitmap_size(const dev_info& dinfo) { // Chunk bitmap area has bitmap of max_chunks rounded off to 4k page - return sisl::round_up(bitset_serialized::nbytes(hs_super_blk::max_chunks_in_pdev(dinfo)), 4096)); + // add 4KB headroom for bitmap serialized header + auto bytes = sisl::round_up(hs_super_blk::max_chunks_in_pdev(dinfo), 8) / 8 + 4096; + return sisl::round_up(bytes, 4096); } static uint64_t total_size(const dev_info& dinfo) { return total_used_size(dinfo) + future_padding_size(dinfo); } @@ -197,13 +199,10 @@ class hs_super_blk { return (dinfo.dev_type == HSDevType::Fast) ? EXTRA_SB_SIZE_FOR_FAST_DEVICE : EXTRA_SB_SIZE_FOR_DATA_DEVICE; } static uint32_t max_chunks_in_pdev(const dev_info& dinfo) { - uint64_t min_chunk_size = - (dinfo.dev_type == HSDevType::Fast) ? MIN_CHUNK_SIZE_FAST_DEVICE : MIN_CHUNK_SIZE_DATA_DEVICE; -#ifdef _PRERELEASE - auto chunk_size = iomgr_flip::instance()->get_test_flip< long >("set_minimum_chunk_size"); - if (chunk_size) { min_chunk_size = chunk_size.get(); } -#endif - return (dinfo.dev_size - 1) / min_chunk_size + 1; + uint64_t min_c_size = min_chunk_size(dinfo.dev_type); + // Do not round up , for a device with 128MB and min_chunk_size = 16MB, we should get 8 chunks + // for a device with 100MB and min_chunk_size = 16MB, we should get 6 chunks, not 7. + return dinfo.dev_size / min_c_size; } static uint32_t min_chunk_size(HSDevType dtype) { uint64_t min_chunk_size = (dtype == HSDevType::Fast) ? MIN_CHUNK_SIZE_FAST_DEVICE : MIN_CHUNK_SIZE_DATA_DEVICE; diff --git a/src/tests/test_pdev.cpp b/src/tests/test_pdev.cpp index e28c8a933..4447c500b 100644 --- a/src/tests/test_pdev.cpp +++ b/src/tests/test_pdev.cpp @@ -44,8 +44,10 @@ SISL_OPTION_GROUP(test_pdev, ::cxxopts::value< uint32_t >()->default_value("2"), "number"), (data_dev_size_mb, "", "data_dev_size_mb", "size of each data device in MB", ::cxxopts::value< uint64_t >()->default_value("1024"), "number"), + // in OrderlyChunkOpsWithRestart UT, we need to create 10 chunks on fast drive + // ensure fast dev has > min_chunk_size_fast(32M) * 10 capacity. (fast_dev_size_mb, "", "fast_dev_size_mb", "size of each fast device in MB", - ::cxxopts::value< uint64_t >()->default_value("100"), "number"), + ::cxxopts::value< uint64_t >()->default_value("400"), "number"), (spdk, "", "spdk", "spdk", ::cxxopts::value< bool >()->default_value("false"), "true or false")); std::vector< std::string > g_data_dev_names;