From 5ab034345d7320fbc86a2133c0c29ec1aca4b71a Mon Sep 17 00:00:00 2001 From: Sage Weil Date: Tue, 17 Jan 2017 10:20:07 -0500 Subject: [PATCH] os/bluestore: return blocks allocated from allocate() Instead of having a separate output argument with the number of blocks allocated, just return it via the return value. Simplifies the calling convention. Signed-off-by: Sage Weil --- src/os/bluestore/Allocator.h | 10 +++++----- src/os/bluestore/BitMapAllocator.cc | 14 ++++++-------- src/os/bluestore/BitMapAllocator.h | 11 +++++++---- src/os/bluestore/BlueFS.cc | 19 ++++++++----------- src/os/bluestore/BlueStore.cc | 28 ++++++++++++---------------- src/os/bluestore/StupidAllocator.cc | 12 ++++-------- src/os/bluestore/StupidAllocator.h | 7 ++++--- 7 files changed, 46 insertions(+), 55 deletions(-) diff --git a/src/os/bluestore/Allocator.h b/src/os/bluestore/Allocator.h index 53acd429f64..7cf64bf82f7 100644 --- a/src/os/bluestore/Allocator.h +++ b/src/os/bluestore/Allocator.h @@ -35,13 +35,13 @@ public: * Apart from that extents can vary between these lower and higher limits according * to free block search algorithm and availability of contiguous space. */ - virtual int allocate(uint64_t want_size, uint64_t alloc_unit, - uint64_t max_alloc_size, int64_t hint, - AllocExtentVector *extents, int *count, uint64_t *ret_len) = 0; + virtual int64_t allocate(uint64_t want_size, uint64_t alloc_unit, + uint64_t max_alloc_size, int64_t hint, + AllocExtentVector *extents, int *count) = 0; int allocate(uint64_t want_size, uint64_t alloc_unit, - int64_t hint, AllocExtentVector *extents, int *count, uint64_t *ret_len) { - return allocate(want_size, alloc_unit, want_size, hint, extents, count, ret_len); + int64_t hint, AllocExtentVector *extents, int *count) { + return allocate(want_size, alloc_unit, want_size, hint, extents, count); } virtual int release( diff --git a/src/os/bluestore/BitMapAllocator.cc b/src/os/bluestore/BitMapAllocator.cc index 7a8331f8f61..74e7c016abb 100644 --- a/src/os/bluestore/BitMapAllocator.cc +++ b/src/os/bluestore/BitMapAllocator.cc @@ -108,9 +108,9 @@ void BitMapAllocator::unreserve(uint64_t unused) m_bit_alloc->unreserve_blocks(nblks); } -int BitMapAllocator::allocate( +int64_t BitMapAllocator::allocate( uint64_t want_size, uint64_t alloc_unit, uint64_t max_alloc_size, - int64_t hint, mempool::bluestore_alloc::vector *extents, int *count, uint64_t *ret_len) + int64_t hint, mempool::bluestore_alloc::vector *extents, int *count) { assert(!(alloc_unit % m_block_size)); @@ -125,27 +125,25 @@ int BitMapAllocator::allocate( << dendl; return allocate_dis(want_size, alloc_unit / m_block_size, - max_alloc_size, hint / m_block_size, extents, count, ret_len); + max_alloc_size, hint / m_block_size, extents, count); } -int BitMapAllocator::allocate_dis( +int64_t BitMapAllocator::allocate_dis( uint64_t want_size, uint64_t alloc_unit, uint64_t max_alloc_size, - int64_t hint, mempool::bluestore_alloc::vector *extents, int *count, uint64_t *ret_len) + int64_t hint, mempool::bluestore_alloc::vector *extents, int *count) { ExtentList block_list = ExtentList(extents, m_block_size, max_alloc_size); int64_t nblks = (want_size + m_block_size - 1) / m_block_size; int64_t num = 0; *count = 0; - *ret_len = 0; num = m_bit_alloc->alloc_blocks_dis_res(nblks, alloc_unit, hint, &block_list); if (num == 0) { return -ENOSPC; } *count = block_list.get_extent_count(); - *ret_len = num * m_block_size; - return 0; + return num * m_block_size; } int BitMapAllocator::release( diff --git a/src/os/bluestore/BitMapAllocator.h b/src/os/bluestore/BitMapAllocator.h index a10c7e4324a..8e86a9c6ca1 100644 --- a/src/os/bluestore/BitMapAllocator.h +++ b/src/os/bluestore/BitMapAllocator.h @@ -21,8 +21,10 @@ class BitMapAllocator : public Allocator { void insert_free(uint64_t offset, uint64_t len); - int allocate_dis(uint64_t want_size, uint64_t alloc_unit, uint64_t max_alloc_size, - int64_t hint, mempool::bluestore_alloc::vector *extents, int *count, uint64_t *ret_len); + int64_t allocate_dis( + uint64_t want_size, uint64_t alloc_unit, uint64_t max_alloc_size, + int64_t hint, mempool::bluestore_alloc::vector *extents, + int *count); public: BitMapAllocator(CephContext* cct, int64_t device_size, int64_t block_size); @@ -31,9 +33,10 @@ public: int reserve(uint64_t need); void unreserve(uint64_t unused); - int allocate( + int64_t allocate( uint64_t want_size, uint64_t alloc_unit, uint64_t max_alloc_size, - int64_t hint, mempool::bluestore_alloc::vector *extents, int *count, uint64_t *ret_len); + int64_t hint, mempool::bluestore_alloc::vector *extents, + int *count); int release( uint64_t offset, uint64_t length); diff --git a/src/os/bluestore/BlueFS.cc b/src/os/bluestore/BlueFS.cc index d3c1d0337e0..601b77c1e81 100644 --- a/src/os/bluestore/BlueFS.cc +++ b/src/os/bluestore/BlueFS.cc @@ -180,12 +180,10 @@ 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 got = 0; - r = alloc[id]->allocate(want, cct->_conf->bluefs_alloc_size, 0, - extents, &count, &got); - - assert(r >= 0); - if (got < want) + int64_t got = alloc[id]->allocate(want, cct->_conf->bluefs_alloc_size, 0, + extents, &count); + assert(got > 0); + if (got < (int64_t)want) alloc[id]->unreserve(want - got); for (int i = 0; i < count; i++) { @@ -1768,16 +1766,15 @@ int BlueFS::_allocate(uint8_t id, uint64_t len, } int count = 0; - uint64_t alloc_len = 0; AllocExtentVector extents; - r = alloc[id]->allocate(left, min_alloc_size, hint, - &extents, &count, &alloc_len); - if (r < 0 || alloc_len < left) { + int64_t alloc_len = alloc[id]->allocate(left, min_alloc_size, hint, + &extents, &count); + if (alloc_len < (int64_t)left) { derr << __func__ << " allocate failed on 0x" << std::hex << left << " min_alloc_size 0x" << min_alloc_size << std::dec << dendl; alloc[id]->dump(); assert(0 == "allocate failed... wtf"); - return r; + return -ENOSPC; } for (int i = 0; i < count; i++) { diff --git a/src/os/bluestore/BlueStore.cc b/src/os/bluestore/BlueStore.cc index ba835c2589f..b0a19b62042 100644 --- a/src/os/bluestore/BlueStore.cc +++ b/src/os/bluestore/BlueStore.cc @@ -3809,17 +3809,16 @@ int BlueStore::_balance_bluefs_freespace(PExtentVector *extents) assert(r == 0); int count = 0; - uint64_t alloc_len = 0; AllocExtentVector exts; - r = alloc->allocate(gift, cct->_conf->bluefs_alloc_size, 0, 0, &exts, - &count, &alloc_len); + int64_t alloc_len = alloc->allocate(gift, cct->_conf->bluefs_alloc_size, + 0, 0, &exts, &count); - if (r < 0 || alloc_len < gift) { + if (alloc_len < (int64_t)gift) { derr << __func__ << " allocate failed on 0x" << std::hex << gift << " min_alloc_size 0x" << min_alloc_size << std::dec << dendl; alloc->dump(); assert(0 == "allocate failed, wtf"); - return r; + return -ENOSPC; } assert(count > 0); @@ -8109,20 +8108,17 @@ int BlueStore::_do_alloc_write( } int count = 0; - uint64_t alloc_len = 0; AllocExtentVector extents; - int r = alloc->allocate(final_length, min_alloc_size, max_alloc_size, - hint, &extents, &count, &alloc_len); - assert(r == 0 && alloc_len == final_length); - need -= final_length; - txc->statfs_delta.allocated() += final_length; - assert(count > 0); - hint = extents[count - 1].end(); - - for (int i = 0; i < count; i++) { - bluestore_pextent_t e = bluestore_pextent_t(extents[i]); + int64_t got = alloc->allocate(final_length, min_alloc_size, max_alloc_size, + hint, &extents, &count); + assert(got == (int64_t)final_length); + need -= got; + txc->statfs_delta.allocated() += got; + for (auto& p : extents) { + bluestore_pextent_t e = bluestore_pextent_t(p); txc->allocated.insert(e.offset, e.length); dblob.extents.push_back(e); + hint = p.end(); } dout(20) << __func__ << " blob " << *b diff --git a/src/os/bluestore/StupidAllocator.cc b/src/os/bluestore/StupidAllocator.cc index 043697149b9..cf7edcead8e 100644 --- a/src/os/bluestore/StupidAllocator.cc +++ b/src/os/bluestore/StupidAllocator.cc @@ -83,7 +83,7 @@ static uint64_t aligned_len(btree_interval_set::iterator p, return p.get_len() - skew; } -int StupidAllocator::allocate_int( +int64_t StupidAllocator::allocate_int( uint64_t want_size, uint64_t alloc_unit, int64_t hint, uint64_t *offset, uint32_t *length) { @@ -201,14 +201,13 @@ int StupidAllocator::allocate_int( return 0; } -int StupidAllocator::allocate( +int64_t StupidAllocator::allocate( uint64_t want_size, uint64_t alloc_unit, uint64_t max_alloc_size, int64_t hint, mempool::bluestore_alloc::vector *extents, - int *count, - uint64_t *ret_len) + int *count) { uint64_t allocated_size = 0; uint64_t offset = 0; @@ -219,7 +218,6 @@ int StupidAllocator::allocate( max_alloc_size = want_size; } *count = 0; - *ret_len = 0; ExtentList block_list = ExtentList(extents, 1, max_alloc_size); @@ -238,12 +236,10 @@ int StupidAllocator::allocate( } *count = block_list.get_extent_count(); - *ret_len = allocated_size; if (allocated_size == 0) { return -ENOSPC; } - - return 0; + return allocated_size; } int StupidAllocator::release( diff --git a/src/os/bluestore/StupidAllocator.h b/src/os/bluestore/StupidAllocator.h index b3b2950005b..6a02993d671 100644 --- a/src/os/bluestore/StupidAllocator.h +++ b/src/os/bluestore/StupidAllocator.h @@ -31,11 +31,12 @@ public: int reserve(uint64_t need); void unreserve(uint64_t unused); - int allocate( + int64_t allocate( uint64_t want_size, uint64_t alloc_unit, uint64_t max_alloc_size, - int64_t hint, mempool::bluestore_alloc::vector *extents, int *count, uint64_t *ret_len); + int64_t hint, mempool::bluestore_alloc::vector *extents, + int *count); - int allocate_int( + int64_t allocate_int( uint64_t want_size, uint64_t alloc_unit, int64_t hint, uint64_t *offset, uint32_t *length); -- 2.39.5