From 4a4dc1a5e0262e344f7e4f49aec779e9493ab543 Mon Sep 17 00:00:00 2001 From: Ming-Hung Tsai Date: Thu, 31 Mar 2016 23:06:08 +0800 Subject: [PATCH 1/8] [btree_node_checker] first draft Spin-off from btree_damage_visitor --- Makefile.in | 1 + .../data-structures/btree_node_checker.cc | 127 +++++++++++ .../data-structures/btree_node_checker.h | 206 ++++++++++++++++++ 3 files changed, 334 insertions(+) create mode 100644 persistent-data/data-structures/btree_node_checker.cc create mode 100644 persistent-data/data-structures/btree_node_checker.h diff --git a/Makefile.in b/Makefile.in index 5e912d0..482f14f 100644 --- a/Makefile.in +++ b/Makefile.in @@ -67,6 +67,7 @@ SOURCE=\ persistent-data/data-structures/bitset.cc \ persistent-data/data-structures/bloom_filter.cc \ persistent-data/data-structures/btree.cc \ + persistent-data/data-structures/btree_node_checker.cc \ persistent-data/error_set.cc \ persistent-data/file_utils.cc \ persistent-data/hex_dump.cc \ diff --git a/persistent-data/data-structures/btree_node_checker.cc b/persistent-data/data-structures/btree_node_checker.cc new file mode 100644 index 0000000..a6c77e0 --- /dev/null +++ b/persistent-data/data-structures/btree_node_checker.cc @@ -0,0 +1,127 @@ +#include "btree_node_checker.h" + +#include + +using persistent_data::btree_detail::btree_node_checker; + +//---------------------------------------------------------------- + +btree_node_checker::error_type btree_node_checker::get_last_error() { + return last_error_; +} + +std::string btree_node_checker::get_last_error_string() { + switch (last_error_) { + case BLOCK_NR_MISMATCH: + return block_nr_mismatch_string(); + case VALUE_SIZES_MISMATCH: + return value_sizes_mismatch_string(); + case MAX_ENTRIES_TOO_LARGE: + return max_entries_too_large_string(); + case MAX_ENTRIES_NOT_DIVISIBLE: + return max_entries_not_divisible_string(); + case NR_ENTRIES_TOO_LARGE: + return nr_entries_too_large_string(); + case NR_ENTRIES_TOO_SMALL: + return nr_entries_too_small_string(); + case KEYS_OUT_OF_ORDER: + return keys_out_of_order_string(); + case PARENT_KEY_MISMATCH: + return parent_key_mismatch_string(); + case LEAF_KEY_OVERLAPPED: + return leaf_key_overlapped_string(); + default: + return std::string(); + } +} + +void btree_node_checker::reset() { + last_error_ = NO_ERROR; +} + +std::string btree_node_checker::block_nr_mismatch_string() { + std::ostringstream out; + out << "block number mismatch: actually " + << error_location_ + << ", claims " << error_block_nr_; + + return out.str(); +} + +std::string btree_node_checker::value_sizes_mismatch_string() { + std::ostringstream out; + out << "value size mismatch: expected " << error_value_sizes_[1] + << ", but got " << error_value_sizes_[0] + << ". This is not the btree you are looking for." + << " (block " << error_location_ << ")"; + + return out.str(); +} + +std::string btree_node_checker::max_entries_too_large_string() { + std::ostringstream out; + out << "max entries too large: " << error_max_entries_ + << " (block " << error_location_ << ")"; + + return out.str(); +} + +std::string btree_node_checker::max_entries_not_divisible_string() { + std::ostringstream out; + out << "max entries is not divisible by 3: " << error_max_entries_ + << " (block " << error_location_ << ")"; + + return out.str(); +} + +std::string btree_node_checker::nr_entries_too_large_string() { + std::ostringstream out; + out << "bad nr_entries: " + << error_nr_entries_ << " < " + << error_max_entries_ + << " (block " << error_location_ << ")"; + + return out.str(); +} + +std::string btree_node_checker::nr_entries_too_small_string() { + std::ostringstream out; + out << "too few entries in btree_node: " + << error_nr_entries_ + << ", expected at least " + << (error_max_entries_ / 3) + << " (block " << error_location_ + << ", max_entries = " << error_max_entries_ << ")"; + + return out.str(); +} + +std::string btree_node_checker::keys_out_of_order_string() { + std::ostringstream out; + out << "keys are out of order, " + << error_keys_[0] << " <= " << error_keys_[1] + << " (block " << error_location_ << ")"; + + return out.str(); +} + +std::string btree_node_checker::parent_key_mismatch_string() { + std::ostringstream out; + out << "parent key mismatch: parent was " << error_keys_[1] + << ", but lowest in node was " << error_keys_[0] + << " (block " << error_location_ << ")"; + + return out.str(); +} + +std::string btree_node_checker::leaf_key_overlapped_string() { + std::ostringstream out; + out << "the last key of the previous leaf was " << error_keys_[1] + << " and the first key of this leaf is " << error_keys_[0] + << " (block " << error_location_ << ")"; + + return out.str(); +} + +//---------------------------------------------------------------- + diff --git a/persistent-data/data-structures/btree_node_checker.h b/persistent-data/data-structures/btree_node_checker.h new file mode 100644 index 0000000..926f613 --- /dev/null +++ b/persistent-data/data-structures/btree_node_checker.h @@ -0,0 +1,206 @@ +#ifndef BTREE_NODE_CHECKER_H +#define BTREE_NODE_CHECKER_H + +#include "block-cache/block_cache.h" +#include "persistent-data/block.h" +#include "persistent-data/data-structures/btree.h" +#include "persistent-data/data-structures/btree_disk_structures.h" + +#include +#include + +using bcache::block_address; + +//---------------------------------------------------------------- + +namespace persistent_data { + namespace btree_detail { + class btree_node_checker { + public: + enum error_type { + NO_ERROR, + BLOCK_NR_MISMATCH, + VALUE_SIZES_MISMATCH, + MAX_ENTRIES_TOO_LARGE, + MAX_ENTRIES_NOT_DIVISIBLE, + NR_ENTRIES_TOO_LARGE, + NR_ENTRIES_TOO_SMALL, + KEYS_OUT_OF_ORDER, + VALUE_SIZE_MISMATCH, + PARENT_KEY_MISMATCH, + LEAF_KEY_OVERLAPPED, + }; + + btree_node_checker(): + last_error_(NO_ERROR), + error_location_(0), + error_block_nr_(0), + error_nr_entries_(0), + error_max_entries_(0), + error_value_sizes_{0, 0}, + error_keys_{0, 0} { + } + + template + bool check_block_nr(btree_detail::node_ref const &n) { + if (n.get_location() != n.get_block_nr()) { + last_error_ = BLOCK_NR_MISMATCH; + error_block_nr_ = n.get_block_nr(); + error_location_ = n.get_location(); + + return false; + } + + return true; + } + + template + bool check_value_size(btree_detail::node_ref const &n) { + if (!n.value_sizes_match()) { + last_error_ = VALUE_SIZES_MISMATCH; + error_location_ = n.get_location(); + error_value_sizes_[0] = n.get_value_size(); + error_value_sizes_[1] = sizeof(typename ValueTraits::disk_type); + return false; + } + + return true; + } + + template + bool check_max_entries(btree_detail::node_ref const &n) { + size_t elt_size = sizeof(uint64_t) + n.get_value_size(); + if (elt_size * n.get_max_entries() + sizeof(node_header) > MD_BLOCK_SIZE) { + last_error_ = MAX_ENTRIES_TOO_LARGE; + error_location_ = n.get_location(); + error_max_entries_ = n.get_max_entries(); + + return false; + } + + if (n.get_max_entries() % 3) { + last_error_ = MAX_ENTRIES_NOT_DIVISIBLE; + error_location_ = n.get_location(); + error_max_entries_ = n.get_max_entries(); + + return false; + } + + return true; + } + + template + bool check_nr_entries(btree_detail::node_ref const &n, + bool is_root) { + if (n.get_nr_entries() > n.get_max_entries()) { + last_error_ = NR_ENTRIES_TOO_LARGE; + error_location_ = n.get_location(); + error_nr_entries_ = n.get_nr_entries(); + error_max_entries_ = n.get_max_entries(); + + return false; + } + + block_address min = n.get_max_entries() / 3; + if (!is_root && (n.get_nr_entries() < min)) { + last_error_ = NR_ENTRIES_TOO_SMALL; + error_location_ = n.get_location(); + error_nr_entries_ = n.get_nr_entries(); + error_max_entries_ = n.get_max_entries(); + + return false; + } + + return true; + } + + template + bool check_ordered_keys(btree_detail::node_ref const &n) { + unsigned nr_entries = n.get_nr_entries(); + + if (nr_entries == 0) + return true; // can only happen if a root node + + uint64_t last_key = n.key_at(0); + + for (unsigned i = 1; i < nr_entries; i++) { + uint64_t k = n.key_at(i); + if (k <= last_key) { + last_error_ = KEYS_OUT_OF_ORDER; + error_location_ = n.get_location(); + error_keys_[0] = k; + error_keys_[1] = last_key; + + return false; + } + last_key = k; + } + + return true; + } + + template + bool check_parent_key(btree_detail::node_ref const &n, + boost::optional key) { + if (!key) + return true; + + if (*key > n.key_at(0)) { + last_error_ = PARENT_KEY_MISMATCH; + error_location_ = n.get_location(); + error_keys_[0] = n.key_at(0); + error_keys_[1] = *key; + + return false; + } + + return true; + } + + template + bool check_leaf_key(btree_detail::node_ref const &n, + boost::optional key) { + if (n.get_nr_entries() == 0) + return true; // can only happen if a root node + + if (key && *key >= n.key_at(0)) { + last_error_ = LEAF_KEY_OVERLAPPED; + error_location_ = n.get_location(); + error_keys_[0] = n.key_at(0); + error_keys_[1] = *key; + + return false; + } + + return true; + } + + error_type get_last_error(); + std::string get_last_error_string(); + void reset(); + + private: + std::string block_nr_mismatch_string(); + std::string value_sizes_mismatch_string(); + std::string max_entries_too_large_string(); + std::string max_entries_not_divisible_string(); + std::string nr_entries_too_large_string(); + std::string nr_entries_too_small_string(); + std::string keys_out_of_order_string(); + std::string parent_key_mismatch_string(); + std::string leaf_key_overlapped_string(); + + error_type last_error_; + block_address error_location_; + block_address error_block_nr_; + uint32_t error_nr_entries_; + uint32_t error_max_entries_; + uint32_t error_value_sizes_[2]; + uint64_t error_keys_[2]; + }; + } +} + +//---------------------------------------------------------------- + +#endif From 6dc9a90fecb3aeff2fa075859d036d9a966e71ea Mon Sep 17 00:00:00 2001 From: Ming-Hung Tsai Date: Thu, 31 Mar 2016 23:08:14 +0800 Subject: [PATCH 2/8] [counting_visitor] fix unnecessary value visiting 1. Do not inherit btree_damage_visitor to avoid unnecessary value visiting. (reverts commit b22495997a8bf427741225ef86edf267ee746d7c) 2. Use btree_node_checker to do node checking --- .../data-structures/btree_counter.h | 92 ++++++++++++------- .../data-structures/btree_damage_visitor.h | 5 +- 2 files changed, 63 insertions(+), 34 deletions(-) diff --git a/persistent-data/data-structures/btree_counter.h b/persistent-data/data-structures/btree_counter.h index 6e52c29..1b8c7a2 100644 --- a/persistent-data/data-structures/btree_counter.h +++ b/persistent-data/data-structures/btree_counter.h @@ -2,44 +2,36 @@ #define PERSISTENT_DATA_DATA_STRUCTURES_BTREE_COUNTER_H #include "persistent-data/data-structures/btree.h" -#include "persistent-data/data-structures/btree_base_visitor.h" -#include "persistent-data/data-structures/btree_damage_visitor.h" +#include "persistent-data/data-structures/btree_node_checker.h" #include "persistent-data/block_counter.h" //---------------------------------------------------------------- namespace persistent_data { namespace btree_count_detail { - template - class counting_visitor : public btree_damage_visitor { - typedef btree_damage_visitor BtreeDamageVisitor; + template + class counting_visitor : public btree::visitor { public: typedef btree tree; - counting_visitor(ValueVisitor &value_visitor, - DamageVisitor &damage_visitor, - block_counter &bc, - ValueCounter &vc) - : BtreeDamageVisitor(value_visitor, damage_visitor, false), - bc_(bc), + counting_visitor(block_counter &bc, ValueCounter &vc) + : bc_(bc), vc_(vc) { } virtual bool visit_internal(node_location const &l, typename tree::internal_node const &n) { - return BtreeDamageVisitor::visit_internal(l, n) ? - visit_node(n) : false; + return check_internal(l, n) ? visit_node(n) : false; } virtual bool visit_internal_leaf(node_location const &l, typename tree::internal_node const &n) { - return BtreeDamageVisitor::visit_internal_leaf(l, n) ? - visit_node(n) : false; + return check_leaf(l, n) ? visit_node(n) : false; } virtual bool visit_leaf(node_location const &l, typename tree::leaf_node const &n) { - if (BtreeDamageVisitor::visit_leaf(l, n) && visit_node(n)) { + if (check_leaf(l, n) && visit_node(n)) { unsigned nr = n.get_nr_entries(); for (unsigned i = 0; i < nr; i++) { @@ -55,7 +47,57 @@ namespace persistent_data { return false; } + typedef typename btree::visitor::error_outcome error_outcome; + + error_outcome error_accessing_node(node_location const &l, block_address b, + std::string const &what) { + return btree::visitor::EXCEPTION_HANDLED; + } + private: + bool check_internal(node_location const &l, + btree_detail::node_ref const &n) { + if (!checker_.check_block_nr(n) || + !checker_.check_value_size(n) || + !checker_.check_max_entries(n) || + !checker_.check_nr_entries(n, l.is_sub_root()) || + !checker_.check_ordered_keys(n) || + !checker_.check_parent_key(n, l.is_sub_root() ? boost::optional() : l.key)) + return false; + + if (l.is_sub_root()) + new_root(l.level()); + + return true; + } + + template + bool check_leaf(node_location const &l, + btree_detail::node_ref const &n) { + if (!checker_.check_block_nr(n) || + !checker_.check_value_size(n) || + !checker_.check_max_entries(n) || + !checker_.check_nr_entries(n, l.is_sub_root()) || + !checker_.check_ordered_keys(n) || + !checker_.check_parent_key(n, l.is_sub_root() ? boost::optional() : l.key)) + return false; + + if (l.is_sub_root()) + new_root(l.level()); + + bool r = checker_.check_leaf_key(n, last_leaf_key_[l.level()]); + if (r && n.get_nr_entries() > 0) + last_leaf_key_[l.level()] = n.key_at(n.get_nr_entries() - 1); + + return r; + } + + void new_root(unsigned level) { + // we're starting a new subtree, so should + // reset the last_leaf value. + last_leaf_key_[level] = boost::optional(); + } + template bool visit_node(Node const &n) { block_address b = n.get_location(); @@ -66,6 +108,8 @@ namespace persistent_data { block_counter &bc_; ValueCounter &vc_; + btree_node_checker checker_; + boost::optional last_leaf_key_[Levels]; }; } @@ -94,21 +138,7 @@ namespace persistent_data { // is not corrupt. template void count_btree_blocks(btree const &tree, block_counter &bc, ValueCounter &vc) { - typedef noop_value_visitor NoopValueVisitor; - NoopValueVisitor noop_vv; - noop_damage_visitor noop_dv; - btree_count_detail::counting_visitor v(noop_vv, noop_dv, bc, vc); - tree.visit_depth_first(v); - } - - template - void count_btree_blocks(btree const &tree, block_counter &bc) { - typedef noop_value_visitor NoopValueVisitor; - NoopValueVisitor noop_vv; - noop_damage_visitor noop_dv; - typedef noop_value_counter NoopValueCounter; - NoopValueCounter vc; - btree_count_detail::counting_visitor v(noop_vv, noop_dv, bc, vc); + btree_count_detail::counting_visitor v(bc, vc); tree.visit_depth_first(v); } } diff --git a/persistent-data/data-structures/btree_damage_visitor.h b/persistent-data/data-structures/btree_damage_visitor.h index 8be623b..5a96d5d 100644 --- a/persistent-data/data-structures/btree_damage_visitor.h +++ b/persistent-data/data-structures/btree_damage_visitor.h @@ -158,9 +158,8 @@ namespace persistent_data { typedef boost::optional maybe_run64; btree_damage_visitor(ValueVisitor &value_visitor, - DamageVisitor &damage_visitor, - bool avoid_repeated_visits = true) - : avoid_repeated_visits_(avoid_repeated_visits), + DamageVisitor &damage_visitor) + : avoid_repeated_visits_(true), value_visitor_(value_visitor), damage_visitor_(damage_visitor) { } From c6c508606857e4c6db70f5794063a46255360d53 Mon Sep 17 00:00:00 2001 From: Ming-Hung Tsai Date: Sat, 2 Apr 2016 16:14:24 +0800 Subject: [PATCH 3/8] [thin_ll_dump] cleanup: use btree_node_checker --- thin-provisioning/thin_ll_dump.cc | 72 ++++--------------------------- 1 file changed, 8 insertions(+), 64 deletions(-) diff --git a/thin-provisioning/thin_ll_dump.cc b/thin-provisioning/thin_ll_dump.cc index 2a20237..f16a745 100644 --- a/thin-provisioning/thin_ll_dump.cc +++ b/thin-provisioning/thin_ll_dump.cc @@ -24,6 +24,8 @@ #include "persistent-data/file_utils.h" #include "persistent-data/data-structures/btree.h" #include "persistent-data/data-structures/btree_counter.h" +#include "persistent-data/data-structures/btree_damage_visitor.h" +#include "persistent-data/data-structures/btree_node_checker.h" #include "persistent-data/data-structures/simple_traits.h" #include "persistent-data/space-maps/core.h" #include "persistent-data/space-maps/disk_structures.h" @@ -33,70 +35,11 @@ #include "version.h" using namespace thin_provisioning; +using namespace persistent_data; //---------------------------------------------------------------- namespace { - // extracted from btree_damage_visitor.h - template - bool check_block_nr(node const &n) { - if (n.get_location() != n.get_block_nr()) { - return false; - } - return true; - } - - // extracted from btree_damage_visitor.h - template - bool check_max_entries(node const &n) { - size_t elt_size = sizeof(uint64_t) + n.get_value_size(); - if (elt_size * n.get_max_entries() + sizeof(node_header) > MD_BLOCK_SIZE) { - return false; - } - - if (n.get_max_entries() % 3) { - return false; - } - - return true; - } - - // extracted from btree_damage_visitor.h - template - bool check_nr_entries(node const &n, bool is_root) { - if (n.get_nr_entries() > n.get_max_entries()) { - return false; - } - - block_address min = n.get_max_entries() / 3; - if (!is_root && (n.get_nr_entries() < min)) { - return false; - } - - return true; - } - - // extracted from btree_damage_visitor.h - template - bool check_ordered_keys(node const &n) { - unsigned nr_entries = n.get_nr_entries(); - - if (nr_entries == 0) - return true; // can only happen if a root node - - uint64_t last_key = n.key_at(0); - - for (unsigned i = 1; i < nr_entries; i++) { - uint64_t k = n.key_at(i); - if (k <= last_key) { - return false; - } - last_key = k; - } - - return true; - } - transaction_manager::ptr open_tm(block_manager<>::ptr bm) { space_map::ptr sm(new core_map(bm->get_nr_blocks())); @@ -136,19 +79,20 @@ namespace { if ((n.get_value_size() == sizeof(mapping_tree_detail::block_traits::disk_type) || n.get_value_size() == sizeof(device_tree_detail::device_details_traits::disk_type)) && !bc_.get_count(n.get_location()) && - check_block_nr(n) && + checker_.check_block_nr(n) && (((flags & INTERNAL_NODE) && !(flags & LEAF_NODE)) || (flags & LEAF_NODE)) && nv_->check_raw(n.raw()) && - check_max_entries(n) && - check_nr_entries(n, true) && - check_ordered_keys(n)) + checker_.check_max_entries(n) && + checker_.check_nr_entries(n, true) && + checker_.check_ordered_keys(n)) return true; return false; } bcache::validator::ptr nv_; block_counter const &bc_; + btree_detail::btree_node_checker checker_; }; //------------------------------------------------------------------- From 1dce79bd5551fef6cac310b2a8e7a69923b7160e Mon Sep 17 00:00:00 2001 From: Ming-Hung Tsai Date: Sat, 2 Apr 2016 17:15:00 +0800 Subject: [PATCH 4/8] [btree_damage_visitor] cleanup: use btree_node_checker --- .../data-structures/btree_damage_visitor.h | 201 ++++-------------- 1 file changed, 40 insertions(+), 161 deletions(-) diff --git a/persistent-data/data-structures/btree_damage_visitor.h b/persistent-data/data-structures/btree_damage_visitor.h index 5a96d5d..94e255b 100644 --- a/persistent-data/data-structures/btree_damage_visitor.h +++ b/persistent-data/data-structures/btree_damage_visitor.h @@ -2,6 +2,7 @@ #define PERSISTENT_DATA_DATA_STRUCTURES_DAMAGE_VISITOR_H #include "persistent-data/data-structures/btree.h" +#include "persistent-data/data-structures/btree_node_checker.h" #include "persistent-data/run.h" //---------------------------------------------------------------- @@ -222,44 +223,55 @@ namespace persistent_data { bool check_internal(node_location const &loc, btree_detail::node_ref const &n) { - if (!already_visited(n) && - check_block_nr(n) && - check_value_size(n) && - check_max_entries(n) && - check_nr_entries(n, loc.is_sub_root()) && - check_ordered_keys(n) && - check_parent_key(loc.is_sub_root() ? boost::optional() : loc.key, n)) { - if (loc.is_sub_root()) - new_root(loc.level()); + if (already_visited(n)) + return false; + else if (!checker_.check_block_nr(n) || + !checker_.check_value_size(n) || + !checker_.check_max_entries(n) || + !checker_.check_nr_entries(n, loc.is_sub_root()) || + !checker_.check_ordered_keys(n) || + !checker_.check_parent_key(n, loc.is_sub_root() ? boost::optional() : loc.key)) { + report_damage(checker_.get_last_error_string()); - good_internal(n.key_at(0)); - return true; + return false; } - return false; + if (loc.is_sub_root()) + new_root(loc.level()); + + good_internal(n.key_at(0)); + + return true; } template bool check_leaf(node_location const &loc, btree_detail::node_ref const &n) { - if (!already_visited(n) && - check_block_nr(n) && - check_value_size(n) && - check_max_entries(n) && - check_nr_entries(n, loc.is_sub_root()) && - check_ordered_keys(n) && - check_parent_key(loc.is_sub_root() ? boost::optional() : loc.key, n)) { - if (loc.is_sub_root()) - new_root(loc.level()); + if (already_visited(n)) + return false; + else if (!checker_.check_block_nr(n) || + !checker_.check_value_size(n) || + !checker_.check_max_entries(n) || + !checker_.check_nr_entries(n, loc.is_sub_root()) || + !checker_.check_ordered_keys(n) || + !checker_.check_parent_key(n, loc.is_sub_root() ? boost::optional() : loc.key)) { + report_damage(checker_.get_last_error_string()); - bool r = check_leaf_key(loc.level(), n); - if (r && n.get_nr_entries() > 0) - good_leaf(n.key_at(0), n.key_at(n.get_nr_entries() - 1) + 1); - - return r; + return false; } - return false; + if (loc.is_sub_root()) + new_root(loc.level()); + + bool r = checker_.check_leaf_key(n, last_leaf_key_[loc.level()]); + if (!r) + report_damage(checker_.get_last_error_string()); + else if (n.get_nr_entries() > 0) { + last_leaf_key_[loc.level()] = n.key_at(n.get_nr_entries() - 1); + good_leaf(n.key_at(0), n.key_at(n.get_nr_entries() - 1) + 1); + } + + return r; } template @@ -276,140 +288,6 @@ namespace persistent_data { return false; } - template - bool check_block_nr(node const &n) { - if (n.get_location() != n.get_block_nr()) { - std::ostringstream out; - out << "block number mismatch: actually " - << n.get_location() - << ", claims " << n.get_block_nr(); - - report_damage(out.str()); - return false; - } - - return true; - } - - template - bool check_value_size(node const &n) { - if (!n.value_sizes_match()) { - report_damage(n.value_mismatch_string()); - return false; - } - - return true; - } - - template - bool check_max_entries(node const &n) { - size_t elt_size = sizeof(uint64_t) + n.get_value_size(); - if (elt_size * n.get_max_entries() + sizeof(node_header) > MD_BLOCK_SIZE) { - std::ostringstream out; - out << "max entries too large: " << n.get_max_entries() - << " (block " << n.get_location() << ")"; - report_damage(out.str()); - return false; - } - - if (n.get_max_entries() % 3) { - std::ostringstream out; - out << "max entries is not divisible by 3: " << n.get_max_entries() - << " (block " << n.get_location() << ")"; - report_damage(out.str()); - return false; - } - - return true; - } - - template - bool check_nr_entries(node const &n, bool is_root) { - if (n.get_nr_entries() > n.get_max_entries()) { - std::ostringstream out; - out << "bad nr_entries: " - << n.get_nr_entries() << " < " - << n.get_max_entries() - << " (block " << n.get_location() << ")"; - report_damage(out.str()); - return false; - } - - block_address min = n.get_max_entries() / 3; - if (!is_root && (n.get_nr_entries() < min)) { - ostringstream out; - out << "too few entries in btree_node: " - << n.get_nr_entries() - << ", expected at least " - << min - << " (block " << n.get_location() - << ", max_entries = " << n.get_max_entries() << ")"; - report_damage(out.str()); - return false; - } - - return true; - } - - template - bool check_ordered_keys(node const &n) { - unsigned nr_entries = n.get_nr_entries(); - - if (nr_entries == 0) - return true; // can only happen if a root node - - uint64_t last_key = n.key_at(0); - - for (unsigned i = 1; i < nr_entries; i++) { - uint64_t k = n.key_at(i); - if (k <= last_key) { - ostringstream out; - out << "keys are out of order, " << k << " <= " << last_key - << " (block " << n.get_location() << ")"; - report_damage(out.str()); - return false; - } - last_key = k; - } - - return true; - } - - template - bool check_parent_key(boost::optional key, node const &n) { - if (!key) - return true; - - if (*key > n.key_at(0)) { - ostringstream out; - out << "parent key mismatch: parent was " << *key - << ", but lowest in node was " << n.key_at(0) - << " (block " << n.get_location() << ")"; - report_damage(out.str()); - return false; - } - - return true; - } - - template - bool check_leaf_key(unsigned level, node const &n) { - if (n.get_nr_entries() == 0) - return true; // can only happen if a root node - - if (last_leaf_key_[level] && *last_leaf_key_[level] >= n.key_at(0)) { - ostringstream out; - out << "the last key of the previous leaf was " << *last_leaf_key_[level] - << " and the first key of this leaf is " << n.key_at(0) - << " (block " << n.get_location() << ")"; - report_damage(out.str()); - return false; - } - - last_leaf_key_[level] = n.key_at(n.get_nr_entries() - 1); - return true; - } - void new_root(unsigned level) { // we're starting a new subtree, so should // reset the last_leaf value. @@ -487,6 +365,7 @@ namespace persistent_data { std::set seen_; boost::optional last_leaf_key_[Levels]; + btree_node_checker checker_; path_tracker path_tracker_; damage_tracker dt_; std::list damage_reasons_; From c8aabf2948d39baa52cb7e7d83f31dbf38baea34 Mon Sep 17 00:00:00 2001 From: Ming-Hung Tsai Date: Sat, 2 Apr 2016 23:15:26 +0800 Subject: [PATCH 5/8] [metadata_counter] fix repeated counting of trees --- thin-provisioning/metadata_counter.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/thin-provisioning/metadata_counter.cc b/thin-provisioning/metadata_counter.cc index bf3f809..e36fde6 100644 --- a/thin-provisioning/metadata_counter.cc +++ b/thin-provisioning/metadata_counter.cc @@ -66,7 +66,6 @@ void thin_provisioning::count_metadata(transaction_manager::ptr tm, count_trees(tm, snap, bc); } - count_trees(tm, sb, bc); count_space_maps(tm, sb, bc); } From f20e2a0f401c1e30c65ff29e76c2af97eb01daf9 Mon Sep 17 00:00:00 2001 From: Ming-Hung Tsai Date: Sat, 2 Apr 2016 23:24:24 +0800 Subject: [PATCH 6/8] [thin_check] cleanup: use metadata_counter --- thin-provisioning/thin_check.cc | 48 ++------------------------------- 1 file changed, 2 insertions(+), 46 deletions(-) diff --git a/thin-provisioning/thin_check.cc b/thin-provisioning/thin_check.cc index 2451db2..f2c57cb 100644 --- a/thin-provisioning/thin_check.cc +++ b/thin-provisioning/thin_check.cc @@ -31,6 +31,7 @@ #include "persistent-data/file_utils.h" #include "thin-provisioning/device_tree.h" #include "thin-provisioning/mapping_tree.h" +#include "thin-provisioning/metadata_counter.h" #include "thin-provisioning/superblock.h" #include "thin-provisioning/commands.h" @@ -169,58 +170,13 @@ namespace { bool clear_needs_check_flag_on_success; }; - void count_trees(transaction_manager::ptr tm, - superblock_detail::superblock &sb, - block_counter &bc) { - - // Count the device tree - { - noop_value_counter vc; - device_tree dtree(*tm, sb.device_details_root_, - device_tree_detail::device_details_traits::ref_counter()); - count_btree_blocks(dtree, bc, vc); - } - - // Count the mapping tree - { - noop_value_counter vc; - mapping_tree mtree(*tm, sb.data_mapping_root_, - mapping_tree_detail::block_traits::ref_counter(tm->get_sm())); - count_btree_blocks(mtree, bc, vc); - } - } - error_state check_space_map_counts(flags const &fs, nested_output &out, superblock_detail::superblock &sb, block_manager<>::ptr bm, transaction_manager::ptr tm) { block_counter bc; - // Count the superblock - bc.inc(superblock_detail::SUPERBLOCK_LOCATION); - count_trees(tm, sb, bc); - - // Count the metadata snap, if present - if (sb.metadata_snap_ != superblock_detail::SUPERBLOCK_LOCATION) { - bc.inc(sb.metadata_snap_); - - superblock_detail::superblock snap = read_superblock(bm, sb.metadata_snap_); - count_trees(tm, snap, bc); - } - - // Count the metadata space map - { - persistent_space_map::ptr metadata_sm = - open_metadata_sm(*tm, static_cast(&sb.metadata_space_map_root_)); - metadata_sm->count_metadata(bc); - } - - // Count the data space map - { - persistent_space_map::ptr data_sm = - open_disk_sm(*tm, static_cast(&sb.data_space_map_root_)); - data_sm->count_metadata(bc); - } + count_metadata(tm, sb, bc); // Finally we need to check the metadata space map agrees // with the counts we've just calculated. From 9322fc9f142ea97e50fea435b1bff91631b5f9d2 Mon Sep 17 00:00:00 2001 From: Ming-Hung Tsai Date: Tue, 5 Apr 2016 15:41:42 +0800 Subject: [PATCH 7/8] [btree_damage_visitor] cleanup: remove redundant statements --- .../data-structures/btree_damage_visitor.h | 22 +++++++++---------- 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/persistent-data/data-structures/btree_damage_visitor.h b/persistent-data/data-structures/btree_damage_visitor.h index 94e255b..1ff16a8 100644 --- a/persistent-data/data-structures/btree_damage_visitor.h +++ b/persistent-data/data-structures/btree_damage_visitor.h @@ -223,6 +223,9 @@ namespace persistent_data { bool check_internal(node_location const &loc, btree_detail::node_ref const &n) { + if (loc.is_sub_root()) + new_root(loc.level()); + if (already_visited(n)) return false; else if (!checker_.check_block_nr(n) || @@ -236,9 +239,6 @@ namespace persistent_data { return false; } - if (loc.is_sub_root()) - new_root(loc.level()); - good_internal(n.key_at(0)); return true; @@ -247,6 +247,9 @@ namespace persistent_data { template bool check_leaf(node_location const &loc, btree_detail::node_ref const &n) { + if (loc.is_sub_root()) + new_root(loc.level()); + if (already_visited(n)) return false; else if (!checker_.check_block_nr(n) || @@ -254,24 +257,19 @@ namespace persistent_data { !checker_.check_max_entries(n) || !checker_.check_nr_entries(n, loc.is_sub_root()) || !checker_.check_ordered_keys(n) || - !checker_.check_parent_key(n, loc.is_sub_root() ? boost::optional() : loc.key)) { + !checker_.check_parent_key(n, loc.is_sub_root() ? boost::optional() : loc.key) || + !checker_.check_leaf_key(n, last_leaf_key_[loc.level()])) { report_damage(checker_.get_last_error_string()); return false; } - if (loc.is_sub_root()) - new_root(loc.level()); - - bool r = checker_.check_leaf_key(n, last_leaf_key_[loc.level()]); - if (!r) - report_damage(checker_.get_last_error_string()); - else if (n.get_nr_entries() > 0) { + if (n.get_nr_entries() > 0) { last_leaf_key_[loc.level()] = n.key_at(n.get_nr_entries() - 1); good_leaf(n.key_at(0), n.key_at(n.get_nr_entries() - 1) + 1); } - return r; + return true; } template From 810e86e675170c04bbf33e0cc3a8809ae2893ca0 Mon Sep 17 00:00:00 2001 From: Ming-Hung Tsai Date: Tue, 5 Apr 2016 17:05:28 +0800 Subject: [PATCH 8/8] [counting_visitor] cleanup: remove redundant statements --- .../data-structures/btree_counter.h | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/persistent-data/data-structures/btree_counter.h b/persistent-data/data-structures/btree_counter.h index 1b8c7a2..ec74e86 100644 --- a/persistent-data/data-structures/btree_counter.h +++ b/persistent-data/data-structures/btree_counter.h @@ -57,6 +57,9 @@ namespace persistent_data { private: bool check_internal(node_location const &l, btree_detail::node_ref const &n) { + if (l.is_sub_root()) + new_root(l.level()); + if (!checker_.check_block_nr(n) || !checker_.check_value_size(n) || !checker_.check_max_entries(n) || @@ -65,31 +68,28 @@ namespace persistent_data { !checker_.check_parent_key(n, l.is_sub_root() ? boost::optional() : l.key)) return false; - if (l.is_sub_root()) - new_root(l.level()); - return true; } template bool check_leaf(node_location const &l, btree_detail::node_ref const &n) { + if (l.is_sub_root()) + new_root(l.level()); + if (!checker_.check_block_nr(n) || !checker_.check_value_size(n) || !checker_.check_max_entries(n) || !checker_.check_nr_entries(n, l.is_sub_root()) || !checker_.check_ordered_keys(n) || - !checker_.check_parent_key(n, l.is_sub_root() ? boost::optional() : l.key)) + !checker_.check_parent_key(n, l.is_sub_root() ? boost::optional() : l.key) || + !checker_.check_leaf_key(n, last_leaf_key_[l.level()])) return false; - if (l.is_sub_root()) - new_root(l.level()); - - bool r = checker_.check_leaf_key(n, last_leaf_key_[l.level()]); - if (r && n.get_nr_entries() > 0) + if (n.get_nr_entries() > 0) last_leaf_key_[l.level()] = n.key_at(n.get_nr_entries() - 1); - return r; + return true; } void new_root(unsigned level) {