Due to the previously fix of __bch_btree_node_alloc, the return value will
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 <[email protected]>
---
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(struct btree *b,
{
struct btree *n = bch_btree_node_alloc(b->c, op, b->level, b->parent);
- 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, struct btree_op *op,
for (i = 0; i < nodes; i++) {
new_nodes[i] = btree_node_alloc_replacement(r[i].b, NULL);
- 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, struct btree_op *op,
if (should_rewrite) {
n = 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 = "cannot allocate new btree root";
c->root = __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
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 <[email protected]> 于2023年2月3日周五 10:28写道：
>
> Due to the previously fix of __bch_btree_node_alloc, the return value will
> 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 <[email protected]>
> ---
> 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(struct btree *b,
> {
> struct btree *n = bch_btree_node_alloc(b->c, op, b->level, b->parent);
>
> - 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, struct btree_op *op,
>
> for (i = 0; i < nodes; i++) {
> new_nodes[i] = btree_node_alloc_replacement(r[i].b, NULL);
> - 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, struct btree_op *op,
> if (should_rewrite) {
> n = 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 = "cannot allocate new btree root";
> c->root = __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
>
> 2023年2月3日 10:43，Zheng Hacker <[email protected]> 写道：
>
> 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.
This was why I suggested you to post an extra cleanup patch like this.
You may just add more changes as you mentioned into this patch and post another updated version.
Thanks.
Coly Li
>
> Thanks,
> Zheng Wang
>
> Zheng Wang <[email protected]> 于2023年2月3日周五 10:28写道：
>>
>> Due to the previously fix of __bch_btree_node_alloc, the return value will
>> 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 <[email protected]>
>> ---
>> 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(struct btree *b,
>> {
>> struct btree *n = bch_btree_node_alloc(b->c, op, b->level, b->parent);
>>
>> - 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, struct btree_op *op,
>>
>> for (i = 0; i < nodes; i++) {
>> new_nodes[i] = btree_node_alloc_replacement(r[i].b, NULL);
>> - 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, struct btree_op *op,
>> if (should_rewrite) {
>> n = 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 = "cannot allocate new btree root";
>> c->root = __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
>>
Coly Li <[email protected]> 于2023年2月3日周五 10:46写道：
>
>
>
> > 2023年2月3日 10:43，Zheng Hacker <[email protected]> 写道：
> >
> > 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.
>
> This was why I suggested you to post an extra cleanup patch like this.
>
> You may just add more changes as you mentioned into this patch and post another updated version.
>
Get it. Will do right now.
Thanks,
Zheng Wang