From 3a96db924968b65afed5fc1355acbf185455c5c4 Mon Sep 17 00:00:00 2001 From: Igor Fedotov Date: Mon, 19 Aug 2019 15:19:25 +0300 Subject: [PATCH] os/bluestore: simplify Allocator::get_fragmentation() signature Remove alloc_unit parameter. Signed-off-by: Igor Fedotov --- src/os/bluestore/Allocator.cc | 6 ++++-- src/os/bluestore/Allocator.h | 2 +- src/os/bluestore/BitmapAllocator.h | 2 +- src/os/bluestore/BlueStore.cc | 2 +- src/os/bluestore/StupidAllocator.cc | 13 ++++++++----- src/os/bluestore/StupidAllocator.h | 5 +++-- src/test/objectstore/Allocator_test.cc | 14 +++++++------- 7 files changed, 25 insertions(+), 19 deletions(-) diff --git a/src/os/bluestore/Allocator.cc b/src/os/bluestore/Allocator.cc index 748589fac22d9..d6f3b03696e7e 100644 --- a/src/os/bluestore/Allocator.cc +++ b/src/os/bluestore/Allocator.cc @@ -13,7 +13,9 @@ class Allocator::SocketHook : public AdminSocketHook { std::string name; public: - explicit SocketHook(Allocator *alloc, const std::string& _name) : alloc(alloc), name(_name) + explicit SocketHook(Allocator *alloc, + const std::string& _name) : + alloc(alloc), name(_name) { AdminSocket *admin_socket = g_ceph_context->get_admin_socket(); if (name.empty()) { @@ -102,7 +104,7 @@ Allocator *Allocator::create(CephContext* cct, string type, { Allocator* alloc = nullptr; if (type == "stupid") { - alloc = new StupidAllocator(cct, name); + alloc = new StupidAllocator(cct, name, block_size); } else if (type == "bitmap") { alloc = new BitmapAllocator(cct, size, block_size, name); } diff --git a/src/os/bluestore/Allocator.h b/src/os/bluestore/Allocator.h index 61299bf58767a..9f679d584e1f8 100644 --- a/src/os/bluestore/Allocator.h +++ b/src/os/bluestore/Allocator.h @@ -52,7 +52,7 @@ public: virtual void init_rm_free(uint64_t offset, uint64_t length) = 0; virtual uint64_t get_free() = 0; - virtual double get_fragmentation(uint64_t alloc_unit) + virtual double get_fragmentation() { return 0.0; } diff --git a/src/os/bluestore/BitmapAllocator.h b/src/os/bluestore/BitmapAllocator.h index ed7f122775fbb..996663b49a2b0 100644 --- a/src/os/bluestore/BitmapAllocator.h +++ b/src/os/bluestore/BitmapAllocator.h @@ -37,7 +37,7 @@ public: void dump() override; void dump(std::function notify) override; - double get_fragmentation(uint64_t) override + double get_fragmentation() override { return _get_fragmentation(); } diff --git a/src/os/bluestore/BlueStore.cc b/src/os/bluestore/BlueStore.cc index 8ef4c62eb9db7..060d334570f02 100644 --- a/src/os/bluestore/BlueStore.cc +++ b/src/os/bluestore/BlueStore.cc @@ -11117,7 +11117,7 @@ void BlueStore::_kv_finalize_thread() _reap_collections(); logger->set(l_bluestore_fragmentation, - (uint64_t)(alloc->get_fragmentation(min_alloc_size) * 1000)); + (uint64_t)(alloc->get_fragmentation() * 1000)); log_latency("kv_final", l_bluestore_kv_final_lat, diff --git a/src/os/bluestore/StupidAllocator.cc b/src/os/bluestore/StupidAllocator.cc index c80c855ad40e4..f75f74468a9ff 100644 --- a/src/os/bluestore/StupidAllocator.cc +++ b/src/os/bluestore/StupidAllocator.cc @@ -10,10 +10,13 @@ #undef dout_prefix #define dout_prefix *_dout << "stupidalloc 0x" << this << " " -StupidAllocator::StupidAllocator(CephContext* cct, const std::string& name) +StupidAllocator::StupidAllocator(CephContext* cct, + const std::string& name, + int64_t _block_size) : Allocator(name), cct(cct), num_free(0), free(10), - last_alloc(0) + last_alloc(0), + block_size(_block_size) { } @@ -253,15 +256,15 @@ uint64_t StupidAllocator::get_free() return num_free; } -double StupidAllocator::get_fragmentation(uint64_t alloc_unit) +double StupidAllocator::get_fragmentation() { - ceph_assert(alloc_unit); + ceph_assert(block_size); double res; uint64_t max_intervals = 0; uint64_t intervals = 0; { std::lock_guard l(lock); - max_intervals = p2roundup(num_free, alloc_unit) / alloc_unit; + max_intervals = p2roundup(num_free, block_size) / block_size; for (unsigned bin = 0; bin < free.size(); ++bin) { intervals += free[bin].num_intervals(); } diff --git a/src/os/bluestore/StupidAllocator.h b/src/os/bluestore/StupidAllocator.h index 23264757c3fd5..d9c4a4478b632 100644 --- a/src/os/bluestore/StupidAllocator.h +++ b/src/os/bluestore/StupidAllocator.h @@ -18,6 +18,7 @@ class StupidAllocator : public Allocator { ceph::mutex lock = ceph::make_mutex("StupidAllocator::lock"); int64_t num_free; ///< total bytes in freelist + int64_t block_size; typedef mempool::bluestore_alloc::pool_allocator< pair> allocator_t; @@ -35,7 +36,7 @@ class StupidAllocator : public Allocator { uint64_t alloc_unit); public: - StupidAllocator(CephContext* cct, const std::string& name = ""); + StupidAllocator(CephContext* cct, const std::string& name, int64_t block_size); ~StupidAllocator() override; int64_t allocate( @@ -50,7 +51,7 @@ public: const interval_set& release_set) override; uint64_t get_free() override; - double get_fragmentation(uint64_t alloc_unit) override; + double get_fragmentation() override; void dump() override; void dump(std::function notify) override; diff --git a/src/test/objectstore/Allocator_test.cc b/src/test/objectstore/Allocator_test.cc index 06285a971f0cf..040a82706623e 100644 --- a/src/test/objectstore/Allocator_test.cc +++ b/src/test/objectstore/Allocator_test.cc @@ -243,7 +243,7 @@ TEST_P(AllocTest, test_alloc_fragmentation) alloc->init_add_free(0, capacity); bool bitmap_alloc = GetParam() == std::string("bitmap"); - EXPECT_EQ(0.0, alloc->get_fragmentation(alloc_unit)); + EXPECT_EQ(0.0, alloc->get_fragmentation()); for (size_t i = 0; i < capacity / alloc_unit; ++i) { @@ -255,7 +255,7 @@ TEST_P(AllocTest, test_alloc_fragmentation) // bitmap fragmentation calculation doesn't provide such constant // estimate if (!bitmap_alloc) { - EXPECT_EQ(0.0, alloc->get_fragmentation(alloc_unit)); + EXPECT_EQ(0.0, alloc->get_fragmentation()); } } EXPECT_EQ(-ENOSPC, alloc->allocate(want_size, alloc_unit, 0, 0, &tmp)); @@ -266,7 +266,7 @@ TEST_P(AllocTest, test_alloc_fragmentation) release_set.insert(allocated[i].offset, allocated[i].length); alloc->release(release_set); } - EXPECT_EQ(1.0, alloc->get_fragmentation(alloc_unit)); + EXPECT_EQ(1.0, alloc->get_fragmentation()); EXPECT_EQ(66u, uint64_t(alloc->get_fragmentation_score() * 100)); for (size_t i = 1; i < allocated.size() / 2; i += 2) @@ -277,10 +277,10 @@ TEST_P(AllocTest, test_alloc_fragmentation) } if (bitmap_alloc) { // fragmentation = one l1 slot is free + one l1 slot is partial - EXPECT_EQ(50U, uint64_t(alloc->get_fragmentation(alloc_unit) * 100)); + EXPECT_EQ(50U, uint64_t(alloc->get_fragmentation() * 100)); } else { // fragmentation approx = 257 intervals / 768 max intervals - EXPECT_EQ(33u, uint64_t(alloc->get_fragmentation(alloc_unit) * 100)); + EXPECT_EQ(33u, uint64_t(alloc->get_fragmentation() * 100)); } EXPECT_EQ(27u, uint64_t(alloc->get_fragmentation_score() * 100)); @@ -294,7 +294,7 @@ TEST_P(AllocTest, test_alloc_fragmentation) // extents that causes some minor fragmentation (minor bug or by-design behavior?). // Hence leaving just two // digits after decimal point due to this. - EXPECT_EQ(0u, uint64_t(alloc->get_fragmentation(alloc_unit) * 100)); + EXPECT_EQ(0u, uint64_t(alloc->get_fragmentation() * 100)); if (bitmap_alloc) { EXPECT_EQ(0u, uint64_t(alloc->get_fragmentation_score() * 100)); } else { @@ -316,7 +316,7 @@ TEST_P(AllocTest, test_dump_fragmentation_score) init_alloc(capacity, alloc_unit); alloc->init_add_free(0, capacity); - EXPECT_EQ(0.0, alloc->get_fragmentation(alloc_unit)); + EXPECT_EQ(0.0, alloc->get_fragmentation()); EXPECT_EQ(0.0, alloc->get_fragmentation_score()); uint64_t allocated_cnt = 0; -- 2.39.5