Re: [PATCH v7 2/2] btrfs: Introduce new mount option to skip block group items scan

[Anand Jain <anand.jain@oracle.com>]

> @@ -2010,6 +2056,9 @@ int btrfs_read_block_groups(struct btrfs_fs_info *info)
>   	int need_clear = 0;
>   	u64 cache_gen;
>   
> +	if (btrfs_test_opt(info, SKIPBG))
> +		return fill_dummy_bgs(info);
> +

   Could it first read the block group if it fails then check mount
   option skip + other required options are set to continue/abort?


> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index f8ec2d8606fd..84d62bd53940 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -2269,11 +2269,15 @@ static int btrfs_read_roots(struct btrfs_fs_info *fs_info)
>   
>   	root = btrfs_read_tree_root(tree_root, &location);
>   	if (IS_ERR(root)) {
> -		ret = PTR_ERR(root);
> -		goto out;
> +		if (!btrfs_test_opt(fs_info, SKIPBG)) {
> +			ret = PTR_ERR(root);
> +			goto out;
> +		}

  Needs a btrfs_warn().

> @@ -2215,9 +2238,12 @@ static int btrfs_statfs(struct dentry *dentry, struct kstatfs *buf)
>   	 * not fit in the free metadata space.  If we aren't ->full then we
>   	 * still can allocate chunks and thus are fine using the currently
>   	 * calculated f_bavail.
> +	 *
> +	 * Or if we're rescuing, set available to 0 anyway.
 >
>   	 */
> -	if (!mixed && block_rsv->space_info->full &&
> -	    total_free_meta - thresh < block_rsv->size)
> +	if (btrfs_test_opt(fs_info, SKIPBG) ||
> +	    (!mixed && block_rsv->space_info->full &&
> +	     total_free_meta - thresh < block_rsv->size))
>   		buf->f_bavail = 0;
>

  I wonder why is this necessary? when RO and nologreply mount options
  are prerequisites of the skip mount option.

  Also its not a good idea that df reports 0 available size.


> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index 0d6e785bcb98..f89625de1fff 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -7594,6 +7594,13 @@ int btrfs_verify_dev_extents(struct btrfs_fs_info *fs_info)
>   	u64 prev_dev_ext_end = 0;
>   	int ret = 0;
>   
> +	/*
> +	 * For rescue=skipbg mount option, we're already RO and are salvaging
> +	 * data, no need for such strict check.
> +	 */
> +	if (btrfs_test_opt(fs_info, SKIPBG))
> +		return 0;
> +

  Here too, can we first verify if the dev extents actually fail, and
  then check if skip + other necessary mount options are set to
  continue/abort the mount. ?

Thanks, Anand