[PATCH] btrfs: Remove redundant code

[PATCH] btrfs: Remove redundant code

[Nikolay Borisov]

insert_into_bitmap has only one caller which always allocates the info struct
passed. As such remove the any NULL checks for info and also remove code
to allocate info in case it was NULL.

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
 fs/btrfs/free-space-cache.c | 13 +------------
 1 file changed, 1 insertion(+), 12 deletions(-)

diff --git a/fs/btrfs/free-space-cache.c b/fs/btrfs/free-space-cache.c
index c5e6180cdb8c..fd24fb99d6dc 100644
--- a/fs/btrfs/free-space-cache.c
+++ b/fs/btrfs/free-space-cache.c
@@ -2101,7 +2101,7 @@ static int insert_into_bitmap(struct btrfs_free_space_ctl *ctl,
 		goto again;
 
 new_bitmap:
-	if (info && info->bitmap) {
+	if (info->bitmap) {
 		add_new_bitmap(ctl, info, offset);
 		added = 1;
 		info = NULL;
@@ -2109,17 +2109,6 @@ static int insert_into_bitmap(struct btrfs_free_space_ctl *ctl,
 	} else {
 		spin_unlock(&ctl->tree_lock);
 
-		/* no pre-allocated info, allocate a new one */
-		if (!info) {
-			info = kmem_cache_zalloc(btrfs_free_space_cachep,
-						 GFP_NOFS);
-			if (!info) {
-				spin_lock(&ctl->tree_lock);
-				ret = -ENOMEM;
-				goto out;
-			}
-		}
-
 		/* allocate the bitmap */
 		info->bitmap = kzalloc(PAGE_SIZE, GFP_NOFS);
 		spin_lock(&ctl->tree_lock);
-- 
2.7.4

Re: [PATCH] btrfs: Remove redundant code

[David Sterba]

On Wed, Jul 12, 2017 at 09:32:15AM +0300, Nikolay Borisov wrote:
> insert_into_bitmap has only one caller which always allocates the info struct
> passed. As such remove the any NULL checks for info and also remove code
> to allocate info in case it was NULL.
> 
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> ---
>  fs/btrfs/free-space-cache.c | 13 +------------
>  1 file changed, 1 insertion(+), 12 deletions(-)
> 
> diff --git a/fs/btrfs/free-space-cache.c b/fs/btrfs/free-space-cache.c
> index c5e6180cdb8c..fd24fb99d6dc 100644
> --- a/fs/btrfs/free-space-cache.c
> +++ b/fs/btrfs/free-space-cache.c
> @@ -2101,7 +2101,7 @@ static int insert_into_bitmap(struct btrfs_free_space_ctl *ctl,
>  		goto again;
>  
>  new_bitmap:
> -	if (info && info->bitmap) {
> +	if (info->bitmap) {
>  		add_new_bitmap(ctl, info, offset);
>  		added = 1;
>  		info = NULL;

What if we reach this point, go back to label "again:", come back to
this check again, then info would be NULL and dereferencing info->bitmap
would crash. Then the below code is still required.

> @@ -2109,17 +2109,6 @@ static int insert_into_bitmap(struct btrfs_free_space_ctl *ctl,
>  	} else {
>  		spin_unlock(&ctl->tree_lock);
>  
> -		/* no pre-allocated info, allocate a new one */
> -		if (!info) {
> -			info = kmem_cache_zalloc(btrfs_free_space_cachep,
> -						 GFP_NOFS);
> -			if (!info) {
> -				spin_lock(&ctl->tree_lock);
> -				ret = -ENOMEM;
> -				goto out;
> -			}
> -		}
> -
>  		/* allocate the bitmap */
>  		info->bitmap = kzalloc(PAGE_SIZE, GFP_NOFS);
>  		spin_lock(&ctl->tree_lock);
> -- 
> 2.7.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] btrfs: remove redundant code

[flyingpeng(彭浩)]

Since leaf is already NULL, and no other branch will go fail_unlock,
the fail_unlock branch is useless.

Signed-off-by: Peng Hao <flyingpeng@tencent.com>
---
 fs/btrfs/disk-io.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 4c3166f3c725..f39165aec175 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -1284,7 +1284,7 @@ struct btrfs_root *btrfs_create_tree(struct btrfs_trans_handle *trans,
        if (IS_ERR(leaf)) {
                ret = PTR_ERR(leaf);
                leaf = NULL;
-               goto fail_unlock;
+               goto fail;
        }

        root->node = leaf;
@@ -1319,9 +1319,6 @@ struct btrfs_root *btrfs_create_tree(struct btrfs_trans_handle *trans,

        return root;

-fail_unlock:
-       if (leaf)
-               btrfs_tree_unlock(leaf);
 fail:
        btrfs_put_root(root);

--
2.27.0

[PATCH] btrfs: remove redundant code

[Hao Peng]

From: Peng Hao <flyingpeng@tencent.com>

Since leaf is already NULL, and no other branch will go fail_unlock,
the fail_unlock branch is useless.

Signed-off-by: Peng Hao <flyingpeng@tencent.com>
---
 fs/btrfs/disk-io.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 4c3166f3c725..f39165aec175 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -1284,7 +1284,7 @@ struct btrfs_root *btrfs_create_tree(struct
btrfs_trans_handle *trans,
        if (IS_ERR(leaf)) {
                ret = PTR_ERR(leaf);
                leaf = NULL;
-               goto fail_unlock;
+               goto fail;
        }

        root->node = leaf;
@@ -1319,9 +1319,6 @@ struct btrfs_root *btrfs_create_tree(struct
btrfs_trans_handle *trans,

        return root;

-fail_unlock:
-       if (leaf)
-               btrfs_tree_unlock(leaf);
 fail:
        btrfs_put_root(root);

--
2.27.0

Re: [PATCH] btrfs: remove redundant code

[David Sterba]

On Fri, Sep 23, 2022 at 10:56:12PM +0800, Hao Peng wrote:
> From: Peng Hao <flyingpeng@tencent.com>
> 
> Since leaf is already NULL, and no other branch will go fail_unlock,
> the fail_unlock branch is useless.

The patch does not aply cleanly, git am says it's corrupted on line 10
but I don't see anything obvious.

The change makes sense, I'll change the subject to "btrfs: simplify
cleanup after error in btrfs_create_tree" and add it to misc-next,
thanks.