From: Adam Kupczyk Date: Mon, 7 Dec 2020 13:57:04 +0000 (+0100) Subject: os/bluestore: Added asserts for allocator regions X-Git-Tag: v16.2.14~68^2~2 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=bdb75c37db353c836a0bd66e52df0a9c955ce841;p=ceph.git os/bluestore: Added asserts for allocator regions Functions release/init_add_free/init_rm_free did not check its input against device size. It is incorrect and had been a problem when you shrink device. Signed-off-by: Adam Kupczyk (cherry picked from commit 9bdb2e3c3105bec2041f282fb7d5104f7d69a8c4) --- diff --git a/src/os/bluestore/Allocator.cc b/src/os/bluestore/Allocator.cc index f92821f0c853..e132cd49442f 100644 --- a/src/os/bluestore/Allocator.cc +++ b/src/os/bluestore/Allocator.cc @@ -109,7 +109,7 @@ public: Allocator::Allocator(const std::string& name, int64_t _capacity, int64_t _block_size) - : capacity(_capacity), block_size(_block_size) + : device_size(_capacity), block_size(_block_size) { asok_hook = new SocketHook(this, name); } @@ -129,7 +129,7 @@ Allocator *Allocator::create(CephContext* cct, string type, { Allocator* alloc = nullptr; if (type == "stupid") { - alloc = new StupidAllocator(cct, name, size, block_size); + alloc = new StupidAllocator(cct, size, block_size, name); } else if (type == "bitmap") { alloc = new BitmapAllocator(cct, size, block_size, name); } else if (type == "avl") { diff --git a/src/os/bluestore/Allocator.h b/src/os/bluestore/Allocator.h index 38ea4f997388..915e9db0ade8 100644 --- a/src/os/bluestore/Allocator.h +++ b/src/os/bluestore/Allocator.h @@ -80,7 +80,7 @@ public: const string& get_name() const; int64_t get_capacity() const { - return capacity; + return device_size; } int64_t get_block_size() const { @@ -90,9 +90,9 @@ public: private: class SocketHook; SocketHook* asok_hook = nullptr; - - int64_t capacity = 0; - int64_t block_size = 0; +protected: + const int64_t device_size = 0; + const int64_t block_size = 0; }; #endif diff --git a/src/os/bluestore/AvlAllocator.cc b/src/os/bluestore/AvlAllocator.cc index 5f17a3689b18..9152c0e32a8f 100644 --- a/src/os/bluestore/AvlAllocator.cc +++ b/src/os/bluestore/AvlAllocator.cc @@ -304,6 +304,7 @@ void AvlAllocator::_release(const interval_set& release_set) for (auto p = release_set.begin(); p != release_set.end(); ++p) { const auto offset = p.get_start(); const auto length = p.get_len(); + ceph_assert(offset + length <= uint64_t(num_total)); ldout(cct, 10) << __func__ << std::hex << " offset 0x" << offset << " length 0x" << length @@ -448,6 +449,7 @@ void AvlAllocator::init_add_free(uint64_t offset, uint64_t length) if (!length) return; std::lock_guard l(lock); + ceph_assert(offset + length <= uint64_t(num_total)); ldout(cct, 10) << __func__ << std::hex << " offset 0x" << offset << " length 0x" << length @@ -460,6 +462,7 @@ void AvlAllocator::init_rm_free(uint64_t offset, uint64_t length) if (!length) return; std::lock_guard l(lock); + ceph_assert(offset + length <= uint64_t(num_total)); ldout(cct, 10) << __func__ << std::hex << " offset 0x" << offset << " length 0x" << length diff --git a/src/os/bluestore/BitmapAllocator.cc b/src/os/bluestore/BitmapAllocator.cc index 0e15c41729fe..5a3967f5e625 100644 --- a/src/os/bluestore/BitmapAllocator.cc +++ b/src/os/bluestore/BitmapAllocator.cc @@ -52,9 +52,10 @@ void BitmapAllocator::release( const interval_set& release_set) { if (cct->_conf->subsys.should_gather()) { - for (auto r : release_set) { - ldout(cct, 10) << __func__ << " 0x" << std::hex << r.first << "~" << r.second - << std::dec << dendl; + for (auto& [offset, len] : release_set) { + ldout(cct, 10) << __func__ << " 0x" << std::hex << offset << "~" << len + << std::dec << dendl; + ceph_assert(offset + len <= (uint64_t)device_size); } } _free_l2(release_set); @@ -70,6 +71,7 @@ void BitmapAllocator::init_add_free(uint64_t offset, uint64_t length) auto mas = get_min_alloc_size(); uint64_t offs = round_up_to(offset, mas); uint64_t l = p2align(offset + length - offs, mas); + ceph_assert(offs + l <= (uint64_t)device_size); _mark_free(offs, l); ldout(cct, 10) << __func__ << " done" << dendl; @@ -81,6 +83,7 @@ void BitmapAllocator::init_rm_free(uint64_t offset, uint64_t length) auto mas = get_min_alloc_size(); uint64_t offs = round_up_to(offset, mas); uint64_t l = p2align(offset + length - offs, mas); + ceph_assert(offs + l <= (uint64_t)device_size); _mark_allocated(offs, l); ldout(cct, 10) << __func__ << " done" << dendl; } diff --git a/src/os/bluestore/BitmapAllocator.h b/src/os/bluestore/BitmapAllocator.h index bb6fa73a1a53..90bcd956f11c 100644 --- a/src/os/bluestore/BitmapAllocator.h +++ b/src/os/bluestore/BitmapAllocator.h @@ -15,7 +15,6 @@ class BitmapAllocator : public Allocator, public AllocatorLevel02 { CephContext* cct; - public: BitmapAllocator(CephContext* _cct, int64_t capacity, int64_t alloc_unit, const std::string& name); ~BitmapAllocator() override diff --git a/src/os/bluestore/StupidAllocator.cc b/src/os/bluestore/StupidAllocator.cc index 805a51fb090b..2c0ca7a43119 100644 --- a/src/os/bluestore/StupidAllocator.cc +++ b/src/os/bluestore/StupidAllocator.cc @@ -11,14 +11,15 @@ #define dout_prefix *_dout << "stupidalloc 0x" << this << " " StupidAllocator::StupidAllocator(CephContext* cct, - const std::string& name, - int64_t _size, - int64_t _block_size) - : Allocator(name, _size, _block_size), cct(cct), num_free(0), + int64_t capacity, + int64_t _block_size, + const std::string& name) + : Allocator(name, capacity, _block_size), + cct(cct), num_free(0), free(10) { ceph_assert(cct != nullptr); - bdev_block_size = cct->_conf->bdev_block_size; + ceph_assert(block_size > 0); } StupidAllocator::~StupidAllocator() @@ -27,8 +28,7 @@ StupidAllocator::~StupidAllocator() unsigned StupidAllocator::_choose_bin(uint64_t orig_len) { - ceph_assert(bdev_block_size > 0); - uint64_t len = orig_len / bdev_block_size; + uint64_t len = orig_len / block_size; int bin = std::min((int)cbits(len), (int)free.size() - 1); ldout(cct, 30) << __func__ << " len 0x" << std::hex << orig_len << std::dec << " -> " << bin << dendl; diff --git a/src/os/bluestore/StupidAllocator.h b/src/os/bluestore/StupidAllocator.h index e1fc101e0879..1ad377a39066 100644 --- a/src/os/bluestore/StupidAllocator.h +++ b/src/os/bluestore/StupidAllocator.h @@ -18,7 +18,6 @@ class StupidAllocator : public Allocator { ceph::mutex lock = ceph::make_mutex("StupidAllocator::lock"); int64_t num_free; ///< total bytes in freelist - uint64_t bdev_block_size; template using allocator_t = mempool::bluestore_alloc::pool_allocator>; @@ -38,9 +37,9 @@ class StupidAllocator : public Allocator { public: StupidAllocator(CephContext* cct, - const std::string& name, int64_t size, - int64_t block_size); + int64_t block_size, + const std::string& name); ~StupidAllocator() override; const char* get_type() const override {