From 8db30b07d0f34af1766e0611e6312893640bbca5 Mon Sep 17 00:00:00 2001 From: Sage Weil Date: Sun, 24 Apr 2016 18:31:59 -0400 Subject: [PATCH] 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 --- src/os/bluestore/BlueStore.cc | 23 ++++++++++++----------- src/os/bluestore/FreelistManager.cc | 17 +++++++++++++++++ src/os/bluestore/FreelistManager.h | 7 ++++--- 3 files changed, 33 insertions(+), 14 deletions(-) diff --git a/src/os/bluestore/BlueStore.cc b/src/os/bluestore/BlueStore.cc index 1389b9c068d..20802ae07f8 100644 --- 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 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 9b0d966ea11..30a17fcb4b9 100644 --- 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 l(lock); + enumerate_p = kv_free.begin(); +} + +bool FreelistManager::enumerate_next(uint64_t *offset, uint64_t *length) +{ + std::lock_guard 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 4e7255e887d..b710fc0d89a 100644 --- 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, -- 2.47.3