os/bluestore: fix BitMapAllocator assert on out-of-bound hint value

Signed-off-by: Igor Fedotov <ifedotov@mirantis.com>

diff --git a/src/os/bluestore/BitMapAllocator.cc b/src/os/bluestore/BitMapAllocator.cc
index 54e986e3ca1e1e890cd7b2505e3af3f2e2bcd714..f2a16d8c900435d053759fd7bfb6acbe98402c8c 100644 (file)
--- a/src/os/bluestore/BitMapAllocator.cc
+++ b/src/os/bluestore/BitMapAllocator.cc
@@ -22,38 +22,39 @@ BitMapAllocator::BitMapAllocator(CephContext* cct, int64_t device_size,
                                 int64_t block_size)
   : cct(cct)
 {
-  assert(ISP2(block_size));
   if (!ISP2(block_size)) {
     derr << __func__ << " block_size " << block_size
          << " not power of 2 aligned!"
          << dendl;
+    assert(ISP2(block_size));
     return;
   }
 
   int64_t zone_size_blks = cct->_conf->bluestore_bitmapallocator_blocks_per_zone;
-  assert(ISP2(zone_size_blks));
   if (!ISP2(zone_size_blks)) {
     derr << __func__ << " zone_size " << zone_size_blks
          << " not power of 2 aligned!"
          << dendl;
+    assert(ISP2(zone_size_blks));
     return;
   }
 
   int64_t span_size = cct->_conf->bluestore_bitmapallocator_span_size;
-  assert(ISP2(span_size));
   if (!ISP2(span_size)) {
     derr << __func__ << " span_size " << span_size
          << " not power of 2 aligned!"
          << dendl;
+    assert(ISP2(span_size));
     return;
   }
 
   m_block_size = block_size;
+  m_total_size = P2ALIGN(device_size, block_size);
   m_bit_alloc = new BitAllocator(cct, device_size / block_size,
                                 zone_size_blks, CONCURRENT, true);
-  assert(m_bit_alloc);
   if (!m_bit_alloc) {
     derr << __func__ << " Unable to intialize Bit Allocator" << dendl;
+    assert(m_bit_alloc);
   }
   dout(10) << __func__ << " instance " << (uint64_t) this
            << " size 0x" << std::hex << device_size << std::dec
@@ -123,7 +124,7 @@ int64_t BitMapAllocator::allocate(
      << " alloc_unit " << alloc_unit
      << " hint " << hint
      << dendl;
-
+  hint = hint % m_total_size; // make hint error-tolerant
   return allocate_dis(want_size, alloc_unit / m_block_size,
                       max_alloc_size, hint / m_block_size, extents);
 }

diff --git a/src/os/bluestore/BitMapAllocator.h b/src/os/bluestore/BitMapAllocator.h
index 21d10f33bf483fd08cbecb75b1821b7d63a9a55e..a39751d4cf6ad55e7bdf8ece26221d62d3d8f6c3 100644 (file)
--- a/src/os/bluestore/BitMapAllocator.h
+++ b/src/os/bluestore/BitMapAllocator.h
@@ -14,6 +14,7 @@ class BitMapAllocator : public Allocator {
   CephContext* cct;
 
   int64_t m_block_size;
+  int64_t m_total_size;
 
   BitAllocator *m_bit_alloc; // Bit allocator instance
 

diff --git a/src/os/bluestore/BlueFS.cc b/src/os/bluestore/BlueFS.cc
index db1dad55451a1ec489d583d74a2d88df25ae33f3..dbb56ed0b202a3e76dd72cc2bbf0e4c75d15cb7b 100644 (file)
--- a/src/os/bluestore/BlueFS.cc
+++ b/src/os/bluestore/BlueFS.cc
@@ -1833,7 +1833,8 @@ int BlueFS::_allocate(uint8_t id, uint64_t len,
                           &extents);
   if (alloc_len < (int64_t)left) {
     derr << __func__ << " allocate failed on 0x" << std::hex << left
-        << " min_alloc_size 0x" << min_alloc_size << std::dec << dendl;
+        << " min_alloc_size 0x" << min_alloc_size 
+         << " hint 0x" <<  hint << std::dec << dendl;
     alloc[id]->dump();
     assert(0 == "allocate failed... wtf");
     return -ENOSPC;

diff --git a/src/test/objectstore/Allocator_test.cc b/src/test/objectstore/Allocator_test.cc
index 65f8e47bd0864e4c5d8091ee58e5b908f92ddc74..076297a2e731feb37330e3d06a41073ec70db424 100644 (file)
--- a/src/test/objectstore/Allocator_test.cc
+++ b/src/test/objectstore/Allocator_test.cc
@@ -259,6 +259,18 @@ TEST_P(AllocTest, test_alloc_hint_bmap)
   ASSERT_EQ(zone_size, allocated);
   EXPECT_EQ(zone_size, (int)extents.size());
   EXPECT_EQ(extents[0].offset, (uint64_t) 0);
+  /*
+   * Verify out-of-bound hint
+   */
+  extents.clear();
+  allocated = alloc->allocate(1, 1, 1, blocks, &extents);
+  ASSERT_EQ(1, allocated);
+  EXPECT_EQ(1, (int)extents.size());
+
+  extents.clear();
+  allocated = alloc->allocate(1, 1, 1, blocks * 3 + 1 , &extents);
+  ASSERT_EQ(1, allocated);
+  EXPECT_EQ(1, (int)extents.size());
 }