os/bluestore/FreelistManager: make safe enumeration interface

Do not expose the internal representation; instead, make a super-simple
enumeration interface. There is only ever one user at a time (mount, and
fsck), so the locking is even superfluous.

Signed-off-by: Sage Weil <sage@redhat.com>

diff --git a/src/os/bluestore/BlueStore.cc b/src/os/bluestore/BlueStore.cc
index 1389b9c068d75f22921ab12b28b18340c4ca4024..20802ae07f8607e8ea42677e46ff8f26a455f022 100644 (file)
--- a/src/os/bluestore/BlueStore.cc
+++ b/src/os/bluestore/BlueStore.cc
@@ -981,11 +981,12 @@ int BlueStore::_open_alloc()
 
   alloc = Allocator::create("stupid");
   uint64_t num = 0, bytes = 0;
-  const auto& fl = fm->get_freelist();
-  for (auto& p : fl) {
-    alloc->init_add_free(p.first, p.second);
+  fm->enumerate_reset();
+  uint64_t offset, length;
+  while (fm->enumerate_next(&offset, &length)) {
+    alloc->init_add_free(offset, length);
     ++num;
-    bytes += p.second;
+    bytes += length;
   }
   dout(10) << __func__ << " loaded " << pretty_si_t(bytes)
           << " in " << num << " extents"
@@ -2354,20 +2355,20 @@ int BlueStore::fsck()
 
   dout(1) << __func__ << " checking freelist vs allocated" << dendl;
   {
-    const auto& free = fm->get_freelist();
-    for (auto p = free.begin();
-        p != free.end(); ++p) {
-      if (used_blocks.intersects(p->first, p->second)) {
-       derr << __func__ << " free extent " << p->first << "~" << p->second
+    fm->enumerate_reset();
+    uint64_t offset, length;
+    while (fm->enumerate_next(&offset, &length)) {
+      if (used_blocks.intersects(offset, length)) {
+       derr << __func__ << " free extent " << offset << "~" << length
             << " intersects allocated blocks" << dendl;
        interval_set<uint64_t> free, overlap;
-       free.insert(p->first, p->second);
+       free.insert(offset, length);
        overlap.intersection_of(free, used_blocks);
        derr << __func__ << " overlap: " << overlap << dendl;
        ++errors;
        continue;
       }
-      used_blocks.insert(p->first, p->second);
+      used_blocks.insert(offset, length);
     }
     if (!used_blocks.contains(0, bdev->get_size())) {
       derr << __func__ << " leaked some space; free+used = "

diff --git a/src/os/bluestore/FreelistManager.cc b/src/os/bluestore/FreelistManager.cc
index 9b0d966ea117e18c26c6cc961507941502ce4725..30a17fcb4b963dd31d8c8d82173279ca77139cb1 100644 (file)
--- a/src/os/bluestore/FreelistManager.cc
+++ b/src/os/bluestore/FreelistManager.cc
@@ -89,6 +89,23 @@ void FreelistManager::dump()
   _dump();
 }
 
+void FreelistManager::enumerate_reset()
+{
+  std::lock_guard<std::mutex> l(lock);
+  enumerate_p = kv_free.begin();
+}
+
+bool FreelistManager::enumerate_next(uint64_t *offset, uint64_t *length)
+{
+  std::lock_guard<std::mutex> l(lock);
+  if (enumerate_p == kv_free.end())
+    return false;
+  *offset = enumerate_p->first;
+  *length = enumerate_p->second;
+  ++enumerate_p;
+  return true;
+}
+
 void FreelistManager::_dump()
 {
   dout(30) << __func__ << " " << total_free

diff --git a/src/os/bluestore/FreelistManager.h b/src/os/bluestore/FreelistManager.h
index 4e7255e887da53a4b54a5aea0cfdef05ce9eb69d..b710fc0d89a27af6a4b97a85310357e9330f2930 100644 (file)
--- a/src/os/bluestore/FreelistManager.h
+++ b/src/os/bluestore/FreelistManager.h
@@ -22,6 +22,8 @@ class FreelistManager {
 
   map_t kv_free;    ///< mirrors our kv values in the db
 
+  map_t::const_iterator enumerate_p;
+
   void _audit();
   void _dump();
 
@@ -40,9 +42,8 @@ public:
     return total_free;
   }
 
-  const map_t& get_freelist() {
-    return kv_free;
-  }
+  void enumerate_reset();
+  bool enumerate_next(uint64_t *offset, uint64_t *length);
 
   void allocate(
     uint64_t offset, uint64_t length,