All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] btrfs-progs: Replace OPEN_CTREE_NO_BLOCK_GROUPS with OPEN_CTREE_PARTIAL
@ 2021-08-12 11:20 Nikolay Borisov
  2021-08-12 11:49 ` Qu Wenruo
  0 siblings, 1 reply; 2+ messages in thread
From: Nikolay Borisov @ 2021-08-12 11:20 UTC (permalink / raw)
  To: linux-btrfs; +Cc: Nikolay Borisov

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 controlledi
by OPEN_CTREE_PARTIAL. Additionally to retain the existing semantics, if an 
error is encountered while reading any of the block groups, simply free the 
block group caches and clearing the uptodate flag on the extent root's node. 

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---

V2:
* Handle the case where we encounter an error on a child node of the exten tree
  while opening the fs with PARTIAL flag. In this case simply delete any block groups 
  read. 

 check/main.c             |  2 +-
 cmds/inspect-dump-tree.c |  2 +-
 cmds/rescue.c            |  4 ++--
 cmds/restore.c           |  2 +-
 kernel-shared/disk-io.c  | 15 ++++++++++-----
 kernel-shared/disk-io.h  |  7 ++++---
 6 files changed, 19 insertions(+), 13 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..dfa4576c6371 100644
--- a/kernel-shared/disk-io.c
+++ b/kernel-shared/disk-io.c
@@ -1044,8 +1044,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)) {
+	if (extent_buffer_uptodate(fs_info->extent_root->node)) {
 		ret = btrfs_read_block_groups(fs_info);
 		/*
 		 * If we don't find any blockgroups (ENOENT) we're either
@@ -1053,9 +1052,15 @@ int btrfs_setup_all_roots(struct btrfs_fs_info *fs_info, u64 root_tree_bytenr,
 		 * anything else is error
 		 */
 		if (ret < 0 && ret != -ENOENT) {
-			errno = -ret;
-			error("failed to read block groups: %m");
-			return ret;
+			if (!(flags & OPEN_CTREE_PARTIAL)) {
+				errno = -ret;
+				error("failed to read block groups: %m");
+				return ret;
+			} else {
+				btrfs_free_block_groups(fs_info);
+				clear_extent_buffer_uptodate(
+						fs_info->extent_root->node);
+			}
 		}
 	}
 
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 */
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH v2] btrfs-progs: Replace OPEN_CTREE_NO_BLOCK_GROUPS with OPEN_CTREE_PARTIAL
  2021-08-12 11:20 [PATCH v2] btrfs-progs: Replace OPEN_CTREE_NO_BLOCK_GROUPS with OPEN_CTREE_PARTIAL Nikolay Borisov
@ 2021-08-12 11:49 ` Qu Wenruo
  0 siblings, 0 replies; 2+ messages in thread
From: Qu Wenruo @ 2021-08-12 11:49 UTC (permalink / raw)
  To: Nikolay Borisov, linux-btrfs



On 2021/8/12 下午7:20, 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 controlledi
> by OPEN_CTREE_PARTIAL. Additionally to retain the existing semantics, if an
> error is encountered while reading any of the block groups, simply free the
> block group caches and clearing the uptodate flag on the extent root's node.
>
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu
> ---
>
> V2:
> * Handle the case where we encounter an error on a child node of the exten tree
>    while opening the fs with PARTIAL flag. In this case simply delete any block groups
>    read.
>
>   check/main.c             |  2 +-
>   cmds/inspect-dump-tree.c |  2 +-
>   cmds/rescue.c            |  4 ++--
>   cmds/restore.c           |  2 +-
>   kernel-shared/disk-io.c  | 15 ++++++++++-----
>   kernel-shared/disk-io.h  |  7 ++++---
>   6 files changed, 19 insertions(+), 13 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..dfa4576c6371 100644
> --- a/kernel-shared/disk-io.c
> +++ b/kernel-shared/disk-io.c
> @@ -1044,8 +1044,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)) {
> +	if (extent_buffer_uptodate(fs_info->extent_root->node)) {
>   		ret = btrfs_read_block_groups(fs_info);
>   		/*
>   		 * If we don't find any blockgroups (ENOENT) we're either
> @@ -1053,9 +1052,15 @@ int btrfs_setup_all_roots(struct btrfs_fs_info *fs_info, u64 root_tree_bytenr,
>   		 * anything else is error
>   		 */
>   		if (ret < 0 && ret != -ENOENT) {
> -			errno = -ret;
> -			error("failed to read block groups: %m");
> -			return ret;
> +			if (!(flags & OPEN_CTREE_PARTIAL)) {
> +				errno = -ret;
> +				error("failed to read block groups: %m");
> +				return ret;
> +			} else {
> +				btrfs_free_block_groups(fs_info);
> +				clear_extent_buffer_uptodate(
> +						fs_info->extent_root->node);
> +			}
>   		}
>   	}
>
> 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 */
>

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2021-08-12 11:50 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-12 11:20 [PATCH v2] btrfs-progs: Replace OPEN_CTREE_NO_BLOCK_GROUPS with OPEN_CTREE_PARTIAL Nikolay Borisov
2021-08-12 11:49 ` Qu Wenruo

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.