[PATCH] Btrfs: remove level==0 check in balance_level

[PATCH] Btrfs: remove level==0 check in balance_level

[Liu Bo]

btrfs_search_slot()
   if (level != 0)
      setup_nodes_for_search()
          balance_level()

It is just impossible to have level=0 in balance_level.

Signed-off-by: Liu Bo <bo.liu@linux.alibaba.com>
---
 fs/btrfs/ctree.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
index 8b31caa60b0a..858085490e23 100644
--- a/fs/btrfs/ctree.c
+++ b/fs/btrfs/ctree.c
@@ -1778,9 +1778,6 @@ static noinline int balance_level(struct btrfs_trans_handle *trans,
 	int orig_slot = path->slots[level];
 	u64 orig_ptr;
 
-	if (level == 0)
-		return 0;
-
 	mid = path->nodes[level];
 
 	WARN_ON(path->locks[level] != BTRFS_WRITE_LOCK &&
-- 
1.8.3.1

Re: [PATCH] Btrfs: remove level==0 check in balance_level

[Nikolay Borisov]

On 12.09.2018 01:06, Liu Bo wrote:
> btrfs_search_slot()
>    if (level != 0)
>       setup_nodes_for_search()
>           balance_level()
> 
> It is just impossible to have level=0 in balance_level.
> 
> Signed-off-by: Liu Bo <bo.liu@linux.alibaba.com>

I concur with the analysis:

Reviewed-by: Nikolay Borisov <nborisov@suse.com>

> ---
>  fs/btrfs/ctree.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
> index 8b31caa60b0a..858085490e23 100644
> --- a/fs/btrfs/ctree.c
> +++ b/fs/btrfs/ctree.c
> @@ -1778,9 +1778,6 @@ static noinline int balance_level(struct btrfs_trans_handle *trans,
>  	int orig_slot = path->slots[level];
>  	u64 orig_ptr;
>  
> -	if (level == 0)
> -		return 0;
> -
>  	mid = path->nodes[level];
>  
>  	WARN_ON(path->locks[level] != BTRFS_WRITE_LOCK &&
> 

Re: [PATCH] Btrfs: remove level==0 check in balance_level

[David Sterba]

On Wed, Sep 12, 2018 at 06:06:23AM +0800, Liu Bo wrote:
> btrfs_search_slot()
>    if (level != 0)
>       setup_nodes_for_search()
>           balance_level()
> 
> It is just impossible to have level=0 in balance_level.

While this is true, what do you think about adding ASSERT(level > 0) ?
This is to catch accidentally passing level 0.

Re: [PATCH] Btrfs: remove level==0 check in balance_level

[Liu Bo]

On Wed, Sep 12, 2018 at 02:52:38PM +0200, David Sterba wrote:
> On Wed, Sep 12, 2018 at 06:06:23AM +0800, Liu Bo wrote:
> > btrfs_search_slot()
> >    if (level != 0)
> >       setup_nodes_for_search()
> >           balance_level()
> > 
> > It is just impossible to have level=0 in balance_level.
> 
> While this is true, what do you think about adding ASSERT(level > 0) ?
> This is to catch accidentally passing level 0.

Sounds good, will update it.

thanks,
-liubo

[PATCH v2] Btrfs: remove level==0 check in balance_level

[Liu Bo]

btrfs_search_slot()
   if (level != 0)
      setup_nodes_for_search()
          balance_level()

It is just impossible to have level=0 in balance_level.

Reviewed-by: Nikolay Borisov <nborisov@suse.com>
Signed-off-by: Liu Bo <bo.liu@linux.alibaba.com>
---
v2: add assertion for level just in case someone breaks the rule in the
future.

 fs/btrfs/ctree.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
index 8b31caa60b0a..ada44c786f2e 100644
--- a/fs/btrfs/ctree.c
+++ b/fs/btrfs/ctree.c
@@ -1778,8 +1778,7 @@ static noinline int balance_level(struct btrfs_trans_handle *trans,
 	int orig_slot = path->slots[level];
 	u64 orig_ptr;
 
-	if (level == 0)
-		return 0;
+	ASSERT(level > 0);
 
 	mid = path->nodes[level];
 
-- 
1.8.3.1

Re: [PATCH v2] Btrfs: remove level==0 check in balance_level

[David Sterba]

On Fri, Sep 14, 2018 at 01:55:59AM +0800, Liu Bo wrote:
> btrfs_search_slot()
>    if (level != 0)
>       setup_nodes_for_search()
>           balance_level()
> 
> It is just impossible to have level=0 in balance_level.
> 
> Reviewed-by: Nikolay Borisov <nborisov@suse.com>
> Signed-off-by: Liu Bo <bo.liu@linux.alibaba.com>

Reviewed-by: David Sterba <dsterba@suse.com>