Re: [RFC PATCH v1 2/3] blk-cgroup: store blkcg in bio instead of blkg
Christoph Hellwig <[email protected]> Tue, 4 Aug 2026 15:32:08 +0200
| Newsgroups | gmane.linux.documentation,gmane.linux.kernel.cgroups,gmane.linux.kernel,gmane.linux.block,gmane.linux.kernel.bcache.devel,gmane.linux.raid,gmane.linux.file-systems,gmane.linux.kernel.mm |
|---|---|
| Message-ID | <[email protected]> |
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? 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.