Re: [PATCH 3/3] btrfs: clean search for device item in finish sprout

[David Sterba <dsterba@suse.cz>]

On Wed, Aug 21, 2019 at 05:26:34PM +0800, Anand Jain wrote:
> No need to btrfs_item_key_to_cpu() as we continue to next leaf. Also keep
> the found_key and search key separate.
> 
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
>  fs/btrfs/volumes.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index a343aa9cf5ba..1db06894aee6 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -2471,6 +2471,7 @@ static int btrfs_finish_sprout(struct btrfs_trans_handle *trans)
>  	struct extent_buffer *leaf;
>  	struct btrfs_dev_item *dev_item;
>  	struct btrfs_device *device;
> +	struct btrfs_key found_key;
>  	struct btrfs_key key;
>  	u8 fs_uuid[BTRFS_FSID_SIZE];
>  	u8 dev_uuid[BTRFS_UUID_SIZE];
> @@ -2498,15 +2499,13 @@ static int btrfs_finish_sprout(struct btrfs_trans_handle *trans)
>  				break;
>  			if (ret < 0)
>  				goto error;
> -			leaf = path->nodes[0];
> -			btrfs_item_key_to_cpu(leaf, &key, path->slots[0]);

This goes back to while, so in this step, 'key' is set up for the next
search, but you remove it.

>  			btrfs_release_path(path);
>  			continue;
>  		}
>  
> -		btrfs_item_key_to_cpu(leaf, &key, path->slots[0]);
> -		if (key.objectid != BTRFS_DEV_ITEMS_OBJECTID ||
> -		    key.type != BTRFS_DEV_ITEM_KEY)
> +		btrfs_item_key_to_cpu(leaf, &found_key, path->slots[0]);

And here 'key' is not updated as well. 'found_key' does not lead to the
same behaviour as it's only set here and check in the if below, but
nothing else.

This would probably lead to an infinite loop in the search slot.

> +		if (found_key.objectid != BTRFS_DEV_ITEMS_OBJECTID ||
> +		    found_key.type != BTRFS_DEV_ITEM_KEY)
>  			break;
>  
>  		dev_item = btrfs_item_ptr(leaf, path->slots[0],
> -- 
> 2.21.0 (Apple Git-120)