[1/2] btrfs-progs: tune: allow --csum to rebuild csum tree

Commit Message

Qu Wenruo Aug. 22, 2023, 7:15 a.m. UTC

[PROBLEM]
Currently "btrfs check --init-csum-tree" goes a very tricky way to
rebuild csum tree, by wiping the csum tree root and let the extent tree
repair code to handle the remaining part of the old csum tree.

We already have a report that such method has caused data corruption on
a seemingly fine btrfs (the report ran "btrfs check --readonly" first,
which has no error reported).

[ENHANCEMENT]
On the other hand, we have a new experimental feature, "btrfstune
--csum", which would go a much safer way to convert data csum, by
inserting new data csum items using the new csum type, then replace the
old csum items with the new ones.

This is way safer, and we have even tested the interrupted cases of such
conversion, and if the fs is fine in the first place, we won't rely on
extent tree repair code at all.

This patch would:

- Fix the spelling of check_csum_change_requirement()

- Remove the csum type check in check_csum_change_requirement()

- Do not verify data csum if we're rebuilding using the same csum type
  As we may want to rebuilt the csum tree to fix the csum mismatch.

  This would require a new flag, VERIFY_SKIP_CSUM for
  read_verify_one_data_sector().

- Skip metadata csum rewrite completely if csum type matches

Signed-off-by: Qu Wenruo <[e-mail]>
---
 tune/change-csum.c | 35 +++++++++++++++++++++++++----------
 tune/main.c        |  3 ++-
 2 files changed, 27 insertions(+), 11 deletions(-)

Patch

diff --git a/tune/change-csum.c b/tune/change-csum.c
index e7bc22526fd1..b5e70c9e4f80 100644
--- a/tune/change-csum.c
+++ b/tune/change-csum.c
@@ -76,12 +76,6 @@  static int check_csum_change_requreiment(struct btrfs_fs_info *fs_info, u16 new_
 		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;
 }
 
@@ -120,13 +114,18 @@  static int get_last_csum_bytenr(struct btrfs_fs_info *fs_info, u64 *result)
 	return 0;
 }
 
+#define VERIFY_OUTPUT_ERROR	(1 << 0)
+#define VERIFY_SKIP_CSUM	(1 << 1)
+
 static int read_verify_one_data_sector(struct btrfs_fs_info *fs_info,
 				       u64 logical, void *data_buf,
 				       const void *old_csums, u16 old_csum_type,
-				       bool output_error)
+				       unsigned int flags)
 {
 	const u32 sectorsize = fs_info->sectorsize;
 	int num_copies = btrfs_num_copies(fs_info, logical, sectorsize);
+	const bool output_error = flags & VERIFY_OUTPUT_ERROR;
+	const bool skip_csum = flags & VERIFY_SKIP_CSUM;
 	bool found_good = false;
 
 	for (int mirror = 1; mirror <= num_copies; mirror++) {
@@ -141,6 +140,11 @@  static int read_verify_one_data_sector(struct btrfs_fs_info *fs_info,
 			error("failed to read logical %llu: %m", logical);
 			continue;
 		}
+		if (skip_csum) {
+			found_good = true;
+			break;
+		}
+
 		btrfs_csum_data(fs_info, fs_info->csum_type, data_buf, csum_has,
 				sectorsize);
 		if (memcmp(csum_has, old_csums, fs_info->csum_size) == 0) {
@@ -169,16 +173,20 @@  static int generate_new_csum_range(struct btrfs_trans_handle *trans,
 	const u32 sectorsize = fs_info->sectorsize;
 	int ret = 0;
 	void *buf;
+	unsigned int flags = VERIFY_OUTPUT_ERROR;
 
 	buf = malloc(fs_info->sectorsize);
 	if (!buf)
 		return -ENOMEM;
 
+	/* We're rebuilding the csum tree, no need to verify the old data csum. */
+	if (fs_info->csum_type == new_csum_type)
+		flags |= VERIFY_SKIP_CSUM;
+
 	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,
-				fs_info->csum_type, true);
-
+				fs_info->csum_type, flags);
 		if (ret < 0) {
 			error("failed to recover a good copy for data at logical %llu",
 			      logical);
@@ -565,6 +573,13 @@  static int change_meta_csums(struct btrfs_fs_info *fs_info, u16 new_csum_type)
 		error("failed to update super flags: %m");
 	}
 
+	/*
+	 * The new csum type matches the old one, it's to rebuild data csum.
+	 * No need to change metadata csum.
+	 */
+	if (fs_info->csum_type == new_csum_type)
+		goto out;
+
 	/*
 	 * Disable metadata csum checks first, as we may hit tree blocks with
 	 * either old or new csums.
@@ -811,7 +826,7 @@  static int determine_csum_type(struct btrfs_fs_info *fs_info, u64 logical,
 	if (!buf)
 		return -ENOMEM;
 	ret = read_verify_one_data_sector(fs_info, logical, buf, csum_expected,
-					  csum_type, false);
+					  csum_type, 0);
 	if (ret < 0)
 		ret = 1;
 	free(buf);
diff --git a/tune/main.c b/tune/main.c
index 73d09c34a897..b611d3171918 100644
--- a/tune/main.c
+++ b/tune/main.c
@@ -121,7 +121,8 @@  static const char * const tune_usage[] = {
 #if EXPERIMENTAL
 	"",
 	"EXPERIMENTAL FEATURES:",
-	OPTLINE("--csum CSUM", "switch checksum for data and metadata to CSUM"),
+	OPTLINE("--csum CSUM", "switch checksum for data and metadata to CSUM, "
+			"would only rebuild csum tree if CSUM is the same as the existing one"),
 #endif
 	NULL
 };