From 7f564d9e2cc3b19e661bffe278516e7ab64d23b0 Mon Sep 17 00:00:00 2001 From: xie xingguo Date: Mon, 11 Jul 2016 10:59:27 +0800 Subject: [PATCH] os/bluestore: fix potential uninitialized nid of onode The _zero() process may implicitly create a new onode, thus we shall call _assign_nid() to initialize the nid properly. And if the onode already has one, _assign_nid() does nothing. So it is proper to call _assign_nid() here under any case. Mark's comments: This passed "ceph_test_objectstore --gtest_filter=*/2". This PR did not appear to have a significant impact on performance tests. Closes #10236 Signed-off-by: xie xingguo os/bluestore: check against we don't overflow Signed-off-by: xie xingguo os/bluestore: try to reap as many collections as we can So if there is one collection getting contiguously stucking, we don't abort at the same point each time. Signed-off-by: xie xingguo os/bluestore: make device size of BitFreelistManager is block-size aligned Otherwise if we try to set past-eof blocks as allocated durint create(), the call to _xor() will trigger the firing of the following assert: assert((length & block_mask) == length); Signed-off-by: xie xingguo --- src/os/bluestore/BitmapFreelistManager.cc | 3 ++- src/os/bluestore/BlueStore.cc | 11 +++++++++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/src/os/bluestore/BitmapFreelistManager.cc b/src/os/bluestore/BitmapFreelistManager.cc index f652a408438..ad651b14574 100644 --- a/src/os/bluestore/BitmapFreelistManager.cc +++ b/src/os/bluestore/BitmapFreelistManager.cc @@ -56,8 +56,9 @@ BitmapFreelistManager::BitmapFreelistManager(KeyValueDB *db, int BitmapFreelistManager::create(uint64_t new_size, KeyValueDB::Transaction txn) { - size = new_size; bytes_per_block = g_conf->bdev_block_size; + assert(ISP2(bytes_per_block)); + size = P2ALIGN(new_size, bytes_per_block); blocks_per_key = g_conf->bluestore_freelist_blocks_per_key; _init_misc(); diff --git a/src/os/bluestore/BlueStore.cc b/src/os/bluestore/BlueStore.cc index 361ebbabe93..c4ba07b251a 100644 --- a/src/os/bluestore/BlueStore.cc +++ b/src/os/bluestore/BlueStore.cc @@ -3322,6 +3322,8 @@ void BlueStore::_reap_collections() removed_colls.swap(removed_collections); } + bool all_reaped = true; + for (list::iterator p = removed_colls.begin(); p != removed_colls.end(); ++p) { @@ -3336,13 +3338,16 @@ void BlueStore::_reap_collections() } return true; })) { - return; + all_reaped = false; + continue; } c->onode_map.clear(); dout(10) << __func__ << " " << c->cid << " done" << dendl; } - dout(10) << __func__ << " all reaped" << dendl; + if (all_reaped) { + dout(10) << __func__ << " all reaped" << dendl; + } } // --------------- @@ -3664,6 +3669,7 @@ int BlueStore::_do_read( } else { uint64_t l = length - pos; if (pr != pr_end) { + assert(pr->first > pos + offset); l = pr->first - (pos + offset); } dout(30) << __func__ << " assemble 0x" << std::hex << pos @@ -6294,6 +6300,7 @@ int BlueStore::_do_zero(TransContext *txc, << dendl; int r = 0; o->exists = true; + _assign_nid(txc, o); _dump_onode(o); -- 2.39.5