os/bluestore: blob's collection initialization in ctor

Set collection on blob creation instead of setting it manually after
creation

Signed-off-by: Pere Diaz Bou <pere-altea@hotmail.com>

diff --git a/src/os/bluestore/BlueStore.cc b/src/os/bluestore/BlueStore.cc
index 626147dff026e57bf4ab97fed6958cb8ff6266f1..9e3c72f6e3d81e807a47086b3c9f96f30398d995 100644 (file)
--- a/src/os/bluestore/BlueStore.cc
+++ b/src/os/bluestore/BlueStore.cc
@@ -2177,10 +2177,7 @@ BlueStore::Blob::~Blob()
     coll_cache->rm_blob();
   }
   SharedBlob* sb = shared_blob.get();
-  if (!sb) {
-    ceph_assert(bc.buffer_map.empty());
-    return;
-  }
+  ceph_assert(sb || (!sb && bc.buffer_map.empty()));
 }
 
 void BlueStore::Blob::dump(Formatter* f) const
@@ -3343,7 +3340,7 @@ void BlueStore::ExtentMap::dup_esb(BlueStore* b, TransContext* txc,
       // dup the blob
       const bluestore_blob_t& blob = e.blob->get_blob();
       ceph_assert(blob.is_shared());
-      ceph_assert(e.blob->is_loaded());
+      ceph_assert(e.blob->is_shared_loaded());
       ceph_assert(!blob.has_unused());
       cb = c->new_blob();
       e.blob->last_encoded_id = n;
@@ -4074,7 +4071,7 @@ void BlueStore::ExtentMap::ExtentDecoder::decode_spanning_blobs(
   unsigned n;
   denc_varint(n, p);
   while (n--) {
-    BlueStore::BlobRef b(c->new_blob());
+    BlueStore::BlobRef b = c->new_blob();
     denc_varint(b->id, p);
     uint64_t sbid = 0;
     b->decode(p, struct_v, &sbid, true, c);
@@ -4513,7 +4510,7 @@ BlueStore::ExtentMap::debug_list_disk_layout()
 
     bluestore_extent_ref_map_t* ref_map = nullptr;
     if (bblob.is_shared()) {
-      ceph_assert(ep->blob->is_loaded());
+      ceph_assert(ep->blob->is_shared_loaded());
       bluestore_shared_blob_t* bsblob = ep->blob->shared_blob->persistent;
       ref_map = &bsblob->ref_map;
     }
@@ -4984,7 +4981,6 @@ void BlueStore::Collection::open_shared_blob(uint64_t sbid, BlobRef b)
   ceph_assert(!b->shared_blob);
   const bluestore_blob_t& blob = b->get_blob();
   if (!blob.is_shared()) {
-    b->collection = this;
     return;
   }
 
@@ -5030,8 +5026,6 @@ void BlueStore::Collection::load_shared_blob(SharedBlobRef sb)
 void BlueStore::Collection::make_blob_shared(uint64_t sbid, BlobRef b)
 {
   ldout(store->cct, 10) << __func__ << " " << *b << dendl;
-  ceph_assert(!b->is_loaded());
-  ceph_assert(!b->shared_blob);
 
   // update blob
   bluestore_blob_t& blob = b->dirty_blob();
@@ -5039,7 +5033,7 @@ void BlueStore::Collection::make_blob_shared(uint64_t sbid, BlobRef b)
   // drop any unused parts, unlikely we could use them in future
   blob.clear_flag(bluestore_blob_t::FLAG_HAS_UNUSED);
   // update shared blob
-  b->shared_blob = new SharedBlob(sbid, this);
+  b->set_shared_blob(new SharedBlob(sbid, this));
   b->shared_blob->loaded = true;
   b->shared_blob->persistent = new bluestore_shared_blob_t(sbid);
   shared_blob_set.add(this, b->shared_blob.get());
@@ -5175,20 +5169,16 @@ void BlueStore::Collection::split_cache(
        dest->cache->add_blob();
        SharedBlob* sb = b->shared_blob.get();
         b->collection = dest;
-       if (sb && sb->collection == dest) {
-         ldout(store->cct, 20) << __func__ << "  already moved " << *sb
-                               << dendl;
-         return;
-       }
-       ldout(store->cct, 20) << __func__ << "  moving " << *b << dendl;
-       if (b->get_sbid()) {
-          ldout(store->cct, 20) << __func__ << "  moving " << *sb << dendl;
-         ldout(store->cct, 20) << __func__
-                               << "   moving registration " << *sb << dendl;
-         shared_blob_set.remove(sb);
-         dest->shared_blob_set.add(dest, sb);
-       }
         if (sb) {
+          if (sb->collection == dest) {
+            ldout(store->cct, 20) << __func__ << "  already moved " << *sb
+              << dendl;
+            return;
+          }
+          ldout(store->cct, 20) << __func__ << "  moving " << *b << dendl;
+          ldout(store->cct, 20) << __func__ << "  moving " << *sb << dendl;
+          shared_blob_set.remove(sb);
+          dest->shared_blob_set.add(dest, sb);
           sb->collection = dest;
         }
       };

diff --git a/src/os/bluestore/BlueStore.h b/src/os/bluestore/BlueStore.h
index 1119a15318726577138f75e38c89e5439f649687..7baaf503e39ac3466888e24858b9862e3b1b8128 100644 (file)
--- a/src/os/bluestore/BlueStore.h
+++ b/src/os/bluestore/BlueStore.h
@@ -615,6 +615,7 @@ public:
       shared_blob = sb;
       ceph_assert(get_cache());
     }
+    Blob(CollectionRef collection) : collection(collection) {}
     BufferSpace bc;
   private:
     mutable bluestore_blob_t blob;  ///< decoded blob metadata
@@ -723,7 +724,7 @@ public:
       if (--nref == 0)
        delete this;
     }
-    bool is_loaded() const {
+    bool is_shared_loaded() const {
       return shared_blob && shared_blob->is_loaded();
     }
     inline BufferCacheShard* get_cache() {
@@ -1623,8 +1624,7 @@ private:
     uint64_t make_blob_unshared(SharedBlob *sb);
 
     BlobRef new_blob() {
-      BlobRef b = new Blob();
-      b->collection = this;
+      BlobRef b = new Blob(this);
       b->get_cache()->add_blob();
       return b;
     }

diff --git a/src/test/objectstore/test_bluestore_types.cc b/src/test/objectstore/test_bluestore_types.cc
index 6e047a09c251bdca9f8b25307cc4c90cf2dc7f24..2d3b6ed92693cf2041097b6f6d2154273acdaadb 100644 (file)
--- a/src/test/objectstore/test_bluestore_types.cc
+++ b/src/test/objectstore/test_bluestore_types.cc
@@ -433,8 +433,7 @@ TEST(Blob, put_ref)
       g_ceph_context, "lru", NULL);
 
     auto coll = ceph::make_ref<BlueStore::Collection>(&store, oc, bc, coll_t());
-    BlueStore::Blob b;
-    b.collection = coll;
+    BlueStore::Blob b(coll.get());
     b.set_shared_blob(new BlueStore::SharedBlob(coll.get()));
     b.dirty_blob().allocated_test(bluestore_pextent_t(0x40715000, 0x2000));
     b.dirty_blob().allocated_test(
@@ -467,8 +466,7 @@ TEST(Blob, put_ref)
   auto coll = ceph::make_ref<BlueStore::Collection>(&store, oc, bc, coll_t());
 
   {
-    BlueStore::Blob B;
-    B.collection = coll;
+    BlueStore::Blob B(coll.get());
     B.set_shared_blob(new BlueStore::SharedBlob(coll.get()));
     bluestore_blob_t& b = B.dirty_blob();
     PExtentVector r;
@@ -489,8 +487,7 @@ TEST(Blob, put_ref)
     ASSERT_EQ(mas*2, b.get_extents()[0].length);
   }
   {
-    BlueStore::Blob B;
-    B.collection = coll;
+    BlueStore::Blob B(coll.get());
     B.set_shared_blob(new BlueStore::SharedBlob(coll.get()));
     bluestore_blob_t& b = B.dirty_blob();
     PExtentVector r;
@@ -514,8 +511,7 @@ TEST(Blob, put_ref)
     ASSERT_EQ(mas*2, b.get_extents()[0].length);
   }
   {
-    BlueStore::Blob B;
-    B.collection = coll;
+    BlueStore::Blob B(coll.get());
     B.set_shared_blob(new BlueStore::SharedBlob(coll.get()));
     bluestore_blob_t& b = B.dirty_blob();
     PExtentVector r;
@@ -553,8 +549,7 @@ TEST(Blob, put_ref)
     ASSERT_EQ(3u, b.get_extents().size());
   }
   {
-    BlueStore::Blob B;
-    B.collection = coll;
+    BlueStore::Blob B(coll.get());
     B.set_shared_blob(new BlueStore::SharedBlob(coll.get()));
     bluestore_blob_t& b = B.dirty_blob();
     PExtentVector r;
@@ -595,8 +590,7 @@ TEST(Blob, put_ref)
     ASSERT_TRUE(b.get_extents()[4].is_valid());
   }
   {
-    BlueStore::Blob B;
-    B.collection = coll;
+    BlueStore::Blob B(coll);
     B.set_shared_blob(new BlueStore::SharedBlob(coll.get()));
     bluestore_blob_t& b = B.dirty_blob();
     PExtentVector r;
@@ -628,8 +622,7 @@ TEST(Blob, put_ref)
     ASSERT_TRUE(b.get_extents()[2].is_valid());
   }
   {
-    BlueStore::Blob B;
-    B.collection = coll;
+    BlueStore::Blob B(coll);
     B.set_shared_blob(new BlueStore::SharedBlob(coll.get()));
     bluestore_blob_t& b = B.dirty_blob();
     PExtentVector r;
@@ -667,8 +660,7 @@ TEST(Blob, put_ref)
     ASSERT_TRUE(b.get_extents()[2].is_valid());
   }
   {
-    BlueStore::Blob B;
-    B.collection = coll;
+    BlueStore::Blob B(coll);
     B.set_shared_blob(new BlueStore::SharedBlob(coll.get()));
     bluestore_blob_t& b = B.dirty_blob();
     PExtentVector r;
@@ -723,8 +715,7 @@ TEST(Blob, put_ref)
     ASSERT_FALSE(b.get_extents()[0].is_valid());
   }
   {
-    BlueStore::Blob B;
-    B.collection = coll;
+    BlueStore::Blob B(coll);
     B.set_shared_blob(new BlueStore::SharedBlob(coll.get()));
     bluestore_blob_t& b = B.dirty_blob();
     PExtentVector r;
@@ -779,8 +770,7 @@ TEST(Blob, put_ref)
     ASSERT_FALSE(b.get_extents()[0].is_valid());
   }
   {
-    BlueStore::Blob B;
-    B.collection = coll;
+    BlueStore::Blob B(coll.get());
     B.set_shared_blob(new BlueStore::SharedBlob(coll.get()));
     bluestore_blob_t& b = B.dirty_blob();
     PExtentVector r;
@@ -823,8 +813,7 @@ TEST(Blob, put_ref)
   }
   // verify csum chunk size if factored in properly
   {
-    BlueStore::Blob B;
-    B.collection = coll;
+    BlueStore::Blob B(coll.get());
     B.set_shared_blob(new BlueStore::SharedBlob(coll.get()));
     bluestore_blob_t& b = B.dirty_blob();
     PExtentVector r;
@@ -842,8 +831,7 @@ TEST(Blob, put_ref)
     ASSERT_EQ(mas*4, b.get_extents()[0].length);
   }
   {
-    BlueStore::Blob B;
-    B.collection = coll;
+    BlueStore::Blob B(coll.get());
     B.set_shared_blob(new BlueStore::SharedBlob(coll.get()));
     bluestore_blob_t& b = B.dirty_blob();
     b.allocated_test(bluestore_pextent_t(0x40101000, 0x4000));
@@ -865,8 +853,7 @@ TEST(Blob, put_ref)
     cout << "r " << r << std::endl;
   }
   {
-    BlueStore::Blob B;
-    B.collection = coll;
+    BlueStore::Blob B(coll.get());
     B.set_shared_blob(new BlueStore::SharedBlob(coll.get()));
     bluestore_blob_t& b = B.dirty_blob();
     b.allocated_test(bluestore_pextent_t(1, 0x5000));
@@ -884,8 +871,7 @@ TEST(Blob, put_ref)
     ASSERT_EQ(0x2000u, r[0].length);
   }
   {
-    BlueStore::Blob B;
-    B.collection = coll;
+    BlueStore::Blob B(coll.get());
     B.set_shared_blob(new BlueStore::SharedBlob(coll.get()));
     bluestore_blob_t& b = B.dirty_blob();
     b.allocated_test(bluestore_pextent_t(1, 0x7000));
@@ -914,8 +900,7 @@ TEST(Blob, put_ref)
       g_ceph_context, "lru", NULL);
 
     auto coll = ceph::make_ref<BlueStore::Collection>(&store, oc, bc, coll_t());
-    BlueStore::Blob B;
-    B.collection = coll;
+    BlueStore::Blob B(coll.get());
     B.set_shared_blob(new BlueStore::SharedBlob(coll.get()));
     bluestore_blob_t& b = B.dirty_blob();
     b.allocated_test(bluestore_pextent_t(1, 0x5000));
@@ -1004,9 +989,8 @@ TEST(Blob, split)
       g_ceph_context, "lru", NULL);
   auto coll = ceph::make_ref<BlueStore::Collection>(&store, oc, bc, coll_t());
   {
-    BlueStore::Blob L, R;
-    L.collection = coll;
-    R.collection = coll;
+    BlueStore::Blob L(coll.get());
+    BlueStore::Blob R(coll.get());
     L.set_shared_blob(new BlueStore::SharedBlob(coll.get()));
     R.set_shared_blob(new BlueStore::SharedBlob(coll.get()));
     L.dirty_blob().allocated_test(bluestore_pextent_t(0x2000, 0x2000));
@@ -1027,9 +1011,8 @@ TEST(Blob, split)
     ASSERT_EQ(0x1000u, R.get_referenced_bytes());
   }
   {
-    BlueStore::Blob L, R;
-    L.collection = coll;
-    R.collection = coll;
+    BlueStore::Blob L(coll.get());
+    BlueStore::Blob R(coll.get());
     L.set_shared_blob(new BlueStore::SharedBlob(coll.get()));
     R.set_shared_blob(new BlueStore::SharedBlob(coll.get()));
     L.dirty_blob().allocated_test(bluestore_pextent_t(0x2000, 0x1000));
@@ -1063,8 +1046,7 @@ TEST(Blob, legacy_decode)
   auto coll = ceph::make_ref<BlueStore::Collection>(&store, oc, bc, coll_t());
   bufferlist bl, bl2;
   {
-    BlueStore::Blob B;
-    B.collection = coll;
+    BlueStore::Blob B(coll.get());
 
     B.set_shared_blob(new BlueStore::SharedBlob(coll.get()));
     B.dirty_blob().allocated_test(bluestore_pextent_t(0x1, 0x2000));
@@ -1110,9 +1092,8 @@ TEST(Blob, legacy_decode)
 
     auto p = bl.front().begin_deep();
     auto p2 = bl2.front().begin_deep();
-    BlueStore::Blob Bres, Bres2;
-    Bres.collection = coll;
-    Bres2.collection = coll;
+    BlueStore::Blob Bres(coll.get());
+    BlueStore::Blob Bres2(coll.get());
     Bres.set_shared_blob(new BlueStore::SharedBlob(coll.get()));
     Bres2.set_shared_blob(new BlueStore::SharedBlob(coll.get()));
 
@@ -1440,7 +1421,6 @@ public:
     ) {
       uint32_t blob_length = (empty_aus + num_aus) * au_size;
       BlueStore::BlobRef b(coll->new_blob());
-      b->collection = coll;
       bluestore_blob_t& bb = b->dirty_blob();
       bb.init_csum(Checksummer::CSUM_CRC32C, csum_order, blob_length);
       for(size_t i = 0; i < num_aus; ++i) {
@@ -1705,8 +1685,7 @@ TEST(ExtentMap, dup_extent_map)
   size_t ext1_offs = 0x0;
   size_t ext1_len = 0x2000;
   size_t ext1_boffs = 0x0;
-  BlueStore::BlobRef b1(coll->new_blob());
-  b1->collection = coll;
+  BlueStore::BlobRef b1 = coll->new_blob();
   auto &_b1 = b1->dirty_blob();
   _b1.init_csum(Checksummer::CSUM_CRC32C, csum_order, ext1_len);
   for(size_t i = 0; i < _b1.get_csum_count(); i++) {