From fccad51d2ef7dfca92dd46e291dd99854dbf9752 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 (cherry picked from commit 88bea46242d4eb6bdfd8309ddf8843c9a14e5f25) --- src/os/bluestore/Allocator.h | 3 --- src/os/bluestore/BitMapAllocator.cc | 31 ++------------------------ src/os/bluestore/BitMapAllocator.h | 3 --- src/os/bluestore/BlueFS.cc | 22 +++++------------- src/os/bluestore/BlueStore.cc | 18 +++++---------- src/os/bluestore/StupidAllocator.cc | 25 --------------------- src/os/bluestore/StupidAllocator.h | 4 ---- src/test/objectstore/Allocator_test.cc | 15 +------------ 8 files changed, 15 insertions(+), 106 deletions(-) diff --git a/src/os/bluestore/Allocator.h b/src/os/bluestore/Allocator.h index 7e4e8648ef3d6..46a0a4ac4c913 100644 --- a/src/os/bluestore/Allocator.h +++ b/src/os/bluestore/Allocator.h @@ -22,9 +22,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 5a369719ff732..9c7e9751674d0 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 7a1d84e6dfc5d..8283a68c5bad5 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 db975a1395030..cb4dc11f02450 100644 --- a/src/os/bluestore/BlueFS.cc +++ b/src/os/bluestore/BlueFS.cc @@ -193,16 +193,12 @@ 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); - if (got < (int64_t)want) { - alloc[id]->unreserve(want - MAX(0, got)); - } - if (got <= 0) { + if (got < 0) { derr << __func__ << " failed to allocate space to return to bluestore" - << dendl; + << dendl; alloc[id]->dump(); return got; } @@ -214,7 +210,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); if (logger) @@ -1863,15 +1859,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(); @@ -1879,9 +1870,8 @@ 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) { - alloc[id]->unreserve(left - alloc_len); + if (alloc_len < (int64_t)left) { + if (alloc_len != 0) { interval_set to_release; 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 6a32c53b0f2ed..f31ee221b5af7 100644 --- a/src/os/bluestore/BlueStore.cc +++ b/src/os/bluestore/BlueStore.cc @@ -5173,17 +5173,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) { @@ -5191,7 +5186,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) { @@ -10321,19 +10315,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 13a7daa2274bb..c7224ea8c4e86 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); - dout(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); - dout(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, @@ -195,9 +172,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 a41feddc1ded3..c994cc718e45c 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 a53c2f7ff3a13..cfa7a04f3f85a 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