diff mbox series

btrfs-progs: Replace OPEN_CTREE_NO_BLOCK_GROUPS with OPEN_CTREE_PARTIAL

Message ID 20210812081546.20724-1-nborisov@suse.com (mailing list archive)
State New, archived
Headers show
Series btrfs-progs: Replace OPEN_CTREE_NO_BLOCK_GROUPS with OPEN_CTREE_PARTIAL | expand

Commit Message

Nikolay Borisov Aug. 12, 2021, 8:15 a.m. UTC 
When OPEN_CTREE_PARTIAL is used errors in loading the extent/csum/log
trees are turned into non-fatal ones and those roots are initialized
with dummy root nodes, which don't have uptodate flag set on the
respective extent buffer. On the other hand reading block groups during
mount is predicated on OPEN_CTREE_NO_BLOCK_GROUPS being unset and
the extent tree's root extent buffer to have the uptodate flag set to
true. Furthermore, OPEN_CTREE_NO_BLOCK_GROUP is used when there is a
high chance the filesystem being opened can be damaged, similarly to
OPEN_CTREE_PARTIAL.

Considering this logic, it's not possible to load block groups when both
OPEN_CTREE_NO_BLOCK_GROUPS and OPEN_CTREE_PARTIAL are passed and the
extent tree is corrupted. This allows to eliminate
OPEN_CTREE_NO_BLOCK_GROUPS and replace its usage with
OPEN_CTREE_PARTIAL, since it's sufficient to check only for the extent
tree's extent buffer's uptodate state, which is controlled by
OPEN_CTREE_PARTIAL.
---
 check/main.c             | 2 +-
 cmds/inspect-dump-tree.c | 2 +-
 cmds/rescue.c            | 4 ++--
 cmds/restore.c           | 2 +-
 kernel-shared/disk-io.c  | 2 +-
 kernel-shared/disk-io.h  | 7 ++++---
 6 files changed, 10 insertions(+), 9 deletions(-)

Comments

Qu Wenruo Aug. 12, 2021, 8:47 a.m. UTC | #1 
On 2021/8/12 下午4:15, Nikolay Borisov wrote:
> When OPEN_CTREE_PARTIAL is used errors in loading the extent/csum/log
> trees are turned into non-fatal ones and those roots are initialized
> with dummy root nodes, which don't have uptodate flag set on the
> respective extent buffer. On the other hand reading block groups during
> mount is predicated on OPEN_CTREE_NO_BLOCK_GROUPS being unset and
> the extent tree's root extent buffer to have the uptodate flag set to
> true. Furthermore, OPEN_CTREE_NO_BLOCK_GROUP is used when there is a
> high chance the filesystem being opened can be damaged, similarly to
> OPEN_CTREE_PARTIAL.
>
> Considering this logic, it's not possible to load block groups when both
> OPEN_CTREE_NO_BLOCK_GROUPS and OPEN_CTREE_PARTIAL are passed and the
> extent tree is corrupted. This allows to eliminate
> OPEN_CTREE_NO_BLOCK_GROUPS and replace its usage with
> OPEN_CTREE_PARTIAL, since it's sufficient to check only for the extent
> tree's extent buffer's uptodate state, which is controlled by
> OPEN_CTREE_PARTIAL.

There is still some differences between PARTIAL and NO_BLOCK_GROUPS.

The major one is, how we handle partially corrupted extent tree (aka,
some leaves are corrupted, but extent tree root is still OK).

In that case, NO_BLOCK_GROUPS will skip the block group item search
completely, thus continue to mount, just with empty block group cache tree.

While PARTIAL will still try to do the block group item search, and if
it hits -EIO, it errors out with -EIO and abort open_ctree().

To merge both, we need to enhance the error handling of
btrfs_read_block_groups() and make sure all call sites utilizing PARTIAL
are OK with partially populated block group cache.

 From a quick glance, I haven't seen obvious call sites that require
block group items while still passes PARTIAL.

Thus I'm fine to merge the two options, but as mentioned, we still need
to change the error handling of btrfs_read_block_groups() in that case.

Thanks,
Qu

> ---
>   check/main.c             | 2 +-
>   cmds/inspect-dump-tree.c | 2 +-
>   cmds/rescue.c            | 4 ++--
>   cmds/restore.c           | 2 +-
>   kernel-shared/disk-io.c  | 2 +-
>   kernel-shared/disk-io.h  | 7 ++++---
>   6 files changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/check/main.c b/check/main.c
> index a88518159830..84dd96fc167a 100644
> --- a/check/main.c
> +++ b/check/main.c
> @@ -10342,7 +10342,7 @@ static int cmd_check(const struct cmd_struct *cmd, int argc, char **argv)
>   			case GETOPT_VAL_INIT_EXTENT:
>   				init_extent_tree = 1;
>   				ctree_flags |= (OPEN_CTREE_WRITES |
> -						OPEN_CTREE_NO_BLOCK_GROUPS);
> +						OPEN_CTREE_PARTIAL);
>   				repair = 1;
>   				break;
>   			case GETOPT_VAL_CHECK_CSUM:
> diff --git a/cmds/inspect-dump-tree.c b/cmds/inspect-dump-tree.c
> index e1c90be7e81f..fca79cd11599 100644
> --- a/cmds/inspect-dump-tree.c
> +++ b/cmds/inspect-dump-tree.c
> @@ -330,7 +330,7 @@ static int cmd_inspect_dump_tree(const struct cmd_struct *cmd,
>   	 * to inspect fs with corrupted extent tree blocks, and show as many good
>   	 * tree blocks as possible.
>   	 */
> -	open_ctree_flags = OPEN_CTREE_PARTIAL | OPEN_CTREE_NO_BLOCK_GROUPS;
> +	open_ctree_flags = OPEN_CTREE_PARTIAL;
>   	cache_tree_init(&block_root);
>   	optind = 0;
>   	while (1) {
> diff --git a/cmds/rescue.c b/cmds/rescue.c
> index a98b255ad328..7ebe9b6c1e54 100644
> --- a/cmds/rescue.c
> +++ b/cmds/rescue.c
> @@ -194,8 +194,8 @@ static int cmd_rescue_zero_log(const struct cmd_struct *cmd,
>   		goto out;
>   	}
>
> -	root = open_ctree(devname, 0, OPEN_CTREE_WRITES | OPEN_CTREE_PARTIAL |
> -			  OPEN_CTREE_NO_BLOCK_GROUPS);
> +	root = open_ctree(devname, 0, OPEN_CTREE_WRITES | OPEN_CTREE_PARTIAL);
> +
>   	if (!root) {
>   		error("could not open ctree");
>   		return 1;
> diff --git a/cmds/restore.c b/cmds/restore.c
> index 39fcc69d8e19..f30d8b7532c0 100644
> --- a/cmds/restore.c
> +++ b/cmds/restore.c
> @@ -1214,7 +1214,7 @@ static struct btrfs_root *open_fs(const char *dev, u64 root_location,
>   		ocf.filename = dev;
>   		ocf.sb_bytenr = bytenr;
>   		ocf.root_tree_bytenr = root_location;
> -		ocf.flags = OPEN_CTREE_PARTIAL | OPEN_CTREE_NO_BLOCK_GROUPS;
> +		ocf.flags = OPEN_CTREE_PARTIAL;
>   		fs_info = open_ctree_fs_info(&ocf);
>   		if (fs_info)
>   			break;
> diff --git a/kernel-shared/disk-io.c b/kernel-shared/disk-io.c
> index 84990a521178..cc635152c46d 100644
> --- a/kernel-shared/disk-io.c
> +++ b/kernel-shared/disk-io.c
> @@ -1045,7 +1045,7 @@ int btrfs_setup_all_roots(struct btrfs_fs_info *fs_info, u64 root_tree_bytenr,
>   	fs_info->generation = generation;
>   	fs_info->last_trans_committed = generation;
>   	if (extent_buffer_uptodate(fs_info->extent_root->node) &&
> -	    !(flags & OPEN_CTREE_NO_BLOCK_GROUPS)) {
> +	    !(flags & OPEN_CTREE_PARTIAL)) {
>   		ret = btrfs_read_block_groups(fs_info);
>   		/*
>   		 * If we don't find any blockgroups (ENOENT) we're either
> diff --git a/kernel-shared/disk-io.h b/kernel-shared/disk-io.h
> index 603ff8a3f945..16e13e4f5524 100644
> --- a/kernel-shared/disk-io.h
> +++ b/kernel-shared/disk-io.h
> @@ -32,7 +32,10 @@
>   enum btrfs_open_ctree_flags {
>   	/* Open filesystem for writes */
>   	OPEN_CTREE_WRITES		= (1U << 0),
> -	/* Allow to open filesystem with some broken tree roots (eg log root) */
> +	/*
> +	 * Allow to open filesystem with some broken tree roots
> +	 * (eg log root/csum/extent tree)
> +	 */
>   	OPEN_CTREE_PARTIAL		= (1U << 1),
>   	/* If primary root pinters are invalid, try backup copies */
>   	OPEN_CTREE_BACKUP_ROOT		= (1U << 2),
> @@ -40,8 +43,6 @@ enum btrfs_open_ctree_flags {
>   	OPEN_CTREE_RECOVER_SUPER	= (1U << 3),
>   	/* Restoring filesystem image */
>   	OPEN_CTREE_RESTORE		= (1U << 4),
> -	/* Do not read block groups (extent tree) */
> -	OPEN_CTREE_NO_BLOCK_GROUPS	= (1U << 5),
>   	/* Open all devices in O_EXCL mode */
>   	OPEN_CTREE_EXCLUSIVE		= (1U << 6),
>   	/* Do not scan devices */
>
diff mbox series

Patch

diff --git a/check/main.c b/check/main.c
index a88518159830..84dd96fc167a 100644
--- a/check/main.c
+++ b/check/main.c
@@ -10342,7 +10342,7 @@  static int cmd_check(const struct cmd_struct *cmd, int argc, char **argv)
 			case GETOPT_VAL_INIT_EXTENT:
 				init_extent_tree = 1;
 				ctree_flags |= (OPEN_CTREE_WRITES |
-						OPEN_CTREE_NO_BLOCK_GROUPS);
+						OPEN_CTREE_PARTIAL);
 				repair = 1;
 				break;
 			case GETOPT_VAL_CHECK_CSUM:
diff --git a/cmds/inspect-dump-tree.c b/cmds/inspect-dump-tree.c
index e1c90be7e81f..fca79cd11599 100644
--- a/cmds/inspect-dump-tree.c
+++ b/cmds/inspect-dump-tree.c
@@ -330,7 +330,7 @@  static int cmd_inspect_dump_tree(const struct cmd_struct *cmd,
 	 * to inspect fs with corrupted extent tree blocks, and show as many good
 	 * tree blocks as possible.
 	 */
-	open_ctree_flags = OPEN_CTREE_PARTIAL | OPEN_CTREE_NO_BLOCK_GROUPS;
+	open_ctree_flags = OPEN_CTREE_PARTIAL;
 	cache_tree_init(&block_root);
 	optind = 0;
 	while (1) {
diff --git a/cmds/rescue.c b/cmds/rescue.c
index a98b255ad328..7ebe9b6c1e54 100644
--- a/cmds/rescue.c
+++ b/cmds/rescue.c
@@ -194,8 +194,8 @@  static int cmd_rescue_zero_log(const struct cmd_struct *cmd,
 		goto out;
 	}
 
-	root = open_ctree(devname, 0, OPEN_CTREE_WRITES | OPEN_CTREE_PARTIAL |
-			  OPEN_CTREE_NO_BLOCK_GROUPS);
+	root = open_ctree(devname, 0, OPEN_CTREE_WRITES | OPEN_CTREE_PARTIAL);
+
 	if (!root) {
 		error("could not open ctree");
 		return 1;
diff --git a/cmds/restore.c b/cmds/restore.c
index 39fcc69d8e19..f30d8b7532c0 100644
--- a/cmds/restore.c
+++ b/cmds/restore.c
@@ -1214,7 +1214,7 @@  static struct btrfs_root *open_fs(const char *dev, u64 root_location,
 		ocf.filename = dev;
 		ocf.sb_bytenr = bytenr;
 		ocf.root_tree_bytenr = root_location;
-		ocf.flags = OPEN_CTREE_PARTIAL | OPEN_CTREE_NO_BLOCK_GROUPS;
+		ocf.flags = OPEN_CTREE_PARTIAL;
 		fs_info = open_ctree_fs_info(&ocf);
 		if (fs_info)
 			break;
diff --git a/kernel-shared/disk-io.c b/kernel-shared/disk-io.c
index 84990a521178..cc635152c46d 100644
--- a/kernel-shared/disk-io.c
+++ b/kernel-shared/disk-io.c
@@ -1045,7 +1045,7 @@  int btrfs_setup_all_roots(struct btrfs_fs_info *fs_info, u64 root_tree_bytenr,
 	fs_info->generation = generation;
 	fs_info->last_trans_committed = generation;
 	if (extent_buffer_uptodate(fs_info->extent_root->node) &&
-	    !(flags & OPEN_CTREE_NO_BLOCK_GROUPS)) {
+	    !(flags & OPEN_CTREE_PARTIAL)) {
 		ret = btrfs_read_block_groups(fs_info);
 		/*
 		 * If we don't find any blockgroups (ENOENT) we're either
diff --git a/kernel-shared/disk-io.h b/kernel-shared/disk-io.h
index 603ff8a3f945..16e13e4f5524 100644
--- a/kernel-shared/disk-io.h
+++ b/kernel-shared/disk-io.h
@@ -32,7 +32,10 @@ 
 enum btrfs_open_ctree_flags {
 	/* Open filesystem for writes */
 	OPEN_CTREE_WRITES		= (1U << 0),
-	/* Allow to open filesystem with some broken tree roots (eg log root) */
+	/*
+	 * Allow to open filesystem with some broken tree roots
+	 * (eg log root/csum/extent tree)
+	 */
 	OPEN_CTREE_PARTIAL		= (1U << 1),
 	/* If primary root pinters are invalid, try backup copies */
 	OPEN_CTREE_BACKUP_ROOT		= (1U << 2),
@@ -40,8 +43,6 @@  enum btrfs_open_ctree_flags {
 	OPEN_CTREE_RECOVER_SUPER	= (1U << 3),
 	/* Restoring filesystem image */
 	OPEN_CTREE_RESTORE		= (1U << 4),
-	/* Do not read block groups (extent tree) */
-	OPEN_CTREE_NO_BLOCK_GROUPS	= (1U << 5),
 	/* Open all devices in O_EXCL mode */
 	OPEN_CTREE_EXCLUSIVE		= (1U << 6),
 	/* Do not scan devices */