[PATCH 0/4] btrfs: rescue= mount options enhancement to support interrupted csum conversion

[PATCH 0/4] btrfs: rescue= mount options enhancement to support interrupted csum conversion

[Qu Wenruo]

[REPO]
https://github.com/adam900710/linux/tree/rescue_changes

[BACKGROUND]
There is an adventurous user using btrfstune to convert a 32T btrfs
from crc32c to xxhash.

However for such huge fs, it takes too long time and the reporter
canceled the conversion.

This makes the reporter unable to mount the fs at all.

[CAUSE]
First of all, for a half converted fs, we will never allow RW mount, so
everything must be done in rescue mode.

There are several different stages of csum conversion, and at different
stage it requires different handling from kernel:

- Generationg new data csums
  At this stage only the super flags (CHANGING_DATA_CSUM flag) is
  preventing the kernel from mounting.

  Intrdoce "rescue=ignoresuperflags" to address this.

- Deleting old data cums
  The same super flags problem, with possible missing data csums.

  Despite the new "rescue=ignoresuperflags", end users will also need
  the existing "rescue=ignoredatacsums" mount option.

- Renaming the objectid of new data cums
  The new csums' objectid will be changed to the regular one.

  During this we can hit data csum mismatch.
  So the same "rescue=ignoresuperflags:ignoredatacsums" can handle it
  already.

- Rewriting metadata csums
  This part is done in-place (no COW), with a new super flags
  (CHANGING_META_CSUM).

  So here introduce a new "rescue=ignoremetacsums" to ignore the
  metadata checksum verification (and rely on the remaining sanity
  checks like tree-checkers).

The first 2 patches are just small cleanups, meanwhile the last two are
the new "rescue=" mount options to handle interrupted csum change.

Qu Wenruo (4):
  btrfs: remove unused Opt enums
  btrfs: output the unrecognized super flags as hex
  btrfs: introduce new "rescue=ignoremetacsums" mount option
  btrfs: introduce new "rescue=ignoresuperflags" mount option

 fs/btrfs/bio.c       |  2 +-
 fs/btrfs/disk-io.c   | 22 ++++++++++++++--------
 fs/btrfs/file-item.c |  2 +-
 fs/btrfs/fs.h        |  4 +++-
 fs/btrfs/messages.c  |  2 +-
 fs/btrfs/super.c     | 27 +++++++++++++++++++++++----
 fs/btrfs/zoned.c     |  2 +-
 7 files changed, 44 insertions(+), 17 deletions(-)

-- 
2.45.2

[PATCH 1/4] btrfs: remove unused Opt enums

[Qu Wenruo]

The following three Opt_* enums are not utilized at all:

- Opt_ignorebadroots
- Opt_ignoredatacsums
- Opt_rescue_all

All those handling are inside "rescue=" mount option groups, and there
is no corresponding token for them, so we can safely remove them.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 fs/btrfs/super.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 549ad700e49e..902423f2839c 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -125,9 +125,6 @@ enum {
 	Opt_rescue,
 	Opt_usebackuproot,
 	Opt_nologreplay,
-	Opt_ignorebadroots,
-	Opt_ignoredatacsums,
-	Opt_rescue_all,
 
 	/* Debugging options */
 	Opt_enospc_debug,
-- 
2.45.2

[PATCH 2/4] btrfs: output the unrecognized super flags as hex

[Qu Wenruo]

Most of the extra suepr flags are beyond 32bits (from CHANGING_FSID_V2
to CHANGING_*_CSUMS), thus using %llu is not only too long and pretty
hard to read.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 fs/btrfs/disk-io.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index ffc9129e23d2..78a11f9357ed 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -2351,7 +2351,7 @@ int btrfs_validate_super(const struct btrfs_fs_info *fs_info,
 		ret = -EINVAL;
 	}
 	if (btrfs_super_flags(sb) & ~BTRFS_SUPER_FLAG_SUPP) {
-		btrfs_err(fs_info, "unrecognized or unsupported super flag: %llu",
+		btrfs_err(fs_info, "unrecognized or unsupported super flag: 0x%llx",
 				btrfs_super_flags(sb) & ~BTRFS_SUPER_FLAG_SUPP);
 		ret = -EINVAL;
 	}
-- 
2.45.2

[PATCH 3/4] btrfs: introduce new "rescue=ignoremetacsums" mount option

[Qu Wenruo]

This patch introduces "rescue=ignoremetacsums" to ignore metadata csums,
meanwhile all the other metadata sanity checks are still kept as is.

This new mount option is mostly to allow the kernel to mount an
interrupted checksum conversion (at the metadata csum overwrite stage).

And since the main part of metadata sanity checks is inside
tree-checker, we shouldn't lose much safety, and the new mount option is
rescue mount option it requires full read-only mount.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 fs/btrfs/bio.c       |  2 +-
 fs/btrfs/disk-io.c   | 16 ++++++++++------
 fs/btrfs/file-item.c |  2 +-
 fs/btrfs/fs.h        |  3 ++-
 fs/btrfs/messages.c  |  2 +-
 fs/btrfs/super.c     | 13 ++++++++++++-
 fs/btrfs/zoned.c     |  2 +-
 7 files changed, 28 insertions(+), 12 deletions(-)

diff --git a/fs/btrfs/bio.c b/fs/btrfs/bio.c
index 477f350a8bd0..07bc19aaa864 100644
--- a/fs/btrfs/bio.c
+++ b/fs/btrfs/bio.c
@@ -732,7 +732,7 @@ static bool btrfs_submit_chunk(struct btrfs_bio *bbio, int mirror_num)
 		 * point, so they are handled as part of the no-checksum case.
 		 */
 		if (inode && !(inode->flags & BTRFS_INODE_NODATASUM) &&
-		    !test_bit(BTRFS_FS_STATE_NO_CSUMS, &fs_info->fs_state) &&
+		    !test_bit(BTRFS_FS_STATE_NO_DATA_CSUMS, &fs_info->fs_state) &&
 		    !btrfs_is_data_reloc_root(inode->root)) {
 			if (should_async_write(bbio) &&
 			    btrfs_wq_submit_bio(bbio, bioc, &smap, mirror_num))
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 78a11f9357ed..74ee9f335dbc 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -367,6 +367,7 @@ int btrfs_validate_extent_buffer(struct extent_buffer *eb,
 	u8 result[BTRFS_CSUM_SIZE];
 	const u8 *header_csum;
 	int ret = 0;
+	bool ignore_csum = btrfs_test_opt(fs_info, IGNOREMETACSUMS);
 
 	ASSERT(check);
 
@@ -399,13 +400,16 @@ int btrfs_validate_extent_buffer(struct extent_buffer *eb,
 
 	if (memcmp(result, header_csum, csum_size) != 0) {
 		btrfs_warn_rl(fs_info,
-"checksum verify failed on logical %llu mirror %u wanted " CSUM_FMT " found " CSUM_FMT " level %d",
+"checksum verify failed on logical %llu mirror %u wanted " CSUM_FMT " found " CSUM_FMT " level %d%s",
 			      eb->start, eb->read_mirror,
 			      CSUM_FMT_VALUE(csum_size, header_csum),
 			      CSUM_FMT_VALUE(csum_size, result),
-			      btrfs_header_level(eb));
-		ret = -EUCLEAN;
-		goto out;
+			      btrfs_header_level(eb),
+			      ignore_csum ? ", ignoring" : "");
+		if (!ignore_csum) {
+			ret = -EUCLEAN;
+			goto out;
+		}
 	}
 
 	if (found_level != check->level) {
@@ -2135,7 +2139,7 @@ static int load_global_roots_objectid(struct btrfs_root *tree_root,
 	/* If we have IGNOREDATACSUMS skip loading these roots. */
 	if (objectid == BTRFS_CSUM_TREE_OBJECTID &&
 	    btrfs_test_opt(fs_info, IGNOREDATACSUMS)) {
-		set_bit(BTRFS_FS_STATE_NO_CSUMS, &fs_info->fs_state);
+		set_bit(BTRFS_FS_STATE_NO_DATA_CSUMS, &fs_info->fs_state);
 		return 0;
 	}
 
@@ -2188,7 +2192,7 @@ static int load_global_roots_objectid(struct btrfs_root *tree_root,
 
 	if (!found || ret) {
 		if (objectid == BTRFS_CSUM_TREE_OBJECTID)
-			set_bit(BTRFS_FS_STATE_NO_CSUMS, &fs_info->fs_state);
+			set_bit(BTRFS_FS_STATE_NO_DATA_CSUMS, &fs_info->fs_state);
 
 		if (!btrfs_test_opt(fs_info, IGNOREBADROOTS))
 			ret = ret ? ret : -ENOENT;
diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
index 55703c833f3d..6c7713fb9a75 100644
--- a/fs/btrfs/file-item.c
+++ b/fs/btrfs/file-item.c
@@ -353,7 +353,7 @@ blk_status_t btrfs_lookup_bio_sums(struct btrfs_bio *bbio)
 	u32 bio_offset = 0;
 
 	if ((inode->flags & BTRFS_INODE_NODATASUM) ||
-	    test_bit(BTRFS_FS_STATE_NO_CSUMS, &fs_info->fs_state))
+	    test_bit(BTRFS_FS_STATE_NO_DATA_CSUMS, &fs_info->fs_state))
 		return BLK_STS_OK;
 
 	/*
diff --git a/fs/btrfs/fs.h b/fs/btrfs/fs.h
index 18e0d3539496..fcef7b81f9fd 100644
--- a/fs/btrfs/fs.h
+++ b/fs/btrfs/fs.h
@@ -98,7 +98,7 @@ enum {
 	/* The btrfs_fs_info created for self-tests */
 	BTRFS_FS_STATE_DUMMY_FS_INFO,
 
-	BTRFS_FS_STATE_NO_CSUMS,
+	BTRFS_FS_STATE_NO_DATA_CSUMS,
 
 	/* Indicates there was an error cleaning up a log tree. */
 	BTRFS_FS_STATE_LOG_CLEANUP_ERROR,
@@ -224,6 +224,7 @@ enum {
 	BTRFS_MOUNT_IGNOREDATACSUMS		= (1UL << 28),
 	BTRFS_MOUNT_NODISCARD			= (1UL << 29),
 	BTRFS_MOUNT_NOSPACECACHE		= (1UL << 30),
+	BTRFS_MOUNT_IGNOREMETACSUMS		= (1UL << 31),
 };
 
 /*
diff --git a/fs/btrfs/messages.c b/fs/btrfs/messages.c
index 210d9c82e2ae..38a857aba446 100644
--- a/fs/btrfs/messages.c
+++ b/fs/btrfs/messages.c
@@ -20,7 +20,7 @@ static const char fs_state_chars[] = {
 	[BTRFS_FS_STATE_TRANS_ABORTED]		= 'A',
 	[BTRFS_FS_STATE_DEV_REPLACING]		= 'R',
 	[BTRFS_FS_STATE_DUMMY_FS_INFO]		= 0,
-	[BTRFS_FS_STATE_NO_CSUMS]		= 'C',
+	[BTRFS_FS_STATE_NO_DATA_CSUMS]		= 'C',
 	[BTRFS_FS_STATE_LOG_CLEANUP_ERROR]	= 'L',
 };
 
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 902423f2839c..386500b9b440 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -175,6 +175,7 @@ enum {
 	Opt_rescue_nologreplay,
 	Opt_rescue_ignorebadroots,
 	Opt_rescue_ignoredatacsums,
+	Opt_rescue_ignoremetacsums,
 	Opt_rescue_parameter_all,
 };
 
@@ -184,7 +185,9 @@ static const struct constant_table btrfs_parameter_rescue[] = {
 	{ "ignorebadroots", Opt_rescue_ignorebadroots },
 	{ "ibadroots", Opt_rescue_ignorebadroots },
 	{ "ignoredatacsums", Opt_rescue_ignoredatacsums },
+	{ "ignoremetacsums", Opt_rescue_ignoremetacsums},
 	{ "idatacsums", Opt_rescue_ignoredatacsums },
+	{ "imetacsums", Opt_rescue_ignoremetacsums},
 	{ "all", Opt_rescue_parameter_all },
 	{}
 };
@@ -570,8 +573,12 @@ static int btrfs_parse_param(struct fs_context *fc, struct fs_parameter *param)
 		case Opt_rescue_ignoredatacsums:
 			btrfs_set_opt(ctx->mount_opt, IGNOREDATACSUMS);
 			break;
+		case Opt_rescue_ignoremetacsums:
+			btrfs_set_opt(ctx->mount_opt, IGNOREMETACSUMS);
+			break;
 		case Opt_rescue_parameter_all:
 			btrfs_set_opt(ctx->mount_opt, IGNOREDATACSUMS);
+			btrfs_set_opt(ctx->mount_opt, IGNOREMETACSUMS);
 			btrfs_set_opt(ctx->mount_opt, IGNOREBADROOTS);
 			btrfs_set_opt(ctx->mount_opt, NOLOGREPLAY);
 			break;
@@ -646,7 +653,8 @@ bool btrfs_check_options(const struct btrfs_fs_info *info, unsigned long *mount_
 	if (!(flags & SB_RDONLY) &&
 	    (check_ro_option(info, *mount_opt, BTRFS_MOUNT_NOLOGREPLAY, "nologreplay") ||
 	     check_ro_option(info, *mount_opt, BTRFS_MOUNT_IGNOREBADROOTS, "ignorebadroots") ||
-	     check_ro_option(info, *mount_opt, BTRFS_MOUNT_IGNOREDATACSUMS, "ignoredatacsums")))
+	     check_ro_option(info, *mount_opt, BTRFS_MOUNT_IGNOREDATACSUMS, "ignoredatacsums") ||
+	     check_ro_option(info, *mount_opt, BTRFS_MOUNT_IGNOREMETACSUMS, "ignoremetacsums")))
 		ret = false;
 
 	if (btrfs_fs_compat_ro(info, FREE_SPACE_TREE) &&
@@ -1062,6 +1070,8 @@ static int btrfs_show_options(struct seq_file *seq, struct dentry *dentry)
 		print_rescue_option(seq, "ignorebadroots", &printed);
 	if (btrfs_test_opt(info, IGNOREDATACSUMS))
 		print_rescue_option(seq, "ignoredatacsums", &printed);
+	if (btrfs_test_opt(info, IGNOREMETACSUMS))
+		print_rescue_option(seq, "ignoremetacsums", &printed);
 	if (btrfs_test_opt(info, FLUSHONCOMMIT))
 		seq_puts(seq, ",flushoncommit");
 	if (btrfs_test_opt(info, DISCARD_SYNC))
@@ -1419,6 +1429,7 @@ static void btrfs_emit_options(struct btrfs_fs_info *info,
 	btrfs_info_if_set(info, old, USEBACKUPROOT, "trying to use backup root at mount time");
 	btrfs_info_if_set(info, old, IGNOREBADROOTS, "ignoring bad roots");
 	btrfs_info_if_set(info, old, IGNOREDATACSUMS, "ignoring data csums");
+	btrfs_info_if_set(info, old, IGNOREMETACSUMS, "ignoring meta csums");
 
 	btrfs_info_if_unset(info, old, NODATACOW, "setting datacow");
 	btrfs_info_if_unset(info, old, SSD, "not using ssd optimizations");
diff --git a/fs/btrfs/zoned.c b/fs/btrfs/zoned.c
index 992a5b7756ca..386daea0ca0b 100644
--- a/fs/btrfs/zoned.c
+++ b/fs/btrfs/zoned.c
@@ -1844,7 +1844,7 @@ void btrfs_finish_ordered_zoned(struct btrfs_ordered_extent *ordered)
 	 * here so that we don't attempt to log the csums later.
 	 */
 	if ((inode->flags & BTRFS_INODE_NODATASUM) ||
-	    test_bit(BTRFS_FS_STATE_NO_CSUMS, &fs_info->fs_state)) {
+	    test_bit(BTRFS_FS_STATE_NO_DATA_CSUMS, &fs_info->fs_state)) {
 		while ((sum = list_first_entry_or_null(&ordered->list,
 						       typeof(*sum), list))) {
 			list_del(&sum->list);
-- 
2.45.2

[PATCH 4/4] btrfs: introduce new "rescue=ignoresuperflags" mount option

[Qu Wenruo]

This new mount option would allow the kernel to skip the super flags
check, it's mostly to allow the kernel to do a rescue mount of an
interrupted checksum conversion.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 fs/btrfs/disk-io.c |  4 +++-
 fs/btrfs/fs.h      |  1 +
 fs/btrfs/super.c   | 13 ++++++++++++-
 3 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 74ee9f335dbc..0df747646c2f 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -2349,12 +2349,14 @@ int btrfs_validate_super(const struct btrfs_fs_info *fs_info,
 	u64 nodesize = btrfs_super_nodesize(sb);
 	u64 sectorsize = btrfs_super_sectorsize(sb);
 	int ret = 0;
+	bool ignore_flags = btrfs_raw_test_opt(fs_info->mount_opt,
+						IGNORESUPERFLAGS);
 
 	if (btrfs_super_magic(sb) != BTRFS_MAGIC) {
 		btrfs_err(fs_info, "no valid FS found");
 		ret = -EINVAL;
 	}
-	if (btrfs_super_flags(sb) & ~BTRFS_SUPER_FLAG_SUPP) {
+	if (!ignore_flags && (btrfs_super_flags(sb) & ~BTRFS_SUPER_FLAG_SUPP)) {
 		btrfs_err(fs_info, "unrecognized or unsupported super flag: 0x%llx",
 				btrfs_super_flags(sb) & ~BTRFS_SUPER_FLAG_SUPP);
 		ret = -EINVAL;
diff --git a/fs/btrfs/fs.h b/fs/btrfs/fs.h
index fcef7b81f9fd..8b14df6ff96c 100644
--- a/fs/btrfs/fs.h
+++ b/fs/btrfs/fs.h
@@ -225,6 +225,7 @@ enum {
 	BTRFS_MOUNT_NODISCARD			= (1UL << 29),
 	BTRFS_MOUNT_NOSPACECACHE		= (1UL << 30),
 	BTRFS_MOUNT_IGNOREMETACSUMS		= (1UL << 31),
+	BTRFS_MOUNT_IGNORESUPERFLAGS		= (1UL << 32),
 };
 
 /*
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 386500b9b440..a66b5c901510 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -176,6 +176,7 @@ enum {
 	Opt_rescue_ignorebadroots,
 	Opt_rescue_ignoredatacsums,
 	Opt_rescue_ignoremetacsums,
+	Opt_rescue_ignoresuperflags,
 	Opt_rescue_parameter_all,
 };
 
@@ -186,8 +187,10 @@ static const struct constant_table btrfs_parameter_rescue[] = {
 	{ "ibadroots", Opt_rescue_ignorebadroots },
 	{ "ignoredatacsums", Opt_rescue_ignoredatacsums },
 	{ "ignoremetacsums", Opt_rescue_ignoremetacsums},
+	{ "ignoresuperflags", Opt_rescue_ignoresuperflags},
 	{ "idatacsums", Opt_rescue_ignoredatacsums },
 	{ "imetacsums", Opt_rescue_ignoremetacsums},
+	{ "isuperflags", Opt_rescue_ignoresuperflags},
 	{ "all", Opt_rescue_parameter_all },
 	{}
 };
@@ -576,9 +579,13 @@ static int btrfs_parse_param(struct fs_context *fc, struct fs_parameter *param)
 		case Opt_rescue_ignoremetacsums:
 			btrfs_set_opt(ctx->mount_opt, IGNOREMETACSUMS);
 			break;
+		case Opt_rescue_ignoresuperflags:
+			btrfs_set_opt(ctx->mount_opt, IGNORESUPERFLAGS);
+			break;
 		case Opt_rescue_parameter_all:
 			btrfs_set_opt(ctx->mount_opt, IGNOREDATACSUMS);
 			btrfs_set_opt(ctx->mount_opt, IGNOREMETACSUMS);
+			btrfs_set_opt(ctx->mount_opt, IGNORESUPERFLAGS);
 			btrfs_set_opt(ctx->mount_opt, IGNOREBADROOTS);
 			btrfs_set_opt(ctx->mount_opt, NOLOGREPLAY);
 			break;
@@ -654,7 +661,8 @@ bool btrfs_check_options(const struct btrfs_fs_info *info, unsigned long *mount_
 	    (check_ro_option(info, *mount_opt, BTRFS_MOUNT_NOLOGREPLAY, "nologreplay") ||
 	     check_ro_option(info, *mount_opt, BTRFS_MOUNT_IGNOREBADROOTS, "ignorebadroots") ||
 	     check_ro_option(info, *mount_opt, BTRFS_MOUNT_IGNOREDATACSUMS, "ignoredatacsums") ||
-	     check_ro_option(info, *mount_opt, BTRFS_MOUNT_IGNOREMETACSUMS, "ignoremetacsums")))
+	     check_ro_option(info, *mount_opt, BTRFS_MOUNT_IGNOREMETACSUMS, "ignoremetacsums") ||
+	     check_ro_option(info, *mount_opt, BTRFS_MOUNT_IGNORESUPERFLAGS, "ignoresuperflags")))
 		ret = false;
 
 	if (btrfs_fs_compat_ro(info, FREE_SPACE_TREE) &&
@@ -1072,6 +1080,8 @@ static int btrfs_show_options(struct seq_file *seq, struct dentry *dentry)
 		print_rescue_option(seq, "ignoredatacsums", &printed);
 	if (btrfs_test_opt(info, IGNOREMETACSUMS))
 		print_rescue_option(seq, "ignoremetacsums", &printed);
+	if (btrfs_test_opt(info, IGNORESUPERFLAGS))
+		print_rescue_option(seq, "ignoresuperflags", &printed);
 	if (btrfs_test_opt(info, FLUSHONCOMMIT))
 		seq_puts(seq, ",flushoncommit");
 	if (btrfs_test_opt(info, DISCARD_SYNC))
@@ -1430,6 +1440,7 @@ static void btrfs_emit_options(struct btrfs_fs_info *info,
 	btrfs_info_if_set(info, old, IGNOREBADROOTS, "ignoring bad roots");
 	btrfs_info_if_set(info, old, IGNOREDATACSUMS, "ignoring data csums");
 	btrfs_info_if_set(info, old, IGNOREMETACSUMS, "ignoring meta csums");
+	btrfs_info_if_set(info, old, IGNORESUPERFLAGS, "ignoring unknown super flags");
 
 	btrfs_info_if_unset(info, old, NODATACOW, "setting datacow");
 	btrfs_info_if_unset(info, old, SSD, "not using ssd optimizations");
-- 
2.45.2

Re: [PATCH 0/4] btrfs: rescue= mount options enhancement to support interrupted csum conversion

[Josef Bacik]

On Tue, Jun 11, 2024 at 02:51:34PM +0930, Qu Wenruo wrote:
> [REPO]
> https://github.com/adam900710/linux/tree/rescue_changes
> 
> [BACKGROUND]
> There is an adventurous user using btrfstune to convert a 32T btrfs
> from crc32c to xxhash.
> 
> However for such huge fs, it takes too long time and the reporter
> canceled the conversion.
> 
> This makes the reporter unable to mount the fs at all.
> 
> [CAUSE]
> First of all, for a half converted fs, we will never allow RW mount, so
> everything must be done in rescue mode.
> 
> There are several different stages of csum conversion, and at different
> stage it requires different handling from kernel:
> 
> - Generationg new data csums
>   At this stage only the super flags (CHANGING_DATA_CSUM flag) is
>   preventing the kernel from mounting.
> 
>   Intrdoce "rescue=ignoresuperflags" to address this.
> 
> - Deleting old data cums
>   The same super flags problem, with possible missing data csums.
> 
>   Despite the new "rescue=ignoresuperflags", end users will also need
>   the existing "rescue=ignoredatacsums" mount option.
> 
> - Renaming the objectid of new data cums
>   The new csums' objectid will be changed to the regular one.
> 
>   During this we can hit data csum mismatch.
>   So the same "rescue=ignoresuperflags:ignoredatacsums" can handle it
>   already.
> 
> - Rewriting metadata csums
>   This part is done in-place (no COW), with a new super flags
>   (CHANGING_META_CSUM).
> 
>   So here introduce a new "rescue=ignoremetacsums" to ignore the
>   metadata checksum verification (and rely on the remaining sanity
>   checks like tree-checkers).
> 
> The first 2 patches are just small cleanups, meanwhile the last two are
> the new "rescue=" mount options to handle interrupted csum change.
> 
> Qu Wenruo (4):
>   btrfs: remove unused Opt enums
>   btrfs: output the unrecognized super flags as hex
>   btrfs: introduce new "rescue=ignoremetacsums" mount option
>   btrfs: introduce new "rescue=ignoresuperflags" mount option
> 

Reviewed-by: Josef Bacik <josef@toxicpanda.com>

Could you update the docs in btrfs-progs/wherever we're keeping the readthedocs
stuff so that these new flags are documented?  Thanks,

Josef

Re: [PATCH 1/4] btrfs: remove unused Opt enums

[David Sterba]

On Tue, Jun 11, 2024 at 02:51:35PM +0930, Qu Wenruo wrote:
> The following three Opt_* enums are not utilized at all:
> 
> - Opt_ignorebadroots
> - Opt_ignoredatacsums
> - Opt_rescue_all

They've been duplicated in the new mount API rewrite and the currently
used enums are in Opt_rescue_* group, like Opt_rescue_parameter_all.

> All those handling are inside "rescue=" mount option groups, and there
> is no corresponding token for them, so we can safely remove them.

Yes.

Re: [PATCH 3/4] btrfs: introduce new "rescue=ignoremetacsums" mount option

[David Sterba]

On Tue, Jun 11, 2024 at 02:51:37PM +0930, Qu Wenruo wrote:
> This patch introduces "rescue=ignoremetacsums" to ignore metadata csums,
> meanwhile all the other metadata sanity checks are still kept as is.
> 
> This new mount option is mostly to allow the kernel to mount an
> interrupted checksum conversion (at the metadata csum overwrite stage).
> 
> And since the main part of metadata sanity checks is inside
> tree-checker, we shouldn't lose much safety, and the new mount option is
> rescue mount option it requires full read-only mount.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>

> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -367,6 +367,7 @@ int btrfs_validate_extent_buffer(struct extent_buffer *eb,
>  	u8 result[BTRFS_CSUM_SIZE];
>  	const u8 *header_csum;
>  	int ret = 0;
> +	bool ignore_csum = btrfs_test_opt(fs_info, IGNOREMETACSUMS);

const

> --- a/fs/btrfs/messages.c
> +++ b/fs/btrfs/messages.c
> @@ -20,7 +20,7 @@ static const char fs_state_chars[] = {
>  	[BTRFS_FS_STATE_TRANS_ABORTED]		= 'A',
>  	[BTRFS_FS_STATE_DEV_REPLACING]		= 'R',
>  	[BTRFS_FS_STATE_DUMMY_FS_INFO]		= 0,
> -	[BTRFS_FS_STATE_NO_CSUMS]		= 'C',
> +	[BTRFS_FS_STATE_NO_DATA_CSUMS]		= 'C',

There should be the status also when the metadata checksums are not
validated, the letters are arbitrary but should reflect the state if
possible, I'd suggest to use 'S' here.

What I'm missing is the sysfs.c update, all options supported by rescue
need to be listed in rescue_opts.

Re: [PATCH 4/4] btrfs: introduce new "rescue=ignoresuperflags" mount option

[David Sterba]

On Tue, Jun 11, 2024 at 02:51:38PM +0930, Qu Wenruo wrote:
> This new mount option would allow the kernel to skip the super flags
> check, it's mostly to allow the kernel to do a rescue mount of an
> interrupted checksum conversion.

This is for future-proofing but the current known super flags should be
at least defined in the kernel code and with a warning or info message.
BTRFS_SUPER_FLAG_CHANGING_FSID and _V2.

Same comment as for the imetacsum, missing sysfs.c entry.

Re: [PATCH 0/4] btrfs: rescue= mount options enhancement to support interrupted csum conversion

[Qu Wenruo]

在 2024/6/12 02:13, Josef Bacik 写道:
[...]
>>
>
> Reviewed-by: Josef Bacik <josef@toxicpanda.com>
>
> Could you update the docs in btrfs-progs/wherever we're keeping the readthedocs
> stuff so that these new flags are documented?  Thanks,

Sure, I'll send out the btrfs-progs updates along with the updated patchset.

Thanks,
Qu
>
> Josef
>

Re: [PATCH 3/4] btrfs: introduce new "rescue=ignoremetacsums" mount option

[Qu Wenruo]

在 2024/6/13 05:08, David Sterba 写道:
> On Tue, Jun 11, 2024 at 02:51:37PM +0930, Qu Wenruo wrote:
>> This patch introduces "rescue=ignoremetacsums" to ignore metadata csums,
>> meanwhile all the other metadata sanity checks are still kept as is.
>>
>> This new mount option is mostly to allow the kernel to mount an
>> interrupted checksum conversion (at the metadata csum overwrite stage).
>>
>> And since the main part of metadata sanity checks is inside
>> tree-checker, we shouldn't lose much safety, and the new mount option is
>> rescue mount option it requires full read-only mount.
>>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>
>> --- a/fs/btrfs/disk-io.c
>> +++ b/fs/btrfs/disk-io.c
>> @@ -367,6 +367,7 @@ int btrfs_validate_extent_buffer(struct extent_buffer *eb,
>>   	u8 result[BTRFS_CSUM_SIZE];
>>   	const u8 *header_csum;
>>   	int ret = 0;
>> +	bool ignore_csum = btrfs_test_opt(fs_info, IGNOREMETACSUMS);
>
> const
>
>> --- a/fs/btrfs/messages.c
>> +++ b/fs/btrfs/messages.c
>> @@ -20,7 +20,7 @@ static const char fs_state_chars[] = {
>>   	[BTRFS_FS_STATE_TRANS_ABORTED]		= 'A',
>>   	[BTRFS_FS_STATE_DEV_REPLACING]		= 'R',
>>   	[BTRFS_FS_STATE_DUMMY_FS_INFO]		= 0,
>> -	[BTRFS_FS_STATE_NO_CSUMS]		= 'C',
>> +	[BTRFS_FS_STATE_NO_DATA_CSUMS]		= 'C',
>
> There should be the status also when the metadata checksums are not
> validated, the letters are arbitrary but should reflect the state if
> possible, I'd suggest to use 'S' here.

I'd prefer to change the NO_DATA_CSUMS one to use 'D' or 'd' (for data),
meanwhile for metadata we go 'M' or 'm'.

But on the other hand, I do not think data/meta csum ignoring really
deserves a dedicated state char.

It's not really that special compared to trans aborted or dummy fs.
(The same for dev-replacing)

I can add it for now, but I believe we need some better discussion on
the fs_state_chars.

>
> What I'm missing is the sysfs.c update, all options supported by rescue
> need to be listed in rescue_opts.
>
That would be updated in the next update.

Thanks,
Qu

Re: [PATCH 4/4] btrfs: introduce new "rescue=ignoresuperflags" mount option

[Qu Wenruo]

在 2024/6/13 05:12, David Sterba 写道:
> On Tue, Jun 11, 2024 at 02:51:38PM +0930, Qu Wenruo wrote:
>> This new mount option would allow the kernel to skip the super flags
>> check, it's mostly to allow the kernel to do a rescue mount of an
>> interrupted checksum conversion.
>
> This is for future-proofing but the current known super flags should be
> at least defined in the kernel code

That's already done in this patch:

https://lore.kernel.org/linux-btrfs/57b836631f9c5dcb24ace616bdb76a37b8d084b2.1717818761.git.wqu@suse.com/T/#u

> and with a warning or info message.
> BTRFS_SUPER_FLAG_CHANGING_FSID and _V2.

I can add some extra warning messages, but currently we do not do any
messages for rescue mount options at all, since user is definitely aware
of some problems then go with rescue.
(And rescue has to be paired with ro, which should already rejected the
mount once when tried without ro, thus the end user definitely knows
what they are doing)

Thus I'm more or less fine without any error messages for now.

Thanks,
Qu
>
> Same comment as for the imetacsum, missing sysfs.c entry.
>

Re: [PATCH 3/4] btrfs: introduce new "rescue=ignoremetacsums" mount option

[David Sterba]

On Fri, Jun 14, 2024 at 06:58:20AM +0930, Qu Wenruo wrote:
> 
> 
> 在 2024/6/13 05:08, David Sterba 写道:
> > On Tue, Jun 11, 2024 at 02:51:37PM +0930, Qu Wenruo wrote:
> >> This patch introduces "rescue=ignoremetacsums" to ignore metadata csums,
> >> meanwhile all the other metadata sanity checks are still kept as is.
> >>
> >> This new mount option is mostly to allow the kernel to mount an
> >> interrupted checksum conversion (at the metadata csum overwrite stage).
> >>
> >> And since the main part of metadata sanity checks is inside
> >> tree-checker, we shouldn't lose much safety, and the new mount option is
> >> rescue mount option it requires full read-only mount.
> >>
> >> Signed-off-by: Qu Wenruo <wqu@suse.com>
> >
> >> --- a/fs/btrfs/disk-io.c
> >> +++ b/fs/btrfs/disk-io.c
> >> @@ -367,6 +367,7 @@ int btrfs_validate_extent_buffer(struct extent_buffer *eb,
> >>   	u8 result[BTRFS_CSUM_SIZE];
> >>   	const u8 *header_csum;
> >>   	int ret = 0;
> >> +	bool ignore_csum = btrfs_test_opt(fs_info, IGNOREMETACSUMS);
> >
> > const
> >
> >> --- a/fs/btrfs/messages.c
> >> +++ b/fs/btrfs/messages.c
> >> @@ -20,7 +20,7 @@ static const char fs_state_chars[] = {
> >>   	[BTRFS_FS_STATE_TRANS_ABORTED]		= 'A',
> >>   	[BTRFS_FS_STATE_DEV_REPLACING]		= 'R',
> >>   	[BTRFS_FS_STATE_DUMMY_FS_INFO]		= 0,
> >> -	[BTRFS_FS_STATE_NO_CSUMS]		= 'C',
> >> +	[BTRFS_FS_STATE_NO_DATA_CSUMS]		= 'C',
> >
> > There should be the status also when the metadata checksums are not
> > validated, the letters are arbitrary but should reflect the state if
> > possible, I'd suggest to use 'S' here.
> 
> I'd prefer to change the NO_DATA_CSUMS one to use 'D' or 'd' (for data),
> meanwhile for metadata we go 'M' or 'm'.

Changing would break backward compatibility, now it's part of user
visible interface. It's not an ABI or API but at least we should do such
changes without considering the consequences.

> But on the other hand, I do not think data/meta csum ignoring really
> deserves a dedicated state char.
> 
> It's not really that special compared to trans aborted or dummy fs.
> (The same for dev-replacing)

The idea of the descriptors is to make it visible that the filesystem is
in some unusual state, skipping checksum verification can make a
difference when reading blocks that would normally not pass the check.

Re: [PATCH 3/4] btrfs: introduce new "rescue=ignoremetacsums" mount option

[Qu Wenruo]

在 2024/6/17 03:47, David Sterba 写道:
> On Fri, Jun 14, 2024 at 06:58:20AM +0930, Qu Wenruo wrote:
>>
>>
>> 在 2024/6/13 05:08, David Sterba 写道:
>>> On Tue, Jun 11, 2024 at 02:51:37PM +0930, Qu Wenruo wrote:
>>>> This patch introduces "rescue=ignoremetacsums" to ignore metadata csums,
>>>> meanwhile all the other metadata sanity checks are still kept as is.
>>>>
>>>> This new mount option is mostly to allow the kernel to mount an
>>>> interrupted checksum conversion (at the metadata csum overwrite stage).
>>>>
>>>> And since the main part of metadata sanity checks is inside
>>>> tree-checker, we shouldn't lose much safety, and the new mount option is
>>>> rescue mount option it requires full read-only mount.
>>>>
>>>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>>>
>>>> --- a/fs/btrfs/disk-io.c
>>>> +++ b/fs/btrfs/disk-io.c
>>>> @@ -367,6 +367,7 @@ int btrfs_validate_extent_buffer(struct extent_buffer *eb,
>>>>    	u8 result[BTRFS_CSUM_SIZE];
>>>>    	const u8 *header_csum;
>>>>    	int ret = 0;
>>>> +	bool ignore_csum = btrfs_test_opt(fs_info, IGNOREMETACSUMS);
>>>
>>> const
>>>
>>>> --- a/fs/btrfs/messages.c
>>>> +++ b/fs/btrfs/messages.c
>>>> @@ -20,7 +20,7 @@ static const char fs_state_chars[] = {
>>>>    	[BTRFS_FS_STATE_TRANS_ABORTED]		= 'A',
>>>>    	[BTRFS_FS_STATE_DEV_REPLACING]		= 'R',
>>>>    	[BTRFS_FS_STATE_DUMMY_FS_INFO]		= 0,
>>>> -	[BTRFS_FS_STATE_NO_CSUMS]		= 'C',
>>>> +	[BTRFS_FS_STATE_NO_DATA_CSUMS]		= 'C',
>>>
>>> There should be the status also when the metadata checksums are not
>>> validated, the letters are arbitrary but should reflect the state if
>>> possible, I'd suggest to use 'S' here.
>>
>> I'd prefer to change the NO_DATA_CSUMS one to use 'D' or 'd' (for data),
>> meanwhile for metadata we go 'M' or 'm'.
>
> Changing would break backward compatibility, now it's part of user
> visible interface. It's not an ABI or API but at least we should do such
> changes without considering the consequences.
>
>> But on the other hand, I do not think data/meta csum ignoring really
>> deserves a dedicated state char.
>>
>> It's not really that special compared to trans aborted or dummy fs.
>> (The same for dev-replacing)
>
> The idea of the descriptors is to make it visible that the filesystem is
> in some unusual state, skipping checksum verification can make a
> difference when reading blocks that would normally not pass the check.

On the other hand, I have already changed the metadata csum mismatch output.
It should be enough to know if we're in a skipping-metadata-csum state.

Furthermore, all of these are in rescue mode with forced RO, which
should already be a thing to mention during report/debugging.

That's why I do not think skipping data/metadata csums really needs such
dedicated handling.

Thanks,
Qu