Re: [RFC PATCH v1 2/3] blk-cgroup: store blkcg in bio instead of blkg

[Christoph Hellwig <hch@lst.de>]

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.