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 <osan...@fb.com>
A couple of really trivial nitpicks below that you should feel free to
ignore ;)
> Signed-off-by: Nikolay Borisov <nbori...@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?
