2022-08-19 08:39:51

by CGEL

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

From: Minghao Chi <[email protected]>

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 <[email protected]>
Signed-off-by: Minghao Chi <[email protected]>
---
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

2022-08-19 09:02:46

by Ryusuke Konishi

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

On Fri, Aug 19, 2022 at 5:17 PM wrote:
>
> From: Minghao Chi <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: Minghao Chi <[email protected]>
> ---
> fs/nilfs2/btree.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)

Will apply, thanks!

Ryusuke Konishi

2022-08-19 21:43:26

by David Laight

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

From: [email protected]
> 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 <[email protected]>
> Signed-off-by: Minghao Chi <[email protected]>
> ---
> 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)

2022-08-19 23:55:58

by Ryusuke Konishi

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

Hi David,

On Sat, Aug 20, 2022 at 6:31 AM David Laight wrote:
>
> From: [email protected]
> > 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 <[email protected]>
> > Signed-off-by: Minghao Chi <[email protected]>
> > ---
> > 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