MIME-Version: 1.0
References: <20230203022759.576832-1-zyytlz.wz@163.com>
In-Reply-To: <20230203022759.576832-1-zyytlz.wz@163.com>
From:   Zheng Hacker <hackerzheng666@gmail.com>
Date:   Fri, 3 Feb 2023 10:43:36 +0800
Message-ID: <CAJedcCzNB+1byPEzEgQ6ELjeoRQcZ=GnZg+1J4+FZvfnoK0H2Q@mail.gmail.com>
Subject: Re: [PATCH] bcache: Remove some unnecessary NULL point check for the
 return value of __bch_btree_node_alloc-related pointer
To:     Zheng Wang <zyytlz.wz@163.com>
Cc:     colyli@suse.de, stable@vger.kernel.org, kent.overstreet@gmail.com,
        linux-bcache@vger.kernel.org, linux-kernel@vger.kernel.org,
        alex000young@gmail.com
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Hello,

After writing the patch, I found there may be more places to replace
IS_ERR_OR_NULL to IS_ERR.
If the alloc of node will never be NULL, the additional NULL check of
nodes after allocation may also
be useless. This patch just fixes the check around the alloc. I'm not
sure about other places for my
limited understanding of the code in bcache.

Thanks,
Zheng Wang

Zheng Wang <zyytlz.wz@163.com> =E4=BA=8E2023=E5=B9=B42=E6=9C=883=E6=97=A5=
=E5=91=A8=E4=BA=94 10:28=E5=86=99=E9=81=93=EF=BC=9A
>
> Due to the previously fix of __bch_btree_node_alloc, the return value wil=
l
> never be a NULL pointer. So IS_ERR is enough to handle the failure
>  situation. Fix it by replacing IS_ERR_OR_NULL check to IS_ERR check.
>
> Fixes: cafe56359144 ("bcache: A block layer cache")
> Signed-off-by: Zheng Wang <zyytlz.wz@163.com>
> ---
>  drivers/md/bcache/btree.c | 6 +++---
>  drivers/md/bcache/super.c | 2 +-
>  2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/md/bcache/btree.c b/drivers/md/bcache/btree.c
> index 147c493a989a..417cd7c436c4 100644
> --- a/drivers/md/bcache/btree.c
> +++ b/drivers/md/bcache/btree.c
> @@ -1138,7 +1138,7 @@ static struct btree *btree_node_alloc_replacement(s=
truct btree *b,
>  {
>         struct btree *n =3D bch_btree_node_alloc(b->c, op, b->level, b->p=
arent);
>
> -       if (!IS_ERR_OR_NULL(n)) {
> +       if (!IS_ERR(n)) {
>                 mutex_lock(&n->write_lock);
>                 bch_btree_sort_into(&b->keys, &n->keys, &b->c->sort);
>                 bkey_copy_key(&n->key, &b->key);
> @@ -1352,7 +1352,7 @@ static int btree_gc_coalesce(struct btree *b, struc=
t btree_op *op,
>
>         for (i =3D 0; i < nodes; i++) {
>                 new_nodes[i] =3D btree_node_alloc_replacement(r[i].b, NUL=
L);
> -               if (IS_ERR_OR_NULL(new_nodes[i]))
> +               if (IS_ERR(new_nodes[i]))
>                         goto out_nocoalesce;
>         }
>
> @@ -1669,7 +1669,7 @@ static int bch_btree_gc_root(struct btree *b, struc=
t btree_op *op,
>         if (should_rewrite) {
>                 n =3D btree_node_alloc_replacement(b, NULL);
>
> -               if (!IS_ERR_OR_NULL(n)) {
> +               if (!IS_ERR(n)) {
>                         bch_btree_node_write_sync(n);
>
>                         bch_btree_set_root(n);
> diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
> index ba3909bb6bea..92de714fe75e 100644
> --- a/drivers/md/bcache/super.c
> +++ b/drivers/md/bcache/super.c
> @@ -2088,7 +2088,7 @@ static int run_cache_set(struct cache_set *c)
>
>                 err =3D "cannot allocate new btree root";
>                 c->root =3D __bch_btree_node_alloc(c, NULL, 0, true, NULL=
);
> -               if (IS_ERR_OR_NULL(c->root))
> +               if (IS_ERR(c->root))
>                         goto err;
>
>                 mutex_lock(&c->root->write_lock);
> --
> 2.25.1
>