[PATCH 0/7] btrfs-progs: tune: add resume support for csum conversion

[PATCH 0/7] btrfs-progs: tune: add resume support for csum conversion

[Qu Wenruo]

[RESUME SUPPORT]
This patchset adds the resume support for "btrfstune --csum".

The main part is in resume for data csum change, as we have 4 different
status during data csum conversion.

Thankfully for data csum conversion, everything is protected by
metadata COW and we can detect the current status by the existence of
both old and new csum items, and their coverage.

For resume of metadata conversion, there is nothing we can really do but
go through all the tree blocks and verify them against both new and old
csum type.

[TESTING]
For the tests, currently errors are injected after every possible
transaction commit, and then resume from that interrupted status.
So far the patchset passes all above tests.

But I'm not sure what's the better way to craft the test case.

Should we go log-writes? Or use pre-built images?

[TODO]
- Test cases for resume

- Support for revert if errors are found
  If we hit data csum mismatch and can not repair from any copy, then
  we should revert back to the old csum.

- Support for pre-cautious metadata check
  We'd better read and very metadata before rewriting them.

- Performance tuning
  We want to take a balance between too frequent commit transaction
  and too large transaction.
  This is especially true for data csum objectid change and maybe
  old data csum deletion.

- UI enhancement
  A btrfs-convert like UI would be better.


Qu Wenruo (7):
  btrfs-progs: tune: implement resume support for metadata checksum
  btrfs-progs: tune: implement resume support for generating new data
    csum
  btrfs-progs: tune: implement resume support for csum tree without any
    new csum item
  btrfs-progs: tune: implement resume support for empty csum tree
  btrfs-progs: tune: implement resume support for half deleted old csums
  btrfs-progs: tune: implement resume support for data csum objectid
    change
  btrfs-progs: tune: reject csum change if the fs is already using the
    target csum type

 tune/change-csum.c | 461 ++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 418 insertions(+), 43 deletions(-)

-- 
2.40.1

[PATCH 1/7] btrfs-progs: tune: implement resume support for metadata checksum

[Qu Wenruo]

For interrupted metadata checksum change, we only need to call the same
change_meta_csums().

Since we don't have any record on the last converted metadata, thus we
have to go through all metadata anyway.
And the existing change_meta_csums() has already implemented the needed
checks to skip converted metadata.

Since we're here, also implement all the surrounding checks, like making
sure the new target csum type matches the interrupted one.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 tune/change-csum.c | 88 ++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 81 insertions(+), 7 deletions(-)

diff --git a/tune/change-csum.c b/tune/change-csum.c
index b5efc3a8807f..7ae618a433cb 100644
--- a/tune/change-csum.c
+++ b/tune/change-csum.c
@@ -45,12 +45,7 @@ static int check_csum_change_requreiment(struct btrfs_fs_info *fs_info)
 		error("no csum change support for extent-tree-v2 feature yet.");
 		return -EOPNOTSUPP;
 	}
-	if (btrfs_super_flags(fs_info->super_copy) &
-	    (BTRFS_SUPER_FLAG_CHANGING_DATA_CSUM |
-	     BTRFS_SUPER_FLAG_CHANGING_META_CSUM)) {
-		error("resume from half converted status is not yet supported");
-		return -EOPNOTSUPP;
-	}
+
 	key.objectid = BTRFS_BALANCE_OBJECTID;
 	key.type = BTRFS_TEMPORARY_ITEM_KEY;
 	key.offset = 0;
@@ -522,7 +517,7 @@ out:
 	return ret;
 }
 
-static int change_meta_csums(struct btrfs_fs_info *fs_info, u32 new_csum_type)
+static int change_meta_csums(struct btrfs_fs_info *fs_info, u16 new_csum_type)
 {
 	struct btrfs_root *extent_root = btrfs_extent_root(fs_info, 0);
 	struct btrfs_path path = { 0 };
@@ -640,6 +635,71 @@ out:
 	return ret;
 }
 
+static int resume_csum_change(struct btrfs_fs_info *fs_info, u16 new_csum_type)
+{
+	const u64 super_flags = btrfs_super_flags(fs_info->super_copy);
+	struct btrfs_root *tree_root = fs_info->tree_root;
+	struct btrfs_path path = { 0 };
+	struct btrfs_key key;
+	int ret;
+
+	if ((super_flags & (BTRFS_SUPER_FLAG_CHANGING_DATA_CSUM |
+			    BTRFS_SUPER_FLAG_CHANGING_META_CSUM)) ==
+	    (BTRFS_SUPER_FLAG_CHANGING_DATA_CSUM |
+	     BTRFS_SUPER_FLAG_CHANGING_META_CSUM)) {
+		error("Invalid super flags, only one bit of CHANGING_DATA_CSUM or CHANGING_META_CSUM can be set");
+		return -EUCLEAN;
+	}
+
+	key.objectid = BTRFS_CSUM_CHANGE_OBJECTID;
+	key.type = BTRFS_TEMPORARY_ITEM_KEY;
+	key.offset = (u64)-1;
+	ret = btrfs_search_slot(NULL, tree_root, &key, &path, 0, 0);
+	if (ret < 0) {
+		errno = -ret;
+		error("failed to locate the csum change item: %m");
+		return ret;
+	}
+	assert(ret > 0);
+	ret = btrfs_previous_item(tree_root, &path, BTRFS_CSUM_CHANGE_OBJECTID,
+				  BTRFS_TEMPORARY_ITEM_KEY);
+	if (ret > 0)
+		ret = -ENOENT;
+	if (ret < 0) {
+		errno = -ret;
+		error("failed to locate the csum change item: %m");
+		btrfs_release_path(&path);
+		return ret;
+	}
+	btrfs_item_key_to_cpu(path.nodes[0], &key, path.slots[0]);
+	btrfs_release_path(&path);
+
+	if (new_csum_type != key.offset) {
+		ret = -EINVAL;
+		error("target csum type mismatch with interrupted csum type, has %s (%u) expect %s (%llu)",
+		      btrfs_super_csum_name(new_csum_type), new_csum_type,
+		      btrfs_super_csum_name(key.offset), key.offset);
+		return ret;
+	}
+
+	if (super_flags & BTRFS_SUPER_FLAG_CHANGING_DATA_CSUM) {
+		error("resume on data checksum changing is not yet supported");
+		return -EOPNOTSUPP;
+	}
+
+	/*
+	 * For metadata resume, just call the same change_meta_csums(), as we
+	 * have no record on previous converted metadata, thus have to go
+	 * through all metadata anyway.
+	 */
+	ret = change_meta_csums(fs_info, new_csum_type);
+	if (ret < 0) {
+		errno = -ret;
+		error("failed to resume metadata csum change: %m");
+	}
+	return ret;
+}
+
 int btrfs_change_csum_type(struct btrfs_fs_info *fs_info, u16 new_csum_type)
 {
 	u16 old_csum_type = fs_info->csum_type;
@@ -650,6 +710,20 @@ int btrfs_change_csum_type(struct btrfs_fs_info *fs_info, u16 new_csum_type)
 	if (ret < 0)
 		return ret;
 
+	if (btrfs_super_flags(fs_info->super_copy) &
+	    (BTRFS_SUPER_FLAG_CHANGING_DATA_CSUM |
+	     BTRFS_SUPER_FLAG_CHANGING_META_CSUM)) {
+		ret = resume_csum_change(fs_info, new_csum_type);
+		if (ret < 0) {
+			errno = -ret;
+			error("failed to resume unfinished csum change: %m");
+			return ret;
+		}
+		printf("converted csum type from %s (%u) to %s (%u)\n",
+		       btrfs_super_csum_name(old_csum_type), old_csum_type,
+		       btrfs_super_csum_name(new_csum_type), new_csum_type);
+		return ret;
+	}
 	/*
 	 * Phase 1, generate new data csums.
 	 *
-- 
2.40.1

[PATCH 2/7] btrfs-progs: tune: implement resume support for generating new data csum

[Qu Wenruo]

If a csum change is interrupted at new data csum generation stage, we
can detect such situation by checking the old and new csum items.

At the new data csum generation stage, old csums are untouched, and only
new csums items (with different objectid) are inserted into the csum
tree.

Thus the old csum items should cover a larger range, while the new csum
items should be a subset of the old csums.

The resume part would start by re-generating the remaining part, then go
through the conversion stages.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 tune/change-csum.c | 226 ++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 193 insertions(+), 33 deletions(-)

diff --git a/tune/change-csum.c b/tune/change-csum.c
index 7ae618a433cb..b95a4117b59b 100644
--- a/tune/change-csum.c
+++ b/tune/change-csum.c
@@ -197,9 +197,9 @@ out:
  * item.
  */
 #define CSUM_CHANGE_BYTES_THRESHOLD	(SZ_2M)
-static int generate_new_data_csums(struct btrfs_fs_info *fs_info, u16 new_csum_type)
+static int generate_new_data_csums_range(struct btrfs_fs_info *fs_info, u64 start,
+					 u16 new_csum_type)
 {
-	struct btrfs_root *tree_root = fs_info->tree_root;
 	struct btrfs_root *csum_root = btrfs_csum_root(fs_info, 0);
 	struct btrfs_trans_handle *trans;
 	struct btrfs_path path = { 0 };
@@ -208,7 +208,7 @@ static int generate_new_data_csums(struct btrfs_fs_info *fs_info, u16 new_csum_t
 	void *csum_buffer;
 	u64 converted_bytes = 0;
 	u64 last_csum;
-	u64 cur = 0;
+	u64 cur = start;
 	int ret;
 
 	ret = get_last_csum_bytenr(fs_info, &last_csum);
@@ -221,34 +221,6 @@ static int generate_new_data_csums(struct btrfs_fs_info *fs_info, u16 new_csum_t
 	if (!csum_buffer)
 		return -ENOMEM;
 
-	trans = btrfs_start_transaction(tree_root, 1);
-	if (IS_ERR(trans)) {
-		ret = PTR_ERR(trans);
-		errno = -ret;
-		error("failed to start transaction: %m");
-		goto out;
-	}
-	key.objectid = BTRFS_CSUM_CHANGE_OBJECTID;
-	key.type = BTRFS_TEMPORARY_ITEM_KEY;
-	key.offset = new_csum_type;
-	ret = btrfs_insert_empty_item(trans, tree_root, &path, &key, 0);
-	btrfs_release_path(&path);
-	if (ret < 0) {
-		errno = -ret;
-		error("failed to insert csum change item: %m");
-		btrfs_abort_transaction(trans, ret);
-		goto out;
-	}
-	btrfs_set_super_flags(fs_info->super_copy,
-			      btrfs_super_flags(fs_info->super_copy) |
-			      BTRFS_SUPER_FLAG_CHANGING_DATA_CSUM);
-	ret = btrfs_commit_transaction(trans, tree_root);
-	if (ret < 0) {
-		errno = -ret;
-		error("failed to commit the initial transaction: %m");
-		goto out;
-	}
-
 	trans = btrfs_start_transaction(csum_root,
 			CSUM_CHANGE_BYTES_THRESHOLD / fs_info->sectorsize *
 			new_csum_size);
@@ -321,6 +293,44 @@ out:
 	return ret;
 }
 
+static int generate_new_data_csums(struct btrfs_fs_info *fs_info, u16 new_csum_type)
+{
+	struct btrfs_root *tree_root = fs_info->tree_root;
+	struct btrfs_trans_handle *trans;
+	struct btrfs_path path = { 0 };
+	struct btrfs_key key;
+	int ret;
+
+	trans = btrfs_start_transaction(tree_root, 1);
+	if (IS_ERR(trans)) {
+		ret = PTR_ERR(trans);
+		errno = -ret;
+		error("failed to start transaction: %m");
+		return ret;
+	}
+	key.objectid = BTRFS_CSUM_CHANGE_OBJECTID;
+	key.type = BTRFS_TEMPORARY_ITEM_KEY;
+	key.offset = new_csum_type;
+	ret = btrfs_insert_empty_item(trans, tree_root, &path, &key, 0);
+	btrfs_release_path(&path);
+	if (ret < 0) {
+		errno = -ret;
+		error("failed to insert csum change item: %m");
+		btrfs_abort_transaction(trans, ret);
+		return ret;
+	}
+	btrfs_set_super_flags(fs_info->super_copy,
+			      btrfs_super_flags(fs_info->super_copy) |
+			      BTRFS_SUPER_FLAG_CHANGING_DATA_CSUM);
+	ret = btrfs_commit_transaction(trans, tree_root);
+	if (ret < 0) {
+		errno = -ret;
+		error("failed to commit the initial transaction: %m");
+		return ret;
+	}
+	return generate_new_data_csums_range(fs_info, 0, new_csum_type);
+}
+
 static int delete_old_data_csums(struct btrfs_fs_info *fs_info)
 {
 	struct btrfs_root *csum_root = btrfs_csum_root(fs_info, 0);
@@ -635,6 +645,152 @@ out:
 	return ret;
 }
 
+/*
+ * Get the first and last csum items which has @objectid as their objectid.
+ *
+ * This would be called to handle data csum resume, which may have both old
+ * and new csums co-exist in the same csum tree.
+ *
+ * Return >0 if there is no such EXTENT_CSUM with given @objectid.
+ * Return 0 if there is such EXTENT_CSUM and populate @first_ret and @last_ret.
+ * Return <0 for errors.
+ */
+static int get_csum_items_range(struct btrfs_fs_info *fs_info,
+				u64 objectid, u64 *first_ret, u64 *last_ret,
+				u32 *last_item_size)
+{
+	struct btrfs_root *csum_root = btrfs_csum_root(fs_info, 0);
+	struct btrfs_path path = { 0 };
+	struct btrfs_key key;
+	int ret;
+
+	key.objectid = objectid;
+	key.type = BTRFS_EXTENT_CSUM_KEY;
+	key.offset = 0;
+
+	ret = btrfs_search_slot(NULL, csum_root, &key, &path, 0, 0);
+	if (ret < 0) {
+		errno = -ret;
+		error("failed to search csum tree: %m");
+		return ret;
+	}
+	if (path.slots[0] >= btrfs_header_nritems(path.nodes[0])) {
+		ret = btrfs_next_leaf(csum_root, &path);
+		if (ret < 0) {
+			errno = -ret;
+			error("failed to search csum tree: %m");
+			btrfs_release_path(&path);
+			return ret;
+		}
+		/*
+		 * There is no next leaf, meaning we didn't find any
+		 * csum item with given objectid.
+		 */
+		if (ret > 0) {
+			btrfs_release_path(&path);
+			return ret;
+		}
+	}
+
+	btrfs_item_key_to_cpu(path.nodes[0], &key, path.slots[0]);
+	btrfs_release_path(&path);
+	if (key.objectid != objectid)
+		return 1;
+	*first_ret = key.offset;
+
+	key.objectid = objectid;
+	key.type = BTRFS_EXTENT_CSUM_KEY;
+	key.offset = (u64)-1;
+
+	ret = btrfs_search_slot(NULL, csum_root, &key, &path, 0, 0);
+	if (ret < 0) {
+		errno = -ret;
+		error("failed to search csum tree: %m");
+		return ret;
+	}
+	assert(ret > 0);
+	ret = btrfs_previous_item(csum_root, &path, objectid,
+				  BTRFS_EXTENT_CSUM_KEY);
+	if (ret < 0) {
+		errno = -ret;
+		error("failed to search csum tree: %m");
+		btrfs_release_path(&path);
+		return ret;
+	}
+	if (ret > 0) {
+		btrfs_release_path(&path);
+		return 1;
+	}
+	btrfs_item_key_to_cpu(path.nodes[0], &key, path.slots[0]);
+	*last_item_size = btrfs_item_size(path.nodes[0], path.slots[0]);
+	btrfs_release_path(&path);
+	*last_ret = key.offset;
+	return 0;
+}
+
+static int resume_data_csum_change(struct btrfs_fs_info *fs_info,
+				   u16 new_csum_type)
+{
+	u64 old_csum_first;
+	u64 old_csum_last;
+	u64 new_csum_first;
+	u64 new_csum_last;
+	bool old_csum_found = false;
+	bool new_csum_found = false;
+	u32 old_last_size;
+	u32 new_last_size;
+	u64 resume_start;
+	int ret;
+
+	ret = get_csum_items_range(fs_info, BTRFS_EXTENT_CSUM_OBJECTID,
+				   &old_csum_first, &old_csum_last,
+				   &old_last_size);
+	if (ret < 0)
+		return ret;
+	if (ret == 0)
+		old_csum_found = true;
+	ret = get_csum_items_range(fs_info, BTRFS_CSUM_CHANGE_OBJECTID,
+				   &new_csum_first, &new_csum_last,
+				   &new_last_size);
+	if (ret < 0)
+		return ret;
+	if (ret == 0)
+		new_csum_found = true;
+
+	/*
+	 * Both old and new csum exist, and new csum is only a subset of the
+	 * old ones.
+	 *
+	 * This means we're still generating new data csums.
+	 */
+	if (old_csum_found && new_csum_found && old_csum_first <= new_csum_first &&
+	    old_csum_last >= new_csum_last) {
+		resume_start = new_csum_last + new_last_size /
+				btrfs_csum_type_size(new_csum_type) *
+				fs_info->sectorsize;
+		goto new_data_csums;
+	}
+
+	/* Other cases are not yet supported. */
+	return -EOPNOTSUPP;
+
+new_data_csums:
+	ret = generate_new_data_csums_range(fs_info, resume_start, new_csum_type);
+	if (ret < 0) {
+		errno = -ret;
+		error("failed to generate new data csums: %m");
+		return ret;
+	}
+	ret = delete_old_data_csums(fs_info);
+	if (ret < 0)
+		return ret;
+	ret = change_csum_objectids(fs_info);
+	if (ret < 0)
+		return ret;
+	ret = change_meta_csums(fs_info, new_csum_type);
+	return ret;
+}
+
 static int resume_csum_change(struct btrfs_fs_info *fs_info, u16 new_csum_type)
 {
 	const u64 super_flags = btrfs_super_flags(fs_info->super_copy);
@@ -683,8 +839,12 @@ static int resume_csum_change(struct btrfs_fs_info *fs_info, u16 new_csum_type)
 	}
 
 	if (super_flags & BTRFS_SUPER_FLAG_CHANGING_DATA_CSUM) {
-		error("resume on data checksum changing is not yet supported");
-		return -EOPNOTSUPP;
+		ret = resume_data_csum_change(fs_info, new_csum_type);
+		if (ret < 0) {
+			errno = -ret;
+			error("failed to resume data checksum change: %m");
+		}
+		return ret;
 	}
 
 	/*
-- 
2.40.1

[PATCH 3/7] btrfs-progs: tune: implement resume support for csum tree without any new csum item

[Qu Wenruo]

There are two possible situations where there is no new csum items at
all:

- We just inserted csum change item
  This means all csums are really old csum type, and we can start
  the conversion from the beginning, only need to skip the csum change
  item insert.

- We finished data csum conversion but not yet started metadata
  conversion
  This means all csums are already new csum type, and we can resume
  by starting changing metadata csums.

To distinguish the two cases, we need to read the first sector, and
verify the data content against both csum types.

If the csum matches with old csum type, we resume from generating new
data csum.
If the csum matches with new csum type, we resume from rewriting
metadata csum.
If the csum doesn't match either csum type, we have some big problems
then.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 tune/change-csum.c | 105 +++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 102 insertions(+), 3 deletions(-)

diff --git a/tune/change-csum.c b/tune/change-csum.c
index b95a4117b59b..05bddaa48d27 100644
--- a/tune/change-csum.c
+++ b/tune/change-csum.c
@@ -115,7 +115,8 @@ static int get_last_csum_bytenr(struct btrfs_fs_info *fs_info, u64 *result)
 
 static int read_verify_one_data_sector(struct btrfs_fs_info *fs_info,
 				       u64 logical, void *data_buf,
-				       const void *old_csums)
+				       const void *old_csums, u16 old_csum_type,
+				       bool output_error)
 {
 	const u32 sectorsize = fs_info->sectorsize;
 	int num_copies = btrfs_num_copies(fs_info, logical, sectorsize);
@@ -138,7 +139,7 @@ static int read_verify_one_data_sector(struct btrfs_fs_info *fs_info,
 		if (memcmp(csum_has, old_csums, fs_info->csum_size) == 0) {
 			found_good = true;
 			break;
-		} else {
+		} else if (output_error){
 			char found[BTRFS_CSUM_STRING_LEN];
 			char want[BTRFS_CSUM_STRING_LEN];
 
@@ -168,7 +169,8 @@ static int generate_new_csum_range(struct btrfs_trans_handle *trans,
 
 	for (u64 cur = logical; cur < logical + length; cur += sectorsize) {
 		ret = read_verify_one_data_sector(fs_info, cur, buf, old_csums +
-				(cur - logical) / sectorsize * fs_info->csum_size);
+				(cur - logical) / sectorsize * fs_info->csum_size,
+				fs_info->csum_type, true);
 
 		if (ret < 0) {
 			error("failed to recover a good copy for data at logical %llu",
@@ -532,8 +534,20 @@ static int change_meta_csums(struct btrfs_fs_info *fs_info, u16 new_csum_type)
 	struct btrfs_root *extent_root = btrfs_extent_root(fs_info, 0);
 	struct btrfs_path path = { 0 };
 	struct btrfs_key key;
+	u64 super_flags;
 	int ret;
 
+	/* Re-set the super flags, this is for resume cases. */
+	super_flags = btrfs_super_flags(fs_info->super_copy);
+	super_flags &= ~BTRFS_SUPER_FLAG_CHANGING_DATA_CSUM;
+	super_flags |= BTRFS_SUPER_FLAG_CHANGING_META_CSUM;
+	btrfs_set_super_flags(fs_info->super_copy, super_flags);
+	ret = write_all_supers(fs_info);
+	if (ret < 0) {
+		errno = -ret;
+		error("failed to update super flags: %m");
+	}
+
 	/*
 	 * Disable metadata csum checks first, as we may hit tree blocks with
 	 * either old or new csums.
@@ -728,6 +742,63 @@ static int get_csum_items_range(struct btrfs_fs_info *fs_info,
 	return 0;
 }
 
+/*
+ * Verify one data sector to determine which csum type matches the csum.
+ *
+ * Return >0 if the current csum type doesn't pass the check (including csum
+ * item too small compared to csum type).
+ * Return 0 if the current csum type passes the check.
+ * Return <0 for other errors.
+ */
+static int determine_csum_type(struct btrfs_fs_info *fs_info, u64 logical,
+			       u16 csum_type)
+{
+	struct btrfs_root *csum_root = btrfs_csum_root(fs_info, logical);
+	struct btrfs_path path = { 0 };
+	struct btrfs_key key;
+	u16 csum_size = btrfs_csum_type_size(csum_type);
+	u8 csum_expected[BTRFS_CSUM_SIZE];
+	void *buf;
+	int ret;
+
+	key.objectid = BTRFS_EXTENT_CSUM_OBJECTID;
+	key.type = BTRFS_EXTENT_CSUM_KEY;
+	key.offset = logical;
+
+	ret = btrfs_search_slot(NULL, csum_root, &key, &path, 0, 0);
+	if (ret > 0)
+		ret = -ENOENT;
+	if (ret < 0) {
+		errno = -ret;
+		error("failed to search csum tree: %m");
+		btrfs_release_path(&path);
+		return ret;
+	}
+
+	/*
+	 * The csum item size is smaller than expected csum size, no
+	 * more need to check.
+	 */
+	if (btrfs_item_size(path.nodes[0], path.slots[0]) < csum_size) {
+		btrfs_release_path(&path);
+		return 1;
+	}
+	read_extent_buffer(path.nodes[0], csum_expected,
+			   btrfs_item_ptr_offset(path.nodes[0], path.slots[0]),
+			   csum_size);
+	btrfs_release_path(&path);
+
+	buf = malloc(fs_info->sectorsize);
+	if (!buf)
+		return -ENOMEM;
+	ret = read_verify_one_data_sector(fs_info, logical, buf, csum_expected,
+					  csum_type, false);
+	if (ret < 0)
+		ret = 1;
+	free(buf);
+	return ret;
+}
+
 static int resume_data_csum_change(struct btrfs_fs_info *fs_info,
 				   u16 new_csum_type)
 {
@@ -757,6 +828,33 @@ static int resume_data_csum_change(struct btrfs_fs_info *fs_info,
 	if (ret == 0)
 		new_csum_found = true;
 
+	/*
+	 * Only old csums exists. This can be one of the two cases:
+	 * - Only the csum change item inserted, no new csum generated.
+	 * - All data csum is converted to the new type.
+	 *
+	 * Here we need to check if the csum item is in old or new type.
+	 */
+	if (old_csum_found && !new_csum_found) {
+		ret = determine_csum_type(fs_info, old_csum_first, fs_info->csum_type);
+		if (ret == 0) {
+			/* All old data csums, restart generation. */
+			resume_start = 0;
+			goto new_data_csums;
+		}
+		ret = determine_csum_type(fs_info, old_csum_first, new_csum_type);
+		if (ret == 0) {
+			/*
+			 * All new data csums, just go metadata csum change, which
+			 * would drop the CHANGING_DATA_CSUM flag for us.
+			 */
+			goto new_meta_csum;
+		}
+		error("The data checksum for logical %llu doesn't match either old or new csum type, unable to resume",
+			old_csum_first);
+		return -EUCLEAN;
+	}
+
 	/*
 	 * Both old and new csum exist, and new csum is only a subset of the
 	 * old ones.
@@ -787,6 +885,7 @@ new_data_csums:
 	ret = change_csum_objectids(fs_info);
 	if (ret < 0)
 		return ret;
+new_meta_csum:
 	ret = change_meta_csums(fs_info, new_csum_type);
 	return ret;
 }
-- 
2.40.1

[PATCH 4/7] btrfs-progs: tune: implement resume support for empty csum tree

[Qu Wenruo]

We have a very rare chance to hit a fs with empty csum tree but still
has CHANGING_DATA_CSUM flag.

The window is very small, but it's still possible, so handle it by
jumping directly to metadata csum change.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 tune/change-csum.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/tune/change-csum.c b/tune/change-csum.c
index 05bddaa48d27..aef3b05a0d9d 100644
--- a/tune/change-csum.c
+++ b/tune/change-csum.c
@@ -828,6 +828,13 @@ static int resume_data_csum_change(struct btrfs_fs_info *fs_info,
 	if (ret == 0)
 		new_csum_found = true;
 
+	/*
+	 * No csum item found at all, this fs has empty csum tree.
+	 * Just go metadata change.
+	 */
+	if (!old_csum_found && !new_csum_found)
+		goto new_meta_csum;
+
 	/*
 	 * Only old csums exists. This can be one of the two cases:
 	 * - Only the csum change item inserted, no new csum generated.
-- 
2.40.1

[PATCH 5/7] btrfs-progs: tune: implement resume support for half deleted old csums

[Qu Wenruo]

If the csum conversion is interrupted when old csums are being deleted,
we should resume by continue deleting the old csums.

The function delete_old_data_csums() can handle half deleted cases
already.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 tune/change-csum.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/tune/change-csum.c b/tune/change-csum.c
index aef3b05a0d9d..2ec2d6cc5271 100644
--- a/tune/change-csum.c
+++ b/tune/change-csum.c
@@ -876,6 +876,15 @@ static int resume_data_csum_change(struct btrfs_fs_info *fs_info,
 		goto new_data_csums;
 	}
 
+	/*
+	 * Both old and new csum exist, and old csum is a subset of the new ones.
+	 *
+	 * This means we're deleting the old csums.
+	 */
+	if (old_csum_found && new_csum_found && new_csum_first <= old_csum_first &&
+	    new_csum_last >= old_csum_last)
+		goto delete_old;
+
 	/* Other cases are not yet supported. */
 	return -EOPNOTSUPP;
 
@@ -886,6 +895,7 @@ new_data_csums:
 		error("failed to generate new data csums: %m");
 		return ret;
 	}
+delete_old:
 	ret = delete_old_data_csums(fs_info);
 	if (ret < 0)
 		return ret;
-- 
2.40.1

[PATCH 6/7] btrfs-progs: tune: implement resume support for data csum objectid change

[Qu Wenruo]

When the csum conversion is interrupted when changing data csum
objectid, we should just resume the objectid conversion.

This situation can be detected by comparing the old and new csum items.
They should both exist but doesn't intersect (interrupted halfway), or
only new csum items exist (interrupted after we have deleted old csums).

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 tune/change-csum.c | 23 +++++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/tune/change-csum.c b/tune/change-csum.c
index 2ec2d6cc5271..dad39c3ec854 100644
--- a/tune/change-csum.c
+++ b/tune/change-csum.c
@@ -885,8 +885,26 @@ static int resume_data_csum_change(struct btrfs_fs_info *fs_info,
 	    new_csum_last >= old_csum_last)
 		goto delete_old;
 
-	/* Other cases are not yet supported. */
-	return -EOPNOTSUPP;
+	/*
+	 * Both csums exist but not covering each other, or only new csum exists.
+	 *
+	 * This means we have already deleted all the old csums, is going to or
+	 * have already started objectid change.
+	 */
+	if ((old_csum_found && new_csum_found && old_csum_last <= new_csum_first) &&
+	    (!old_csum_found && new_csum_found))
+		goto change;
+
+	/* The remaining cases should not be possible. */
+	error("unexpected resume condition:");
+	error("old csum found=%d start=%llu last=%llu new csum found=%d start=%llu last=%llu",
+		old_csum_found,
+		old_csum_found ? old_csum_first : 0,
+		old_csum_found ? old_csum_last : 0,
+		new_csum_found,
+		new_csum_found ? new_csum_first : 0,
+		new_csum_found ? new_csum_last : 0);
+	return -EUCLEAN;
 
 new_data_csums:
 	ret = generate_new_data_csums_range(fs_info, resume_start, new_csum_type);
@@ -899,6 +917,7 @@ delete_old:
 	ret = delete_old_data_csums(fs_info);
 	if (ret < 0)
 		return ret;
+change:
 	ret = change_csum_objectids(fs_info);
 	if (ret < 0)
 		return ret;
-- 
2.40.1

[PATCH 7/7] btrfs-progs: tune: reject csum change if the fs is already using the target csum type

[Qu Wenruo]

Currently "btrfstune --csum" allows us to change the csum to the same
one, this is good for testing but not good for end users, as if the end
user interrupts it, they have to resume the change (even it's to the
same csum type) until it finished, or kernel would reject such fs.

Furthermore, we never change the super block csum type until we
completely changed the csum type, thus for resume cases, the fs would
still show up as using the old csum type thus won't cause any problem
resuming.

So here we just reject the csum conversion if the target csum type is
the same as the existing csum type.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 tune/change-csum.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/tune/change-csum.c b/tune/change-csum.c
index dad39c3ec854..ef3d663e038e 100644
--- a/tune/change-csum.c
+++ b/tune/change-csum.c
@@ -29,7 +29,7 @@
 #include "common/utils.h"
 #include "tune/tune.h"
 
-static int check_csum_change_requreiment(struct btrfs_fs_info *fs_info)
+static int check_csum_change_requreiment(struct btrfs_fs_info *fs_info, u16 new_csum_type)
 {
 	struct btrfs_root *tree_root = fs_info->tree_root;
 	struct btrfs_root *dev_root = fs_info->dev_root;
@@ -75,6 +75,12 @@ static int check_csum_change_requreiment(struct btrfs_fs_info *fs_info)
 		error("running dev-replace detected, please finish or cancel it.");
 		return -EINVAL;
 	}
+
+	if (fs_info->csum_type == new_csum_type) {
+		error("the fs is already using csum type %s (%u)",
+		      btrfs_super_csum_name(new_csum_type), new_csum_type);
+		return -EINVAL;
+	}
 	return 0;
 }
 
@@ -1001,7 +1007,7 @@ int btrfs_change_csum_type(struct btrfs_fs_info *fs_info, u16 new_csum_type)
 	int ret;
 
 	/* Phase 0, check conflicting features. */
-	ret = check_csum_change_requreiment(fs_info);
+	ret = check_csum_change_requreiment(fs_info, new_csum_type);
 	if (ret < 0)
 		return ret;
 
-- 
2.40.1

Re: [PATCH 0/7] btrfs-progs: tune: add resume support for csum conversion

[Anand Jain]

Why not use the generation numbers "From" and "To" for converting
to the new checksum and keeping track of the generation number of
the most recent successful conversion?

Thanks, Anand


On 24/05/2023 15:41, Qu Wenruo wrote:
> [RESUME SUPPORT]
> This patchset adds the resume support for "btrfstune --csum".
> 
> The main part is in resume for data csum change, as we have 4 different
> status during data csum conversion.
> 
> Thankfully for data csum conversion, everything is protected by
> metadata COW and we can detect the current status by the existence of
> both old and new csum items, and their coverage.
> 
> For resume of metadata conversion, there is nothing we can really do but
> go through all the tree blocks and verify them against both new and old
> csum type.
> 
> [TESTING]
> For the tests, currently errors are injected after every possible
> transaction commit, and then resume from that interrupted status.
> So far the patchset passes all above tests.
> 
> But I'm not sure what's the better way to craft the test case.
> 
> Should we go log-writes? Or use pre-built images?
> 
> [TODO]
> - Test cases for resume
> 
> - Support for revert if errors are found
>    If we hit data csum mismatch and can not repair from any copy, then
>    we should revert back to the old csum.
> 
> - Support for pre-cautious metadata check
>    We'd better read and very metadata before rewriting them.
> 
> - Performance tuning
>    We want to take a balance between too frequent commit transaction
>    and too large transaction.
>    This is especially true for data csum objectid change and maybe
>    old data csum deletion.
> 
> - UI enhancement
>    A btrfs-convert like UI would be better.
> 
> 
> Qu Wenruo (7):
>    btrfs-progs: tune: implement resume support for metadata checksum
>    btrfs-progs: tune: implement resume support for generating new data
>      csum
>    btrfs-progs: tune: implement resume support for csum tree without any
>      new csum item
>    btrfs-progs: tune: implement resume support for empty csum tree
>    btrfs-progs: tune: implement resume support for half deleted old csums
>    btrfs-progs: tune: implement resume support for data csum objectid
>      change
>    btrfs-progs: tune: reject csum change if the fs is already using the
>      target csum type
> 
>   tune/change-csum.c | 461 ++++++++++++++++++++++++++++++++++++++++-----
>   1 file changed, 418 insertions(+), 43 deletions(-)
> 

Re: [PATCH 0/7] btrfs-progs: tune: add resume support for csum conversion

[Qu Wenruo]

On 2023/5/24 16:42, Anand Jain wrote:
>
>
> Why not use the generation numbers "From" and "To" for converting
> to the new checksum and keeping track of the generation number of
> the most recent successful conversion?

Firstly, generation only makes sense for COW based operations (data csum
modification part), thus it doesn't work for metadata csum rewrite,
which happen in place.

Secondly, there is only one major generation number, and how do you
define "from" and "to"?

>
> Thanks, Anand
>
>
> On 24/05/2023 15:41, Qu Wenruo wrote:
>> [RESUME SUPPORT]
>> This patchset adds the resume support for "btrfstune --csum".
>>
>> The main part is in resume for data csum change, as we have 4 different
>> status during data csum conversion.
>>
>> Thankfully for data csum conversion, everything is protected by
>> metadata COW and we can detect the current status by the existence of
>> both old and new csum items, and their coverage.
>>
>> For resume of metadata conversion, there is nothing we can really do but
>> go through all the tree blocks and verify them against both new and old
>> csum type.
>>
>> [TESTING]
>> For the tests, currently errors are injected after every possible
>> transaction commit, and then resume from that interrupted status.
>> So far the patchset passes all above tests.
>>
>> But I'm not sure what's the better way to craft the test case.
>>
>> Should we go log-writes? Or use pre-built images?
>>
>> [TODO]
>> - Test cases for resume
>>
>> - Support for revert if errors are found
>>    If we hit data csum mismatch and can not repair from any copy, then
>>    we should revert back to the old csum.
>>
>> - Support for pre-cautious metadata check
>>    We'd better read and very metadata before rewriting them.
>>
>> - Performance tuning
>>    We want to take a balance between too frequent commit transaction
>>    and too large transaction.
>>    This is especially true for data csum objectid change and maybe
>>    old data csum deletion.
>>
>> - UI enhancement
>>    A btrfs-convert like UI would be better.
>>
>>
>> Qu Wenruo (7):
>>    btrfs-progs: tune: implement resume support for metadata checksum
>>    btrfs-progs: tune: implement resume support for generating new data
>>      csum
>>    btrfs-progs: tune: implement resume support for csum tree without any
>>      new csum item
>>    btrfs-progs: tune: implement resume support for empty csum tree
>>    btrfs-progs: tune: implement resume support for half deleted old csums
>>    btrfs-progs: tune: implement resume support for data csum objectid
>>      change
>>    btrfs-progs: tune: reject csum change if the fs is already using the
>>      target csum type
>>
>>   tune/change-csum.c | 461 ++++++++++++++++++++++++++++++++++++++++-----
>>   1 file changed, 418 insertions(+), 43 deletions(-)
>>
>

Re: [PATCH 0/7] btrfs-progs: tune: add resume support for csum conversion

[David Sterba]

On Wed, May 24, 2023 at 03:41:23PM +0800, Qu Wenruo wrote:
> [RESUME SUPPORT]
> This patchset adds the resume support for "btrfstune --csum".

Great, thanks.

> The main part is in resume for data csum change, as we have 4 different
> status during data csum conversion.
> 
> Thankfully for data csum conversion, everything is protected by
> metadata COW and we can detect the current status by the existence of
> both old and new csum items, and their coverage.
> 
> For resume of metadata conversion, there is nothing we can really do but
> go through all the tree blocks and verify them against both new and old
> csum type.
> 
> [TESTING]
> For the tests, currently errors are injected after every possible
> transaction commit, and then resume from that interrupted status.
> So far the patchset passes all above tests.

So you've inserted some "if/return EINVAL", right?

> But I'm not sure what's the better way to craft the test case.
> 
> Should we go log-writes? Or use pre-built images?

Log writes maybe, the device mapper targets are already used in the test
suite. You could use your testing injection code to generate them.

But it's clear that we could generate them only once, or we'd have to
store the image generators (possible).

Alternatively, can we create some error injection framework? In the
simplest form, define injection points in the code watching for some
conndition and trigger them from outside eg. set by an environment
variable. Maybe there's something better.

> [TODO]
> - Test cases for resume
> 
> - Support for revert if errors are found
>   If we hit data csum mismatch and can not repair from any copy, then
>   we should revert back to the old csum.
> 
> - Support for pre-cautious metadata check
>   We'd better read and very metadata before rewriting them.

Read and verify? Agreed.

> - Performance tuning
>   We want to take a balance between too frequent commit transaction
>   and too large transaction.
>   This is especially true for data csum objectid change and maybe
>   old data csum deletion.
> 
> - UI enhancement
>   A btrfs-convert like UI would be better.

Yeah it's growing beyond what one simple option can handle. The separate
command group for btrfstune is in the work, so far still on the code
level, the subcommands are roughly matching the conversion features but
not yet finalized.

Re: [PATCH 0/7] btrfs-progs: tune: add resume support for csum conversion

[Qu Wenruo]

On 2023/5/24 23:55, David Sterba wrote:
> On Wed, May 24, 2023 at 03:41:23PM +0800, Qu Wenruo wrote:
>> [RESUME SUPPORT]
>> This patchset adds the resume support for "btrfstune --csum".
>
> Great, thanks.
>
>> The main part is in resume for data csum change, as we have 4 different
>> status during data csum conversion.
>>
>> Thankfully for data csum conversion, everything is protected by
>> metadata COW and we can detect the current status by the existence of
>> both old and new csum items, and their coverage.
>>
>> For resume of metadata conversion, there is nothing we can really do but
>> go through all the tree blocks and verify them against both new and old
>> csum type.
>>
>> [TESTING]
>> For the tests, currently errors are injected after every possible
>> transaction commit, and then resume from that interrupted status.
>> So far the patchset passes all above tests.
>
> So you've inserted some "if/return EINVAL", right?

Yes for my local tests.

>
>> But I'm not sure what's the better way to craft the test case.
>>
>> Should we go log-writes? Or use pre-built images?
>
> Log writes maybe, the device mapper targets are already used in the test
> suite. You could use your testing injection code to generate them.

Log writes allows us to resume at each FUA/FLUSH, thus doesn't need the
error injection part.

It has the best coverage for data csum part, but not for metadata
rewrites, as it doesn't generate any FUA/FLUSH.

>
> But it's clear that we could generate them only once, or we'd have to
> store the image generators (possible).

Pre-built images are much better and controllable for us to test
metadata resume and other corner cases, but less coverage overall.

>
> Alternatively, can we create some error injection framework? In the
> simplest form, define injection points in the code watching for some
> conndition and trigger them from outside eg. set by an environment
> variable. Maybe there's something better.

For injection framework, we at least need some way to trigger them by
some possibility.
Which can already be a little too complex.

>
>> [TODO]
>> - Test cases for resume
>>
>> - Support for revert if errors are found
>>    If we hit data csum mismatch and can not repair from any copy, then
>>    we should revert back to the old csum.
>>
>> - Support for pre-cautious metadata check
>>    We'd better read and very metadata before rewriting them.
>
> Read and verify? Agreed.

It's already read-and-verify during metadata rewrites, the pre-cautious
part is reading the whole metadata, before doing metadata rewrites.

But the best practice is "btrfs check --readonly" before calling
"btrfstune --csum".

Thanks,
Qu


>
>> - Performance tuning
>>    We want to take a balance between too frequent commit transaction
>>    and too large transaction.
>>    This is especially true for data csum objectid change and maybe
>>    old data csum deletion.
>>
>> - UI enhancement
>>    A btrfs-convert like UI would be better.
>
> Yeah it's growing beyond what one simple option can handle. The separate
> command group for btrfstune is in the work, so far still on the code
> level, the subcommands are roughly matching the conversion features but
> not yet finalized.

Re: [PATCH 0/7] btrfs-progs: tune: add resume support for csum conversion

[David Sterba]

On Thu, May 25, 2023 at 06:24:42AM +0800, Qu Wenruo wrote:
> > Alternatively, can we create some error injection framework? In the
> > simplest form, define injection points in the code watching for some
> > conndition and trigger them from outside eg. set by an environment
> > variable. Maybe there's something better.
> 
> For injection framework, we at least need some way to trigger them by
> some possibility.
> Which can already be a little too complex.

I've implemented the environment variable approach, it's in devel and
example injection points are in the checksum switch. It's really simple
and I'm not sure I like it but at least we have a starting point.