nilfs2: delete unnecessary checks before brelse()

[PATCH] nilfs2: delete unnecessary checks before brelse()

Posted by cgel.zte@gmail.com 3 years, 7 months ago

From: Minghao Chi <chi.minghao@zte.com.cn>

The brelse() function tests whether its argument is NULL
and then returns immediately.
Thus remove the tests which are not needed around the shown calls.

Reported-by: Zeal Robot <zealci@zte.com.cn>
Signed-off-by: Minghao Chi <chi.minghao@zte.com.cn>
---
 fs/nilfs2/btree.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/fs/nilfs2/btree.c b/fs/nilfs2/btree.c
index f544c22fff78..b3b5a46488c2 100644
--- a/fs/nilfs2/btree.c
+++ b/fs/nilfs2/btree.c
@@ -1668,8 +1668,7 @@ static int nilfs_btree_check_delete(struct nilfs_bmap *btree, __u64 key)
 	maxkey = nilfs_btree_node_get_key(node, nchildren - 1);
 	nextmaxkey = (nchildren > 1) ?
 		nilfs_btree_node_get_key(node, nchildren - 2) : 0;
-	if (bh != NULL)
-		brelse(bh);
+	brelse(bh);
 
 	return (maxkey == key) && (nextmaxkey < NILFS_BMAP_LARGE_LOW);
 }
@@ -1717,8 +1716,7 @@ static int nilfs_btree_gather_data(struct nilfs_bmap *btree,
 		ptrs[i] = le64_to_cpu(dptrs[i]);
 	}
 
-	if (bh != NULL)
-		brelse(bh);
+	brelse(bh);
 
 	return nitems;
 }
-- 
2.25.1

RE: [PATCH] nilfs2: delete unnecessary checks before brelse()

Posted by David Laight 3 years, 7 months ago

From: cgel.zte@gmail.com
> Sent: 19 August 2022 09:17
> 
> The brelse() function tests whether its argument is NULL
> and then returns immediately.
> Thus remove the tests which are not needed around the shown calls.

The test is the caller makes sense if it is a 'hot' path
and bh is normally NULL.

	David

> 
> Reported-by: Zeal Robot <zealci@zte.com.cn>
> Signed-off-by: Minghao Chi <chi.minghao@zte.com.cn>
> ---
>  fs/nilfs2/btree.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/nilfs2/btree.c b/fs/nilfs2/btree.c
> index f544c22fff78..b3b5a46488c2 100644
> --- a/fs/nilfs2/btree.c
> +++ b/fs/nilfs2/btree.c
> @@ -1668,8 +1668,7 @@ static int nilfs_btree_check_delete(struct nilfs_bmap *btree, __u64 key)
>  	maxkey = nilfs_btree_node_get_key(node, nchildren - 1);
>  	nextmaxkey = (nchildren > 1) ?
>  		nilfs_btree_node_get_key(node, nchildren - 2) : 0;
> -	if (bh != NULL)
> -		brelse(bh);
> +	brelse(bh);
> 
>  	return (maxkey == key) && (nextmaxkey < NILFS_BMAP_LARGE_LOW);
>  }
> @@ -1717,8 +1716,7 @@ static int nilfs_btree_gather_data(struct nilfs_bmap *btree,
>  		ptrs[i] = le64_to_cpu(dptrs[i]);
>  	}
> 
> -	if (bh != NULL)
> -		brelse(bh);
> +	brelse(bh);
> 
>  	return nitems;
>  }
> --
> 2.25.1

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

Re: [PATCH] nilfs2: delete unnecessary checks before brelse()

Posted by Ryusuke Konishi 3 years, 7 months ago

Hi David,

On Sat, Aug 20, 2022 at 6:31 AM David Laight wrote:
>
> From: cgel.zte@gmail.com
> > Sent: 19 August 2022 09:17
> >
> > The brelse() function tests whether its argument is NULL
> > and then returns immediately.
> > Thus remove the tests which are not needed around the shown calls.
>

> The test is the caller makes sense if it is a 'hot' path
> and bh is normally NULL.
>
>         David

Should the commit message say "inline function" instead of "function"?

or do you have other intentions?

These tests indeed seem meaningless, since brelse() is an inline
function that tests whether bh is NULL or not, like this

 static inline void brelse(struct buffer_head *bh)
 {
        if (bh)
                __brelse(bh);
 }

and the corresponding parts are equivalent to:

  if (bh != NULL)
          if (bh)
                  __brelse(bh);


Regards,
Ryusuke Konishi

>
> >
> > Reported-by: Zeal Robot <zealci@zte.com.cn>
> > Signed-off-by: Minghao Chi <chi.minghao@zte.com.cn>
> > ---
> >  fs/nilfs2/btree.c | 6 ++----
> >  1 file changed, 2 insertions(+), 4 deletions(-)
> >
> > diff --git a/fs/nilfs2/btree.c b/fs/nilfs2/btree.c
> > index f544c22fff78..b3b5a46488c2 100644
> > --- a/fs/nilfs2/btree.c
> > +++ b/fs/nilfs2/btree.c
> > @@ -1668,8 +1668,7 @@ static int nilfs_btree_check_delete(struct nilfs_bmap *btree, __u64 key)
> >       maxkey = nilfs_btree_node_get_key(node, nchildren - 1);
> >       nextmaxkey = (nchildren > 1) ?
> >               nilfs_btree_node_get_key(node, nchildren - 2) : 0;
> > -     if (bh != NULL)
> > -             brelse(bh);
> > +     brelse(bh);
> >
> >       return (maxkey == key) && (nextmaxkey < NILFS_BMAP_LARGE_LOW);
> >  }
> > @@ -1717,8 +1716,7 @@ static int nilfs_btree_gather_data(struct nilfs_bmap *btree,
> >               ptrs[i] = le64_to_cpu(dptrs[i]);
> >       }
> >
> > -     if (bh != NULL)
> > -             brelse(bh);
> > +     brelse(bh);
> >
> >       return nitems;
> >  }
> > --
> > 2.25.1

Re: [PATCH] nilfs2: delete unnecessary checks before brelse()

Posted by Ryusuke Konishi 3 years, 7 months ago

On Fri, Aug 19, 2022 at 5:17 PM wrote:
>
> From: Minghao Chi <chi.minghao@zte.com.cn>
>
> The brelse() function tests whether its argument is NULL
> and then returns immediately.
> Thus remove the tests which are not needed around the shown calls.
>
> Reported-by: Zeal Robot <zealci@zte.com.cn>
> Signed-off-by: Minghao Chi <chi.minghao@zte.com.cn>
> ---
>  fs/nilfs2/btree.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)

Will apply, thanks!

Ryusuke Konishi