os/bluestore/BlueFS: fix reclaim_blocks

We need to return all extents to the caller.  The current code
fails to assign *offset so it appears like a single extent from
the start of the device, which is very wrong.

Fixes: http://tracker.ceph.com/issues/18368
Signed-off-by: Sage Weil <sage@redhat.com>

diff --git a/src/os/bluestore/BlueFS.cc b/src/os/bluestore/BlueFS.cc
index 1262706b5f0913edad8ff0f3768994a129c4c3ea..f193b3e3d7b7fd127b33633a8683e70d9490daa4 100644 (file)
--- a/src/os/bluestore/BlueFS.cc
+++ b/src/os/bluestore/BlueFS.cc
@@ -170,7 +170,7 @@ void BlueFS::add_block_extent(unsigned id, uint64_t offset, uint64_t length)
 }
 
 int BlueFS::reclaim_blocks(unsigned id, uint64_t want,
-                          uint64_t *offset, uint32_t *length)
+                          AllocExtentVector *extents)
 {
   std::unique_lock<std::mutex> l(lock);
   dout(1) << __func__ << " bdev " << id
@@ -180,30 +180,27 @@ int BlueFS::reclaim_blocks(unsigned id, uint64_t want,
   int r = alloc[id]->reserve(want);
   assert(r == 0); // caller shouldn't ask for more than they can get
   int count = 0;
-  uint64_t alloc_len = 0;;
-  AllocExtentVector extents = AllocExtentVector(want / cct->_conf->bluefs_alloc_size);
-
+  uint64_t got = 0;
   r = alloc[id]->allocate(want, cct->_conf->bluefs_alloc_size, 0,
-                          &extents, &count, &alloc_len);
+                          extents, &count, &got);
 
-  *length = alloc_len;
   assert(r >= 0);
-  if (*length < want) 
-    alloc[id]->unreserve(want - *length);
+  if (got < want)
+    alloc[id]->unreserve(want - got);
 
   for (int i = 0; i < count; i++) {
-    block_all[id].erase(extents[i].offset, extents[i].length);
-    block_total[id] -= extents[i].length;
-    log_t.op_alloc_rm(id, extents[i].offset, extents[i].length);
+    block_all[id].erase((*extents)[i].offset, (*extents)[i].length);
+    block_total[id] -= (*extents)[i].length;
+    log_t.op_alloc_rm(id, (*extents)[i].offset, (*extents)[i].length);
   }
 
   r = _flush_and_sync_log(l);
   assert(r == 0);
 
   if (logger)
-    logger->inc(l_bluefs_reclaim_bytes, *length);
+    logger->inc(l_bluefs_reclaim_bytes, got);
   dout(1) << __func__ << " bdev " << id << " want 0x" << std::hex << want
-         << " got 0x" << *offset << "~" << *length << std::dec << dendl;
+         << " got " << *extents << dendl;
   return 0;
 }
 

diff --git a/src/os/bluestore/BlueFS.h b/src/os/bluestore/BlueFS.h
index f2bb86f2d3d1d5269b7cb12d8a19d4024177e174..f9754306e5008e97d07a9299eefa2f76e793d6d3 100644 (file)
--- a/src/os/bluestore/BlueFS.h
+++ b/src/os/bluestore/BlueFS.h
@@ -389,7 +389,7 @@ public:
 
   /// reclaim block space
   int reclaim_blocks(unsigned bdev, uint64_t want,
-                    uint64_t *offset, uint32_t *length);
+                    AllocExtentVector *extents);
 
   void flush(FileWriter *h) {
     std::lock_guard<std::mutex> l(lock);

diff --git a/src/os/bluestore/BlueStore.cc b/src/os/bluestore/BlueStore.cc
index ba448cf35784759835e777318997dbd96f815325..461721a788affb6fd30243022dc9e038fcf79572 100644 (file)
--- a/src/os/bluestore/BlueStore.cc
+++ b/src/os/bluestore/BlueStore.cc
@@ -3715,18 +3715,17 @@ int BlueStore::_balance_bluefs_freespace(vector<bluestore_pextent_t> *extents)
             << " (" << pretty_si_t(reclaim) << ")" << dendl;
 
     while (reclaim > 0) {
-      uint64_t offset = 0;
-      uint32_t length = 0;
-
       // NOTE: this will block and do IO.
+      AllocExtentVector extents;
       int r = bluefs->reclaim_blocks(bluefs_shared_bdev, reclaim,
-                                  &offset, &length);
+                                    &extents);
       assert(r >= 0);
 
-      bluefs_extents.erase(offset, length);
-      bluefs_extents_reclaiming.insert(offset, length);
-
-      reclaim -= length;
+      for (auto e : extents) {
+       bluefs_extents.erase(e.offset, e.length);
+       bluefs_extents_reclaiming.insert(e.offset, e.length);
+       reclaim -= e.length;
+      }
     }
 
     ret = 1;