[PATCH 1/3] btrfs: dev stat drop useless goto

[PATCH 1/3] btrfs: dev stat drop useless goto

[Anand Jain]

In the function btrfs_init_dev_stats() goto out is not needed, because the
alloc has failed. So just return -ENOMEM.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 fs/btrfs/volumes.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 9b684adad81c..bd279db0f760 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -7289,10 +7289,8 @@ int btrfs_init_dev_stats(struct btrfs_fs_info *fs_info)
 	int i;
 
 	path = btrfs_alloc_path();
-	if (!path) {
-		ret = -ENOMEM;
-		goto out;
-	}
+	if (!path)
+		return -ENOMEM;
 
 	mutex_lock(&fs_devices->device_list_mutex);
 	list_for_each_entry(device, &fs_devices->devices, dev_list) {
@@ -7332,7 +7330,6 @@ int btrfs_init_dev_stats(struct btrfs_fs_info *fs_info)
 	}
 	mutex_unlock(&fs_devices->device_list_mutex);
 
-out:
 	btrfs_free_path(path);
 	return ret < 0 ? ret : 0;
 }
-- 
2.21.0 (Apple Git-120)

[PATCH 2/3] btrfs: dev stats item key conversion per cpu type is not needed

[Anand Jain]

%found_key is not used, drop it.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 fs/btrfs/volumes.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index bd279db0f760..a343aa9cf5ba 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -7278,7 +7278,6 @@ void btrfs_init_devices_late(struct btrfs_fs_info *fs_info)
 int btrfs_init_dev_stats(struct btrfs_fs_info *fs_info)
 {
 	struct btrfs_key key;
-	struct btrfs_key found_key;
 	struct btrfs_root *dev_root = fs_info->dev_root;
 	struct btrfs_fs_devices *fs_devices = fs_info->fs_devices;
 	struct extent_buffer *eb;
@@ -7310,7 +7309,6 @@ int btrfs_init_dev_stats(struct btrfs_fs_info *fs_info)
 		}
 		slot = path->slots[0];
 		eb = path->nodes[0];
-		btrfs_item_key_to_cpu(eb, &found_key, slot);
 		item_size = btrfs_item_size_nr(eb, slot);
 
 		ptr = btrfs_item_ptr(eb, slot,
-- 
2.21.0 (Apple Git-120)

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

[Anand Jain]

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

Re: [PATCH 2/3] btrfs: dev stats item key conversion per cpu type is not needed

[David Sterba]

On Wed, Aug 21, 2019 at 05:26:33PM +0800, Anand Jain wrote:
> %found_key is not used, drop it.

Patches that remove dead/unused code should say why, eg. in this case
the variable hasn't been used since the beginning, but in other cases it
may point to code that needs closer attention.

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

[David Sterba]

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)

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

[David Sterba]

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)

Re: [PATCH 1/3] btrfs: dev stat drop useless goto

[David Sterba]

On Wed, Aug 21, 2019 at 05:26:32PM +0800, Anand Jain wrote:
> In the function btrfs_init_dev_stats() goto out is not needed, because the
> alloc has failed. So just return -ENOMEM.
> 
> Signed-off-by: Anand Jain <anand.jain@oracle.com>

I'll add 1 and 2 to misc-next, 3 seems to be buggy.