From 7d1f46b2b70a97b59bd891b1ebd293e79a18c727 Mon Sep 17 00:00:00 2001 From: Igor Fedotov Date: Mon, 19 Dec 2016 14:37:06 +0000 Subject: [PATCH] os/bluestore: remove duplicate sbid member for Blob, use one from shared_blob member Signed-off-by: Igor Fedotov --- src/os/bluestore/BlueStore.cc | 43 ++++++++++---------- src/os/bluestore/BlueStore.h | 19 ++++++--- src/os/bluestore/bluestore_types.cc | 4 -- src/os/bluestore/bluestore_types.h | 8 ---- src/test/objectstore/test_bluestore_types.cc | 1 - 5 files changed, 36 insertions(+), 39 deletions(-) diff --git a/src/os/bluestore/BlueStore.cc b/src/os/bluestore/BlueStore.cc index 56e8bc258fa6b..4473ba982ba97 100644 --- a/src/os/bluestore/BlueStore.cc +++ b/src/os/bluestore/BlueStore.cc @@ -1814,7 +1814,7 @@ bool BlueStore::ExtentMap::encode_some(uint32_t offset, uint32_t length, denc_varint(0, bound); // logical_offset denc_varint(0, bound); // len denc_varint(0, bound); // blob_offset - p->blob->bound_encode(bound, struct_v, false); + p->blob->bound_encode(bound, struct_v, p->blob->shared_blob->sbid, false); } { @@ -1866,7 +1866,7 @@ bool BlueStore::ExtentMap::encode_some(uint32_t offset, uint32_t length, } pos = p->logical_end(); if (include_blob) { - p->blob->encode(app, struct_v, false); + p->blob->encode(app, struct_v, p->blob->shared_blob->sbid, false); } } } @@ -1926,9 +1926,10 @@ void BlueStore::ExtentMap::decode_some(bufferlist& bl) assert(le->blob); } else { Blob *b = new Blob(); - b->decode(p, struct_v, false); + uint64_t sbid = 0; + b->decode(p, struct_v, &sbid, false); blobs[n] = b; - onode->c->open_shared_blob(b); + onode->c->open_shared_blob(sbid, b); le->assign_blob(b); } // we build ref_map dynamically for non-spanning blobs @@ -1951,7 +1952,7 @@ void BlueStore::ExtentMap::bound_encode_spanning_blobs(size_t& p) denc_varint((uint32_t)0, key_size); p += spanning_blob_map.size() * key_size; for (const auto& i : spanning_blob_map) { - i.second->bound_encode(p, struct_v, true); + i.second->bound_encode(p, struct_v, i.second->shared_blob->sbid, true); } } @@ -1963,7 +1964,7 @@ void BlueStore::ExtentMap::encode_spanning_blobs( denc_varint(spanning_blob_map.size(), p); for (auto& i : spanning_blob_map) { denc_varint(i.second->id, p); - i.second->encode(p, struct_v, true); + i.second->encode(p, struct_v, i.second->shared_blob->sbid, true); } } @@ -1980,8 +1981,9 @@ void BlueStore::ExtentMap::decode_spanning_blobs( BlobRef b(new Blob()); denc_varint(b->id, p); spanning_blob_map[b->id] = b; - b->decode(p, struct_v, true); - c->open_shared_blob(b); + uint64_t sbid = 0; + b->decode(p, struct_v, &sbid, true); + c->open_shared_blob(sbid, b); } } @@ -2313,7 +2315,7 @@ BlueStore::Collection::Collection(BlueStore *ns, Cache *c, coll_t cid) { } -void BlueStore::Collection::open_shared_blob(BlobRef b) +void BlueStore::Collection::open_shared_blob(uint64_t sbid, BlobRef b) { assert(!b->shared_blob); const bluestore_blob_t& blob = b->get_blob(); @@ -2322,14 +2324,14 @@ void BlueStore::Collection::open_shared_blob(BlobRef b) return; } - b->shared_blob = shared_blob_set.lookup(blob.sbid); + b->shared_blob = shared_blob_set.lookup(sbid); if (b->shared_blob) { - dout(10) << __func__ << " sbid 0x" << std::hex << blob.sbid << std::dec + dout(10) << __func__ << " sbid 0x" << std::hex << sbid << std::dec << " had " << *b->shared_blob << dendl; } else { - b->shared_blob = new SharedBlob(blob.sbid, cache); + b->shared_blob = new SharedBlob(sbid, cache); shared_blob_set.add(b->shared_blob.get()); - dout(10) << __func__ << " sbid 0x" << std::hex << blob.sbid << std::dec + dout(10) << __func__ << " sbid 0x" << std::hex << sbid << std::dec << " opened " << *b->shared_blob << dendl; } } @@ -2354,7 +2356,7 @@ void BlueStore::Collection::load_shared_blob(SharedBlobRef sb) sb->loaded = true; } -void BlueStore::Collection::make_blob_shared(BlobRef b) +void BlueStore::Collection::make_blob_shared(uint64_t sbid, BlobRef b) { dout(10) << __func__ << " " << *b << dendl; bluestore_blob_t& blob = b->dirty_blob(); @@ -2365,7 +2367,7 @@ void BlueStore::Collection::make_blob_shared(BlobRef b) // update shared blob b->shared_blob->loaded = true; // we are new and therefore up to date - b->shared_blob->sbid = blob.sbid; + b->shared_blob->sbid = sbid; shared_blob_set.add(b->shared_blob.get()); for (auto p : blob.extents) { if (p.is_valid()) { @@ -4435,18 +4437,18 @@ int BlueStore::fsck(bool deep) } } if (blob.is_shared()) { - if (blob.sbid > blobid_max) { + if (i.first->shared_blob->sbid > blobid_max) { derr << __func__ << " " << oid << " blob " << blob - << " sbid " << blob.sbid << " > blobid_max " + << " sbid " << i.first->shared_blob->sbid << " > blobid_max " << blobid_max << dendl; ++errors; - } else if (blob.sbid == 0) { + } else if (i.first->shared_blob->sbid == 0) { derr << __func__ << " " << oid << " blob " << blob << " marked as shared but has uninitialized sbid" << dendl; ++errors; } - sb_info_t& sbi = sb_info[blob.sbid]; + sb_info_t& sbi = sb_info[i.first->shared_blob->sbid]; sbi.sb = i.first->shared_blob; sbi.oids.push_back(oid); sbi.compressed = blob.is_compressed(); @@ -8676,8 +8678,7 @@ int BlueStore::_do_clone_range( const bluestore_blob_t& blob = e.blob->get_blob(); // make sure it is shared if (!blob.is_shared()) { - e.blob->dirty_blob().sbid = _assign_blobid(txc); - c->make_blob_shared(e.blob); + c->make_blob_shared(_assign_blobid(txc), e.blob); dirtied_oldo = true; // fixme: overkill } else if (!e.blob->shared_blob->loaded) { c->load_shared_blob(e.blob->shared_blob); diff --git a/src/os/bluestore/BlueStore.h b/src/os/bluestore/BlueStore.h index d1673edfc5bd8..b92777f5cd2be 100644 --- a/src/os/bluestore/BlueStore.h +++ b/src/os/bluestore/BlueStore.h @@ -528,22 +528,31 @@ public: } } #else - void bound_encode(size_t& p, uint64_t struct_v, bool include_ref_map) const { + void bound_encode(size_t& p, uint64_t struct_v, uint64_t sbid, bool include_ref_map) const { denc(blob, p, struct_v); + if (blob.is_shared()) { + denc(sbid, p); + } if (include_ref_map) { ref_map.bound_encode(p); } } - void encode(bufferlist::contiguous_appender& p, uint64_t struct_v, + void encode(bufferlist::contiguous_appender& p, uint64_t struct_v, uint64_t sbid, bool include_ref_map) const { denc(blob, p, struct_v); + if (blob.is_shared()) { + denc(sbid, p); + } if (include_ref_map) { ref_map.encode(p); } } - void decode(bufferptr::iterator& p, uint64_t struct_v, + void decode(bufferptr::iterator& p, uint64_t struct_v, uint64_t* sbid, bool include_ref_map) { denc(blob, p, struct_v); + if (blob.is_shared()) { + denc(*sbid, p); + } if (include_ref_map) { ref_map.decode(p); } @@ -1105,9 +1114,9 @@ public: // open = SharedBlob is instantiated // shared = blob_t shared flag is set; SharedBlob is hashed. // loaded = SharedBlob::shared_blob_t is loaded from kv store - void open_shared_blob(BlobRef b); + void open_shared_blob(uint64_t sbid, BlobRef b); void load_shared_blob(SharedBlobRef sb); - void make_blob_shared(BlobRef b); + void make_blob_shared(uint64_t sbid, BlobRef b); BlobRef new_blob() { BlobRef b = new Blob; diff --git a/src/os/bluestore/bluestore_types.cc b/src/os/bluestore/bluestore_types.cc index 0781c1135ac5b..01630cbc1df87 100644 --- a/src/os/bluestore/bluestore_types.cc +++ b/src/os/bluestore/bluestore_types.cc @@ -400,7 +400,6 @@ void bluestore_blob_t::dump(Formatter *f) const f->dump_object("extent", p); } f->close_section(); - f->dump_unsigned("shared_blob_id", sbid); f->dump_unsigned("compressed_length_original", compressed_length_orig); f->dump_unsigned("compressed_length", compressed_length); f->dump_unsigned("flags", flags); @@ -434,9 +433,6 @@ void bluestore_blob_t::generate_test_instances(list& ls) ostream& operator<<(ostream& out, const bluestore_blob_t& o) { out << "blob(" << o.extents; - if (o.sbid) { - out << " sbid 0x" << std::hex << o.sbid << std::dec; - } if (o.is_compressed()) { out << " clen 0x" << std::hex << o.compressed_length_orig diff --git a/src/os/bluestore/bluestore_types.h b/src/os/bluestore/bluestore_types.h index 1cb4159647795..2e684791bf511 100644 --- a/src/os/bluestore/bluestore_types.h +++ b/src/os/bluestore/bluestore_types.h @@ -280,7 +280,6 @@ struct bluestore_blob_t { static string get_flags_string(unsigned flags); vector extents;///< raw data position on device - uint64_t sbid = 0; ///< shared blob id (if shared) uint32_t compressed_length_orig = 0;///< original length of compressed blob if any uint32_t compressed_length = 0; ///< compressed length if any uint32_t flags = 0; ///< FLAG_* @@ -301,7 +300,6 @@ struct bluestore_blob_t { assert(struct_v == 1); denc(extents, p); denc_varint(flags, p); - denc_varint(sbid, p); denc_varint_lowz(compressed_length_orig, p); denc_varint_lowz(compressed_length, p); denc(csum_type, p); @@ -315,9 +313,6 @@ struct bluestore_blob_t { assert(struct_v == 1); denc(extents, p); denc_varint(flags, p); - if (is_shared()) { - denc_varint(sbid, p); - } if (is_compressed()) { denc_varint_lowz(compressed_length_orig, p); denc_varint_lowz(compressed_length, p); @@ -338,9 +333,6 @@ struct bluestore_blob_t { assert(struct_v == 1); denc(extents, p); denc_varint(flags, p); - if (is_shared()) { - denc_varint(sbid, p); - } if (is_compressed()) { denc_varint_lowz(compressed_length_orig, p); denc_varint_lowz(compressed_length, p); diff --git a/src/test/objectstore/test_bluestore_types.cc b/src/test/objectstore/test_bluestore_types.cc index ca3bcaad6b5e5..4c63bef32c725 100644 --- a/src/test/objectstore/test_bluestore_types.cc +++ b/src/test/objectstore/test_bluestore_types.cc @@ -688,7 +688,6 @@ TEST(Blob, put_ref) b.extents.push_back(bluestore_pextent_t(0x40118000, 0x7000)); b.set_flag(bluestore_blob_t::FLAG_SHARED); b.init_csum(Checksummer::CSUM_CRC32C, 12, 0x1e000); - b.sbid = 0xcf92e; cout << "before: " << B << std::endl; vector r; -- 2.39.5