Merge pull request ceph#15289 from ifed01/wip-bluestore-fix-bmap-assert

os/bluestore: fix BitMapAllocator assert on out-of-bound hint value Reviewed-by: Varada Kari <[email protected]>
zmc · May 26, 2017 · 5f003d6 · 5f003d6
2 parents c867a4d + 6f8e5d2
commit 5f003d6
Show file tree

Hide file tree

Showing 4 changed files with 21 additions and 6 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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());
 }