Re: [PATCH] Btrfs: fix extent_from_logical to deal with skinny metadata

[Wang Shilong <wangsl.fnst@cn.fujitsu.com>]

Hello Josef,

I have sent a patch before to address this issue:
       https://patchwork.kernel.org/patch/3472211/

I introduced another function btrfs_previous_extent_item() to
search previous extent item, I think you must miss my path before
giving this patch ^_^.

Thanks,
Wang

On 01/24/2014 05:06 AM, Josef Bacik wrote:
> I don't think this is an issue and I've not seen it in practice but
> extent_from_logical will fail to find a skinny extent because it uses
> btrfs_previous_item and gives it the normal extent item type.  This is just not
> a place to use btrfs_previous_item since we care about either normal extents or
> skinny extents, so open code btrfs_previous_item to properly check.  This would
> only affect metadata and the only place this is used for metadata is scrub and
> I'm pretty sure it's just for printing stuff out, not actually doing any work so
> hopefully it was never a problem other than a cosmetic one.  Thanks,
>
> Signed-off-by: Josef Bacik <jbacik@fb.com>
> ---
>   fs/btrfs/backref.c | 41 +++++++++++++++++++++++++++++++++--------
>   1 file changed, 33 insertions(+), 8 deletions(-)
>
> diff --git a/fs/btrfs/backref.c b/fs/btrfs/backref.c
> index 34a8952..dcf2448 100644
> --- a/fs/btrfs/backref.c
> +++ b/fs/btrfs/backref.c
> @@ -1302,20 +1302,45 @@ int extent_from_logical(struct btrfs_fs_info *fs_info, u64 logical,
>   	ret = btrfs_search_slot(NULL, fs_info->extent_root, &key, path, 0, 0);
>   	if (ret < 0)
>   		return ret;
> -	ret = btrfs_previous_item(fs_info->extent_root, path,
> -					0, BTRFS_EXTENT_ITEM_KEY);
> -	if (ret < 0)
> -		return ret;
>   
> -	btrfs_item_key_to_cpu(path->nodes[0], found_key, path->slots[0]);
> +	while (1) {
> +		u32 nritems;
> +		if (path->slots[0] == 0) {
> +			btrfs_set_path_blocking(path);
> +			ret = btrfs_prev_leaf(fs_info->extent_root, path);
> +			if (ret != 0) {
> +				if (ret > 0) {
> +					pr_debug("logical %llu is not within "
> +						 "any extent\n", logical);
> +					ret = -ENOENT;
> +				}
> +				return ret;
> +			}
> +		} else {
> +			path->slots[0]--;
> +		}
> +		nritems = btrfs_header_nritems(path->nodes[0]);
> +		if (nritems == 0) {
> +			pr_debug("logical %llu is not within any extent\n",
> +				 logical);
> +			return -ENOENT;
> +		}
> +		if (path->slots[0] == nritems)
> +			path->slots[0]--;
> +
> +		btrfs_item_key_to_cpu(path->nodes[0], found_key,
> +				      path->slots[0]);
> +		if (found_key->type == BTRFS_EXTENT_ITEM_KEY ||
> +		    found_key->type == BTRFS_METADATA_ITEM_KEY)
> +			break;
> +	}
> +
>   	if (found_key->type == BTRFS_METADATA_ITEM_KEY)
>   		size = fs_info->extent_root->leafsize;
>   	else if (found_key->type == BTRFS_EXTENT_ITEM_KEY)
>   		size = found_key->offset;
>   
> -	if ((found_key->type != BTRFS_EXTENT_ITEM_KEY &&
> -	     found_key->type != BTRFS_METADATA_ITEM_KEY) ||
> -	    found_key->objectid > logical ||
> +	if (found_key->objectid > logical ||
>   	    found_key->objectid + size <= logical) {
>   		pr_debug("logical %llu is not within any extent\n", logical);
>   		return -ENOENT;