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>
(cherry picked from commit 882ad33b527c9ea2224dcdc28f9fa3f459ee4712)

diff --git a/src/os/bluestore/BlueFS.cc b/src/os/bluestore/BlueFS.cc
index 97079d0aa70a8fb7391669a16714b55dae82503b..e4b8c94ec87e192536a3c1bf3e91c10f4dfb9055 100644 (file)
--- a/src/os/bluestore/BlueFS.cc
+++ b/src/os/bluestore/BlueFS.cc
@@ -168,7 +168,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
@@ -178,30 +178,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 / g_conf->bluefs_alloc_size);
-
+  uint64_t got = 0;
   r = alloc[id]->allocate(want, g_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 ed4c9edbc21e91ebda100556f0388bdc6d5e7520..2adf45cf950df43020f1cb52ec4de6ed7a05d5c4 100644 (file)
--- a/src/os/bluestore/BlueFS.h
+++ b/src/os/bluestore/BlueFS.h
@@ -388,7 +388,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 57569e0c7ac65eea4ad14da29d245ee87727292a..fe73545cca093fa6ebf71f48fab9e5df60b0b2db 100644 (file)
--- a/src/os/bluestore/BlueStore.cc
+++ b/src/os/bluestore/BlueStore.cc
@@ -3655,18 +3655,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;