From 88bea46242d4eb6bdfd8309ddf8843c9a14e5f25 Mon Sep 17 00:00:00 2001 From: Igor Fedotov Date: Mon, 7 May 2018 16:26:03 +0300 Subject: [PATCH] os/bluestore: get rid off allocator's reserve method Signed-off-by: Igor Fedotov --- src/os/bluestore/Allocator.h | 3 --- src/os/bluestore/BitMapAllocator.cc | 31 ++------------------- src/os/bluestore/BitMapAllocator.h | 3 --- src/os/bluestore/BlueFS.cc | 28 +++++++------------ src/os/bluestore/BlueStore.cc | 37 +++++++++++--------------- src/os/bluestore/StupidAllocator.cc | 25 ----------------- src/os/bluestore/StupidAllocator.h | 4 --- src/test/objectstore/Allocator_test.cc | 15 +---------- 8 files changed, 27 insertions(+), 119 deletions(-) diff --git a/src/os/bluestore/Allocator.h b/src/os/bluestore/Allocator.h index d3a48db6501..6594d8aeb60 100644 --- a/src/os/bluestore/Allocator.h +++ b/src/os/bluestore/Allocator.h @@ -20,9 +20,6 @@ class Allocator { public: virtual ~Allocator() {} - virtual int reserve(uint64_t need) = 0; - virtual void unreserve(uint64_t unused) = 0; - /* * Allocate required number of blocks in n number of extents. * Min and Max number of extents are limited by: diff --git a/src/os/bluestore/BitMapAllocator.cc b/src/os/bluestore/BitMapAllocator.cc index 90395e6712e..4ef5e61cdd2 100644 --- a/src/os/bluestore/BitMapAllocator.cc +++ b/src/os/bluestore/BitMapAllocator.cc @@ -80,35 +80,6 @@ void BitMapAllocator::insert_free(uint64_t off, uint64_t len) len / m_block_size); } -int BitMapAllocator::reserve(uint64_t need) -{ - int nblks = need / m_block_size; // apply floor - assert(!(need % m_block_size)); - dout(10) << __func__ << " instance " << (uint64_t) this - << " num_used " << m_bit_alloc->get_used_blocks() - << " total " << m_bit_alloc->total_blocks() - << dendl; - - if (!m_bit_alloc->reserve_blocks(nblks)) { - return -ENOSPC; - } - return 0; -} - -void BitMapAllocator::unreserve(uint64_t unused) -{ - int nblks = unused / m_block_size; - assert(!(unused % m_block_size)); - - dout(10) << __func__ << " instance " << (uint64_t) this - << " unused " << nblks - << " num used " << m_bit_alloc->get_used_blocks() - << " total " << m_bit_alloc->total_blocks() - << dendl; - - m_bit_alloc->unreserve_blocks(nblks); -} - int64_t BitMapAllocator::allocate( uint64_t want_size, uint64_t alloc_unit, uint64_t max_alloc_size, int64_t hint, PExtentVector *extents) @@ -119,6 +90,8 @@ int64_t BitMapAllocator::allocate( assert(!max_alloc_size || max_alloc_size >= alloc_unit); + //FIXME: reproduce reserve/unreserve + dout(10) << __func__ <<" instance "<< (uint64_t) this << " want_size " << want_size << " alloc_unit " << alloc_unit diff --git a/src/os/bluestore/BitMapAllocator.h b/src/os/bluestore/BitMapAllocator.h index 7a1d84e6dfc..8283a68c5ba 100644 --- a/src/os/bluestore/BitMapAllocator.h +++ b/src/os/bluestore/BitMapAllocator.h @@ -27,9 +27,6 @@ public: BitMapAllocator(CephContext* cct, int64_t device_size, int64_t block_size); ~BitMapAllocator() override; - int reserve(uint64_t need) override; - void unreserve(uint64_t unused) override; - int64_t allocate( uint64_t want_size, uint64_t alloc_unit, uint64_t max_alloc_size, int64_t hint, PExtentVector *extents) override; diff --git a/src/os/bluestore/BlueFS.cc b/src/os/bluestore/BlueFS.cc index 567262c2fbd..4c485231880 100644 --- a/src/os/bluestore/BlueFS.cc +++ b/src/os/bluestore/BlueFS.cc @@ -215,19 +215,15 @@ int BlueFS::reclaim_blocks(unsigned id, uint64_t want, << " want 0x" << std::hex << want << std::dec << dendl; assert(id < alloc.size()); assert(alloc[id]); - int r = alloc[id]->reserve(want); - assert(r == 0); // caller shouldn't ask for more than they can get + int64_t got = alloc[id]->allocate(want, cct->_conf->bluefs_alloc_size, 0, extents); assert(got != 0); - if (got < (int64_t)want) { - alloc[id]->unreserve(want - std::max(0, got)); - if (got < 0) { - derr << __func__ << " failed to allocate space to return to bluestore" - << dendl; - alloc[id]->dump(); - return got; - } + if (got < 0) { + derr << __func__ << " failed to allocate space to return to bluestore" + << dendl; + alloc[id]->dump(); + return got; } for (auto& p : *extents) { @@ -236,7 +232,7 @@ int BlueFS::reclaim_blocks(unsigned id, uint64_t want, } flush_bdev(); - r = _flush_and_sync_log(l); + int r = _flush_and_sync_log(l); assert(r == 0); logger->inc(l_bluefs_reclaim_bytes, got); @@ -1969,15 +1965,10 @@ int BlueFS::_allocate(uint8_t id, uint64_t len, uint64_t min_alloc_size = cct->_conf->bluefs_alloc_size; uint64_t left = round_up_to(len, min_alloc_size); - int r = -ENOSPC; int64_t alloc_len = 0; PExtentVector extents; if (alloc[id]) { - r = alloc[id]->reserve(left); - } - - if (r == 0) { uint64_t hint = 0; if (!node->extents.empty() && node->extents.back().bdev == id) { hint = node->extents.back().end(); @@ -1985,10 +1976,9 @@ int BlueFS::_allocate(uint8_t id, uint64_t len, extents.reserve(4); // 4 should be (more than) enough for most allocations alloc_len = alloc[id]->allocate(left, min_alloc_size, hint, &extents); } - if (r < 0 || (alloc_len < (int64_t)left)) { - if (r == 0) { + if (alloc_len < (int64_t)left) { + if (alloc_len != 0) { interval_set to_release; - alloc[id]->unreserve(left - alloc_len); for (auto& p : extents) { to_release.insert(p.offset, p.length); } diff --git a/src/os/bluestore/BlueStore.cc b/src/os/bluestore/BlueStore.cc index b1cfed2651e..36a8fb98364 100644 --- a/src/os/bluestore/BlueStore.cc +++ b/src/os/bluestore/BlueStore.cc @@ -5138,17 +5138,12 @@ int BlueStore::_balance_bluefs_freespace(PExtentVector *extents) dout(10) << __func__ << " gifting " << gift << " (" << byte_u_t(gift) << ")" << dendl; - // fixme: just do one allocation to start... - int r = alloc->reserve(gift); - assert(r == 0); - int64_t alloc_len = alloc->allocate(gift, cct->_conf->bluefs_alloc_size, 0, 0, extents); if (alloc_len <= 0) { dout(0) << __func__ << " no allocate on 0x" << std::hex << gift << " min_alloc_size 0x" << min_alloc_size << std::dec << dendl; - alloc->unreserve(gift); _dump_alloc_on_rebalance_failure(); return 0; } else if (alloc_len < (int64_t)gift) { @@ -5156,7 +5151,6 @@ int BlueStore::_balance_bluefs_freespace(PExtentVector *extents) << " min_alloc_size 0x" << min_alloc_size << " allocated 0x" << alloc_len << std::dec << dendl; - alloc->unreserve(gift - alloc_len); _dump_alloc_on_rebalance_failure(); } for (auto& e : *extents) { @@ -6397,20 +6391,21 @@ int BlueStore::_fsck(bool deep, bool repair) if (!e->is_valid()) { continue; } - int r = alloc->reserve(e->length); - if (r != 0) { - derr << __func__ << " failed to reserve 0x" << std::hex << e->length - << " bytes, misreferences repair's incomplete" << std::dec << dendl; - bypass_rest = true; - break; - } PExtentVector exts; int64_t alloc_len = alloc->allocate(e->length, min_alloc_size, 0, 0, &exts); if (alloc_len < (int64_t)e->length) { derr << __func__ << " allocate failed on 0x" << std::hex << e->length << " min_alloc_size 0x" << min_alloc_size << std::dec << dendl; - assert(0 == "allocate failed, wtf"); + if (alloc_len > 0) { + interval_set release_set; + for (auto e : exts) { + release_set.insert(e.offset, e.length); + } + alloc->release(release_set); + } + bypass_rest = true; + break; } expected_statfs.allocated += e->length; if (compressed) { @@ -6728,8 +6723,6 @@ void BlueStore::inject_leaked(uint64_t len) txn = db->get_transaction(); PExtentVector exts; - int r = alloc->reserve(len); - assert(r == 0); int64_t alloc_len = alloc->allocate(len, min_alloc_size, min_alloc_size * 256, 0, &exts); assert(alloc_len >= (int64_t)len); @@ -10563,19 +10556,19 @@ int BlueStore::_do_alloc_write( need += wi.blob_length; } } - int r = alloc->reserve(need); - if (r < 0) { - derr << __func__ << " failed to reserve 0x" << std::hex << need << std::dec - << dendl; - return r; - } PExtentVector prealloc; prealloc.reserve(2 * wctx->writes.size());; int prealloc_left = 0; prealloc_left = alloc->allocate( need, min_alloc_size, need, 0, &prealloc); + if (prealloc_left < 0) { + derr << __func__ << " failed to allocate 0x" << std::hex << need << std::dec + << dendl; + return -ENOSPC; + } assert(prealloc_left == (int64_t)need); + dout(20) << __func__ << " prealloc " << prealloc << dendl; auto prealloc_pos = prealloc.begin(); diff --git a/src/os/bluestore/StupidAllocator.cc b/src/os/bluestore/StupidAllocator.cc index 99b9d030a16..dd17c7718f0 100644 --- a/src/os/bluestore/StupidAllocator.cc +++ b/src/os/bluestore/StupidAllocator.cc @@ -12,7 +12,6 @@ StupidAllocator::StupidAllocator(CephContext* cct) : cct(cct), num_free(0), - num_reserved(0), free(10), last_alloc(0) { @@ -48,28 +47,6 @@ void StupidAllocator::_insert_free(uint64_t off, uint64_t len) } } -int StupidAllocator::reserve(uint64_t need) -{ - std::lock_guard l(lock); - ldout(cct, 10) << __func__ << " need 0x" << std::hex << need - << " num_free 0x" << num_free - << " num_reserved 0x" << num_reserved << std::dec << dendl; - if ((int64_t)need > num_free - num_reserved) - return -ENOSPC; - num_reserved += need; - return 0; -} - -void StupidAllocator::unreserve(uint64_t unused) -{ - std::lock_guard l(lock); - ldout(cct, 10) << __func__ << " unused 0x" << std::hex << unused - << " num_free 0x" << num_free - << " num_reserved 0x" << num_reserved << std::dec << dendl; - assert(num_reserved >= (int64_t)unused); - num_reserved -= unused; -} - /// return the effective length of the extent if we align to alloc_unit uint64_t StupidAllocator::_aligned_len( StupidAllocator::interval_set_t::iterator p, @@ -196,9 +173,7 @@ int64_t StupidAllocator::allocate_int( } num_free -= *length; - num_reserved -= *length; assert(num_free >= 0); - assert(num_reserved >= 0); last_alloc = *offset + *length; return 0; } diff --git a/src/os/bluestore/StupidAllocator.h b/src/os/bluestore/StupidAllocator.h index a41feddc1de..c994cc718e4 100644 --- a/src/os/bluestore/StupidAllocator.h +++ b/src/os/bluestore/StupidAllocator.h @@ -17,7 +17,6 @@ class StupidAllocator : public Allocator { std::mutex lock; int64_t num_free; ///< total bytes in freelist - int64_t num_reserved; ///< reserved bytes typedef mempool::bluestore_alloc::pool_allocator< pair> allocator_t; @@ -38,9 +37,6 @@ public: StupidAllocator(CephContext* cct); ~StupidAllocator() override; - int reserve(uint64_t need) override; - void unreserve(uint64_t unused) override; - int64_t allocate( uint64_t want_size, uint64_t alloc_unit, uint64_t max_alloc_size, int64_t hint, PExtentVector *extents) override; diff --git a/src/test/objectstore/Allocator_test.cc b/src/test/objectstore/Allocator_test.cc index a53c2f7ff3a..cfa7a04f3f8 100644 --- a/src/test/objectstore/Allocator_test.cc +++ b/src/test/objectstore/Allocator_test.cc @@ -57,7 +57,6 @@ TEST_P(AllocTest, test_alloc_min_alloc) { init_alloc(blocks, block_size); alloc->init_add_free(block_size, block_size); - EXPECT_EQ(alloc->reserve(block_size), 0); PExtentVector extents; EXPECT_EQ(block_size, alloc->allocate(block_size, block_size, 0, (int64_t) 0, &extents)); @@ -68,7 +67,6 @@ TEST_P(AllocTest, test_alloc_min_alloc) */ { alloc->init_add_free(0, block_size * 4); - EXPECT_EQ(alloc->reserve(block_size * 4), 0); PExtentVector extents; EXPECT_EQ(4*block_size, alloc->allocate(4 * (uint64_t)block_size, (uint64_t) block_size, @@ -83,7 +81,6 @@ TEST_P(AllocTest, test_alloc_min_alloc) { alloc->init_add_free(0, block_size * 2); alloc->init_add_free(3 * block_size, block_size * 2); - EXPECT_EQ(alloc->reserve(block_size * 4), 0); PExtentVector extents; EXPECT_EQ(4*block_size, @@ -109,7 +106,6 @@ TEST_P(AllocTest, test_alloc_min_max_alloc) */ { alloc->init_add_free(0, block_size * 4); - EXPECT_EQ(alloc->reserve(block_size * 4), 0); PExtentVector extents; EXPECT_EQ(4*block_size, alloc->allocate(4 * (uint64_t)block_size, (uint64_t) block_size, @@ -127,7 +123,6 @@ TEST_P(AllocTest, test_alloc_min_max_alloc) */ { alloc->init_add_free(0, block_size * 4); - EXPECT_EQ(alloc->reserve(block_size * 4), 0); PExtentVector extents; EXPECT_EQ(4*block_size, alloc->allocate(4 * (uint64_t)block_size, (uint64_t) block_size, @@ -143,7 +138,6 @@ TEST_P(AllocTest, test_alloc_min_max_alloc) */ { alloc->init_add_free(0, block_size * 1024); - EXPECT_EQ(alloc->reserve(block_size * 1024), 0); PExtentVector extents; EXPECT_EQ(1024 * block_size, alloc->allocate(1024 * (uint64_t)block_size, @@ -160,7 +154,6 @@ TEST_P(AllocTest, test_alloc_min_max_alloc) */ { alloc->init_add_free(0, block_size * 16); - EXPECT_EQ(alloc->reserve(block_size * 16), 0); PExtentVector extents; EXPECT_EQ(16 * block_size, alloc->allocate(16 * (uint64_t)block_size, (uint64_t) block_size, @@ -183,7 +176,6 @@ TEST_P(AllocTest, test_alloc_failure) alloc->init_add_free(0, block_size * 256); alloc->init_add_free(block_size * 512, block_size * 256); - EXPECT_EQ(alloc->reserve(block_size * 512), 0); PExtentVector extents; EXPECT_EQ(512 * block_size, alloc->allocate(512 * (uint64_t)block_size, @@ -192,7 +184,6 @@ TEST_P(AllocTest, test_alloc_failure) alloc->init_add_free(0, block_size * 256); alloc->init_add_free(block_size * 512, block_size * 256); extents.clear(); - EXPECT_EQ(alloc->reserve(block_size * 512), 0); EXPECT_EQ(-ENOSPC, alloc->allocate(512 * (uint64_t)block_size, (uint64_t) block_size * 512, @@ -209,7 +200,6 @@ TEST_P(AllocTest, test_alloc_big) alloc->init_add_free(2*block_size, (blocks-2)*block_size); for (int64_t big = mas; big < 1048576*128; big*=2) { cout << big << std::endl; - EXPECT_EQ(alloc->reserve(big), 0); PExtentVector extents; EXPECT_EQ(big, alloc->allocate(big, mas, 0, &extents)); @@ -231,7 +221,6 @@ TEST_P(AllocTest, test_alloc_hint_bmap) alloc->init_add_free(0, blocks); PExtentVector extents; - alloc->reserve(blocks); allocated = alloc->allocate(1, 1, 1, zone_size, &extents); ASSERT_EQ(1, allocated); @@ -285,7 +274,6 @@ TEST_P(AllocTest, test_alloc_non_aligned_len) alloc->init_add_free(2097152, 1064960); alloc->init_add_free(3670016, 2097152); - EXPECT_EQ(0, alloc->reserve(want_size)); PExtentVector extents; EXPECT_EQ(want_size, alloc->allocate(want_size, alloc_unit, 0, &extents)); } @@ -307,12 +295,11 @@ TEST_P(AllocTest, test_alloc_fragmentation) for (size_t i = 0; i < capacity / alloc_unit; ++i) { tmp.clear(); - EXPECT_EQ(0, alloc->reserve(want_size)); EXPECT_EQ(want_size, alloc->allocate(want_size, alloc_unit, 0, 0, &tmp)); allocated.insert(allocated.end(), tmp.begin(), tmp.end()); EXPECT_EQ(0.0, alloc->get_fragmentation(alloc_unit)); } - EXPECT_EQ(-ENOSPC, alloc->reserve(want_size)); + EXPECT_EQ(-ENOSPC, alloc->allocate(want_size, alloc_unit, 0, 0, &tmp)); for (size_t i = 0; i < allocated.size(); i += 2) { -- 2.39.5