From f057a766c54d5eab8657084326e4b79e5b6af7e0 Mon Sep 17 00:00:00 2001 From: shosseinimotlagh Date: Thu, 19 Sep 2024 11:56:25 -0700 Subject: [PATCH] Add assert --- src/engine/blkalloc/blk.h | 6 +++++- src/engine/blkstore/blkstore.hpp | 9 ++++++--- src/homeblks/volume/tests/vol_gtest.cpp | 2 +- src/homeblks/volume/tool/hs_svc_tool.cpp | 2 +- 4 files changed, 13 insertions(+), 6 deletions(-) diff --git a/src/engine/blkalloc/blk.h b/src/engine/blkalloc/blk.h index 5429f77bf..a5e71296d 100644 --- a/src/engine/blkalloc/blk.h +++ b/src/engine/blkalloc/blk.h @@ -57,7 +57,11 @@ struct BlkId { static constexpr uint64_t s_chunk_num_mask{(static_cast< uint64_t >(1) << CHUNK_NUM_BITS) - 1}; public: - [[nodiscard]] static constexpr blk_count_t max_blks_in_op() { return (1 << NBLKS_BITS); } + [[nodiscard]] static constexpr blk_count_t max_blks_in_op() { + static_assert(NBLKS_BITS <= std::numeric_limits< blk_count_t >::digits, + "NBLKS_BITS is too large and may cause overflow in blk_count_t"); + return (1 << NBLKS_BITS); + } [[nodiscard]] static constexpr uint64_t max_id_int() { return (1ull << (BLK_NUM_BITS + NBLKS_BITS + CHUNK_NUM_BITS)) - 1; } diff --git a/src/engine/blkstore/blkstore.hpp b/src/engine/blkstore/blkstore.hpp index 9c534b137..345156d9f 100644 --- a/src/engine/blkstore/blkstore.hpp +++ b/src/engine/blkstore/blkstore.hpp @@ -325,11 +325,14 @@ class BlkStore { BlkAllocStatus alloc_contiguous_blk(const uint32_t size, blk_alloc_hints& hints, BlkId* const out_blkid) { // Allocate a block from the device manager assert(size % m_pagesz == 0); - const uint32_t nblks{static_cast< uint32_t >(size / m_pagesz)}; + static uint32_t max_suppoted_size = m_pagesz * std::numeric_limits< blk_count_t >::max(); + HS_REL_ASSERT_LE(size, max_suppoted_size, "size {} more than max size limit of {}", size, max_suppoted_size); + + const blk_count_t nblks{static_cast< blk_count_t >(size / m_pagesz)}; hints.is_contiguous = true; HS_DBG_ASSERT_LE(nblks, BlkId::max_blks_in_op(), "nblks {} more than max blks {}", nblks, BlkId::max_blks_in_op()); - return (m_vdev.alloc_contiguous_blk(static_cast< blk_count_t >(nblks), hints, out_blkid)); + return (m_vdev.alloc_contiguous_blk(nblks, hints, out_blkid)); } /* Allocate a new block of the size based on the hints provided */ @@ -342,7 +345,7 @@ class BlkStore { uint32_t nblks{static_cast< uint32_t >(size / m_pagesz)}; if (nblks <= BlkId::max_blks_in_op()) { auto max_val = std::numeric_limits< blk_count_t >::max(); - HS_DBG_ASSERT_LE(nblks, max_val, "max_blks_in_op must be less than {}", max_val); + HS_DBG_ASSERT_LE(nblks, max_val, "nblks must be less than {}", max_val); return (m_vdev.alloc_blk(static_cast< blk_count_t >(nblks), hints, out_blkid)); } else { while (nblks != 0) { diff --git a/src/homeblks/volume/tests/vol_gtest.cpp b/src/homeblks/volume/tests/vol_gtest.cpp index 517e3e7f1..d8aa4ef17 100644 --- a/src/homeblks/volume/tests/vol_gtest.cpp +++ b/src/homeblks/volume/tests/vol_gtest.cpp @@ -2607,7 +2607,7 @@ SISL_OPTION_GROUP( (oob_unmap_enable, "", "oob_unmap_enable", "out-of-band unmap enable 0 or 1", ::cxxopts::value< uint32_t >()->default_value("0"), "flag"), (max_disk_capacity, "", "max_disk_capacity", "max disk capacity", - ::cxxopts::value< uint64_t >()->default_value("10"), "GB"), + ::cxxopts::value< uint64_t >()->default_value("20"), "GB"), (max_volume, "", "max_volume", "max volume", ::cxxopts::value< uint64_t >()->default_value("50"), "number"), (max_num_writes, "", "max_num_writes", "max num of writes", ::cxxopts::value< uint64_t >()->default_value("100000"), "number"), diff --git a/src/homeblks/volume/tool/hs_svc_tool.cpp b/src/homeblks/volume/tool/hs_svc_tool.cpp index 531d622ca..82a3b0aa6 100644 --- a/src/homeblks/volume/tool/hs_svc_tool.cpp +++ b/src/homeblks/volume/tool/hs_svc_tool.cpp @@ -117,7 +117,7 @@ SISL_OPTION_GROUP(hs_svc_tool, (device_list, "", "device_list", "List of device paths", ::cxxopts::value< std::vector< std::string > >(), "path [...]"), (dev_size_gb, "", "dev_size_gb", "size of each device in GB", - ::cxxopts::value< uint64_t >()->default_value("5"), "number"), + ::cxxopts::value< uint64_t >()->default_value("10"), "number"), (zero_boot_sb, "", "zero_boot_sb", "mark homestore init state", ::cxxopts::value< bool >()->default_value("false"), "true or false"), (spdk, "", "spdk", "spdk", ::cxxopts::value< bool >()->default_value("false"), "true or false"));