From c5d458df94f940760c2af03265e65ce7df58630c Mon Sep 17 00:00:00 2001 From: Sage Weil Date: Fri, 30 Dec 2016 12:22:42 -0500 Subject: [PATCH] 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 (cherry picked from commit 882ad33b527c9ea2224dcdc28f9fa3f459ee4712) --- src/os/bluestore/BlueFS.cc | 23 ++++++++++------------- src/os/bluestore/BlueFS.h | 2 +- src/os/bluestore/BlueStore.cc | 15 +++++++-------- 3 files changed, 18 insertions(+), 22 deletions(-) diff --git a/src/os/bluestore/BlueFS.cc b/src/os/bluestore/BlueFS.cc index 97079d0aa70a8..e4b8c94ec87e1 100644 --- 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 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 ed4c9edbc21e9..2adf45cf950df 100644 --- 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 l(lock); diff --git a/src/os/bluestore/BlueStore.cc b/src/os/bluestore/BlueStore.cc index 57569e0c7ac65..fe73545cca093 100644 --- a/src/os/bluestore/BlueStore.cc +++ b/src/os/bluestore/BlueStore.cc @@ -3655,18 +3655,17 @@ int BlueStore::_balance_bluefs_freespace(vector *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; -- 2.39.5