From e7fa012701fb018b92c76183a37ee672a944d205 Mon Sep 17 00:00:00 2001
From: Ming-Hung Tsai <mtsai@redhat.com>
Date: Thu, 9 Sep 2021 09:58:45 +0800
Subject: [PATCH] [btree] Fix ref-counting on overwritten values

---
 era/era_detail.h                             | 10 +++++++++-
 persistent-data/data-structures/btree.tcc    | 17 +++++++++--------
 persistent-data/space-maps/disk_structures.h | 10 ++++++++++
 thin-provisioning/device_tree.h              |  8 ++++++++
 thin-provisioning/mapping_tree.h             |  8 ++++++++
 5 files changed, 44 insertions(+), 9 deletions(-)

diff --git a/era/era_detail.h b/era/era_detail.h
index 80961d5..a2c1851 100644
--- a/era/era_detail.h
+++ b/era/era_detail.h
@@ -22,6 +22,14 @@ namespace era {
 		uint64_t writeset_root;
 	};
 
+	inline bool operator==(era_detail const& lhs, era_detail const& rhs) {
+		return lhs.nr_bits == rhs.nr_bits && lhs.writeset_root == rhs.writeset_root;
+	}
+
+	inline bool operator!=(era_detail const& lhs, era_detail const& rhs) {
+		return !(lhs == rhs);
+	}
+
 	struct era_detail_ref_counter {
 		era_detail_ref_counter(persistent_data::transaction_manager::ptr tm)
 			: tm_(tm) {
@@ -31,7 +39,7 @@ namespace era {
 			tm_->get_sm()->inc(d.writeset_root);
 		}
 
-		void dec(persistent_data::block_address b) {
+		void dec(era_detail const &d) {
 			// I don't think we ever do this in the tools
 			throw std::runtime_error("not implemented");
 		}
diff --git a/persistent-data/data-structures/btree.tcc b/persistent-data/data-structures/btree.tcc
index 27c9adc..3dc0704 100644
--- a/persistent-data/data-structures/btree.tcc
+++ b/persistent-data/data-structures/btree.tcc
@@ -693,9 +693,15 @@ namespace persistent_data {
 		leaf_node n = spine.template get_node<ValueTraits>();
 		if (need_insert)
 			n.insert_at(index, key[Levels - 1], value);
-		else
-			// FIXME: check if we're overwriting with the same value.
-			n.set_value(index, value);
+		else {
+			typename ValueTraits::value_type old_value = n.value_at(index);
+			if (value != old_value) {
+				// do decrement the old value if it already exists
+				rc_.dec(old_value);
+
+				n.set_value(index, value);
+			}
+		}
 
 		root_ = spine.get_root();
 
@@ -981,11 +987,6 @@ namespace persistent_data {
 		if (i < 0 || leaf.key_at(i) != key)
 			i++;
 
-		// do decrement the old value if it already exists
-		// FIXME: I'm not sure about this, I don't understand the |inc| reference
-		if (static_cast<unsigned>(i) < leaf.get_nr_entries() && leaf.key_at(i) == key && inc) {
-			// dec old entry
-		}
 		*index = i;
 
 		return ((static_cast<unsigned>(i) >= leaf.get_nr_entries()) ||
diff --git a/persistent-data/space-maps/disk_structures.h b/persistent-data/space-maps/disk_structures.h
index aa8fbe5..50249d8 100644
--- a/persistent-data/space-maps/disk_structures.h
+++ b/persistent-data/space-maps/disk_structures.h
@@ -42,6 +42,16 @@ namespace persistent_data {
 			uint32_t none_free_before_;
 		};
 
+		inline bool operator==(index_entry const& lhs, index_entry const& rhs) {
+			// The return value doesn't matter, since the ref-counts of bitmap blocks
+			// are managed by shadow operations.
+			return false;
+		}
+
+		inline bool operator!=(index_entry const& lhs, index_entry const& rhs) {
+			return !(lhs == rhs);
+		}
+
 		struct index_entry_traits {
 			typedef index_entry_disk disk_type;
 			typedef index_entry value_type;
diff --git a/thin-provisioning/device_tree.h b/thin-provisioning/device_tree.h
index ee241a5..ffa98ff 100644
--- a/thin-provisioning/device_tree.h
+++ b/thin-provisioning/device_tree.h
@@ -25,6 +25,14 @@ namespace thin_provisioning {
 			uint32_t snapshotted_time_;
 		};
 
+		inline bool operator==(device_details const& lhs, device_details const& rhs) {
+			return false; // device_details are not compariable
+		}
+
+		inline bool operator!=(device_details const& lhs, device_details const& rhs) {
+			return !(lhs == rhs);
+		}
+
 		struct device_details_traits {
 			typedef device_details_disk disk_type;
 			typedef device_details value_type;
diff --git a/thin-provisioning/mapping_tree.h b/thin-provisioning/mapping_tree.h
index 24207ed..6281f2a 100644
--- a/thin-provisioning/mapping_tree.h
+++ b/thin-provisioning/mapping_tree.h
@@ -24,6 +24,14 @@ namespace thin_provisioning {
 			uint32_t time_;
 		};
 
+		inline bool operator==(block_time const& lhs, block_time const& rhs) {
+			return lhs.block_ == rhs.block_;
+		}
+
+		inline bool operator!=(block_time const& lhs, block_time const& rhs) {
+			return !(lhs == rhs);
+		}
+
 		class block_time_ref_counter {
 		public:
 			block_time_ref_counter(space_map::ptr sm);