Hi, 在 2026/8/4 21:32, Christoph Hellwig 写道: > On Tue, Aug 04, 2026 at 05:19:24PM +0800, Tao Cui wrote: >> While reading 2/3, one spot in bio_pinned_blkg() made me wonder, so I >> gave it a try — and the WARN_ON_ONCE triggers every time for me. >> >> I may well be missing something, but my worry is that the bio's ref on >> the blkg keeps the object alive, not its entry in the radix tree. >> blkg_destroy() runs throtl_pd_offline (which only schedules an async >> flush) before radix_tree_delete(), so the queued bio ends up dispatched >> (blk_throtl_dispatch_work_fn -> blk_cgroup_bio_start -> >> bio_pinned_blkg) after the blkg is already gone from the tree, and >> blkg_lookup() returns NULL. >> >> I applied the series and wrote a small reproducer: >> >> - null_blk, cgroup v2, a child cgroup with io.max rbps=4096; >> - a read issued in the child cgroup gets throttled and queued, pinning >> the blkg; >> - migrate the reader out and rmdir the cgroup; the queued bio is then >> flushed after the blkg has left the tree. > Can you add this to blktests? > >> Maybe keeping the pinned blkg pointer in the bio would sidestep this, so >> the lookup can't miss?
The problem here is that blkg_destroy can be called while blkg is still pinned by blkg_get, in this case remove the cgroup directly remove the blkg from radix tree, that's why blkg_lookup can't find this blkg anymore, and the extra blkg ref is leaked :( > That would grow the bio, which we try hard to avoid. I think the way to > avoid this is to have active/passive refcounts on the blkg, where an > active one keeps it in the radix tree, but a 0 passive one would prevent > the caller from getting a new reference to it. The users who rely on the > pin for the I/O completion path would then just keep the active reference > and use a pure lookup without getting a new passive reference in the > completion path. This would remove the need for BIO_BLKG_REF which > feels a bit kludgy and eats up precious bio flag space. The problem here is that remove a cgroup can also remove the blkg from radix tree, even through it still has active refcounts. I think this can be fixed by checking the cgroup online_pin first, if it's zero, we can search the blkg from the request_queue blkg list, where blkg will not be removed until blkg_free_workfn(). What's better, if we can convert the blkg list to hash table with key as blk-cgroup, it will be much better as we can lookup from this table instead of blkcg radix tree. > -- Thanks, Kuai
