From 72fe4df3f8156d5c906a2a9993c1386518bd549d Mon Sep 17 00:00:00 2001 From: xie xingguo Date: Fri, 8 Jul 2016 08:59:08 +0800 Subject: [PATCH 1/5] os/bluestore: check against that we don't access violation Some day we might want make this interface more applicable by handling the abnormal cases more gracefully... Signed-off-by: xie xingguo --- src/os/bluestore/bluestore_types.h | 1 + 1 file changed, 1 insertion(+) diff --git a/src/os/bluestore/bluestore_types.h b/src/os/bluestore/bluestore_types.h index 44e0e8b5d2759..20a632439f557 100644 --- a/src/os/bluestore/bluestore_types.h +++ b/src/os/bluestore/bluestore_types.h @@ -283,6 +283,7 @@ struct bluestore_blob_t { } b_len += b_off; while (b_len) { + assert(p != extents.end()); if (!p->is_valid()) { return false; } From a7228eb8b9b0f5b85130b8185772afa9a5ea6141 Mon Sep 17 00:00:00 2001 From: xie xingguo Date: Thu, 7 Jul 2016 20:09:41 +0800 Subject: [PATCH 2/5] os/bluestore: drop unused member _bytes Signed-off-by: xie xingguo --- src/os/bluestore/bluestore_types.h | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/os/bluestore/bluestore_types.h b/src/os/bluestore/bluestore_types.h index 20a632439f557..f0e2702e96bdd 100644 --- a/src/os/bluestore/bluestore_types.h +++ b/src/os/bluestore/bluestore_types.h @@ -610,9 +610,7 @@ struct bluestore_wal_transaction_t { list ops; interval_set released; ///< allocations to release after wal - int64_t _bytes; ///< cached byte count - - bluestore_wal_transaction_t() : seq(0), _bytes(-1) {} + bluestore_wal_transaction_t() : seq(0) {} void encode(bufferlist& bl) const; void decode(bufferlist::iterator& p); From 359c3863a5b7a622923ac6e0a5a90fa038b0948f Mon Sep 17 00:00:00 2001 From: xie xingguo Date: Fri, 8 Jul 2016 09:48:03 +0800 Subject: [PATCH 3/5] os/bluestore: assert to confirm that we don't access violation Signed-off-by: xie xingguo --- src/os/bluestore/bluestore_types.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/os/bluestore/bluestore_types.h b/src/os/bluestore/bluestore_types.h index f0e2702e96bdd..2c1846da2beda 100644 --- a/src/os/bluestore/bluestore_types.h +++ b/src/os/bluestore/bluestore_types.h @@ -362,6 +362,7 @@ struct bluestore_blob_t { assert(p != extents.end()); } while (x_len > 0) { + assert(p != extents.end()); uint64_t l = MIN(p->length - x_off, x_len); f(p->offset + x_off, l); x_off = 0; @@ -382,6 +383,7 @@ struct bluestore_blob_t { bufferlist::iterator it = bl.begin(); uint64_t x_len = bl.length(); while (x_len > 0) { + assert(p != extents.end()); uint64_t l = MIN(p->length - x_off, x_len); bufferlist t; it.copy(l, t); From ed5f7c4df7be444200b96f19b61d3d7f95e009d6 Mon Sep 17 00:00:00 2001 From: xie xingguo Date: Fri, 8 Jul 2016 14:13:46 +0800 Subject: [PATCH 4/5] os/bluestore: drop unnecessary increase of iterator We are going to break anyway. Signed-off-by: xie xingguo --- src/os/bluestore/bluestore_types.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/os/bluestore/bluestore_types.cc b/src/os/bluestore/bluestore_types.cc index 61f0d9e683a3c..4c710c577a8eb 100644 --- a/src/os/bluestore/bluestore_types.cc +++ b/src/os/bluestore/bluestore_types.cc @@ -941,7 +941,7 @@ void bluestore_onode_t::punch_hole( p->second.blob, p->second.offset + p->second.length - keep, keep); - extent_map.erase(p++); + extent_map.erase(p); break; } } From ff0329634a29f51174e372d1289745b3bf2883b6 Mon Sep 17 00:00:00 2001 From: xie xingguo Date: Fri, 8 Jul 2016 15:21:57 +0800 Subject: [PATCH 5/5] os/bluestore: release compressor if comp_mode turned out to be none Signed-off-by: xie xingguo --- src/os/bluestore/BlueStore.cc | 72 +++++++++++++++++++---------------- 1 file changed, 39 insertions(+), 33 deletions(-) diff --git a/src/os/bluestore/BlueStore.cc b/src/os/bluestore/BlueStore.cc index 978a020fea74e..c2fa342e3bea7 100644 --- a/src/os/bluestore/BlueStore.cc +++ b/src/os/bluestore/BlueStore.cc @@ -1429,42 +1429,48 @@ void BlueStore::_set_compression() comp_min_blob_size = g_conf->bluestore_compression_min_blob_size; comp_max_blob_size = g_conf->bluestore_compression_max_blob_size; - const char *alg = 0; - if (g_conf->bluestore_compression_algorithm == "snappy") { - alg = "snappy"; - } else if (g_conf->bluestore_compression_algorithm == "zlib") { - alg = "zlib"; - } else if (g_conf->bluestore_compression_algorithm.length()) { - derr << __func__ << " unrecognized compression algorithm '" - << g_conf->bluestore_compression_algorithm << "'" << dendl; - } - if (alg) { - compressor = Compressor::create(cct, alg); - if (!compressor) { - derr << __func__ << " unable to initialize " << alg << " compressor" - << dendl; - } + if (g_conf->bluestore_compression == "force") { + comp_mode = COMP_FORCE; + } else if (g_conf->bluestore_compression == "aggressive") { + comp_mode = COMP_AGGRESSIVE; + } else if (g_conf->bluestore_compression == "passive") { + comp_mode = COMP_PASSIVE; + } else if (g_conf->bluestore_compression == "none") { + comp_mode = COMP_NONE; } else { - compressor = nullptr; - } - CompressionMode m = COMP_NONE; - if (compressor) { - if (g_conf->bluestore_compression == "force") { - m = COMP_FORCE; - } else if (g_conf->bluestore_compression == "aggressive") { - m = COMP_AGGRESSIVE; - } else if (g_conf->bluestore_compression == "passive") { - m = COMP_PASSIVE; - } else if (g_conf->bluestore_compression == "none") { - m = COMP_NONE; - } else { - derr << __func__ << " unrecognized value '" - << g_conf->bluestore_compression - << "' for bluestore_compression, reverting to 'none'" << dendl; - m = COMP_NONE; + derr << __func__ << " unrecognized value '" + << g_conf->bluestore_compression + << "' for bluestore_compression, reverting to 'none'" + << dendl; + comp_mode = COMP_NONE; + } + + compressor = nullptr; + if (comp_mode != COMP_NONE) { + const char *alg = 0; + if (g_conf->bluestore_compression_algorithm == "snappy") { + alg = "snappy"; + } else if (g_conf->bluestore_compression_algorithm == "zlib") { + alg = "zlib"; + } else if (g_conf->bluestore_compression_algorithm.length()) { + derr << __func__ << " unrecognized compression algorithm '" + << g_conf->bluestore_compression_algorithm << "'" + << ", reverting compression mode to 'none'" + << dendl; + comp_mode = COMP_NONE; + } + + if (alg) { + compressor = Compressor::create(cct, alg); + if (!compressor) { + derr << __func__ << " unable to initialize " << alg << " compressor" + << ", reverting compression mode to 'none'" + << dendl; + comp_mode = COMP_NONE; + } } } - comp_mode = m; + dout(10) << __func__ << " mode " << get_comp_mode_name(comp_mode) << " alg " << (compressor ? compressor->get_type() : "(none)") << dendl;