[3/3] btrfs: clean search for device item in finish sprout
diff mbox series

Message ID 20190821092634.6778-3-anand.jain@oracle.com
State New
Headers show
Series
  • [1/3] btrfs: dev stat drop useless goto
Related show

Commit Message

Anand Jain Aug. 21, 2019, 9:26 a.m. UTC
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(-)

Comments

David Sterba Aug. 21, 2019, 1:54 p.m. UTC | #1
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;

The declaration should be in the scope of use, ie. inside the while
loop.

>  	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]);
>  			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]);
> +		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)
David Sterba Aug. 21, 2019, 2:01 p.m. UTC | #2
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)

Patch
diff mbox series

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]);
 			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]);
+		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],