From: Adam Kupczyk <akupczyk@ibm.com>
Date: Thu, 25 May 2023 19:26:28 +0000 (+0000)
Subject: os/bluestore: Fix tracking of num_blobs
X-Git-Tag: v19.0.0~486^2~22
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=34b04b065d6dab6979b09f1481cb1f4ea72cc73c;p=ceph.git

os/bluestore: Fix tracking of num_blobs

By moving BufferSpace from SharedBlob to Blob tracking of num_blobs get broken.
Fixed that and reinforced by adding asserts to BufferCacheShard destructor.

Signed-off-by: Adam Kupczyk <akupczyk@ibm.com>
---

diff --git a/src/os/bluestore/BlueStore.cc b/src/os/bluestore/BlueStore.cc
index 060b6e2f3f06..67e3a4a534a6 100644
--- a/src/os/bluestore/BlueStore.cc
+++ b/src/os/bluestore/BlueStore.cc
@@ -2146,9 +2146,6 @@ BlueStore::SharedBlob::SharedBlob(uint64_t i, Collection *_coll)
   : coll(_coll), sbid_unloaded(i)
 {
   ceph_assert(sbid_unloaded > 0);
-  if (get_cache()) {
-    get_cache()->add_blob();
-  }
 }
 
 BlueStore::SharedBlob::~SharedBlob()
@@ -4088,16 +4085,17 @@ 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->shared_blob = new SharedBlob(this);
+    b->set_shared_blob(new SharedBlob(this));
     return;
   }
 
-  b->shared_blob = shared_blob_set.lookup(sbid);
-  if (b->shared_blob) {
+  SharedBlobRef sb = shared_blob_set.lookup(sbid);
+  if (sb) {
+    b->set_shared_blob(sb);
     ldout(store->cct, 10) << __func__ << " sbid 0x" << std::hex << sbid
 			  << std::dec << " had " << *b->shared_blob << dendl;
   } else {
-    b->shared_blob = new SharedBlob(sbid, this);
+    b->set_shared_blob(new SharedBlob(sbid, this));
     shared_blob_set.add(this, b->shared_blob.get());
     ldout(store->cct, 10) << __func__ << " sbid 0x" << std::hex << sbid
 			  << std::dec << " opened " << *b->shared_blob
@@ -16631,8 +16629,10 @@ int BlueStore::_do_remove(
 	// but now those 2 blobs share it.
 	// This is illegal, as empty shared blobs should be unique.
 	// Fixing by re-creation.
+
+	// Here we skip set_shared_blob() because e.blob is already in BufferCacheShard
+	// and cannot do add_blob() twice
 	e.blob->shared_blob = new SharedBlob(c.get());
-	dout(20) << __func__ << " recreated empty shared blob " << e << dendl;
       }
       h->extent_map.dirty_range(e.logical_offset, 1);
     }
diff --git a/src/os/bluestore/BlueStore.h b/src/os/bluestore/BlueStore.h
index 24b5cd143b86..18a4ed9ea56b 100644
--- a/src/os/bluestore/BlueStore.h
+++ b/src/os/bluestore/BlueStore.h
@@ -497,9 +497,6 @@ public:
     };
 
     SharedBlob(Collection *_coll) : coll(_coll), sbid_unloaded(0) {
-      if (get_cache()) {
-	get_cache()->add_blob();
-      }
     }
     SharedBlob(uint64_t i, Collection *_coll);
     ~SharedBlob();
@@ -601,6 +598,14 @@ public:
     int16_t id = -1;                ///< id, for spanning blobs only, >= 0
     int16_t last_encoded_id = -1;   ///< (ephemeral) used during encoding only
     SharedBlobRef shared_blob;      ///< shared blob state (if any)
+
+    void set_shared_blob(SharedBlobRef sb) {
+      ceph_assert((bool)sb);
+      ceph_assert(!shared_blob);
+      shared_blob = sb;
+      ceph_assert(shared_blob->get_cache());
+      shared_blob->get_cache()->add_blob();
+    }
     BufferSpace bc;
   private:
     mutable bluestore_blob_t blob;  ///< decoded blob metadata
@@ -651,7 +656,7 @@ public:
 			uint32_t *length0);
 
     void dup(Blob& o) {
-      o.shared_blob = shared_blob;
+      o.set_shared_blob(shared_blob);
       o.blob = blob;
 #ifdef CACHE_BLOB_BL
       o.blob_bl = blob_bl;
@@ -1435,6 +1440,10 @@ private:
 
   public:
     BufferCacheShard(CephContext* cct) : CacheShard(cct) {}
+    virtual ~BufferCacheShard() {
+      ceph_assert(num_blobs == 0);
+      ceph_assert(num_extents == 0);
+    }
     static BufferCacheShard *create(CephContext* cct, std::string type, 
                                     PerfCounters *logger);
     virtual void _add(Buffer *b, int level, Buffer *near) = 0;
@@ -1550,7 +1559,7 @@ private:
 
     BlobRef new_blob() {
       BlobRef b = new Blob();
-      b->shared_blob = new SharedBlob(this);
+      b->set_shared_blob(new SharedBlob(this));
       return b;
     }