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 <xie.xingguo@zte.com.cn>
os/bluestore: check against we don't overflow

Signed-off-by: xie xingguo <xie.xingguo@zte.com.cn>
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 <xie.xingguo@zte.com.cn>
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 <xie.xingguo@zte.com.cn>

diff --git a/src/os/bluestore/BitmapFreelistManager.cc b/src/os/bluestore/BitmapFreelistManager.cc
index f652a40843842f2ac62a4814fae7e44995992a66..ad651b145740324569e988ddd438a1e756774f17 100644 (file)
--- 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 361ebbabe93b8cc1e72bca4ac79506ab590e51c2..c4ba07b251af50f52d98df5c919c8388634b5ee9 100644 (file)
--- 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<CollectionRef>::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);