diff mbox

[5/6] btrfs-progs: check: Add support for freespace tree fixing

Message ID 1529060762-4372-6-git-send-email-nborisov@suse.com (mailing list archive)
State New, archived
Headers show

Commit Message

Nikolay Borisov June 15, 2018, 11:06 a.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  | 61 +++++++++++++++++++++++++++++++++++++----------------------
 extent-tree.c |  9 +++++++++
 2 files changed, 47 insertions(+), 23 deletions(-)

Comments

Omar Sandoval Sept. 21, 2018, 8:42 p.m. UTC | #1
On Fri, Jun 15, 2018 at 02:06:01PM +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.
> 
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> ---
>  check/main.c  | 61 +++++++++++++++++++++++++++++++++++++----------------------
>  extent-tree.c |  9 +++++++++
>  2 files changed, 47 insertions(+), 23 deletions(-)
> 
> diff --git a/check/main.c b/check/main.c
> index 3a5efaf615a9..44d734ff4254 100644
> --- a/check/main.c
> +++ b/check/main.c
> @@ -5321,19 +5321,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;
> -	}
> -
> -	if (ctx.progress_enabled) {
> -		ctx.tp = TASK_FREE_SPACE;
> -		task_start(ctx.info);
> -	}
> -
>  	while (1) {
>  		cache = btrfs_lookup_first_block_group(root->fs_info, start);
>  		if (!cache)
> @@ -5383,11 +5370,11 @@ static int check_space_cache(struct btrfs_root *root)
>  		}
>  	}
>  
> -	task_stop(ctx.info);
>  
>  	return error ? -EINVAL : 0;
>  }
>  
> +

Stray newline.

>  /*
>   * Check data checksum for [@bytenr, @bytenr + @num_bytes).
>   *
> @@ -9338,7 +9325,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");
> @@ -9365,6 +9351,41 @@ 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;
> +	}
> +
> +	if (ctx.progress_enabled) {
> +		ctx.tp = TASK_FREE_SPACE;
> +		task_start(ctx.info);
> +	}
> +
> +	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:
> +	task_stop(ctx.info);
> +	return ret ? -EINVAL : 0;
> +}
> +
>  const char * const cmd_check_usage[] = {
>  	"btrfs check [options] <device>",
>  	"Check structural integrity of a filesystem (unmounted).",
> @@ -9768,15 +9789,9 @@ int cmd_check(int argc, char **argv)
>  		else
>  			fprintf(stderr, "checking free space cache\n");
>  	}
> -	ret = check_space_cache(root);
> +
> +	ret = validate_free_space_cache(root);
>  	err |= !!ret;
> -	if (ret) {
> -		if (btrfs_fs_compat_ro(info, 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

This approach seems reasonable.

> diff --git a/extent-tree.c b/extent-tree.c
> index b9d51b388c9a..40117f81352e 100644
> --- a/extent-tree.c
> +++ b/extent-tree.c
> @@ -29,6 +29,7 @@
>  #include "crc32c.h"
>  #include "volumes.h"
>  #include "free-space-cache.h"
> +#include "free-space-tree.h"
>  #include "utils.h"
>  
>  #define PENDING_EXTENT_INSERT 0
> @@ -2292,6 +2293,11 @@ static int __free_extent(struct btrfs_trans_handle *trans,
>  			BUG_ON(ret);
>  		}
>  
> +		ret = add_to_free_space_tree(trans, bytenr, num_bytes);
> +		if (ret) {
> +			goto fail;
> +		}
> +
>  		update_block_group(trans->fs_info, bytenr, num_bytes, 0,
>  				   mark_free);
>  	}
> @@ -2630,6 +2636,9 @@ static int alloc_reserved_tree_block(struct btrfs_trans_handle *trans,
>  	btrfs_mark_buffer_dirty(leaf);
>  	btrfs_free_path(path);
>  
> +	ret = remove_from_free_space_tree(trans, ins->objectid, fs_info->nodesize);
> +	if (ret)
> +		return ret;
>  	ret = update_block_group(fs_info, ins->objectid, fs_info->nodesize,
>  				 1, 0);
>  	return ret;

Related to my comment on patch 4, the extent-tree.c changes should be a
separate patch that comes before we add the compat bit, assuming it
works with commands that open_ctree(..., OPEN_CTREE_WRITES) and modify
the fs.
diff mbox

Patch

diff --git a/check/main.c b/check/main.c
index 3a5efaf615a9..44d734ff4254 100644
--- a/check/main.c
+++ b/check/main.c
@@ -5321,19 +5321,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;
-	}
-
-	if (ctx.progress_enabled) {
-		ctx.tp = TASK_FREE_SPACE;
-		task_start(ctx.info);
-	}
-
 	while (1) {
 		cache = btrfs_lookup_first_block_group(root->fs_info, start);
 		if (!cache)
@@ -5383,11 +5370,11 @@  static int check_space_cache(struct btrfs_root *root)
 		}
 	}
 
-	task_stop(ctx.info);
 
 	return error ? -EINVAL : 0;
 }
 
+
 /*
  * Check data checksum for [@bytenr, @bytenr + @num_bytes).
  *
@@ -9338,7 +9325,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");
@@ -9365,6 +9351,41 @@  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;
+	}
+
+	if (ctx.progress_enabled) {
+		ctx.tp = TASK_FREE_SPACE;
+		task_start(ctx.info);
+	}
+
+	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:
+	task_stop(ctx.info);
+	return ret ? -EINVAL : 0;
+}
+
 const char * const cmd_check_usage[] = {
 	"btrfs check [options] <device>",
 	"Check structural integrity of a filesystem (unmounted).",
@@ -9768,15 +9789,9 @@  int cmd_check(int argc, char **argv)
 		else
 			fprintf(stderr, "checking free space cache\n");
 	}
-	ret = check_space_cache(root);
+
+	ret = validate_free_space_cache(root);
 	err |= !!ret;
-	if (ret) {
-		if (btrfs_fs_compat_ro(info, 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
diff --git a/extent-tree.c b/extent-tree.c
index b9d51b388c9a..40117f81352e 100644
--- a/extent-tree.c
+++ b/extent-tree.c
@@ -29,6 +29,7 @@ 
 #include "crc32c.h"
 #include "volumes.h"
 #include "free-space-cache.h"
+#include "free-space-tree.h"
 #include "utils.h"
 
 #define PENDING_EXTENT_INSERT 0
@@ -2292,6 +2293,11 @@  static int __free_extent(struct btrfs_trans_handle *trans,
 			BUG_ON(ret);
 		}
 
+		ret = add_to_free_space_tree(trans, bytenr, num_bytes);
+		if (ret) {
+			goto fail;
+		}
+
 		update_block_group(trans->fs_info, bytenr, num_bytes, 0,
 				   mark_free);
 	}
@@ -2630,6 +2636,9 @@  static int alloc_reserved_tree_block(struct btrfs_trans_handle *trans,
 	btrfs_mark_buffer_dirty(leaf);
 	btrfs_free_path(path);
 
+	ret = remove_from_free_space_tree(trans, ins->objectid, fs_info->nodesize);
+	if (ret)
+		return ret;
 	ret = update_block_group(fs_info, ins->objectid, fs_info->nodesize,
 				 1, 0);
 	return ret;