[08/10] btrfs-progs: check: Add support for freespace tree fixing
diff mbox series

Message ID 1538405181-25231-9-git-send-email-nborisov@suse.com
State New
Headers show
Series
  • Freespace tree repair support v2
Related show

Commit Message

Nikolay Borisov Oct. 1, 2018, 2:46 p.m. UTC
Now that all the prerequisite code for proper support of free space
tree repair is in, it's time to wire it in. This is achieved by first
hooking the freespace tree to the __free_extent/alloc_reserved_tree_block
functions. And then introducing a wrapper function to contains the
existing check_space_cache and the newly introduced repair code.
Finally, it's important to note that FST repair code first clears the
existing FST in case of any problem found and rebuilds it from scratch.

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
 check/main.c | 47 ++++++++++++++++++++++++++++++-----------------
 1 file changed, 30 insertions(+), 17 deletions(-)

Comments

Omar Sandoval Oct. 4, 2018, 7:16 p.m. UTC | #1
On Mon, Oct 01, 2018 at 05:46:19PM +0300, Nikolay Borisov wrote:
> Now that all the prerequisite code for proper support of free space
> tree repair is in, it's time to wire it in. This is achieved by first
> hooking the freespace tree to the __free_extent/alloc_reserved_tree_block
> functions. And then introducing a wrapper function to contains the
> existing check_space_cache and the newly introduced repair code.
> Finally, it's important to note that FST repair code first clears the
> existing FST in case of any problem found and rebuilds it from scratch.

Reviewed-by: Omar Sandoval <osandov@fb.com>

A couple of really trivial nitpicks below that you should feel free to
ignore ;)

> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> ---
>  check/main.c | 47 ++++++++++++++++++++++++++++++-----------------
>  1 file changed, 30 insertions(+), 17 deletions(-)
> 
> diff --git a/check/main.c b/check/main.c
> index b361cd7e26a0..4daf85aad82c 100644
> --- a/check/main.c
> +++ b/check/main.c
> @@ -5392,14 +5392,6 @@ static int check_space_cache(struct btrfs_root *root)
>  	int ret;
>  	int error = 0;
>  
> -	if (btrfs_super_cache_generation(root->fs_info->super_copy) != -1ULL &&
> -	    btrfs_super_generation(root->fs_info->super_copy) !=
> -	    btrfs_super_cache_generation(root->fs_info->super_copy)) {
> -		printf("cache and super generation don't match, space cache "
> -		       "will be invalidated\n");
> -		return 0;
> -	}
> -
>  	while (1) {
>  		ctx.item_count++;
>  		cache = btrfs_lookup_first_block_group(root->fs_info, start);
> @@ -9417,7 +9409,6 @@ static int do_clear_free_space_cache(struct btrfs_fs_info *fs_info,
>  			ret = 1;
>  			goto close_out;
>  		}
> -		printf("Clearing free space cache\n");

Just out of curiosity, why did you delete this message? The one in the
v2 case is still there.

>  		ret = clear_free_space_cache(fs_info);
>  		if (ret) {
>  			error("failed to clear free space cache");
> @@ -9444,6 +9435,35 @@ static int do_clear_free_space_cache(struct btrfs_fs_info *fs_info,
>  	return ret;
>  }
>  
> +static int validate_free_space_cache(struct btrfs_root *root)

At first glance, I wouldn't know what the difference is between
check_space_cache() and validate_free_space_cache(); they sound like the
same thing. Maybe rename this to check_and_repair_space_cache() or just
fold the rebuild into check_space_cache(), to be more in line with the
other check steps in fsck?

Patch
diff mbox series

diff --git a/check/main.c b/check/main.c
index b361cd7e26a0..4daf85aad82c 100644
--- a/check/main.c
+++ b/check/main.c
@@ -5392,14 +5392,6 @@  static int check_space_cache(struct btrfs_root *root)
 	int ret;
 	int error = 0;
 
-	if (btrfs_super_cache_generation(root->fs_info->super_copy) != -1ULL &&
-	    btrfs_super_generation(root->fs_info->super_copy) !=
-	    btrfs_super_cache_generation(root->fs_info->super_copy)) {
-		printf("cache and super generation don't match, space cache "
-		       "will be invalidated\n");
-		return 0;
-	}
-
 	while (1) {
 		ctx.item_count++;
 		cache = btrfs_lookup_first_block_group(root->fs_info, start);
@@ -9417,7 +9409,6 @@  static int do_clear_free_space_cache(struct btrfs_fs_info *fs_info,
 			ret = 1;
 			goto close_out;
 		}
-		printf("Clearing free space cache\n");
 		ret = clear_free_space_cache(fs_info);
 		if (ret) {
 			error("failed to clear free space cache");
@@ -9444,6 +9435,35 @@  static int do_clear_free_space_cache(struct btrfs_fs_info *fs_info,
 	return ret;
 }
 
+static int validate_free_space_cache(struct btrfs_root *root)
+{
+
+	int ret;
+
+	if (btrfs_super_cache_generation(root->fs_info->super_copy) != -1ULL &&
+	    btrfs_super_generation(root->fs_info->super_copy) !=
+	    btrfs_super_cache_generation(root->fs_info->super_copy)) {
+		printf("cache and super generation don't match, space cache "
+		       "will be invalidated\n");
+		return 0;
+	}
+
+	ret = check_space_cache(root);
+	if (ret && btrfs_fs_compat_ro(global_info, FREE_SPACE_TREE)
+	    && repair) {
+		ret = do_clear_free_space_cache(global_info, 2);
+		if (ret)
+			goto out;
+
+		ret = btrfs_create_free_space_tree(global_info);
+		if (ret)
+			error("couldn't repair freespace tree");
+	}
+
+out:
+	return ret ? -EINVAL : 0;
+}
+
 const char * const cmd_check_usage[] = {
 	"btrfs check [options] <device>",
 	"Check structural integrity of a filesystem (unmounted).",
@@ -9877,16 +9897,9 @@  int cmd_check(int argc, char **argv)
 		task_start(ctx.info, &ctx.start_time, &ctx.item_count);
 	}
 
-	ret = check_space_cache(root);
+	ret = validate_free_space_cache(root);
 	task_stop(ctx.info);
 	err |= !!ret;
-	if (ret) {
-		if (is_free_space_tree)
-			error("errors found in free space tree");
-		else
-			error("errors found in free space cache");
-		goto out;
-	}
 
 	/*
 	 * We used to have to have these hole extents in between our real