os/bluestore: fix improper access to a BitmapFastAllocator::last_pos

from multiple threads.

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

diff --git a/src/os/bluestore/BitmapFastAllocator.cc b/src/os/bluestore/BitmapFastAllocator.cc
index 9139e817e65c09926584d8b4ae86c96f2b58f9fe..2733ccd060d432f84d22b011f86227b1b45e3785 100755 (executable)
--- a/src/os/bluestore/BitmapFastAllocator.cc
+++ b/src/os/bluestore/BitmapFastAllocator.cc
@@ -24,11 +24,7 @@ int64_t BitmapFastAllocator::allocate(
 {
   uint64_t allocated = 0;
 
-  if (hint != 0) {
-    last_pos = hint;
-  }
-
-  _allocate_l2(want_size, alloc_unit, max_alloc_size, &last_pos,
+  _allocate_l2(want_size, alloc_unit, max_alloc_size, hint,
     &allocated, extents);
   if (!allocated) {
     return -ENOSPC;
@@ -69,5 +65,5 @@ void BitmapFastAllocator::init_rm_free(uint64_t offset, uint64_t length)
 void BitmapFastAllocator::shutdown()
 {
   ldout(cct, 1) << __func__ << dendl;
-  last_pos = 0;
+  _shutdown();
 }

diff --git a/src/os/bluestore/BitmapFastAllocator.h b/src/os/bluestore/BitmapFastAllocator.h
index ac58f2a5a636857c900d1c9786b5be4c8d23a9b5..9807780b50c8d7ef60eb3f4964fee4893d9fa206 100755 (executable)
--- a/src/os/bluestore/BitmapFastAllocator.h
+++ b/src/os/bluestore/BitmapFastAllocator.h
@@ -15,7 +15,6 @@
 class BitmapFastAllocator : public Allocator,
   public AllocatorLevel02<AllocatorLevel01Loose> {
   CephContext* cct;
-  uint64_t last_pos = 0;
 
 public:
   BitmapFastAllocator(CephContext* _cct, int64_t capacity, int64_t alloc_unit);

diff --git a/src/os/bluestore/fastbmap_allocator_impl.h b/src/os/bluestore/fastbmap_allocator_impl.h
index d8f7b1d8b76bd10397c2b3b9e79f67dab879ff0a..fe8e94e33b7444b59c9a9a7cedea65e0195dac59 100755 (executable)
--- a/src/os/bluestore/fastbmap_allocator_impl.h
+++ b/src/os/bluestore/fastbmap_allocator_impl.h
@@ -544,6 +544,7 @@ protected:
   slot_vector_t l2;
   uint64_t l2_granularity = 0; // space per entry
   uint64_t available = 0;
+  uint64_t last_pos = 0;
 
   enum {
     CHILD_PER_SLOT = bits_per_slot, // 64
@@ -640,28 +641,30 @@ protected:
   void _allocate_l2(uint64_t length,
     uint64_t min_length,
     uint64_t max_length,
-    uint64_t* pos_start,
+    uint64_t hint,
+    
     uint64_t* allocated,
     interval_vector_t* res)
   {
     uint64_t prev_allocated = *allocated;
     uint64_t d = CHILD_PER_SLOT;
-    assert((*pos_start % d) == 0);
     assert(min_length <= l2_granularity);
     assert(max_length == 0 || max_length >= min_length);
     assert(max_length == 0 || (max_length % min_length) == 0);
 
     uint64_t l1_w = slotset_width * l1._children_per_slot();
 
-    auto last_pos = *pos_start;
-
-    auto l2_pos = *pos_start;
     std::lock_guard<std::mutex> l(lock);
 
     if (available < min_length) {
       return;
     }
-    auto pos = *pos_start / d;
+    if (hint != 0) {
+      last_pos = (hint / d) < l2.size() ? p2align(hint, d) : 0;
+    }
+    auto l2_pos = last_pos;
+    auto last_pos0 = last_pos;
+    auto pos = last_pos / d;
     auto pos_end = l2.size();
     // outer loop below is intended to optimize the performance by
     // avoiding 'modulo' operations inside the internal loop.
@@ -687,7 +690,7 @@ protected:
          bool empty = l1._allocate_l1(length,
            min_length,
            max_length,
-           (l2_pos + free_pos)* l1_w,
+           (l2_pos + free_pos) * l1_w,
            (l2_pos + free_pos + 1) * l1_w,
            allocated,
            res);
@@ -707,14 +710,13 @@ protected:
       }
       l2_pos = 0;
       pos = 0;
-      pos_end = *pos_start;
+      pos_end = last_pos0 / d;
     }
 
     ++l2_allocs;
     auto allocated_here = *allocated - prev_allocated;
     assert(available >= allocated_here);
     available -= allocated_here;
-    *pos_start = last_pos;
   }
 
 #ifndef NON_CEPH_BUILD
@@ -770,6 +772,10 @@ protected:
     available += l1._free_l1(o, len);
     _mark_l2_free(l2_pos, l2_pos_end);
   }
+  void _shutdown()
+  {
+    last_pos = 0;
+  }
 };
 
 #endif

diff --git a/src/test/objectstore/fastbmap_allocator_test.cc b/src/test/objectstore/fastbmap_allocator_test.cc
index acddb3f3723a5e949a4ada5d1e329b71891982ff..64db85f2044ebd4ad9cb6bfbf5b7d38384860141 100755 (executable)
--- a/src/test/objectstore/fastbmap_allocator_test.cc
+++ b/src/test/objectstore/fastbmap_allocator_test.cc
@@ -36,8 +36,8 @@ public:
     interval_vector_t* res)
   {
     uint64_t allocated = 0;
-    uint64_t last_pos = 0; // do hint support so far
-    _allocate_l2(length, min_length, 0, &last_pos, &allocated, res);
+    uint64_t hint = 0; // trigger internal l2 hint support
+    _allocate_l2(length, min_length, 0, hint, &allocated, res);
     *allocated0 += allocated;
   }
   void free_l2(const interval_vector_t& r)