* [PATCH] blk-cgroup: fix blkg->online set on radix_tree_insert failure
@ 2026-05-06 13:11 Tao Cui
2026-05-07 4:42 ` [PATCH v2] blk-cgroup: fix leaks and online flag " Tao Cui
2026-05-07 6:12 ` [PATCH v3] " Tao Cui
0 siblings, 2 replies; 4+ messages in thread
From: Tao Cui @ 2026-05-06 13:11 UTC (permalink / raw)
To: tj, josef, axboe, cgroups; +Cc: Tao Cui
In blkg_create(), blkg->online was set to true unconditionally outside
the radix_tree_insert success check. When the insertion fails, the blkg
is not in the radix tree nor the hash/list, yet it is incorrectly marked
online. This state inconsistency could cause future code paths that
depend on blkg->online to behave incorrectly.
Move blkg->online = true inside the if (likely(!ret)) block so that it
is only set when the blkg is fully initialized and inserted.
Signed-off-by: Tao Cui <cuitao@kylinos.cn>
---
block/blk-cgroup.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
index 554c87bb4a86..539301f4f645 100644
--- a/block/blk-cgroup.c
+++ b/block/blk-cgroup.c
@@ -431,8 +431,8 @@ static struct blkcg_gq *blkg_create(struct blkcg *blkcg, struct gendisk *disk,
blkg->pd[i]->online = true;
}
}
+ blkg->online = true;
}
- blkg->online = true;
spin_unlock(&blkcg->lock);
if (!ret)
--
2.43.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH v2] blk-cgroup: fix leaks and online flag on radix_tree_insert failure
2026-05-06 13:11 [PATCH] blk-cgroup: fix blkg->online set on radix_tree_insert failure Tao Cui
@ 2026-05-07 4:42 ` Tao Cui
2026-05-07 6:12 ` [PATCH v3] " Tao Cui
1 sibling, 0 replies; 4+ messages in thread
From: Tao Cui @ 2026-05-07 4:42 UTC (permalink / raw)
To: cuitao; +Cc: axboe, cgroups, josef, tj
When radix_tree_insert() fails in blkg_create(), the error path has two
issues:
1. blkg->online is set to true unconditionally, even when the blkg was
never fully inserted. Move the assignment inside the success block.
2. The error path calls blkg_put() without first calling
percpu_ref_kill(). Because the refcount is still in percpu mode,
percpu_ref_put() only does this_cpu_sub() without checking for zero,
so blkg_release() is never triggered. This permanently leaks the
blkg memory, its percpu iostat, policy data, the parent blkg
reference, and the cgroup css reference — the latter preventing the
cgroup from ever being destroyed.
Fix by adding percpu_ref_kill() before blkg_put(), matching the pattern
used in blkg_destroy().
Signed-off-by: Tao Cui <cuitao@kylinos.cn>
---
v2:
- Also fix the percpu_ref leak on the radix_tree_insert() error path by
adding percpu_ref_kill() before blkg_put(), as pointed out by the
Sashiko AI review.
---
block/blk-cgroup.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
index 554c87bb4a86..845211f318fc 100644
--- a/block/blk-cgroup.c
+++ b/block/blk-cgroup.c
@@ -431,14 +431,15 @@ static struct blkcg_gq *blkg_create(struct blkcg *blkcg, struct gendisk *disk,
blkg->pd[i]->online = true;
}
}
+ blkg->online = true;
}
- blkg->online = true;
spin_unlock(&blkcg->lock);
if (!ret)
return blkg;
/* @blkg failed fully initialized, use the usual release path */
+ percpu_ref_kill(&blkg->refcnt);
blkg_put(blkg);
return ERR_PTR(ret);
--
2.43.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH v3] blk-cgroup: fix leaks and online flag on radix_tree_insert failure
2026-05-06 13:11 [PATCH] blk-cgroup: fix blkg->online set on radix_tree_insert failure Tao Cui
2026-05-07 4:42 ` [PATCH v2] blk-cgroup: fix leaks and online flag " Tao Cui
@ 2026-05-07 6:12 ` Tao Cui
2026-05-08 0:27 ` Tejun Heo
1 sibling, 1 reply; 4+ messages in thread
From: Tao Cui @ 2026-05-07 6:12 UTC (permalink / raw)
To: cuitao; +Cc: axboe, cgroups, josef, tj
When radix_tree_insert() fails in blkg_create(), the error path has two
issues:
1. blkg->online is set to true unconditionally, even when the blkg was
never fully inserted. Move the assignment inside the success block.
2. The error path calls blkg_put() without first calling
percpu_ref_kill(). Because the refcount is still in percpu mode,
percpu_ref_put() only does this_cpu_sub() without checking for zero,
so blkg_release() is never triggered. This permanently leaks the
blkg memory, its percpu iostat, policy data, the parent blkg
reference, and the cgroup css reference — the latter preventing the
cgroup from ever being destroyed.
Fix by replacing blkg_put() with percpu_ref_kill(), matching the pattern
used in blkg_destroy().
Signed-off-by: Tao Cui <cuitao@kylinos.cn>
---
v3:
- Remove the redundant blkg_put() after percpu_ref_kill() to avoid a
double-put that causes the refcount to go negative and bypass
blkg_release(), as pointed out by the sashiko AI review.
v2:
- Also fix the percpu_ref leak on the radix_tree_insert() error path by
adding percpu_ref_kill() before blkg_put(), as pointed out by the
sashiko AI review.
---
block/blk-cgroup.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
index 554c87bb4a86..9fe850deef3b 100644
--- a/block/blk-cgroup.c
+++ b/block/blk-cgroup.c
@@ -431,15 +431,15 @@ static struct blkcg_gq *blkg_create(struct blkcg *blkcg, struct gendisk *disk,
blkg->pd[i]->online = true;
}
}
+ blkg->online = true;
}
- blkg->online = true;
spin_unlock(&blkcg->lock);
if (!ret)
return blkg;
/* @blkg failed fully initialized, use the usual release path */
- blkg_put(blkg);
+ percpu_ref_kill(&blkg->refcnt);
return ERR_PTR(ret);
err_put_css:
--
2.43.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v3] blk-cgroup: fix leaks and online flag on radix_tree_insert failure
2026-05-07 6:12 ` [PATCH v3] " Tao Cui
@ 2026-05-08 0:27 ` Tejun Heo
0 siblings, 0 replies; 4+ messages in thread
From: Tejun Heo @ 2026-05-08 0:27 UTC (permalink / raw)
To: Tao Cui; +Cc: axboe, cgroups, josef
On Thu, May 07, 2026 at 02:12:29PM +0800, Tao Cui wrote:
> When radix_tree_insert() fails in blkg_create(), the error path has two
> issues:
>
> 1. blkg->online is set to true unconditionally, even when the blkg was
> never fully inserted. Move the assignment inside the success block.
>
> 2. The error path calls blkg_put() without first calling
> percpu_ref_kill(). Because the refcount is still in percpu mode,
> percpu_ref_put() only does this_cpu_sub() without checking for zero,
> so blkg_release() is never triggered. This permanently leaks the
> blkg memory, its percpu iostat, policy data, the parent blkg
> reference, and the cgroup css reference — the latter preventing the
> cgroup from ever being destroyed.
>
> Fix by replacing blkg_put() with percpu_ref_kill(), matching the pattern
> used in blkg_destroy().
>
> Signed-off-by: Tao Cui <cuitao@kylinos.cn>
Acked-by: Tejun Heo <tj@kernel.org>
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-05-08 0:27 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-06 13:11 [PATCH] blk-cgroup: fix blkg->online set on radix_tree_insert failure Tao Cui
2026-05-07 4:42 ` [PATCH v2] blk-cgroup: fix leaks and online flag " Tao Cui
2026-05-07 6:12 ` [PATCH v3] " Tao Cui
2026-05-08 0:27 ` Tejun Heo
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox