From: Igor Fedotov <igor.fedotov@croit.io>
Date: Mon, 10 Jul 2023 21:22:58 +0000 (+0300)
Subject: os/bluestore: fix btree allocator
X-Git-Tag: testing/wip-vshankar-testing-20250110.122509-reef-debug~56^2
X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=c1808e6dfe2590848ebca0dcc74548807d5fd596;p=ceph-ci.git

os/bluestore: fix btree allocator

Fixes: https://tracker.ceph.com/issues/61949

Signed-off-by: Igor Fedotov <igor.fedotov@croit.io>
(cherry picked from commit c4c3e34f819ef13a476093969bb634658b84e026)
---

diff --git a/src/os/bluestore/BtreeAllocator.cc b/src/os/bluestore/BtreeAllocator.cc
index 2455ec111b1..2647b875992 100644
--- a/src/os/bluestore/BtreeAllocator.cc
+++ b/src/os/bluestore/BtreeAllocator.cc
@@ -132,14 +132,17 @@ void BtreeAllocator::_process_range_removal(uint64_t start, uint64_t end,
 
   // | left <|////|  right |
   if (left_over && right_over) {
-    // add the spin-off right seg
-    range_seg_t seg_after{end, seg_whole.end};
-    range_tree.emplace_hint(rs, seg_after.start, seg_after.end);
-    range_size_tree.emplace(seg_after);
     // shink the left seg in offset tree
+    // this should be done before calling any emplace/emplace_hint
+    // on range_tree since they invalidate rs iterator.
     rs->second = start;
     // insert the shrinked left seg back into size tree
     range_size_tree.emplace(seg_whole.start, start);
+
+    // add the spin-off right seg
+    range_seg_t seg_after{end, seg_whole.end};
+    range_tree.emplace_hint(rs, seg_after.start, seg_after.end);
+    range_size_tree.emplace(seg_after);
   } else if (left_over) {
     // | left <|///////////|
     // shrink the left seg in the offset tree
@@ -167,8 +170,11 @@ void BtreeAllocator::_remove_from_tree(uint64_t start, uint64_t size)
   ceph_assert(size != 0);
   ceph_assert(size <= num_free);
 
-  auto rs = range_tree.find(start);
-  /* Make sure we completely overlap with someone */
+  // Make sure we completely overlap with someone
+  auto rs = range_tree.lower_bound(start);
+  if ((rs == range_tree.end() || rs->first > start) && rs != range_tree.begin()) {
+    --rs;
+  }
   ceph_assert(rs != range_tree.end());
   ceph_assert(rs->first <= start);
   ceph_assert(rs->second >= end);
@@ -192,6 +198,11 @@ void BtreeAllocator::_try_remove_from_tree(uint64_t start, uint64_t size,
 
   do {
 
+    //FIXME: this is apparently wrong since _process_range_removal might
+    // invalidate existing iterators.
+    // Not a big deal so far since this method is not in use - it's called
+    // when making Hybrid allocator from a regular one. Which isn't an option
+    // for BtreeAllocator for now.
     auto next_rs = rs;
     ++next_rs;
 
diff --git a/src/test/objectstore/Allocator_bench.cc b/src/test/objectstore/Allocator_bench.cc
index 0d04a854e9a..76598d2a1b2 100644
--- a/src/test/objectstore/Allocator_bench.cc
+++ b/src/test/objectstore/Allocator_bench.cc
@@ -365,4 +365,4 @@ TEST_P(AllocTest, mempoolAccounting)
 INSTANTIATE_TEST_SUITE_P(
   Allocator,
   AllocTest,
-  ::testing::Values("stupid", "bitmap", "avl", "btree", "hybrid"));
+  ::testing::Values("stupid", "bitmap", "avl", "hybrid", "btree"));
diff --git a/src/test/objectstore/Allocator_test.cc b/src/test/objectstore/Allocator_test.cc
index 186063aa7f7..0e76c479002 100644
--- a/src/test/objectstore/Allocator_test.cc
+++ b/src/test/objectstore/Allocator_test.cc
@@ -631,7 +631,29 @@ TEST_P(AllocTest, test_alloc_50656_first_fit)
   EXPECT_EQ(got, 0x400000);
 }
 
+TEST_P(AllocTest, test_init_rm_free_unbound)
+{
+  int64_t block_size = 1024;
+  int64_t capacity = 4 * 1024 * block_size;
+
+  {
+    init_alloc(capacity, block_size);
+
+    alloc->init_add_free(0, block_size * 2);
+    alloc->init_add_free(block_size * 3, block_size * 3);
+    alloc->init_add_free(block_size * 7, block_size * 2);
+
+    alloc->init_rm_free(block_size * 4, block_size);
+    ASSERT_EQ(alloc->get_free(), block_size * 6);
+
+    auto cb = [&](size_t off, size_t len) {
+      cout << std::hex << "0x" << off << "~" << len << std::dec << std::endl;
+    };
+    alloc->foreach(cb);
+  }
+}
+
 INSTANTIATE_TEST_SUITE_P(
   Allocator,
   AllocTest,
-  ::testing::Values("stupid", "bitmap", "avl", "hybrid"));
+  ::testing::Values("stupid", "bitmap", "avl", "hybrid", "btree"));