* [RFC PATCH v1 0/3] blk-cgroup: store blkcg in bio before blkcg_mutex conversion
@ 2026-08-04 6:53 Yu Kuai
2026-08-04 6:53 ` [RFC PATCH v1 1/3] blk-cgroup: add helpers for bio cgroup state Yu Kuai
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Yu Kuai @ 2026-08-04 6:53 UTC (permalink / raw)
To: Jens Axboe
Cc: Tejun Heo, Johannes Weiner, Michal Koutný, Jonathan Corbet,
Yu Kuai, Josef Bacik, Coly Li, Kent Overstreet, Alasdair Kergon,
Mike Snitzer, Mikulas Patocka, Benjamin Marzinski, Song Liu,
Dan Williams, Vishal Verma, Dave Jiang, Alison Schofield,
Pankaj Gupta, Andreas Gruenbacher, Matthew Wilcox, Jan Kara,
Andrew Morton, Chris Li, Kairui Song, Christoph Hellwig,
Nilay Shroff, Tao Cui, cgroups, linux-doc, linux-kernel,
linux-block, linux-bcache, dm-devel, linux-raid, nvdimm,
virtualization, gfs2, linux-fsdevel, linux-mm
From: Yu Kuai <yukuai@fygo.io>
This RFC is a preparatory series for the blkcg_mutex conversion proposed in
the blkcg_mutex RFC v2 series [1]. That series moves queue-local blkg
topology synchronization from q->queue_lock to q->blkcg_mutex. A direct
conversion is awkward while bios store queue-local blkg references, because
bio allocation, bio_set_dev(), remap and clone paths can run in contexts
where creating a missing blkg must not sleep or is unnecessary.
This set makes the bio association queue-independent by storing the blkcg
in the bio instead of a blkg. A blkg is looked up from the bio's blkcg and
current bdev only when blkcg policy or accounting code needs it. The first
successful policy lookup pins the blkg with a bio-owned reference tracked by
BIO_BLKG_REF; the reference is dropped when the bio cgroup association is
cleared or when bio_set_dev() changes the lookup key.
Patch 1 adds accessors for the current bio cgroup state and factors out the
release path. Patch 2 switches the stored bio association from blkg to blkcg,
adds lookup/create helpers for policy users, and converts the blkcg policies
and cloned-bio users. Patch 3 moves async bio punt state from blkg to blkcg
so punted bios do not instantiate a queue-local blkg when no policy needs one.
This is v1 of this preparatory set; there is no previous version.
Related series:
[1] RFC v2 blk-cgroup: protect blkgs with blkcg_mutex
https://lore.kernel.org/r/20260724123037.3004560-1-yukuai@kernel.org
Yu Kuai (3):
blk-cgroup: add helpers for bio cgroup state
blk-cgroup: store blkcg in bio instead of blkg
blk-cgroup: move async bio punt state to blkcg
Documentation/admin-guide/cgroup-v2.rst | 2 +-
block/bfq-cgroup.c | 14 +-
block/bfq-iosched.c | 18 +-
block/bio.c | 22 +--
block/blk-cgroup-fc-appid.c | 10 +-
block/blk-cgroup.c | 251 +++++++++++++++---------
block/blk-cgroup.h | 40 +++-
block/blk-crypto-fallback.c | 2 +-
block/blk-iocost.c | 12 +-
block/blk-iolatency.c | 11 +-
block/blk-ioprio.c | 2 +-
block/blk-throttle.c | 2 +-
block/blk-throttle.h | 2 +-
drivers/md/bcache/request.c | 2 +-
drivers/md/dm.c | 2 +-
drivers/md/md.c | 2 +-
drivers/nvdimm/nd_virtio.c | 2 +-
fs/gfs2/lops.c | 3 +-
include/linux/bio.h | 20 +-
include/linux/blk_types.h | 9 +-
include/linux/writeback.h | 2 +-
mm/page_io.c | 10 +-
22 files changed, 268 insertions(+), 172 deletions(-)
base-commit: f2690679ecf3a3151688ebef766dd2512ff95854
--
2.51.0
^ permalink raw reply [flat|nested] 12+ messages in thread* [RFC PATCH v1 1/3] blk-cgroup: add helpers for bio cgroup state 2026-08-04 6:53 [RFC PATCH v1 0/3] blk-cgroup: store blkcg in bio before blkcg_mutex conversion Yu Kuai @ 2026-08-04 6:53 ` Yu Kuai 2026-08-04 10:52 ` Jan Kara 2026-08-04 6:53 ` [RFC PATCH v1 2/3] blk-cgroup: store blkcg in bio instead of blkg Yu Kuai 2026-08-04 6:53 ` [RFC PATCH v1 3/3] blk-cgroup: move async bio punt state to blkcg Yu Kuai 2 siblings, 1 reply; 12+ messages in thread From: Yu Kuai @ 2026-08-04 6:53 UTC (permalink / raw) To: Jens Axboe Cc: Tejun Heo, Johannes Weiner, Michal Koutný, Jonathan Corbet, Yu Kuai, Josef Bacik, Coly Li, Kent Overstreet, Alasdair Kergon, Mike Snitzer, Mikulas Patocka, Benjamin Marzinski, Song Liu, Dan Williams, Vishal Verma, Dave Jiang, Alison Schofield, Pankaj Gupta, Andreas Gruenbacher, Matthew Wilcox, Jan Kara, Andrew Morton, Chris Li, Kairui Song, Christoph Hellwig, Nilay Shroff, Tao Cui, cgroups, linux-doc, linux-kernel, linux-block, linux-bcache, dm-devel, linux-raid, nvdimm, virtualization, gfs2, linux-fsdevel, linux-mm From: Yu Kuai <yukuai@fygo.io> blk-cgroup users open-code bio->bi_blkg throughout the policy and accounting paths. Some users need the blkg itself, while others immediately dereference it to get the associated blkcg. The bio release paths also open-code the CONFIG_BLK_CGROUP guarded blkg reference drop and field clear. Add bio_blkg(), bio_blkcg() and bio_clear_blkg() helpers. Convert the read-side users to use the accessors, and use bio_clear_blkg() from bio_uninit() and bio_endio() so the release path no longer needs to know about CONFIG_BLK_CGROUP. Keep the direct bio->bi_blkg stores in the association and initialization paths, as those paths still assign the stored association. This keeps the current behavior unchanged while preparing for changing what cgroup state a bio stores internally. Signed-off-by: Yu Kuai <yukuai@fygo.io> --- block/bfq-cgroup.c | 4 ++-- block/bio.c | 14 ++------------ block/blk-cgroup-fc-appid.c | 11 +++++++++-- block/blk-cgroup.c | 19 ++++++++++--------- block/blk-cgroup.h | 19 ++++++++++++++++++- block/blk-iocost.c | 6 +++--- block/blk-iolatency.c | 6 +++--- block/blk-ioprio.c | 2 +- block/blk-throttle.c | 2 +- block/blk-throttle.h | 2 +- include/linux/bio.h | 10 ++++++++++ 11 files changed, 60 insertions(+), 35 deletions(-) diff --git a/block/bfq-cgroup.c b/block/bfq-cgroup.c index e82ff03bda02..7e65fe6844ee 100644 --- a/block/bfq-cgroup.c +++ b/block/bfq-cgroup.c @@ -363,7 +363,7 @@ void bfqg_and_blkg_put(struct bfq_group *bfqg) void bfqg_stats_update_legacy_io(struct request_queue *q, struct request *rq) { - struct bfq_group *bfqg = blkg_to_bfqg(rq->bio->bi_blkg); + struct bfq_group *bfqg = blkg_to_bfqg(bio_blkg(rq->bio)); if (!bfqg) return; @@ -606,7 +606,7 @@ static void bfq_link_bfqg(struct bfq_data *bfqd, struct bfq_group *bfqg) struct bfq_group *bfq_bio_bfqg(struct bfq_data *bfqd, struct bio *bio) { - struct blkcg_gq *blkg = bio->bi_blkg; + struct blkcg_gq *blkg = bio_blkg(bio); struct bfq_group *bfqg; while (blkg) { diff --git a/block/bio.c b/block/bio.c index 6a2f6fc3413e..c207b248edba 100644 --- a/block/bio.c +++ b/block/bio.c @@ -179,12 +179,7 @@ static inline gfp_t try_alloc_gfp(gfp_t gfp) void bio_uninit(struct bio *bio) { -#ifdef CONFIG_BLK_CGROUP - if (bio->bi_blkg) { - blkg_put(bio->bi_blkg); - bio->bi_blkg = NULL; - } -#endif + bio_clear_blkg(bio); if (bio_integrity(bio)) bio_integrity_free(bio); @@ -1803,17 +1798,12 @@ void bio_endio(struct bio *bio) goto again; } -#ifdef CONFIG_BLK_CGROUP /* * Release cgroup info. We shouldn't have to do this here, but quite * a few callers of bio_init fail to call bio_uninit, so we cover up * for that here at least for now. */ - if (bio->bi_blkg) { - blkg_put(bio->bi_blkg); - bio->bi_blkg = NULL; - } -#endif + bio_clear_blkg(bio); if (bio->bi_end_io) bio->bi_end_io(bio); diff --git a/block/blk-cgroup-fc-appid.c b/block/blk-cgroup-fc-appid.c index 3ec21333f393..b2e16e9a7a6c 100644 --- a/block/blk-cgroup-fc-appid.c +++ b/block/blk-cgroup-fc-appid.c @@ -50,8 +50,15 @@ EXPORT_SYMBOL_GPL(blkcg_set_fc_appid); */ char *blkcg_get_fc_appid(struct bio *bio) { - if (!bio->bi_blkg || bio->bi_blkg->blkcg->fc_app_id[0] == '\0') + struct blkcg *blkcg; + + if (!bio_blkg(bio)) return NULL; - return bio->bi_blkg->blkcg->fc_app_id; + + blkcg = bio_blkcg(bio); + if (blkcg->fc_app_id[0] == '\0') + return NULL; + + return blkcg->fc_app_id; } EXPORT_SYMBOL_GPL(blkcg_get_fc_appid); diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c index 354637f3b158..753a3bdd0e8c 100644 --- a/block/blk-cgroup.c +++ b/block/blk-cgroup.c @@ -239,7 +239,7 @@ static void blkg_async_bio_workfn(struct work_struct *work) */ void blkcg_punt_bio_submit(struct bio *bio) { - struct blkcg_gq *blkg = bio->bi_blkg; + struct blkcg_gq *blkg = bio_blkg(bio); if (blkg->parent) { spin_lock(&blkg->async_bio_lock); @@ -275,9 +275,9 @@ subsys_initcall(blkcg_punt_bio_init); */ struct cgroup_subsys_state *bio_blkcg_css(struct bio *bio) { - if (!bio || !bio->bi_blkg) + if (!bio || !bio_blkg(bio)) return NULL; - return &bio->bi_blkg->blkcg->css; + return &bio_blkcg(bio)->css; } EXPORT_SYMBOL_GPL(bio_blkcg_css); @@ -2118,8 +2118,8 @@ static inline struct blkcg_gq *blkg_tryget_closest(struct bio *bio, void bio_associate_blkg_from_css(struct bio *bio, struct cgroup_subsys_state *css) { - if (bio->bi_blkg) - blkg_put(bio->bi_blkg); + if (bio_blkg(bio)) + blkg_put(bio_blkg(bio)); if (css && css->parent) { bio->bi_blkg = blkg_tryget_closest(bio, css); @@ -2146,7 +2146,7 @@ void bio_associate_blkg(struct bio *bio) if (blk_op_is_passthrough(bio->bi_opf)) return; - if (bio->bi_blkg) { + if (bio_blkg(bio)) { css = bio_blkcg_css(bio); bio_associate_blkg_from_css(bio, css); } else { @@ -2170,7 +2170,7 @@ EXPORT_SYMBOL_GPL(bio_associate_blkg); */ void bio_clone_blkg_association(struct bio *dst, struct bio *src) { - if (src->bi_blkg) + if (bio_blkg(src)) bio_associate_blkg_from_css(dst, bio_blkcg_css(src)); } EXPORT_SYMBOL_GPL(bio_clone_blkg_association); @@ -2186,7 +2186,8 @@ static int blk_cgroup_io_type(struct bio *bio) void blk_cgroup_bio_start(struct bio *bio) { - struct blkcg *blkcg = bio->bi_blkg->blkcg; + struct blkcg_gq *blkg = bio_blkg(bio); + struct blkcg *blkcg = bio_blkcg(bio); int rwd = blk_cgroup_io_type(bio), cpu; struct blkg_iostat_set *bis; unsigned long flags; @@ -2199,7 +2200,7 @@ void blk_cgroup_bio_start(struct bio *bio) return; cpu = get_cpu(); - bis = per_cpu_ptr(bio->bi_blkg->iostat_cpu, cpu); + bis = per_cpu_ptr(blkg->iostat_cpu, cpu); flags = u64_stats_update_begin_irqsave(&bis->sync); /* diff --git a/block/blk-cgroup.h b/block/blk-cgroup.h index 615390f751aa..1e80b0a73233 100644 --- a/block/blk-cgroup.h +++ b/block/blk-cgroup.h @@ -124,6 +124,11 @@ static inline struct blkcg *css_to_blkcg(struct cgroup_subsys_state *css) return css ? container_of(css, struct blkcg, css) : NULL; } +static inline struct blkcg *bio_blkcg(struct bio *bio) +{ + return bio_blkg(bio)->blkcg; +} + /* * A blkcg_gq (blkg) is association between a block cgroup (blkcg) and a * request_queue (q). This is used by blkcg policies which need to track @@ -343,6 +348,16 @@ static inline void blkg_put(struct blkcg_gq *blkg) percpu_ref_put(&blkg->refcnt); } +static inline void bio_clear_blkg(struct bio *bio) +{ + struct blkcg_gq *blkg = bio_blkg(bio); + + if (blkg) { + blkg_put(blkg); + bio->bi_blkg = NULL; + } +} + /** * blkg_for_each_descendant_pre - pre-order walk of a blkg's descendants * @d_blkg: loop cursor pointing to the current descendant @@ -455,7 +470,7 @@ static inline void blkcg_clear_delay(struct blkcg_gq *blkg) */ static inline bool blk_cgroup_mergeable(struct request *rq, struct bio *bio) { - return rq->bio->bi_blkg == bio->bi_blkg && + return bio_blkg(rq->bio) == bio_blkg(bio) && bio_issue_as_root_blkg(rq->bio) == bio_issue_as_root_blkg(bio); } @@ -481,6 +496,7 @@ struct blkcg_policy { struct blkcg { }; +static inline struct blkcg *bio_blkcg(struct bio *bio) { return NULL; } static inline struct blkcg_gq *blkg_lookup(struct blkcg *blkcg, void *key) { return NULL; } static inline void blkg_init_queue(struct request_queue *q) { } static inline int blkcg_init_disk(struct gendisk *disk) { return 0; } @@ -497,6 +513,7 @@ static inline struct blkg_policy_data *blkg_to_pd(struct blkcg_gq *blkg, static inline struct blkcg_gq *pd_to_blkg(struct blkg_policy_data *pd) { return NULL; } static inline void blkg_get(struct blkcg_gq *blkg) { } static inline void blkg_put(struct blkcg_gq *blkg) { } +static inline void bio_clear_blkg(struct bio *bio) { } static inline void blk_cgroup_bio_start(struct bio *bio) { } static inline bool blk_cgroup_mergeable(struct request *rq, struct bio *bio) { return true; } diff --git a/block/blk-iocost.c b/block/blk-iocost.c index 8b2aeba2e1e3..d4470476bcd0 100644 --- a/block/blk-iocost.c +++ b/block/blk-iocost.c @@ -2686,7 +2686,7 @@ iocg_handle_over_budget(struct rq_qos *rqos, struct ioc_gq *iocg, static void ioc_rqos_throttle(struct rq_qos *rqos, struct bio *bio) { - struct blkcg_gq *blkg = bio->bi_blkg; + struct blkcg_gq *blkg = bio_blkg(bio); struct ioc *ioc = rqos_to_ioc(rqos); struct ioc_gq *iocg = blkg_to_iocg(blkg); struct ioc_now now; @@ -2775,7 +2775,7 @@ static void ioc_rqos_throttle(struct rq_qos *rqos, struct bio *bio) static void ioc_rqos_merge(struct rq_qos *rqos, struct request *rq, struct bio *bio) { - struct ioc_gq *iocg = blkg_to_iocg(bio->bi_blkg); + struct ioc_gq *iocg = blkg_to_iocg(bio_blkg(bio)); struct ioc *ioc = rqos_to_ioc(rqos); sector_t bio_end = bio_end_sector(bio); struct ioc_now now; @@ -2833,7 +2833,7 @@ static void ioc_rqos_merge(struct rq_qos *rqos, struct request *rq, static void ioc_rqos_done_bio(struct rq_qos *rqos, struct bio *bio) { - struct ioc_gq *iocg = blkg_to_iocg(bio->bi_blkg); + struct ioc_gq *iocg = blkg_to_iocg(bio_blkg(bio)); if (iocg && bio->bi_iocost_cost) atomic64_add(bio->bi_iocost_cost, &iocg->done_vtime); diff --git a/block/blk-iolatency.c b/block/blk-iolatency.c index cef02b6c5fa9..c0d8d5f6bdba 100644 --- a/block/blk-iolatency.c +++ b/block/blk-iolatency.c @@ -463,7 +463,7 @@ static void check_scale_change(struct iolatency_grp *iolat) static void blkcg_iolatency_throttle(struct rq_qos *rqos, struct bio *bio) { struct blk_iolatency *blkiolat = BLKIOLATENCY(rqos); - struct blkcg_gq *blkg = bio->bi_blkg; + struct blkcg_gq *blkg = bio_blkg(bio); bool issue_as_root = bio_issue_as_root_blkg(bio); if (!blkiolat->enabled) @@ -590,11 +590,11 @@ static void blkcg_iolatency_done_bio(struct rq_qos *rqos, struct bio *bio) bool issue_as_root = bio_issue_as_root_blkg(bio); int inflight = 0; - blkg = bio->bi_blkg; + blkg = bio_blkg(bio); if (!blkg || !bio_flagged(bio, BIO_QOS_THROTTLED)) return; - iolat = blkg_to_lat(bio->bi_blkg); + iolat = blkg_to_lat(blkg); if (!iolat) return; diff --git a/block/blk-ioprio.c b/block/blk-ioprio.c index 8fa8bca35062..5e3f8e49550d 100644 --- a/block/blk-ioprio.c +++ b/block/blk-ioprio.c @@ -132,7 +132,7 @@ static struct blkcg_policy ioprio_policy = { void blkcg_set_ioprio(struct bio *bio) { - struct ioprio_blkcg *blkcg = blkcg_to_ioprio_blkcg(bio->bi_blkg->blkcg); + struct ioprio_blkcg *blkcg = blkcg_to_ioprio_blkcg(bio_blkcg(bio)); u16 prio; if (!blkcg || blkcg->prio_policy == POLICY_NO_CHANGE) diff --git a/block/blk-throttle.c b/block/blk-throttle.c index ffc3b70065d4..3828c3857900 100644 --- a/block/blk-throttle.c +++ b/block/blk-throttle.c @@ -1792,7 +1792,7 @@ static bool tg_within_limit(struct throtl_grp *tg, struct bio *bio, bool rw) bool __blk_throtl_bio(struct bio *bio) { struct request_queue *q = bdev_get_queue(bio->bi_bdev); - struct blkcg_gq *blkg = bio->bi_blkg; + struct blkcg_gq *blkg = bio_blkg(bio); struct throtl_qnode *qn = NULL; struct throtl_grp *tg = blkg_to_tg(blkg); struct throtl_service_queue *sq; diff --git a/block/blk-throttle.h b/block/blk-throttle.h index 9d7a42c039a1..609a126c7ccb 100644 --- a/block/blk-throttle.h +++ b/block/blk-throttle.h @@ -173,7 +173,7 @@ static inline bool blk_should_throtl(struct bio *bio) if (!blk_throtl_activated(bio->bi_bdev->bd_queue)) return false; - tg = blkg_to_tg(bio->bi_blkg); + tg = blkg_to_tg(bio_blkg(bio)); if (!cgroup_subsys_on_dfl(io_cgrp_subsys)) { if (!bio_flagged(bio, BIO_CGROUP_ACCT)) { bio_set_flag(bio, BIO_CGROUP_ACCT); diff --git a/include/linux/bio.h b/include/linux/bio.h index 8f33f717b14f..dc4baa3602b7 100644 --- a/include/linux/bio.h +++ b/include/linux/bio.h @@ -503,12 +503,22 @@ static inline void bio_release_pages(struct bio *bio, bool mark_dirty) disk_devt((bio)->bi_bdev->bd_disk) #ifdef CONFIG_BLK_CGROUP +static inline struct blkcg_gq *bio_blkg(struct bio *bio) +{ + return bio->bi_blkg; +} + void bio_associate_blkg(struct bio *bio); void bio_associate_blkg_from_css(struct bio *bio, struct cgroup_subsys_state *css); void bio_clone_blkg_association(struct bio *dst, struct bio *src); void blkcg_punt_bio_submit(struct bio *bio); #else /* CONFIG_BLK_CGROUP */ +static inline struct blkcg_gq *bio_blkg(struct bio *bio) +{ + return NULL; +} + static inline void bio_associate_blkg(struct bio *bio) { } static inline void bio_associate_blkg_from_css(struct bio *bio, struct cgroup_subsys_state *css) -- 2.51.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [RFC PATCH v1 1/3] blk-cgroup: add helpers for bio cgroup state 2026-08-04 6:53 ` [RFC PATCH v1 1/3] blk-cgroup: add helpers for bio cgroup state Yu Kuai @ 2026-08-04 10:52 ` Jan Kara 2026-08-04 13:25 ` Christoph Hellwig 0 siblings, 1 reply; 12+ messages in thread From: Jan Kara @ 2026-08-04 10:52 UTC (permalink / raw) To: Yu Kuai Cc: Jens Axboe, Tejun Heo, Johannes Weiner, Michal Koutný, Jonathan Corbet, Yu Kuai, Josef Bacik, Coly Li, Kent Overstreet, Alasdair Kergon, Mike Snitzer, Mikulas Patocka, Benjamin Marzinski, Song Liu, Dan Williams, Vishal Verma, Dave Jiang, Alison Schofield, Pankaj Gupta, Andreas Gruenbacher, Matthew Wilcox, Jan Kara, Andrew Morton, Chris Li, Kairui Song, Christoph Hellwig, Nilay Shroff, Tao Cui, cgroups, linux-doc, linux-kernel, linux-block, linux-bcache, dm-devel, linux-raid, nvdimm, virtualization, gfs2, linux-fsdevel, linux-mm On Tue 04-08-26 14:53:11, Yu Kuai wrote: > From: Yu Kuai <yukuai@fygo.io> > > blk-cgroup users open-code bio->bi_blkg throughout the policy and > accounting paths. Some users need the blkg itself, while others > immediately dereference it to get the associated blkcg. The bio release > paths also open-code the CONFIG_BLK_CGROUP guarded blkg reference drop > and field clear. > > Add bio_blkg(), bio_blkcg() and bio_clear_blkg() helpers. Convert the > read-side users to use the accessors, and use bio_clear_blkg() from > bio_uninit() and bio_endio() so the release path no longer needs to know > about CONFIG_BLK_CGROUP. Keep the direct bio->bi_blkg stores in the > association and initialization paths, as those paths still assign the > stored association. > > This keeps the current behavior unchanged while preparing for changing > what cgroup state a bio stores internally. > > Signed-off-by: Yu Kuai <yukuai@fygo.io> Mostly looks good. Just I think bio_blkcg() should gracefully handle the case where bio->bi_blkg is NULL (and return NULL in that case). That way you can also get rid of somewhat odd pattern: if (!bio_blkg(bio)) return ...; do something with bio_blkcg(bio) You can then just check bio_blkcg(bio) directly which is much more obvious. Honza > --- > block/bfq-cgroup.c | 4 ++-- > block/bio.c | 14 ++------------ > block/blk-cgroup-fc-appid.c | 11 +++++++++-- > block/blk-cgroup.c | 19 ++++++++++--------- > block/blk-cgroup.h | 19 ++++++++++++++++++- > block/blk-iocost.c | 6 +++--- > block/blk-iolatency.c | 6 +++--- > block/blk-ioprio.c | 2 +- > block/blk-throttle.c | 2 +- > block/blk-throttle.h | 2 +- > include/linux/bio.h | 10 ++++++++++ > 11 files changed, 60 insertions(+), 35 deletions(-) > > diff --git a/block/bfq-cgroup.c b/block/bfq-cgroup.c > index e82ff03bda02..7e65fe6844ee 100644 > --- a/block/bfq-cgroup.c > +++ b/block/bfq-cgroup.c > @@ -363,7 +363,7 @@ void bfqg_and_blkg_put(struct bfq_group *bfqg) > > void bfqg_stats_update_legacy_io(struct request_queue *q, struct request *rq) > { > - struct bfq_group *bfqg = blkg_to_bfqg(rq->bio->bi_blkg); > + struct bfq_group *bfqg = blkg_to_bfqg(bio_blkg(rq->bio)); > > if (!bfqg) > return; > @@ -606,7 +606,7 @@ static void bfq_link_bfqg(struct bfq_data *bfqd, struct bfq_group *bfqg) > > struct bfq_group *bfq_bio_bfqg(struct bfq_data *bfqd, struct bio *bio) > { > - struct blkcg_gq *blkg = bio->bi_blkg; > + struct blkcg_gq *blkg = bio_blkg(bio); > struct bfq_group *bfqg; > > while (blkg) { > diff --git a/block/bio.c b/block/bio.c > index 6a2f6fc3413e..c207b248edba 100644 > --- a/block/bio.c > +++ b/block/bio.c > @@ -179,12 +179,7 @@ static inline gfp_t try_alloc_gfp(gfp_t gfp) > > void bio_uninit(struct bio *bio) > { > -#ifdef CONFIG_BLK_CGROUP > - if (bio->bi_blkg) { > - blkg_put(bio->bi_blkg); > - bio->bi_blkg = NULL; > - } > -#endif > + bio_clear_blkg(bio); > if (bio_integrity(bio)) > bio_integrity_free(bio); > > @@ -1803,17 +1798,12 @@ void bio_endio(struct bio *bio) > goto again; > } > > -#ifdef CONFIG_BLK_CGROUP > /* > * Release cgroup info. We shouldn't have to do this here, but quite > * a few callers of bio_init fail to call bio_uninit, so we cover up > * for that here at least for now. > */ > - if (bio->bi_blkg) { > - blkg_put(bio->bi_blkg); > - bio->bi_blkg = NULL; > - } > -#endif > + bio_clear_blkg(bio); > > if (bio->bi_end_io) > bio->bi_end_io(bio); > diff --git a/block/blk-cgroup-fc-appid.c b/block/blk-cgroup-fc-appid.c > index 3ec21333f393..b2e16e9a7a6c 100644 > --- a/block/blk-cgroup-fc-appid.c > +++ b/block/blk-cgroup-fc-appid.c > @@ -50,8 +50,15 @@ EXPORT_SYMBOL_GPL(blkcg_set_fc_appid); > */ > char *blkcg_get_fc_appid(struct bio *bio) > { > - if (!bio->bi_blkg || bio->bi_blkg->blkcg->fc_app_id[0] == '\0') > + struct blkcg *blkcg; > + > + if (!bio_blkg(bio)) > return NULL; > - return bio->bi_blkg->blkcg->fc_app_id; > + > + blkcg = bio_blkcg(bio); > + if (blkcg->fc_app_id[0] == '\0') > + return NULL; > + > + return blkcg->fc_app_id; > } > EXPORT_SYMBOL_GPL(blkcg_get_fc_appid); > diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c > index 354637f3b158..753a3bdd0e8c 100644 > --- a/block/blk-cgroup.c > +++ b/block/blk-cgroup.c > @@ -239,7 +239,7 @@ static void blkg_async_bio_workfn(struct work_struct *work) > */ > void blkcg_punt_bio_submit(struct bio *bio) > { > - struct blkcg_gq *blkg = bio->bi_blkg; > + struct blkcg_gq *blkg = bio_blkg(bio); > > if (blkg->parent) { > spin_lock(&blkg->async_bio_lock); > @@ -275,9 +275,9 @@ subsys_initcall(blkcg_punt_bio_init); > */ > struct cgroup_subsys_state *bio_blkcg_css(struct bio *bio) > { > - if (!bio || !bio->bi_blkg) > + if (!bio || !bio_blkg(bio)) > return NULL; > - return &bio->bi_blkg->blkcg->css; > + return &bio_blkcg(bio)->css; > } > EXPORT_SYMBOL_GPL(bio_blkcg_css); > > @@ -2118,8 +2118,8 @@ static inline struct blkcg_gq *blkg_tryget_closest(struct bio *bio, > void bio_associate_blkg_from_css(struct bio *bio, > struct cgroup_subsys_state *css) > { > - if (bio->bi_blkg) > - blkg_put(bio->bi_blkg); > + if (bio_blkg(bio)) > + blkg_put(bio_blkg(bio)); > > if (css && css->parent) { > bio->bi_blkg = blkg_tryget_closest(bio, css); > @@ -2146,7 +2146,7 @@ void bio_associate_blkg(struct bio *bio) > if (blk_op_is_passthrough(bio->bi_opf)) > return; > > - if (bio->bi_blkg) { > + if (bio_blkg(bio)) { > css = bio_blkcg_css(bio); > bio_associate_blkg_from_css(bio, css); > } else { > @@ -2170,7 +2170,7 @@ EXPORT_SYMBOL_GPL(bio_associate_blkg); > */ > void bio_clone_blkg_association(struct bio *dst, struct bio *src) > { > - if (src->bi_blkg) > + if (bio_blkg(src)) > bio_associate_blkg_from_css(dst, bio_blkcg_css(src)); > } > EXPORT_SYMBOL_GPL(bio_clone_blkg_association); > @@ -2186,7 +2186,8 @@ static int blk_cgroup_io_type(struct bio *bio) > > void blk_cgroup_bio_start(struct bio *bio) > { > - struct blkcg *blkcg = bio->bi_blkg->blkcg; > + struct blkcg_gq *blkg = bio_blkg(bio); > + struct blkcg *blkcg = bio_blkcg(bio); > int rwd = blk_cgroup_io_type(bio), cpu; > struct blkg_iostat_set *bis; > unsigned long flags; > @@ -2199,7 +2200,7 @@ void blk_cgroup_bio_start(struct bio *bio) > return; > > cpu = get_cpu(); > - bis = per_cpu_ptr(bio->bi_blkg->iostat_cpu, cpu); > + bis = per_cpu_ptr(blkg->iostat_cpu, cpu); > flags = u64_stats_update_begin_irqsave(&bis->sync); > > /* > diff --git a/block/blk-cgroup.h b/block/blk-cgroup.h > index 615390f751aa..1e80b0a73233 100644 > --- a/block/blk-cgroup.h > +++ b/block/blk-cgroup.h > @@ -124,6 +124,11 @@ static inline struct blkcg *css_to_blkcg(struct cgroup_subsys_state *css) > return css ? container_of(css, struct blkcg, css) : NULL; > } > > +static inline struct blkcg *bio_blkcg(struct bio *bio) > +{ > + return bio_blkg(bio)->blkcg; > +} > + > /* > * A blkcg_gq (blkg) is association between a block cgroup (blkcg) and a > * request_queue (q). This is used by blkcg policies which need to track > @@ -343,6 +348,16 @@ static inline void blkg_put(struct blkcg_gq *blkg) > percpu_ref_put(&blkg->refcnt); > } > > +static inline void bio_clear_blkg(struct bio *bio) > +{ > + struct blkcg_gq *blkg = bio_blkg(bio); > + > + if (blkg) { > + blkg_put(blkg); > + bio->bi_blkg = NULL; > + } > +} > + > /** > * blkg_for_each_descendant_pre - pre-order walk of a blkg's descendants > * @d_blkg: loop cursor pointing to the current descendant > @@ -455,7 +470,7 @@ static inline void blkcg_clear_delay(struct blkcg_gq *blkg) > */ > static inline bool blk_cgroup_mergeable(struct request *rq, struct bio *bio) > { > - return rq->bio->bi_blkg == bio->bi_blkg && > + return bio_blkg(rq->bio) == bio_blkg(bio) && > bio_issue_as_root_blkg(rq->bio) == bio_issue_as_root_blkg(bio); > } > > @@ -481,6 +496,7 @@ struct blkcg_policy { > struct blkcg { > }; > > +static inline struct blkcg *bio_blkcg(struct bio *bio) { return NULL; } > static inline struct blkcg_gq *blkg_lookup(struct blkcg *blkcg, void *key) { return NULL; } > static inline void blkg_init_queue(struct request_queue *q) { } > static inline int blkcg_init_disk(struct gendisk *disk) { return 0; } > @@ -497,6 +513,7 @@ static inline struct blkg_policy_data *blkg_to_pd(struct blkcg_gq *blkg, > static inline struct blkcg_gq *pd_to_blkg(struct blkg_policy_data *pd) { return NULL; } > static inline void blkg_get(struct blkcg_gq *blkg) { } > static inline void blkg_put(struct blkcg_gq *blkg) { } > +static inline void bio_clear_blkg(struct bio *bio) { } > static inline void blk_cgroup_bio_start(struct bio *bio) { } > static inline bool blk_cgroup_mergeable(struct request *rq, struct bio *bio) { return true; } > > diff --git a/block/blk-iocost.c b/block/blk-iocost.c > index 8b2aeba2e1e3..d4470476bcd0 100644 > --- a/block/blk-iocost.c > +++ b/block/blk-iocost.c > @@ -2686,7 +2686,7 @@ iocg_handle_over_budget(struct rq_qos *rqos, struct ioc_gq *iocg, > > static void ioc_rqos_throttle(struct rq_qos *rqos, struct bio *bio) > { > - struct blkcg_gq *blkg = bio->bi_blkg; > + struct blkcg_gq *blkg = bio_blkg(bio); > struct ioc *ioc = rqos_to_ioc(rqos); > struct ioc_gq *iocg = blkg_to_iocg(blkg); > struct ioc_now now; > @@ -2775,7 +2775,7 @@ static void ioc_rqos_throttle(struct rq_qos *rqos, struct bio *bio) > static void ioc_rqos_merge(struct rq_qos *rqos, struct request *rq, > struct bio *bio) > { > - struct ioc_gq *iocg = blkg_to_iocg(bio->bi_blkg); > + struct ioc_gq *iocg = blkg_to_iocg(bio_blkg(bio)); > struct ioc *ioc = rqos_to_ioc(rqos); > sector_t bio_end = bio_end_sector(bio); > struct ioc_now now; > @@ -2833,7 +2833,7 @@ static void ioc_rqos_merge(struct rq_qos *rqos, struct request *rq, > > static void ioc_rqos_done_bio(struct rq_qos *rqos, struct bio *bio) > { > - struct ioc_gq *iocg = blkg_to_iocg(bio->bi_blkg); > + struct ioc_gq *iocg = blkg_to_iocg(bio_blkg(bio)); > > if (iocg && bio->bi_iocost_cost) > atomic64_add(bio->bi_iocost_cost, &iocg->done_vtime); > diff --git a/block/blk-iolatency.c b/block/blk-iolatency.c > index cef02b6c5fa9..c0d8d5f6bdba 100644 > --- a/block/blk-iolatency.c > +++ b/block/blk-iolatency.c > @@ -463,7 +463,7 @@ static void check_scale_change(struct iolatency_grp *iolat) > static void blkcg_iolatency_throttle(struct rq_qos *rqos, struct bio *bio) > { > struct blk_iolatency *blkiolat = BLKIOLATENCY(rqos); > - struct blkcg_gq *blkg = bio->bi_blkg; > + struct blkcg_gq *blkg = bio_blkg(bio); > bool issue_as_root = bio_issue_as_root_blkg(bio); > > if (!blkiolat->enabled) > @@ -590,11 +590,11 @@ static void blkcg_iolatency_done_bio(struct rq_qos *rqos, struct bio *bio) > bool issue_as_root = bio_issue_as_root_blkg(bio); > int inflight = 0; > > - blkg = bio->bi_blkg; > + blkg = bio_blkg(bio); > if (!blkg || !bio_flagged(bio, BIO_QOS_THROTTLED)) > return; > > - iolat = blkg_to_lat(bio->bi_blkg); > + iolat = blkg_to_lat(blkg); > if (!iolat) > return; > > diff --git a/block/blk-ioprio.c b/block/blk-ioprio.c > index 8fa8bca35062..5e3f8e49550d 100644 > --- a/block/blk-ioprio.c > +++ b/block/blk-ioprio.c > @@ -132,7 +132,7 @@ static struct blkcg_policy ioprio_policy = { > > void blkcg_set_ioprio(struct bio *bio) > { > - struct ioprio_blkcg *blkcg = blkcg_to_ioprio_blkcg(bio->bi_blkg->blkcg); > + struct ioprio_blkcg *blkcg = blkcg_to_ioprio_blkcg(bio_blkcg(bio)); > u16 prio; > > if (!blkcg || blkcg->prio_policy == POLICY_NO_CHANGE) > diff --git a/block/blk-throttle.c b/block/blk-throttle.c > index ffc3b70065d4..3828c3857900 100644 > --- a/block/blk-throttle.c > +++ b/block/blk-throttle.c > @@ -1792,7 +1792,7 @@ static bool tg_within_limit(struct throtl_grp *tg, struct bio *bio, bool rw) > bool __blk_throtl_bio(struct bio *bio) > { > struct request_queue *q = bdev_get_queue(bio->bi_bdev); > - struct blkcg_gq *blkg = bio->bi_blkg; > + struct blkcg_gq *blkg = bio_blkg(bio); > struct throtl_qnode *qn = NULL; > struct throtl_grp *tg = blkg_to_tg(blkg); > struct throtl_service_queue *sq; > diff --git a/block/blk-throttle.h b/block/blk-throttle.h > index 9d7a42c039a1..609a126c7ccb 100644 > --- a/block/blk-throttle.h > +++ b/block/blk-throttle.h > @@ -173,7 +173,7 @@ static inline bool blk_should_throtl(struct bio *bio) > if (!blk_throtl_activated(bio->bi_bdev->bd_queue)) > return false; > > - tg = blkg_to_tg(bio->bi_blkg); > + tg = blkg_to_tg(bio_blkg(bio)); > if (!cgroup_subsys_on_dfl(io_cgrp_subsys)) { > if (!bio_flagged(bio, BIO_CGROUP_ACCT)) { > bio_set_flag(bio, BIO_CGROUP_ACCT); > diff --git a/include/linux/bio.h b/include/linux/bio.h > index 8f33f717b14f..dc4baa3602b7 100644 > --- a/include/linux/bio.h > +++ b/include/linux/bio.h > @@ -503,12 +503,22 @@ static inline void bio_release_pages(struct bio *bio, bool mark_dirty) > disk_devt((bio)->bi_bdev->bd_disk) > > #ifdef CONFIG_BLK_CGROUP > +static inline struct blkcg_gq *bio_blkg(struct bio *bio) > +{ > + return bio->bi_blkg; > +} > + > void bio_associate_blkg(struct bio *bio); > void bio_associate_blkg_from_css(struct bio *bio, > struct cgroup_subsys_state *css); > void bio_clone_blkg_association(struct bio *dst, struct bio *src); > void blkcg_punt_bio_submit(struct bio *bio); > #else /* CONFIG_BLK_CGROUP */ > +static inline struct blkcg_gq *bio_blkg(struct bio *bio) > +{ > + return NULL; > +} > + > static inline void bio_associate_blkg(struct bio *bio) { } > static inline void bio_associate_blkg_from_css(struct bio *bio, > struct cgroup_subsys_state *css) > -- > 2.51.0 > -- Jan Kara <jack@suse.com> SUSE Labs, CR ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC PATCH v1 1/3] blk-cgroup: add helpers for bio cgroup state 2026-08-04 10:52 ` Jan Kara @ 2026-08-04 13:25 ` Christoph Hellwig 2026-08-04 15:07 ` yu kuai 0 siblings, 1 reply; 12+ messages in thread From: Christoph Hellwig @ 2026-08-04 13:25 UTC (permalink / raw) To: Jan Kara Cc: Yu Kuai, Jens Axboe, Tejun Heo, Johannes Weiner, Michal Koutný, Jonathan Corbet, Yu Kuai, Josef Bacik, Coly Li, Kent Overstreet, Alasdair Kergon, Mike Snitzer, Mikulas Patocka, Benjamin Marzinski, Song Liu, Dan Williams, Vishal Verma, Dave Jiang, Alison Schofield, Pankaj Gupta, Andreas Gruenbacher, Matthew Wilcox, Andrew Morton, Chris Li, Kairui Song, Christoph Hellwig, Nilay Shroff, Tao Cui, cgroups, linux-doc, linux-kernel, linux-block, linux-bcache, dm-devel, linux-raid, nvdimm, virtualization, gfs2, linux-fsdevel, linux-mm On Tue, Aug 04, 2026 at 12:52:16PM +0200, Jan Kara wrote: > Mostly looks good. Just I think bio_blkcg() should gracefully handle the > case where bio->bi_blkg is NULL (and return NULL in that case). That way > you can also get rid of somewhat odd pattern: > > if (!bio_blkg(bio)) > return ...; > do something with bio_blkcg(bio) > > You can then just check bio_blkcg(bio) directly which is much more obvious. Yes. Looking at the whole series I'm also not sure that this makes too much sense as a split out patch as the next one touches more than half of the callsite anyway. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC PATCH v1 1/3] blk-cgroup: add helpers for bio cgroup state 2026-08-04 13:25 ` Christoph Hellwig @ 2026-08-04 15:07 ` yu kuai 0 siblings, 0 replies; 12+ messages in thread From: yu kuai @ 2026-08-04 15:07 UTC (permalink / raw) To: Christoph Hellwig, Jan Kara, yu kuai Cc: Yu Kuai, Jens Axboe, Tejun Heo, Johannes Weiner, Michal Koutný, Jonathan Corbet, Josef Bacik, Coly Li, Kent Overstreet, Alasdair Kergon, Mike Snitzer, Mikulas Patocka, Benjamin Marzinski, Song Liu, Dan Williams, Vishal Verma, Dave Jiang, Alison Schofield, Pankaj Gupta, Andreas Gruenbacher, Matthew Wilcox, Andrew Morton, Chris Li, Kairui Song, Nilay Shroff, Tao Cui, cgroups, linux-doc, linux-kernel, linux-block, linux-bcache, dm-devel, linux-raid, nvdimm, virtualization, gfs2, linux-fsdevel, linux-mm Hi, 在 2026/8/4 21:25, Christoph Hellwig 写道: > On Tue, Aug 04, 2026 at 12:52:16PM +0200, Jan Kara wrote: >> Mostly looks good. Just I think bio_blkcg() should gracefully handle the >> case where bio->bi_blkg is NULL (and return NULL in that case). That way >> you can also get rid of somewhat odd pattern: >> >> if (!bio_blkg(bio)) >> return ...; >> do something with bio_blkcg(bio) >> >> You can then just check bio_blkcg(bio) directly which is much more obvious. > Yes. Looking at the whole series I'm also not sure that this makes too > much sense as a split out patch as the next one touches more than half > of the callsite anyway. Yes, this make sense, I'm trying not to cook a huge patch, but patch 2 is grow much bigger than I was expected. Unless I figure out a nicer way to split patch 2, I'll merge them in the next version. > -- Thanks, Kuai ^ permalink raw reply [flat|nested] 12+ messages in thread
* [RFC PATCH v1 2/3] blk-cgroup: store blkcg in bio instead of blkg 2026-08-04 6:53 [RFC PATCH v1 0/3] blk-cgroup: store blkcg in bio before blkcg_mutex conversion Yu Kuai 2026-08-04 6:53 ` [RFC PATCH v1 1/3] blk-cgroup: add helpers for bio cgroup state Yu Kuai @ 2026-08-04 6:53 ` Yu Kuai 2026-08-04 9:19 ` Tao Cui 2026-08-04 6:53 ` [RFC PATCH v1 3/3] blk-cgroup: move async bio punt state to blkcg Yu Kuai 2 siblings, 1 reply; 12+ messages in thread From: Yu Kuai @ 2026-08-04 6:53 UTC (permalink / raw) To: Jens Axboe Cc: Tejun Heo, Johannes Weiner, Michal Koutný, Jonathan Corbet, Yu Kuai, Josef Bacik, Coly Li, Kent Overstreet, Alasdair Kergon, Mike Snitzer, Mikulas Patocka, Benjamin Marzinski, Song Liu, Dan Williams, Vishal Verma, Dave Jiang, Alison Schofield, Pankaj Gupta, Andreas Gruenbacher, Matthew Wilcox, Jan Kara, Andrew Morton, Chris Li, Kairui Song, Christoph Hellwig, Nilay Shroff, Tao Cui, cgroups, linux-doc, linux-kernel, linux-block, linux-bcache, dm-devel, linux-raid, nvdimm, virtualization, gfs2, linux-fsdevel, linux-mm From: Yu Kuai <yukuai@fygo.io> A bio currently stores a queue-local blkg reference. This forces bio association and remap paths to look up or create a blkg even when the bio will never enter a blkcg policy. Store the blkcg css association in the bio instead, and derive the blkg from the bio's blkcg and current bdev when a policy needs it. The first successful policy lookup pins the blkg, records the pin with BIO_BLKG_REF, and drops it from bio_clear_blkcg() or when bio_set_dev() changes the lookup key. Keep lookup-only users from creating missing blkgs by using bio_blkg_lookup(), and rename the bio cgroup association helpers to match the stored blkcg state. Signed-off-by: Yu Kuai <yukuai@fygo.io> --- Documentation/admin-guide/cgroup-v2.rst | 2 +- block/bfq-cgroup.c | 14 +- block/bfq-iosched.c | 18 ++- block/bio.c | 12 +- block/blk-cgroup-fc-appid.c | 5 +- block/blk-cgroup.c | 206 +++++++++++++++--------- block/blk-cgroup.h | 23 ++- block/blk-crypto-fallback.c | 2 +- block/blk-iocost.c | 10 +- block/blk-iolatency.c | 7 +- drivers/md/bcache/request.c | 2 +- drivers/md/dm.c | 2 +- drivers/md/md.c | 2 +- drivers/nvdimm/nd_virtio.c | 2 +- fs/gfs2/lops.c | 3 +- include/linux/bio.h | 30 ++-- include/linux/blk_types.h | 9 +- include/linux/writeback.h | 2 +- mm/page_io.c | 10 +- 19 files changed, 218 insertions(+), 143 deletions(-) diff --git a/Documentation/admin-guide/cgroup-v2.rst b/Documentation/admin-guide/cgroup-v2.rst index 14b8c571c0d1..bbd79931d5ab 100644 --- a/Documentation/admin-guide/cgroup-v2.rst +++ b/Documentation/admin-guide/cgroup-v2.rst @@ -3235,7 +3235,7 @@ the configuration, the bio may be executed at a lower priority and if the writeback session is holding shared resources, e.g. a journal entry, may lead to priority inversion. There is no one easy solution for the problem. Filesystems can try to work around specific problem -cases by skipping wbc_init_bio() and using bio_associate_blkg() +cases by skipping wbc_init_bio() and using bio_associate_blkcg() directly. diff --git a/block/bfq-cgroup.c b/block/bfq-cgroup.c index 7e65fe6844ee..3ac3b4c05402 100644 --- a/block/bfq-cgroup.c +++ b/block/bfq-cgroup.c @@ -363,11 +363,13 @@ void bfqg_and_blkg_put(struct bfq_group *bfqg) void bfqg_stats_update_legacy_io(struct request_queue *q, struct request *rq) { - struct bfq_group *bfqg = blkg_to_bfqg(bio_blkg(rq->bio)); + struct blkcg_gq *blkg = bio_blkg_lookup(rq->bio); + struct bfq_group *bfqg; - if (!bfqg) + if (!blkg) return; + bfqg = blkg_to_bfqg(blkg); blkg_rwstat_add(&bfqg->stats.bytes, rq->cmd_flags, blk_rq_bytes(rq)); blkg_rwstat_add(&bfqg->stats.ios, rq->cmd_flags, 1); } @@ -606,7 +608,7 @@ static void bfq_link_bfqg(struct bfq_data *bfqd, struct bfq_group *bfqg) struct bfq_group *bfq_bio_bfqg(struct bfq_data *bfqd, struct bio *bio) { - struct blkcg_gq *blkg = bio_blkg(bio); + struct blkcg_gq *blkg = bio_blkg_lookup(bio); struct bfq_group *bfqg; while (blkg) { @@ -614,14 +616,16 @@ struct bfq_group *bfq_bio_bfqg(struct bfq_data *bfqd, struct bio *bio) blkg = blkg->parent; continue; } + bfqg = blkg_to_bfqg(blkg); if (bfqg->pd.online) { - bio_associate_blkg_from_css(bio, &blkg->blkcg->css); + bio_associate_blkcg_from_css(bio, &blkg->blkcg->css); return bfqg; } blkg = blkg->parent; } - bio_associate_blkg_from_css(bio, + + bio_associate_blkcg_from_css(bio, &bfqg_to_blkg(bfqd->root_group)->blkcg->css); return bfqd->root_group; } diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c index 0f75301b3115..3d51d743552c 100644 --- a/block/bfq-iosched.c +++ b/block/bfq-iosched.c @@ -128,6 +128,7 @@ #include "elevator.h" #include "blk.h" +#include "blk-cgroup.h" #include "blk-mq.h" #include "blk-mq-sched.h" #include "bfq-iosched.h" @@ -2452,15 +2453,15 @@ static bool bfq_bio_merge(struct request_queue *q, struct bio *bio, struct request *free = NULL; bool ret; +#ifdef CONFIG_BFQ_GROUP_IOSCHED + if (bic && bio_blkg_lookup(bio) == NULL) + return false; +#endif + spin_lock_irq(&bfqd->lock); if (bic) { - /* - * Make sure cgroup info is uptodate for current process before - * considering the merge. - */ bfq_bic_update_cgroup(bic, bio); - bfqd->bio_bfqq = bic_to_bfqq(bic, op_is_sync(bio->bi_opf), bfq_actuator_index(bfqd, bio)); } else { @@ -6245,6 +6246,13 @@ static void bfq_insert_request(struct blk_mq_hw_ctx *hctx, struct request *rq, LIST_HEAD(free); #ifdef CONFIG_BFQ_GROUP_IOSCHED + /* + * Pin the blkg used to look up bfqg. If this is the first IO for + * the blkcg on this queue, create the bfqg before holding bfqd->lock. + */ + if (rq->bio && !bio_flagged(rq->bio, BIO_BLKG_REF)) + bio_blkg(rq->bio); + if (!cgroup_subsys_on_dfl(io_cgrp_subsys) && rq->bio) bfqg_stats_update_legacy_io(q, rq); #endif diff --git a/block/bio.c b/block/bio.c index c207b248edba..db33c993c296 100644 --- a/block/bio.c +++ b/block/bio.c @@ -179,7 +179,7 @@ static inline gfp_t try_alloc_gfp(gfp_t gfp) void bio_uninit(struct bio *bio) { - bio_clear_blkg(bio); + bio_clear_blkcg(bio); if (bio_integrity(bio)) bio_integrity_free(bio); @@ -228,10 +228,10 @@ void bio_init(struct bio *bio, struct block_device *bdev, struct bio_vec *table, bio->bi_end_io = NULL; bio->bi_private = NULL; #ifdef CONFIG_BLK_CGROUP - bio->bi_blkg = NULL; + bio->bi_blkcg = NULL; bio->issue_time_ns = 0; if (bdev) - bio_associate_blkg(bio); + bio_associate_blkcg(bio); #ifdef CONFIG_BLK_CGROUP_IOCOST bio->bi_iocost_cost = 0; #endif @@ -276,7 +276,7 @@ void bio_reset(struct bio *bio, struct block_device *bdev, blk_opf_t opf) bio->bi_io_vec = bv; bio->bi_bdev = bdev; if (bio->bi_bdev) - bio_associate_blkg(bio); + bio_associate_blkcg(bio); bio->bi_opf = opf; } EXPORT_SYMBOL(bio_reset); @@ -860,7 +860,7 @@ static int __bio_clone(struct bio *bio, struct bio *bio_src, gfp_t gfp) if (bio->bi_bdev == bio_src->bi_bdev && bio_flagged(bio_src, BIO_REMAPPED)) bio_set_flag(bio, BIO_REMAPPED); - bio_clone_blkg_association(bio, bio_src); + bio_clone_blkcg_association(bio, bio_src); } if (bio_crypt_clone(bio, bio_src, gfp) < 0) @@ -1803,7 +1803,7 @@ void bio_endio(struct bio *bio) * a few callers of bio_init fail to call bio_uninit, so we cover up * for that here at least for now. */ - bio_clear_blkg(bio); + bio_clear_blkcg(bio); if (bio->bi_end_io) bio->bi_end_io(bio); diff --git a/block/blk-cgroup-fc-appid.c b/block/blk-cgroup-fc-appid.c index b2e16e9a7a6c..7589c6209989 100644 --- a/block/blk-cgroup-fc-appid.c +++ b/block/blk-cgroup-fc-appid.c @@ -50,12 +50,11 @@ EXPORT_SYMBOL_GPL(blkcg_set_fc_appid); */ char *blkcg_get_fc_appid(struct bio *bio) { - struct blkcg *blkcg; + struct blkcg *blkcg = bio_blkcg(bio); - if (!bio_blkg(bio)) + if (!blkcg) return NULL; - blkcg = bio_blkcg(bio); if (blkcg->fc_app_id[0] == '\0') return NULL; diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c index 753a3bdd0e8c..93ab57e0a9f1 100644 --- a/block/blk-cgroup.c +++ b/block/blk-cgroup.c @@ -241,13 +241,13 @@ void blkcg_punt_bio_submit(struct bio *bio) { struct blkcg_gq *blkg = bio_blkg(bio); - if (blkg->parent) { + if (blkg && blkg->parent) { spin_lock(&blkg->async_bio_lock); bio_list_add(&blkg->async_bios, bio); spin_unlock(&blkg->async_bio_lock); queue_work(blkcg_punt_bio_wq, &blkg->async_bio_work); } else { - /* never bounce for the root cgroup */ + /* Never bounce if there is no non-root blkg to queue on. */ submit_bio(bio); } } @@ -275,7 +275,7 @@ subsys_initcall(blkcg_punt_bio_init); */ struct cgroup_subsys_state *bio_blkcg_css(struct bio *bio) { - if (!bio || !bio_blkg(bio)) + if (!bio || !bio_blkcg(bio)) return NULL; return &bio_blkcg(bio)->css; } @@ -2051,129 +2051,181 @@ void blkcg_add_delay(struct blkcg_gq *blkg, u64 now, u64 delta) atomic64_add(delta, &blkg->delay_nsec); } -static inline struct blkcg_gq *blkg_lookup_tryget(struct blkcg_gq *blkg) +/* + * Return the blkg pinned by @bio through BIO_BLKG_REF. The returned blkg is + * already owned by @bio and no extra reference is acquired. If the pinned + * blkg can't be found, fall back to the root blkg. + */ +static struct blkcg_gq *bio_pinned_blkg(struct bio *bio) { -retry: - if (blkg_tryget(blkg)) - return blkg; + struct request_queue *q = bdev_get_queue(bio->bi_bdev); + struct blkcg_gq *blkg; - blkg = blkg->parent; - if (blkg) - goto retry; + rcu_read_lock(); + blkg = blkg_lookup(bio_blkcg(bio), q); + rcu_read_unlock(); - return NULL; + if (WARN_ON_ONCE(!blkg)) + return q->root_blkg; + return blkg; } + /** - * blkg_tryget_closest - try and get a blkg ref on the closet blkg + * bio_blkg_lookup - look up a blkg associated with a bio * @bio: target bio - * @css: target css * - * As the failure mode here is to walk up the blkg tree, this ensure that the - * blkg->parent pointers are always valid. This returns the blkg that it ended - * up taking a reference on or %NULL if no reference was taken. + * Look up the queue-local blkg for @bio's current device and blkcg without + * creating a missing blkg. The first successful lookup pins the blkg to @bio; + * later lookups reuse the bio-owned reference. */ -static inline struct blkcg_gq *blkg_tryget_closest(struct bio *bio, - struct cgroup_subsys_state *css) +struct blkcg_gq *bio_blkg_lookup(struct bio *bio) { - struct request_queue *q = bio->bi_bdev->bd_queue; - struct blkcg *blkcg = css_to_blkcg(css); + struct blkcg *blkcg = bio_blkcg(bio); + struct request_queue *q; struct blkcg_gq *blkg; + if (bio_flagged(bio, BIO_BLKG_REF)) + return bio_pinned_blkg(bio); + + if (!blkcg || !bio->bi_bdev) + return NULL; + + q = bdev_get_queue(bio->bi_bdev); rcu_read_lock(); blkg = blkg_lookup(blkcg, q); - if (likely(blkg)) - blkg = blkg_lookup_tryget(blkg); + if (blkg && blkg_tryget(blkg)) + bio_set_flag(bio, BIO_BLKG_REF); + else + blkg = NULL; rcu_read_unlock(); - if (blkg) - return blkg; + return blkg; +} +EXPORT_SYMBOL_GPL(bio_blkg_lookup); + +/** + * bio_put_blkg_ref - drop the blkg reference pinned by a bio + * @bio: target bio + * + * Drop the bio-owned blkg reference acquired by bio_blkg(), if any. + */ +void bio_put_blkg_ref(struct bio *bio) +{ + if (bio_flagged(bio, BIO_BLKG_REF)) { + struct blkcg_gq *blkg = bio_pinned_blkg(bio); + + blkg_put(blkg); + bio_clear_flag(bio, BIO_BLKG_REF); + } +} +EXPORT_SYMBOL_GPL(bio_put_blkg_ref); + +/** + * bio_blkg - look up the blkg associated with a bio + * @bio: target bio + * + * Look up the queue-local blkg for @bio's current device and blkcg. If this + * is the first policy use of @bio, create the missing blkg hierarchy if + * necessary, pin the exact blkg, and mark @bio so bio_clear_blkcg() can drop + * the reference when the bio completes. + */ +struct blkcg_gq *bio_blkg(struct bio *bio) +{ + struct blkcg *blkcg = bio_blkcg(bio); + struct gendisk *disk; + struct request_queue *q; + struct blkcg_gq *blkg; + + if (!blkcg || !bio->bi_bdev) + return NULL; + + if (bio_flagged(bio, BIO_BLKG_REF)) + return bio_pinned_blkg(bio); + + disk = bio->bi_bdev->bd_disk; + q = disk->queue; - /* - * Fast path failed, we're probably issuing IO in this cgroup the first - * time, hold lock to create new blkg. - */ spin_lock_irq(&q->queue_lock); - blkg = blkg_lookup_create(blkcg, bio->bi_bdev->bd_disk); - if (blkg) - blkg = blkg_lookup_tryget(blkg); + blkg = blkg_lookup_create(blkcg, disk); + if (blkg && blkg->blkcg == blkcg && blkg_tryget(blkg)) + bio_set_flag(bio, BIO_BLKG_REF); + else + blkg = NULL; spin_unlock_irq(&q->queue_lock); return blkg; } +EXPORT_SYMBOL_GPL(bio_blkg); /** - * bio_associate_blkg_from_css - associate a bio with a specified css + * bio_associate_blkcg_from_css - associate a bio with a specified css * @bio: target bio * @css: target css * - * Associate @bio with the blkg found by combining the css's blkg and the - * request_queue of the @bio. An association failure is handled by walking up - * the blkg tree. Therefore, the blkg associated can be anything between @blkg - * and q->root_blkg. This situation only happens when a cgroup is dying and - * then the remaining bios will spill to the closest alive blkg. + * Associate @bio with the blkcg found from @css. The queue-local blkg is + * created and pinned by bio_blkg() when blkcg policies need it. * - * A reference will be taken on the blkg and will be released when @bio is + * A reference will be taken on the blkcg and will be released when @bio is * freed. */ -void bio_associate_blkg_from_css(struct bio *bio, +void bio_associate_blkcg_from_css(struct bio *bio, struct cgroup_subsys_state *css) { - if (bio_blkg(bio)) - blkg_put(bio_blkg(bio)); + struct blkcg *blkcg; - if (css && css->parent) { - bio->bi_blkg = blkg_tryget_closest(bio, css); - } else { - blkg_get(bdev_get_queue(bio->bi_bdev)->root_blkg); - bio->bi_blkg = bdev_get_queue(bio->bi_bdev)->root_blkg; - } + if (!css || !css->parent) + css = &blkcg_root.css; + + blkcg = css_to_blkcg(css); + if (bio_blkcg(bio) == blkcg) + return; + + css_get(css); + bio_clear_blkcg(bio); + bio->bi_blkcg = blkcg; } -EXPORT_SYMBOL_GPL(bio_associate_blkg_from_css); +EXPORT_SYMBOL_GPL(bio_associate_blkcg_from_css); /** - * bio_associate_blkg - associate a bio with a blkg + * bio_associate_blkcg - associate a bio with a blkcg * @bio: target bio * - * Associate @bio with the blkg found from the bio's css and request_queue. - * If one is not found, bio_lookup_blkg() creates the blkg. If a blkg is - * already associated, the css is reused and association redone as the - * request_queue may have changed. + * Associate @bio with the blkcg found from the bio's css. If a blkcg is + * already associated, keep it as blkcg association is not queue-local. */ -void bio_associate_blkg(struct bio *bio) +void bio_associate_blkcg(struct bio *bio) { struct cgroup_subsys_state *css; if (blk_op_is_passthrough(bio->bi_opf)) return; - if (bio_blkg(bio)) { - css = bio_blkcg_css(bio); - bio_associate_blkg_from_css(bio, css); - } else { - rcu_read_lock(); - css = blkcg_css(); - if (!css_tryget_online(css)) - css = NULL; - rcu_read_unlock(); + if (bio_blkcg(bio)) + return; - bio_associate_blkg_from_css(bio, css); - if (css) - css_put(css); - } + rcu_read_lock(); + css = blkcg_css(); + if (!css_tryget_online(css)) + css = NULL; + rcu_read_unlock(); + + bio_associate_blkcg_from_css(bio, css); + if (css) + css_put(css); } -EXPORT_SYMBOL_GPL(bio_associate_blkg); +EXPORT_SYMBOL_GPL(bio_associate_blkcg); /** - * bio_clone_blkg_association - clone blkg association from src to dst bio + * bio_clone_blkcg_association - clone blkcg association from src to dst bio * @dst: destination bio * @src: source bio */ -void bio_clone_blkg_association(struct bio *dst, struct bio *src) +void bio_clone_blkcg_association(struct bio *dst, struct bio *src) { - if (bio_blkg(src)) - bio_associate_blkg_from_css(dst, bio_blkcg_css(src)); + if (bio_blkcg(src)) + bio_associate_blkcg_from_css(dst, bio_blkcg_css(src)); } -EXPORT_SYMBOL_GPL(bio_clone_blkg_association); +EXPORT_SYMBOL_GPL(bio_clone_blkcg_association); static int blk_cgroup_io_type(struct bio *bio) { @@ -2186,19 +2238,25 @@ static int blk_cgroup_io_type(struct bio *bio) void blk_cgroup_bio_start(struct bio *bio) { - struct blkcg_gq *blkg = bio_blkg(bio); struct blkcg *blkcg = bio_blkcg(bio); + struct blkcg_gq *blkg; int rwd = blk_cgroup_io_type(bio), cpu; struct blkg_iostat_set *bis; unsigned long flags; if (!cgroup_subsys_on_dfl(io_cgrp_subsys)) return; + if (!blkcg) + return; /* Root-level stats are sourced from system-wide IO stats */ if (!cgroup_parent(blkcg->css.cgroup)) return; + blkg = bio_blkg_lookup(bio); + if (!blkg) + return; + cpu = get_cpu(); bis = per_cpu_ptr(blkg->iostat_cpu, cpu); flags = u64_stats_update_begin_irqsave(&bis->sync); diff --git a/block/blk-cgroup.h b/block/blk-cgroup.h index 1e80b0a73233..b6fb85db4d3d 100644 --- a/block/blk-cgroup.h +++ b/block/blk-cgroup.h @@ -126,7 +126,7 @@ static inline struct blkcg *css_to_blkcg(struct cgroup_subsys_state *css) static inline struct blkcg *bio_blkcg(struct bio *bio) { - return bio_blkg(bio)->blkcg; + return bio->bi_blkcg; } /* @@ -281,6 +281,9 @@ static inline struct blkcg_gq *blkg_lookup(struct blkcg *blkcg, return blkg; } +struct blkcg_gq *bio_blkg_lookup(struct bio *bio); +struct blkcg_gq *bio_blkg(struct bio *bio); + /** * blkg_to_pd - get policy private data * @blkg: blkg of interest @@ -348,13 +351,15 @@ static inline void blkg_put(struct blkcg_gq *blkg) percpu_ref_put(&blkg->refcnt); } -static inline void bio_clear_blkg(struct bio *bio) +static inline void bio_clear_blkcg(struct bio *bio) { - struct blkcg_gq *blkg = bio_blkg(bio); + struct blkcg *blkcg = bio_blkcg(bio); + + bio_put_blkg_ref(bio); - if (blkg) { - blkg_put(blkg); - bio->bi_blkg = NULL; + if (blkcg) { + css_put(&blkcg->css); + bio->bi_blkcg = NULL; } } @@ -470,7 +475,7 @@ static inline void blkcg_clear_delay(struct blkcg_gq *blkg) */ static inline bool blk_cgroup_mergeable(struct request *rq, struct bio *bio) { - return bio_blkg(rq->bio) == bio_blkg(bio) && + return bio_blkcg(rq->bio) == bio_blkcg(bio) && bio_issue_as_root_blkg(rq->bio) == bio_issue_as_root_blkg(bio); } @@ -497,6 +502,8 @@ struct blkcg { }; static inline struct blkcg *bio_blkcg(struct bio *bio) { return NULL; } +static inline struct blkcg_gq *bio_blkg_lookup(struct bio *bio) { return NULL; } +static inline struct blkcg_gq *bio_blkg(struct bio *bio) { return NULL; } static inline struct blkcg_gq *blkg_lookup(struct blkcg *blkcg, void *key) { return NULL; } static inline void blkg_init_queue(struct request_queue *q) { } static inline int blkcg_init_disk(struct gendisk *disk) { return 0; } @@ -513,7 +520,7 @@ static inline struct blkg_policy_data *blkg_to_pd(struct blkcg_gq *blkg, static inline struct blkcg_gq *pd_to_blkg(struct blkg_policy_data *pd) { return NULL; } static inline void blkg_get(struct blkcg_gq *blkg) { } static inline void blkg_put(struct blkcg_gq *blkg) { } -static inline void bio_clear_blkg(struct bio *bio) { } +static inline void bio_clear_blkcg(struct bio *bio) { } static inline void blk_cgroup_bio_start(struct bio *bio) { } static inline bool blk_cgroup_mergeable(struct request *rq, struct bio *bio) { return true; } diff --git a/block/blk-crypto-fallback.c b/block/blk-crypto-fallback.c index 2a5c52ab74b4..5ef4baab444b 100644 --- a/block/blk-crypto-fallback.c +++ b/block/blk-crypto-fallback.c @@ -187,7 +187,7 @@ static struct bio *blk_crypto_alloc_enc_bio(struct bio *bio_src, bio->bi_write_hint = bio_src->bi_write_hint; bio->bi_write_stream = bio_src->bi_write_stream; bio->bi_iter.bi_sector = bio_src->bi_iter.bi_sector; - bio_clone_blkg_association(bio, bio_src); + bio_clone_blkcg_association(bio, bio_src); /* * Move page array up in the allocated memory for the bio vecs as far as diff --git a/block/blk-iocost.c b/block/blk-iocost.c index d4470476bcd0..62ffd759bb95 100644 --- a/block/blk-iocost.c +++ b/block/blk-iocost.c @@ -2775,7 +2775,7 @@ static void ioc_rqos_throttle(struct rq_qos *rqos, struct bio *bio) static void ioc_rqos_merge(struct rq_qos *rqos, struct request *rq, struct bio *bio) { - struct ioc_gq *iocg = blkg_to_iocg(bio_blkg(bio)); + struct ioc_gq *iocg = blkg_to_iocg(bio_blkg_lookup(bio)); struct ioc *ioc = rqos_to_ioc(rqos); sector_t bio_end = bio_end_sector(bio); struct ioc_now now; @@ -2833,9 +2833,13 @@ static void ioc_rqos_merge(struct rq_qos *rqos, struct request *rq, static void ioc_rqos_done_bio(struct rq_qos *rqos, struct bio *bio) { - struct ioc_gq *iocg = blkg_to_iocg(bio_blkg(bio)); + struct ioc_gq *iocg; + + if (!bio->bi_iocost_cost) + return; - if (iocg && bio->bi_iocost_cost) + iocg = blkg_to_iocg(bio_blkg_lookup(bio)); + if (iocg) atomic64_add(bio->bi_iocost_cost, &iocg->done_vtime); } diff --git a/block/blk-iolatency.c b/block/blk-iolatency.c index c0d8d5f6bdba..7ad18a538d7e 100644 --- a/block/blk-iolatency.c +++ b/block/blk-iolatency.c @@ -590,8 +590,11 @@ static void blkcg_iolatency_done_bio(struct rq_qos *rqos, struct bio *bio) bool issue_as_root = bio_issue_as_root_blkg(bio); int inflight = 0; - blkg = bio_blkg(bio); - if (!blkg || !bio_flagged(bio, BIO_QOS_THROTTLED)) + if (!bio_flagged(bio, BIO_QOS_THROTTLED)) + return; + + blkg = bio_blkg_lookup(bio); + if (!blkg) return; iolat = blkg_to_lat(blkg); diff --git a/drivers/md/bcache/request.c b/drivers/md/bcache/request.c index 3fa3b13a410f..c0f945b8d941 100644 --- a/drivers/md/bcache/request.c +++ b/drivers/md/bcache/request.c @@ -848,7 +848,7 @@ static CLOSURE_CALLBACK(cached_dev_read_done) s->iop.bio->bi_iter.bi_sector = s->cache_miss->bi_iter.bi_sector; s->iop.bio->bi_iter.bi_size = s->insert_bio_sectors << 9; - bio_clone_blkg_association(s->iop.bio, s->cache_miss); + bio_clone_blkcg_association(s->iop.bio, s->cache_miss); bch_bio_map(s->iop.bio, NULL); bio_copy_data(s->cache_miss, s->iop.bio); diff --git a/drivers/md/dm.c b/drivers/md/dm.c index d413bfaf3527..cd68eec77f5a 100644 --- a/drivers/md/dm.c +++ b/drivers/md/dm.c @@ -1373,7 +1373,7 @@ void dm_submit_bio_remap(struct bio *clone, struct bio *tgt_clone) if (!tgt_clone) tgt_clone = clone; - bio_clone_blkg_association(tgt_clone, io->orig_bio); + bio_clone_blkcg_association(tgt_clone, io->orig_bio); /* * Account io->origin_bio to DM dev on behalf of target diff --git a/drivers/md/md.c b/drivers/md/md.c index d1465bcd86c8..af55f8efa46b 100644 --- a/drivers/md/md.c +++ b/drivers/md/md.c @@ -9355,7 +9355,7 @@ void md_submit_discard_bio(struct mddev *mddev, struct md_rdev *rdev, return; bio_chain(discard_bio, bio); - bio_clone_blkg_association(discard_bio, bio); + bio_clone_blkcg_association(discard_bio, bio); mddev_trace_remap(mddev, discard_bio, bio->bi_iter.bi_sector); submit_bio_noacct(discard_bio); } diff --git a/drivers/nvdimm/nd_virtio.c b/drivers/nvdimm/nd_virtio.c index 4176046627be..54e4adb0ccb7 100644 --- a/drivers/nvdimm/nd_virtio.c +++ b/drivers/nvdimm/nd_virtio.c @@ -121,7 +121,7 @@ int async_pmem_flush(struct nd_region *nd_region, struct bio *bio) if (!child) return -ENOMEM; - bio_clone_blkg_association(child, bio); + bio_clone_blkcg_association(child, bio); child->bi_iter.bi_sector = -1; bio_chain(child, bio); submit_bio(child); diff --git a/fs/gfs2/lops.c b/fs/gfs2/lops.c index 6dabe73ad790..6512dbd9516f 100644 --- a/fs/gfs2/lops.c +++ b/fs/gfs2/lops.c @@ -484,7 +484,7 @@ static struct bio *gfs2_chain_bio(struct bio *prev, unsigned int nr_iovecs, struct bio *new; new = bio_alloc(prev->bi_bdev, nr_iovecs, opf, GFP_NOIO); - bio_clone_blkg_association(new, prev); + bio_clone_blkcg_association(new, prev); new->bi_iter.bi_sector = sector; bio_chain(new, prev); submit_bio(prev); @@ -1114,4 +1114,3 @@ const struct gfs2_log_operations *gfs2_log_ops[] = { &gfs2_revoke_lops, NULL, }; - diff --git a/include/linux/bio.h b/include/linux/bio.h index dc4baa3602b7..e5799fdf431d 100644 --- a/include/linux/bio.h +++ b/include/linux/bio.h @@ -503,28 +503,20 @@ static inline void bio_release_pages(struct bio *bio, bool mark_dirty) disk_devt((bio)->bi_bdev->bd_disk) #ifdef CONFIG_BLK_CGROUP -static inline struct blkcg_gq *bio_blkg(struct bio *bio) -{ - return bio->bi_blkg; -} - -void bio_associate_blkg(struct bio *bio); -void bio_associate_blkg_from_css(struct bio *bio, +void bio_associate_blkcg(struct bio *bio); +void bio_associate_blkcg_from_css(struct bio *bio, struct cgroup_subsys_state *css); -void bio_clone_blkg_association(struct bio *dst, struct bio *src); +void bio_clone_blkcg_association(struct bio *dst, struct bio *src); +void bio_put_blkg_ref(struct bio *bio); void blkcg_punt_bio_submit(struct bio *bio); #else /* CONFIG_BLK_CGROUP */ -static inline struct blkcg_gq *bio_blkg(struct bio *bio) -{ - return NULL; -} - -static inline void bio_associate_blkg(struct bio *bio) { } -static inline void bio_associate_blkg_from_css(struct bio *bio, +static inline void bio_associate_blkcg(struct bio *bio) { } +static inline void bio_associate_blkcg_from_css(struct bio *bio, struct cgroup_subsys_state *css) { } -static inline void bio_clone_blkg_association(struct bio *dst, +static inline void bio_clone_blkcg_association(struct bio *dst, struct bio *src) { } +static inline void bio_put_blkg_ref(struct bio *bio) { } static inline void blkcg_punt_bio_submit(struct bio *bio) { submit_bio(bio); @@ -534,10 +526,12 @@ static inline void blkcg_punt_bio_submit(struct bio *bio) static inline void bio_set_dev(struct bio *bio, struct block_device *bdev) { bio_clear_flag(bio, BIO_REMAPPED); - if (bio->bi_bdev != bdev) + if (bio->bi_bdev != bdev) { + bio_put_blkg_ref(bio); bio_clear_flag(bio, BIO_BPS_THROTTLED); + } bio->bi_bdev = bdev; - bio_associate_blkg(bio); + bio_associate_blkcg(bio); } /* diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h index 8808ee76e73c..5f95c2e0e90b 100644 --- a/include/linux/blk_types.h +++ b/include/linux/blk_types.h @@ -246,12 +246,10 @@ struct bio { void *bi_private; #ifdef CONFIG_BLK_CGROUP /* - * Represents the association of the css and request_queue for the bio. - * If a bio goes direct to device, it will not have a blkg as it will - * not have a request_queue associated with it. The reference is put - * on release of the bio. + * Represents the blkcg css association for the bio. The reference is + * put on release of the bio. */ - struct blkcg_gq *bi_blkg; + struct blkcg *bi_blkcg; /* Time that this bio was issued. */ u64 issue_time_ns; #ifdef CONFIG_BLK_CGROUP_IOCOST @@ -309,6 +307,7 @@ enum { BIO_TRACE_COMPLETION, /* bio_endio() should trace the final completion * of this bio. */ BIO_CGROUP_ACCT, /* has been accounted to a cgroup */ + BIO_BLKG_REF, /* bio pins the associated blkg */ BIO_QOS_THROTTLED, /* bio went through rq_qos throttle path */ /* * This bio has completed bps throttling at the single tg granularity, diff --git a/include/linux/writeback.h b/include/linux/writeback.h index 62552a2ce5b9..4f869fe9cc90 100644 --- a/include/linux/writeback.h +++ b/include/linux/writeback.h @@ -262,7 +262,7 @@ static inline void wbc_init_bio(struct writeback_control *wbc, struct bio *bio) * regular writeback instead of writing things out itself. */ if (wbc->wb) - bio_associate_blkg_from_css(bio, wbc->wb->blkcg_css); + bio_associate_blkcg_from_css(bio, wbc->wb->blkcg_css); } void inode_switch_wbs_work_fn(struct work_struct *work); diff --git a/mm/page_io.c b/mm/page_io.c index b23f494fcc83..112e50475605 100644 --- a/mm/page_io.c +++ b/mm/page_io.c @@ -316,7 +316,7 @@ static inline void count_swpout_vm_event(struct folio *folio) } #if defined(CONFIG_MEMCG) && defined(CONFIG_BLK_CGROUP) -static void bio_associate_blkg_from_page(struct bio *bio, struct folio *folio) +static void bio_associate_blkcg_from_page(struct bio *bio, struct folio *folio) { struct cgroup_subsys_state *css; struct mem_cgroup *memcg; @@ -331,12 +331,12 @@ static void bio_associate_blkg_from_page(struct bio *bio, struct folio *folio) css = NULL; rcu_read_unlock(); - bio_associate_blkg_from_css(bio, css); + bio_associate_blkcg_from_css(bio, css); if (css) css_put(css); } #else -#define bio_associate_blkg_from_page(bio, folio) do { } while (0) +#define bio_associate_blkcg_from_page(bio, folio) do { } while (0) #endif /* CONFIG_MEMCG && CONFIG_BLK_CGROUP */ struct swap_iocb { @@ -436,7 +436,7 @@ static void swap_writepage_bdev_sync(struct folio *folio, bio.bi_iter.bi_sector = swap_folio_sector(folio); bio_add_folio_nofail(&bio, folio, folio_size(folio), 0); - bio_associate_blkg_from_page(&bio, folio); + bio_associate_blkcg_from_page(&bio, folio); count_swpout_vm_event(folio); folio_start_writeback(folio); @@ -456,7 +456,7 @@ static void swap_writepage_bdev_async(struct folio *folio, bio->bi_end_io = end_swap_bio_write; bio_add_folio_nofail(bio, folio, folio_size(folio), 0); - bio_associate_blkg_from_page(bio, folio); + bio_associate_blkcg_from_page(bio, folio); count_swpout_vm_event(folio); folio_start_writeback(folio); folio_unlock(folio); -- 2.51.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [RFC PATCH v1 2/3] blk-cgroup: store blkcg in bio instead of blkg 2026-08-04 6:53 ` [RFC PATCH v1 2/3] blk-cgroup: store blkcg in bio instead of blkg Yu Kuai @ 2026-08-04 9:19 ` Tao Cui 2026-08-04 13:32 ` Christoph Hellwig 0 siblings, 1 reply; 12+ messages in thread From: Tao Cui @ 2026-08-04 9:19 UTC (permalink / raw) To: Yu Kuai, Jens Axboe Cc: cui.tao, Tejun Heo, Johannes Weiner, Michal Koutný, Jonathan Corbet, Yu Kuai, Josef Bacik, Coly Li, Kent Overstreet, Alasdair Kergon, Mike Snitzer, Mikulas Patocka, Benjamin Marzinski, Song Liu, Dan Williams, Vishal Verma, Dave Jiang, Alison Schofield, Pankaj Gupta, Andreas Gruenbacher, Matthew Wilcox, Jan Kara, Andrew Morton, Chris Li, Kairui Song, Christoph Hellwig, Nilay Shroff, cgroups, linux-doc, linux-kernel, linux-block, linux-bcache, dm-devel, linux-raid, nvdimm, virtualization, gfs2, linux-fsdevel, linux-mm Hi Kuai, 在 2026/8/4 14:53, Yu Kuai 写道: > From: Yu Kuai <yukuai@fygo.io> > > A bio currently stores a queue-local blkg reference. This forces bio > association and remap paths to look up or create a blkg even when the bio > will never enter a blkcg policy. > > Store the blkcg css association in the bio instead, and derive the blkg > from the bio's blkcg and current bdev when a policy needs it. The first > successful policy lookup pins the blkg, records the pin with BIO_BLKG_REF, > and drops it from bio_clear_blkcg() or when bio_set_dev() changes the > lookup key. > > Keep lookup-only users from creating missing blkgs by using > bio_blkg_lookup(), and rename the bio cgroup association helpers to match > the stored blkcg state. > > Signed-off-by: Yu Kuai <yukuai@fygo.io> > --- > Documentation/admin-guide/cgroup-v2.rst | 2 +- > block/bfq-cgroup.c | 14 +- > block/bfq-iosched.c | 18 ++- > block/bio.c | 12 +- > block/blk-cgroup-fc-appid.c | 5 +- > block/blk-cgroup.c | 206 +++++++++++++++--------- > block/blk-cgroup.h | 23 ++- > block/blk-crypto-fallback.c | 2 +- > block/blk-iocost.c | 10 +- > block/blk-iolatency.c | 7 +- > drivers/md/bcache/request.c | 2 +- > drivers/md/dm.c | 2 +- > drivers/md/md.c | 2 +- > drivers/nvdimm/nd_virtio.c | 2 +- > fs/gfs2/lops.c | 3 +- > include/linux/bio.h | 30 ++-- > include/linux/blk_types.h | 9 +- > include/linux/writeback.h | 2 +- > mm/page_io.c | 10 +- > 19 files changed, 218 insertions(+), 143 deletions(-) > > diff --git a/Documentation/admin-guide/cgroup-v2.rst b/Documentation/admin-guide/cgroup-v2.rst > index 14b8c571c0d1..bbd79931d5ab 100644 > --- a/Documentation/admin-guide/cgroup-v2.rst > +++ b/Documentation/admin-guide/cgroup-v2.rst > @@ -3235,7 +3235,7 @@ the configuration, the bio may be executed at a lower priority and if > the writeback session is holding shared resources, e.g. a journal > entry, may lead to priority inversion. There is no one easy solution > for the problem. Filesystems can try to work around specific problem > -cases by skipping wbc_init_bio() and using bio_associate_blkg() > +cases by skipping wbc_init_bio() and using bio_associate_blkcg() > directly. > > > diff --git a/block/bfq-cgroup.c b/block/bfq-cgroup.c > index 7e65fe6844ee..3ac3b4c05402 100644 > --- a/block/bfq-cgroup.c > +++ b/block/bfq-cgroup.c > @@ -363,11 +363,13 @@ void bfqg_and_blkg_put(struct bfq_group *bfqg) > > void bfqg_stats_update_legacy_io(struct request_queue *q, struct request *rq) > { > - struct bfq_group *bfqg = blkg_to_bfqg(bio_blkg(rq->bio)); > + struct blkcg_gq *blkg = bio_blkg_lookup(rq->bio); > + struct bfq_group *bfqg; > > - if (!bfqg) > + if (!blkg) > return; > > + bfqg = blkg_to_bfqg(blkg); > blkg_rwstat_add(&bfqg->stats.bytes, rq->cmd_flags, blk_rq_bytes(rq)); > blkg_rwstat_add(&bfqg->stats.ios, rq->cmd_flags, 1); > } > @@ -606,7 +608,7 @@ static void bfq_link_bfqg(struct bfq_data *bfqd, struct bfq_group *bfqg) > > struct bfq_group *bfq_bio_bfqg(struct bfq_data *bfqd, struct bio *bio) > { > - struct blkcg_gq *blkg = bio_blkg(bio); > + struct blkcg_gq *blkg = bio_blkg_lookup(bio); > struct bfq_group *bfqg; > > while (blkg) { > @@ -614,14 +616,16 @@ struct bfq_group *bfq_bio_bfqg(struct bfq_data *bfqd, struct bio *bio) > blkg = blkg->parent; > continue; > } > + > bfqg = blkg_to_bfqg(blkg); > if (bfqg->pd.online) { > - bio_associate_blkg_from_css(bio, &blkg->blkcg->css); > + bio_associate_blkcg_from_css(bio, &blkg->blkcg->css); > return bfqg; > } > blkg = blkg->parent; > } > - bio_associate_blkg_from_css(bio, > + > + bio_associate_blkcg_from_css(bio, > &bfqg_to_blkg(bfqd->root_group)->blkcg->css); > return bfqd->root_group; > } > diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c > index 0f75301b3115..3d51d743552c 100644 > --- a/block/bfq-iosched.c > +++ b/block/bfq-iosched.c > @@ -128,6 +128,7 @@ > > #include "elevator.h" > #include "blk.h" > +#include "blk-cgroup.h" > #include "blk-mq.h" > #include "blk-mq-sched.h" > #include "bfq-iosched.h" > @@ -2452,15 +2453,15 @@ static bool bfq_bio_merge(struct request_queue *q, struct bio *bio, > struct request *free = NULL; > bool ret; > > +#ifdef CONFIG_BFQ_GROUP_IOSCHED > + if (bic && bio_blkg_lookup(bio) == NULL) > + return false; > +#endif > + > spin_lock_irq(&bfqd->lock); > > if (bic) { > - /* > - * Make sure cgroup info is uptodate for current process before > - * considering the merge. > - */ > bfq_bic_update_cgroup(bic, bio); > - > bfqd->bio_bfqq = bic_to_bfqq(bic, op_is_sync(bio->bi_opf), > bfq_actuator_index(bfqd, bio)); > } else { > @@ -6245,6 +6246,13 @@ static void bfq_insert_request(struct blk_mq_hw_ctx *hctx, struct request *rq, > LIST_HEAD(free); > > #ifdef CONFIG_BFQ_GROUP_IOSCHED > + /* > + * Pin the blkg used to look up bfqg. If this is the first IO for > + * the blkcg on this queue, create the bfqg before holding bfqd->lock. > + */ > + if (rq->bio && !bio_flagged(rq->bio, BIO_BLKG_REF)) > + bio_blkg(rq->bio); > + > if (!cgroup_subsys_on_dfl(io_cgrp_subsys) && rq->bio) > bfqg_stats_update_legacy_io(q, rq); > #endif > diff --git a/block/bio.c b/block/bio.c > index c207b248edba..db33c993c296 100644 > --- a/block/bio.c > +++ b/block/bio.c > @@ -179,7 +179,7 @@ static inline gfp_t try_alloc_gfp(gfp_t gfp) > > void bio_uninit(struct bio *bio) > { > - bio_clear_blkg(bio); > + bio_clear_blkcg(bio); > if (bio_integrity(bio)) > bio_integrity_free(bio); > > @@ -228,10 +228,10 @@ void bio_init(struct bio *bio, struct block_device *bdev, struct bio_vec *table, > bio->bi_end_io = NULL; > bio->bi_private = NULL; > #ifdef CONFIG_BLK_CGROUP > - bio->bi_blkg = NULL; > + bio->bi_blkcg = NULL; > bio->issue_time_ns = 0; > if (bdev) > - bio_associate_blkg(bio); > + bio_associate_blkcg(bio); > #ifdef CONFIG_BLK_CGROUP_IOCOST > bio->bi_iocost_cost = 0; > #endif > @@ -276,7 +276,7 @@ void bio_reset(struct bio *bio, struct block_device *bdev, blk_opf_t opf) > bio->bi_io_vec = bv; > bio->bi_bdev = bdev; > if (bio->bi_bdev) > - bio_associate_blkg(bio); > + bio_associate_blkcg(bio); > bio->bi_opf = opf; > } > EXPORT_SYMBOL(bio_reset); > @@ -860,7 +860,7 @@ static int __bio_clone(struct bio *bio, struct bio *bio_src, gfp_t gfp) > if (bio->bi_bdev == bio_src->bi_bdev && > bio_flagged(bio_src, BIO_REMAPPED)) > bio_set_flag(bio, BIO_REMAPPED); > - bio_clone_blkg_association(bio, bio_src); > + bio_clone_blkcg_association(bio, bio_src); > } > > if (bio_crypt_clone(bio, bio_src, gfp) < 0) > @@ -1803,7 +1803,7 @@ void bio_endio(struct bio *bio) > * a few callers of bio_init fail to call bio_uninit, so we cover up > * for that here at least for now. > */ > - bio_clear_blkg(bio); > + bio_clear_blkcg(bio); > > if (bio->bi_end_io) > bio->bi_end_io(bio); > diff --git a/block/blk-cgroup-fc-appid.c b/block/blk-cgroup-fc-appid.c > index b2e16e9a7a6c..7589c6209989 100644 > --- a/block/blk-cgroup-fc-appid.c > +++ b/block/blk-cgroup-fc-appid.c > @@ -50,12 +50,11 @@ EXPORT_SYMBOL_GPL(blkcg_set_fc_appid); > */ > char *blkcg_get_fc_appid(struct bio *bio) > { > - struct blkcg *blkcg; > + struct blkcg *blkcg = bio_blkcg(bio); > > - if (!bio_blkg(bio)) > + if (!blkcg) > return NULL; > > - blkcg = bio_blkcg(bio); > if (blkcg->fc_app_id[0] == '\0') > return NULL; > > diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c > index 753a3bdd0e8c..93ab57e0a9f1 100644 > --- a/block/blk-cgroup.c > +++ b/block/blk-cgroup.c > @@ -241,13 +241,13 @@ void blkcg_punt_bio_submit(struct bio *bio) > { > struct blkcg_gq *blkg = bio_blkg(bio); > > - if (blkg->parent) { > + if (blkg && blkg->parent) { > spin_lock(&blkg->async_bio_lock); > bio_list_add(&blkg->async_bios, bio); > spin_unlock(&blkg->async_bio_lock); > queue_work(blkcg_punt_bio_wq, &blkg->async_bio_work); > } else { > - /* never bounce for the root cgroup */ > + /* Never bounce if there is no non-root blkg to queue on. */ > submit_bio(bio); > } > } > @@ -275,7 +275,7 @@ subsys_initcall(blkcg_punt_bio_init); > */ > struct cgroup_subsys_state *bio_blkcg_css(struct bio *bio) > { > - if (!bio || !bio_blkg(bio)) > + if (!bio || !bio_blkcg(bio)) > return NULL; > return &bio_blkcg(bio)->css; > } > @@ -2051,129 +2051,181 @@ void blkcg_add_delay(struct blkcg_gq *blkg, u64 now, u64 delta) > atomic64_add(delta, &blkg->delay_nsec); > } > > -static inline struct blkcg_gq *blkg_lookup_tryget(struct blkcg_gq *blkg) > +/* > + * Return the blkg pinned by @bio through BIO_BLKG_REF. The returned blkg is > + * already owned by @bio and no extra reference is acquired. If the pinned > + * blkg can't be found, fall back to the root blkg. > + */ > +static struct blkcg_gq *bio_pinned_blkg(struct bio *bio) > { > -retry: > - if (blkg_tryget(blkg)) > - return blkg; > + struct request_queue *q = bdev_get_queue(bio->bi_bdev); > + struct blkcg_gq *blkg; > > - blkg = blkg->parent; > - if (blkg) > - goto retry; > + rcu_read_lock(); > + blkg = blkg_lookup(bio_blkcg(bio), q); > + rcu_read_unlock(); > > - return NULL; > + if (WARN_ON_ONCE(!blkg)) > + return q->root_blkg; > + return blkg; > } 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. Reproduces on boot: WARNING: CPU: 0 PID: 11 at block/blk-cgroup.c:2061 bio_pinned_blkg+0x65/0xa0 Workqueue: kthrotld blk_throtl_dispatch_work_fn Call Trace: blk_cgroup_bio_start+0x49/0xe0 submit_bio_noacct_nocheck+0x2f/0x350 blk_throtl_dispatch_work_fn+0xd2/0x110 process_one_work+0x1a2/0x3f0 worker_thread+0x172/0x2e0 kthread+0xdd/0x110 ret_from_fork+0x1bd/0x220 Maybe keeping the pinned blkg pointer in the bio would sidestep this, so the lookup can't miss? Thanks, Tao > + > /** > - * blkg_tryget_closest - try and get a blkg ref on the closet blkg > + * bio_blkg_lookup - look up a blkg associated with a bio > * @bio: target bio > - * @css: target css > * > - * As the failure mode here is to walk up the blkg tree, this ensure that the > - * blkg->parent pointers are always valid. This returns the blkg that it ended > - * up taking a reference on or %NULL if no reference was taken. > + * Look up the queue-local blkg for @bio's current device and blkcg without > + * creating a missing blkg. The first successful lookup pins the blkg to @bio; > + * later lookups reuse the bio-owned reference. > */ > -static inline struct blkcg_gq *blkg_tryget_closest(struct bio *bio, > - struct cgroup_subsys_state *css) > +struct blkcg_gq *bio_blkg_lookup(struct bio *bio) > { > - struct request_queue *q = bio->bi_bdev->bd_queue; > - struct blkcg *blkcg = css_to_blkcg(css); > + struct blkcg *blkcg = bio_blkcg(bio); > + struct request_queue *q; > struct blkcg_gq *blkg; > > + if (bio_flagged(bio, BIO_BLKG_REF)) > + return bio_pinned_blkg(bio); > + > + if (!blkcg || !bio->bi_bdev) > + return NULL; > + > + q = bdev_get_queue(bio->bi_bdev); > rcu_read_lock(); > blkg = blkg_lookup(blkcg, q); > - if (likely(blkg)) > - blkg = blkg_lookup_tryget(blkg); > + if (blkg && blkg_tryget(blkg)) > + bio_set_flag(bio, BIO_BLKG_REF); > + else > + blkg = NULL; > rcu_read_unlock(); > > - if (blkg) > - return blkg; > + return blkg; > +} > +EXPORT_SYMBOL_GPL(bio_blkg_lookup); > + > +/** > + * bio_put_blkg_ref - drop the blkg reference pinned by a bio > + * @bio: target bio > + * > + * Drop the bio-owned blkg reference acquired by bio_blkg(), if any. > + */ > +void bio_put_blkg_ref(struct bio *bio) > +{ > + if (bio_flagged(bio, BIO_BLKG_REF)) { > + struct blkcg_gq *blkg = bio_pinned_blkg(bio); > + > + blkg_put(blkg); > + bio_clear_flag(bio, BIO_BLKG_REF); > + } > +} > +EXPORT_SYMBOL_GPL(bio_put_blkg_ref); > + > +/** > + * bio_blkg - look up the blkg associated with a bio > + * @bio: target bio > + * > + * Look up the queue-local blkg for @bio's current device and blkcg. If this > + * is the first policy use of @bio, create the missing blkg hierarchy if > + * necessary, pin the exact blkg, and mark @bio so bio_clear_blkcg() can drop > + * the reference when the bio completes. > + */ > +struct blkcg_gq *bio_blkg(struct bio *bio) > +{ > + struct blkcg *blkcg = bio_blkcg(bio); > + struct gendisk *disk; > + struct request_queue *q; > + struct blkcg_gq *blkg; > + > + if (!blkcg || !bio->bi_bdev) > + return NULL; > + > + if (bio_flagged(bio, BIO_BLKG_REF)) > + return bio_pinned_blkg(bio); > + > + disk = bio->bi_bdev->bd_disk; > + q = disk->queue; > > - /* > - * Fast path failed, we're probably issuing IO in this cgroup the first > - * time, hold lock to create new blkg. > - */ > spin_lock_irq(&q->queue_lock); > - blkg = blkg_lookup_create(blkcg, bio->bi_bdev->bd_disk); > - if (blkg) > - blkg = blkg_lookup_tryget(blkg); > + blkg = blkg_lookup_create(blkcg, disk); > + if (blkg && blkg->blkcg == blkcg && blkg_tryget(blkg)) > + bio_set_flag(bio, BIO_BLKG_REF); > + else > + blkg = NULL; > spin_unlock_irq(&q->queue_lock); > > return blkg; > } > +EXPORT_SYMBOL_GPL(bio_blkg); > > /** > - * bio_associate_blkg_from_css - associate a bio with a specified css > + * bio_associate_blkcg_from_css - associate a bio with a specified css > * @bio: target bio > * @css: target css > * > - * Associate @bio with the blkg found by combining the css's blkg and the > - * request_queue of the @bio. An association failure is handled by walking up > - * the blkg tree. Therefore, the blkg associated can be anything between @blkg > - * and q->root_blkg. This situation only happens when a cgroup is dying and > - * then the remaining bios will spill to the closest alive blkg. > + * Associate @bio with the blkcg found from @css. The queue-local blkg is > + * created and pinned by bio_blkg() when blkcg policies need it. > * > - * A reference will be taken on the blkg and will be released when @bio is > + * A reference will be taken on the blkcg and will be released when @bio is > * freed. > */ > -void bio_associate_blkg_from_css(struct bio *bio, > +void bio_associate_blkcg_from_css(struct bio *bio, > struct cgroup_subsys_state *css) > { > - if (bio_blkg(bio)) > - blkg_put(bio_blkg(bio)); > + struct blkcg *blkcg; > > - if (css && css->parent) { > - bio->bi_blkg = blkg_tryget_closest(bio, css); > - } else { > - blkg_get(bdev_get_queue(bio->bi_bdev)->root_blkg); > - bio->bi_blkg = bdev_get_queue(bio->bi_bdev)->root_blkg; > - } > + if (!css || !css->parent) > + css = &blkcg_root.css; > + > + blkcg = css_to_blkcg(css); > + if (bio_blkcg(bio) == blkcg) > + return; > + > + css_get(css); > + bio_clear_blkcg(bio); > + bio->bi_blkcg = blkcg; > } > -EXPORT_SYMBOL_GPL(bio_associate_blkg_from_css); > +EXPORT_SYMBOL_GPL(bio_associate_blkcg_from_css); > > /** > - * bio_associate_blkg - associate a bio with a blkg > + * bio_associate_blkcg - associate a bio with a blkcg > * @bio: target bio > * > - * Associate @bio with the blkg found from the bio's css and request_queue. > - * If one is not found, bio_lookup_blkg() creates the blkg. If a blkg is > - * already associated, the css is reused and association redone as the > - * request_queue may have changed. > + * Associate @bio with the blkcg found from the bio's css. If a blkcg is > + * already associated, keep it as blkcg association is not queue-local. > */ > -void bio_associate_blkg(struct bio *bio) > +void bio_associate_blkcg(struct bio *bio) > { > struct cgroup_subsys_state *css; > > if (blk_op_is_passthrough(bio->bi_opf)) > return; > > - if (bio_blkg(bio)) { > - css = bio_blkcg_css(bio); > - bio_associate_blkg_from_css(bio, css); > - } else { > - rcu_read_lock(); > - css = blkcg_css(); > - if (!css_tryget_online(css)) > - css = NULL; > - rcu_read_unlock(); > + if (bio_blkcg(bio)) > + return; > > - bio_associate_blkg_from_css(bio, css); > - if (css) > - css_put(css); > - } > + rcu_read_lock(); > + css = blkcg_css(); > + if (!css_tryget_online(css)) > + css = NULL; > + rcu_read_unlock(); > + > + bio_associate_blkcg_from_css(bio, css); > + if (css) > + css_put(css); > } > -EXPORT_SYMBOL_GPL(bio_associate_blkg); > +EXPORT_SYMBOL_GPL(bio_associate_blkcg); > > /** > - * bio_clone_blkg_association - clone blkg association from src to dst bio > + * bio_clone_blkcg_association - clone blkcg association from src to dst bio > * @dst: destination bio > * @src: source bio > */ > -void bio_clone_blkg_association(struct bio *dst, struct bio *src) > +void bio_clone_blkcg_association(struct bio *dst, struct bio *src) > { > - if (bio_blkg(src)) > - bio_associate_blkg_from_css(dst, bio_blkcg_css(src)); > + if (bio_blkcg(src)) > + bio_associate_blkcg_from_css(dst, bio_blkcg_css(src)); > } > -EXPORT_SYMBOL_GPL(bio_clone_blkg_association); > +EXPORT_SYMBOL_GPL(bio_clone_blkcg_association); > > static int blk_cgroup_io_type(struct bio *bio) > { > @@ -2186,19 +2238,25 @@ static int blk_cgroup_io_type(struct bio *bio) > > void blk_cgroup_bio_start(struct bio *bio) > { > - struct blkcg_gq *blkg = bio_blkg(bio); > struct blkcg *blkcg = bio_blkcg(bio); > + struct blkcg_gq *blkg; > int rwd = blk_cgroup_io_type(bio), cpu; > struct blkg_iostat_set *bis; > unsigned long flags; > > if (!cgroup_subsys_on_dfl(io_cgrp_subsys)) > return; > + if (!blkcg) > + return; > > /* Root-level stats are sourced from system-wide IO stats */ > if (!cgroup_parent(blkcg->css.cgroup)) > return; > > + blkg = bio_blkg_lookup(bio); > + if (!blkg) > + return; > + > cpu = get_cpu(); > bis = per_cpu_ptr(blkg->iostat_cpu, cpu); > flags = u64_stats_update_begin_irqsave(&bis->sync); > diff --git a/block/blk-cgroup.h b/block/blk-cgroup.h > index 1e80b0a73233..b6fb85db4d3d 100644 > --- a/block/blk-cgroup.h > +++ b/block/blk-cgroup.h > @@ -126,7 +126,7 @@ static inline struct blkcg *css_to_blkcg(struct cgroup_subsys_state *css) > > static inline struct blkcg *bio_blkcg(struct bio *bio) > { > - return bio_blkg(bio)->blkcg; > + return bio->bi_blkcg; > } > > /* > @@ -281,6 +281,9 @@ static inline struct blkcg_gq *blkg_lookup(struct blkcg *blkcg, > return blkg; > } > > +struct blkcg_gq *bio_blkg_lookup(struct bio *bio); > +struct blkcg_gq *bio_blkg(struct bio *bio); > + > /** > * blkg_to_pd - get policy private data > * @blkg: blkg of interest > @@ -348,13 +351,15 @@ static inline void blkg_put(struct blkcg_gq *blkg) > percpu_ref_put(&blkg->refcnt); > } > > -static inline void bio_clear_blkg(struct bio *bio) > +static inline void bio_clear_blkcg(struct bio *bio) > { > - struct blkcg_gq *blkg = bio_blkg(bio); > + struct blkcg *blkcg = bio_blkcg(bio); > + > + bio_put_blkg_ref(bio); > > - if (blkg) { > - blkg_put(blkg); > - bio->bi_blkg = NULL; > + if (blkcg) { > + css_put(&blkcg->css); > + bio->bi_blkcg = NULL; > } > } > > @@ -470,7 +475,7 @@ static inline void blkcg_clear_delay(struct blkcg_gq *blkg) > */ > static inline bool blk_cgroup_mergeable(struct request *rq, struct bio *bio) > { > - return bio_blkg(rq->bio) == bio_blkg(bio) && > + return bio_blkcg(rq->bio) == bio_blkcg(bio) && > bio_issue_as_root_blkg(rq->bio) == bio_issue_as_root_blkg(bio); > } > > @@ -497,6 +502,8 @@ struct blkcg { > }; > > static inline struct blkcg *bio_blkcg(struct bio *bio) { return NULL; } > +static inline struct blkcg_gq *bio_blkg_lookup(struct bio *bio) { return NULL; } > +static inline struct blkcg_gq *bio_blkg(struct bio *bio) { return NULL; } > static inline struct blkcg_gq *blkg_lookup(struct blkcg *blkcg, void *key) { return NULL; } > static inline void blkg_init_queue(struct request_queue *q) { } > static inline int blkcg_init_disk(struct gendisk *disk) { return 0; } > @@ -513,7 +520,7 @@ static inline struct blkg_policy_data *blkg_to_pd(struct blkcg_gq *blkg, > static inline struct blkcg_gq *pd_to_blkg(struct blkg_policy_data *pd) { return NULL; } > static inline void blkg_get(struct blkcg_gq *blkg) { } > static inline void blkg_put(struct blkcg_gq *blkg) { } > -static inline void bio_clear_blkg(struct bio *bio) { } > +static inline void bio_clear_blkcg(struct bio *bio) { } > static inline void blk_cgroup_bio_start(struct bio *bio) { } > static inline bool blk_cgroup_mergeable(struct request *rq, struct bio *bio) { return true; } > > diff --git a/block/blk-crypto-fallback.c b/block/blk-crypto-fallback.c > index 2a5c52ab74b4..5ef4baab444b 100644 > --- a/block/blk-crypto-fallback.c > +++ b/block/blk-crypto-fallback.c > @@ -187,7 +187,7 @@ static struct bio *blk_crypto_alloc_enc_bio(struct bio *bio_src, > bio->bi_write_hint = bio_src->bi_write_hint; > bio->bi_write_stream = bio_src->bi_write_stream; > bio->bi_iter.bi_sector = bio_src->bi_iter.bi_sector; > - bio_clone_blkg_association(bio, bio_src); > + bio_clone_blkcg_association(bio, bio_src); > > /* > * Move page array up in the allocated memory for the bio vecs as far as > diff --git a/block/blk-iocost.c b/block/blk-iocost.c > index d4470476bcd0..62ffd759bb95 100644 > --- a/block/blk-iocost.c > +++ b/block/blk-iocost.c > @@ -2775,7 +2775,7 @@ static void ioc_rqos_throttle(struct rq_qos *rqos, struct bio *bio) > static void ioc_rqos_merge(struct rq_qos *rqos, struct request *rq, > struct bio *bio) > { > - struct ioc_gq *iocg = blkg_to_iocg(bio_blkg(bio)); > + struct ioc_gq *iocg = blkg_to_iocg(bio_blkg_lookup(bio)); > struct ioc *ioc = rqos_to_ioc(rqos); > sector_t bio_end = bio_end_sector(bio); > struct ioc_now now; > @@ -2833,9 +2833,13 @@ static void ioc_rqos_merge(struct rq_qos *rqos, struct request *rq, > > static void ioc_rqos_done_bio(struct rq_qos *rqos, struct bio *bio) > { > - struct ioc_gq *iocg = blkg_to_iocg(bio_blkg(bio)); > + struct ioc_gq *iocg; > + > + if (!bio->bi_iocost_cost) > + return; > > - if (iocg && bio->bi_iocost_cost) > + iocg = blkg_to_iocg(bio_blkg_lookup(bio)); > + if (iocg) > atomic64_add(bio->bi_iocost_cost, &iocg->done_vtime); > } > > diff --git a/block/blk-iolatency.c b/block/blk-iolatency.c > index c0d8d5f6bdba..7ad18a538d7e 100644 > --- a/block/blk-iolatency.c > +++ b/block/blk-iolatency.c > @@ -590,8 +590,11 @@ static void blkcg_iolatency_done_bio(struct rq_qos *rqos, struct bio *bio) > bool issue_as_root = bio_issue_as_root_blkg(bio); > int inflight = 0; > > - blkg = bio_blkg(bio); > - if (!blkg || !bio_flagged(bio, BIO_QOS_THROTTLED)) > + if (!bio_flagged(bio, BIO_QOS_THROTTLED)) > + return; > + > + blkg = bio_blkg_lookup(bio); > + if (!blkg) > return; > > iolat = blkg_to_lat(blkg); > diff --git a/drivers/md/bcache/request.c b/drivers/md/bcache/request.c > index 3fa3b13a410f..c0f945b8d941 100644 > --- a/drivers/md/bcache/request.c > +++ b/drivers/md/bcache/request.c > @@ -848,7 +848,7 @@ static CLOSURE_CALLBACK(cached_dev_read_done) > s->iop.bio->bi_iter.bi_sector = > s->cache_miss->bi_iter.bi_sector; > s->iop.bio->bi_iter.bi_size = s->insert_bio_sectors << 9; > - bio_clone_blkg_association(s->iop.bio, s->cache_miss); > + bio_clone_blkcg_association(s->iop.bio, s->cache_miss); > bch_bio_map(s->iop.bio, NULL); > > bio_copy_data(s->cache_miss, s->iop.bio); > diff --git a/drivers/md/dm.c b/drivers/md/dm.c > index d413bfaf3527..cd68eec77f5a 100644 > --- a/drivers/md/dm.c > +++ b/drivers/md/dm.c > @@ -1373,7 +1373,7 @@ void dm_submit_bio_remap(struct bio *clone, struct bio *tgt_clone) > if (!tgt_clone) > tgt_clone = clone; > > - bio_clone_blkg_association(tgt_clone, io->orig_bio); > + bio_clone_blkcg_association(tgt_clone, io->orig_bio); > > /* > * Account io->origin_bio to DM dev on behalf of target > diff --git a/drivers/md/md.c b/drivers/md/md.c > index d1465bcd86c8..af55f8efa46b 100644 > --- a/drivers/md/md.c > +++ b/drivers/md/md.c > @@ -9355,7 +9355,7 @@ void md_submit_discard_bio(struct mddev *mddev, struct md_rdev *rdev, > return; > > bio_chain(discard_bio, bio); > - bio_clone_blkg_association(discard_bio, bio); > + bio_clone_blkcg_association(discard_bio, bio); > mddev_trace_remap(mddev, discard_bio, bio->bi_iter.bi_sector); > submit_bio_noacct(discard_bio); > } > diff --git a/drivers/nvdimm/nd_virtio.c b/drivers/nvdimm/nd_virtio.c > index 4176046627be..54e4adb0ccb7 100644 > --- a/drivers/nvdimm/nd_virtio.c > +++ b/drivers/nvdimm/nd_virtio.c > @@ -121,7 +121,7 @@ int async_pmem_flush(struct nd_region *nd_region, struct bio *bio) > > if (!child) > return -ENOMEM; > - bio_clone_blkg_association(child, bio); > + bio_clone_blkcg_association(child, bio); > child->bi_iter.bi_sector = -1; > bio_chain(child, bio); > submit_bio(child); > diff --git a/fs/gfs2/lops.c b/fs/gfs2/lops.c > index 6dabe73ad790..6512dbd9516f 100644 > --- a/fs/gfs2/lops.c > +++ b/fs/gfs2/lops.c > @@ -484,7 +484,7 @@ static struct bio *gfs2_chain_bio(struct bio *prev, unsigned int nr_iovecs, > struct bio *new; > > new = bio_alloc(prev->bi_bdev, nr_iovecs, opf, GFP_NOIO); > - bio_clone_blkg_association(new, prev); > + bio_clone_blkcg_association(new, prev); > new->bi_iter.bi_sector = sector; > bio_chain(new, prev); > submit_bio(prev); > @@ -1114,4 +1114,3 @@ const struct gfs2_log_operations *gfs2_log_ops[] = { > &gfs2_revoke_lops, > NULL, > }; > - > diff --git a/include/linux/bio.h b/include/linux/bio.h > index dc4baa3602b7..e5799fdf431d 100644 > --- a/include/linux/bio.h > +++ b/include/linux/bio.h > @@ -503,28 +503,20 @@ static inline void bio_release_pages(struct bio *bio, bool mark_dirty) > disk_devt((bio)->bi_bdev->bd_disk) > > #ifdef CONFIG_BLK_CGROUP > -static inline struct blkcg_gq *bio_blkg(struct bio *bio) > -{ > - return bio->bi_blkg; > -} > - > -void bio_associate_blkg(struct bio *bio); > -void bio_associate_blkg_from_css(struct bio *bio, > +void bio_associate_blkcg(struct bio *bio); > +void bio_associate_blkcg_from_css(struct bio *bio, > struct cgroup_subsys_state *css); > -void bio_clone_blkg_association(struct bio *dst, struct bio *src); > +void bio_clone_blkcg_association(struct bio *dst, struct bio *src); > +void bio_put_blkg_ref(struct bio *bio); > void blkcg_punt_bio_submit(struct bio *bio); > #else /* CONFIG_BLK_CGROUP */ > -static inline struct blkcg_gq *bio_blkg(struct bio *bio) > -{ > - return NULL; > -} > - > -static inline void bio_associate_blkg(struct bio *bio) { } > -static inline void bio_associate_blkg_from_css(struct bio *bio, > +static inline void bio_associate_blkcg(struct bio *bio) { } > +static inline void bio_associate_blkcg_from_css(struct bio *bio, > struct cgroup_subsys_state *css) > { } > -static inline void bio_clone_blkg_association(struct bio *dst, > +static inline void bio_clone_blkcg_association(struct bio *dst, > struct bio *src) { } > +static inline void bio_put_blkg_ref(struct bio *bio) { } > static inline void blkcg_punt_bio_submit(struct bio *bio) > { > submit_bio(bio); > @@ -534,10 +526,12 @@ static inline void blkcg_punt_bio_submit(struct bio *bio) > static inline void bio_set_dev(struct bio *bio, struct block_device *bdev) > { > bio_clear_flag(bio, BIO_REMAPPED); > - if (bio->bi_bdev != bdev) > + if (bio->bi_bdev != bdev) { > + bio_put_blkg_ref(bio); > bio_clear_flag(bio, BIO_BPS_THROTTLED); > + } > bio->bi_bdev = bdev; > - bio_associate_blkg(bio); > + bio_associate_blkcg(bio); > } > > /* > diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h > index 8808ee76e73c..5f95c2e0e90b 100644 > --- a/include/linux/blk_types.h > +++ b/include/linux/blk_types.h > @@ -246,12 +246,10 @@ struct bio { > void *bi_private; > #ifdef CONFIG_BLK_CGROUP > /* > - * Represents the association of the css and request_queue for the bio. > - * If a bio goes direct to device, it will not have a blkg as it will > - * not have a request_queue associated with it. The reference is put > - * on release of the bio. > + * Represents the blkcg css association for the bio. The reference is > + * put on release of the bio. > */ > - struct blkcg_gq *bi_blkg; > + struct blkcg *bi_blkcg; > /* Time that this bio was issued. */ > u64 issue_time_ns; > #ifdef CONFIG_BLK_CGROUP_IOCOST > @@ -309,6 +307,7 @@ enum { > BIO_TRACE_COMPLETION, /* bio_endio() should trace the final completion > * of this bio. */ > BIO_CGROUP_ACCT, /* has been accounted to a cgroup */ > + BIO_BLKG_REF, /* bio pins the associated blkg */ > BIO_QOS_THROTTLED, /* bio went through rq_qos throttle path */ > /* > * This bio has completed bps throttling at the single tg granularity, > diff --git a/include/linux/writeback.h b/include/linux/writeback.h > index 62552a2ce5b9..4f869fe9cc90 100644 > --- a/include/linux/writeback.h > +++ b/include/linux/writeback.h > @@ -262,7 +262,7 @@ static inline void wbc_init_bio(struct writeback_control *wbc, struct bio *bio) > * regular writeback instead of writing things out itself. > */ > if (wbc->wb) > - bio_associate_blkg_from_css(bio, wbc->wb->blkcg_css); > + bio_associate_blkcg_from_css(bio, wbc->wb->blkcg_css); > } > > void inode_switch_wbs_work_fn(struct work_struct *work); > diff --git a/mm/page_io.c b/mm/page_io.c > index b23f494fcc83..112e50475605 100644 > --- a/mm/page_io.c > +++ b/mm/page_io.c > @@ -316,7 +316,7 @@ static inline void count_swpout_vm_event(struct folio *folio) > } > > #if defined(CONFIG_MEMCG) && defined(CONFIG_BLK_CGROUP) > -static void bio_associate_blkg_from_page(struct bio *bio, struct folio *folio) > +static void bio_associate_blkcg_from_page(struct bio *bio, struct folio *folio) > { > struct cgroup_subsys_state *css; > struct mem_cgroup *memcg; > @@ -331,12 +331,12 @@ static void bio_associate_blkg_from_page(struct bio *bio, struct folio *folio) > css = NULL; > rcu_read_unlock(); > > - bio_associate_blkg_from_css(bio, css); > + bio_associate_blkcg_from_css(bio, css); > if (css) > css_put(css); > } > #else > -#define bio_associate_blkg_from_page(bio, folio) do { } while (0) > +#define bio_associate_blkcg_from_page(bio, folio) do { } while (0) > #endif /* CONFIG_MEMCG && CONFIG_BLK_CGROUP */ > > struct swap_iocb { > @@ -436,7 +436,7 @@ static void swap_writepage_bdev_sync(struct folio *folio, > bio.bi_iter.bi_sector = swap_folio_sector(folio); > bio_add_folio_nofail(&bio, folio, folio_size(folio), 0); > > - bio_associate_blkg_from_page(&bio, folio); > + bio_associate_blkcg_from_page(&bio, folio); > count_swpout_vm_event(folio); > > folio_start_writeback(folio); > @@ -456,7 +456,7 @@ static void swap_writepage_bdev_async(struct folio *folio, > bio->bi_end_io = end_swap_bio_write; > bio_add_folio_nofail(bio, folio, folio_size(folio), 0); > > - bio_associate_blkg_from_page(bio, folio); > + bio_associate_blkcg_from_page(bio, folio); > count_swpout_vm_event(folio); > folio_start_writeback(folio); > folio_unlock(folio); ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC PATCH v1 2/3] blk-cgroup: store blkcg in bio instead of blkg 2026-08-04 9:19 ` Tao Cui @ 2026-08-04 13:32 ` Christoph Hellwig 2026-08-04 15:30 ` yu kuai 0 siblings, 1 reply; 12+ messages in thread From: Christoph Hellwig @ 2026-08-04 13:32 UTC (permalink / raw) To: Tao Cui Cc: Yu Kuai, Jens Axboe, Tejun Heo, Johannes Weiner, Michal Koutný, Jonathan Corbet, Yu Kuai, Josef Bacik, Coly Li, Kent Overstreet, Alasdair Kergon, Mike Snitzer, Mikulas Patocka, Benjamin Marzinski, Song Liu, Dan Williams, Vishal Verma, Dave Jiang, Alison Schofield, Pankaj Gupta, Andreas Gruenbacher, Matthew Wilcox, Jan Kara, Andrew Morton, Chris Li, Kairui Song, Christoph Hellwig, Nilay Shroff, cgroups, linux-doc, linux-kernel, linux-block, linux-bcache, dm-devel, linux-raid, nvdimm, virtualization, gfs2, linux-fsdevel, linux-mm 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. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC PATCH v1 2/3] blk-cgroup: store blkcg in bio instead of blkg 2026-08-04 13:32 ` Christoph Hellwig @ 2026-08-04 15:30 ` yu kuai 2026-08-04 15:47 ` Christoph Hellwig 2026-08-05 0:58 ` Tao Cui 0 siblings, 2 replies; 12+ messages in thread From: yu kuai @ 2026-08-04 15:30 UTC (permalink / raw) To: Christoph Hellwig, Tao Cui, yu kuai Cc: Yu Kuai, Jens Axboe, Tejun Heo, Johannes Weiner, Michal Koutný, Jonathan Corbet, Josef Bacik, Coly Li, Kent Overstreet, Alasdair Kergon, Mike Snitzer, Mikulas Patocka, Benjamin Marzinski, Song Liu, Dan Williams, Vishal Verma, Dave Jiang, Alison Schofield, Pankaj Gupta, Andreas Gruenbacher, Matthew Wilcox, Jan Kara, Andrew Morton, Chris Li, Kairui Song, Nilay Shroff, cgroups, linux-doc, linux-kernel, linux-block, linux-bcache, dm-devel, linux-raid, nvdimm, virtualization, gfs2, linux-fsdevel, linux-mm 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC PATCH v1 2/3] blk-cgroup: store blkcg in bio instead of blkg 2026-08-04 15:30 ` yu kuai @ 2026-08-04 15:47 ` Christoph Hellwig 2026-08-05 0:58 ` Tao Cui 1 sibling, 0 replies; 12+ messages in thread From: Christoph Hellwig @ 2026-08-04 15:47 UTC (permalink / raw) To: yu kuai Cc: Christoph Hellwig, Tao Cui, Yu Kuai, Jens Axboe, Tejun Heo, Johannes Weiner, Michal Koutný, Jonathan Corbet, Josef Bacik, Coly Li, Kent Overstreet, Alasdair Kergon, Mike Snitzer, Mikulas Patocka, Benjamin Marzinski, Song Liu, Dan Williams, Vishal Verma, Dave Jiang, Alison Schofield, Pankaj Gupta, Andreas Gruenbacher, Matthew Wilcox, Jan Kara, Andrew Morton, Chris Li, Kairui Song, Nilay Shroff, cgroups, linux-doc, linux-kernel, linux-block, linux-bcache, dm-devel, linux-raid, nvdimm, virtualization, gfs2, linux-fsdevel, linux-mm On Tue, Aug 04, 2026 at 11:30:55PM +0800, yu kuai wrote: > 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. Yes, I think you can really easily do that with the rhashtable. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC PATCH v1 2/3] blk-cgroup: store blkcg in bio instead of blkg 2026-08-04 15:30 ` yu kuai 2026-08-04 15:47 ` Christoph Hellwig @ 2026-08-05 0:58 ` Tao Cui 1 sibling, 0 replies; 12+ messages in thread From: Tao Cui @ 2026-08-05 0:58 UTC (permalink / raw) To: yukuai, Christoph Hellwig Cc: cui.tao, Yu Kuai, Jens Axboe, Tejun Heo, Johannes Weiner, Michal Koutný, Jonathan Corbet, Josef Bacik, Coly Li, Kent Overstreet, Alasdair Kergon, Mike Snitzer, Mikulas Patocka, Benjamin Marzinski, Song Liu, Dan Williams, Vishal Verma, Dave Jiang, Alison Schofield, Pankaj Gupta, Andreas Gruenbacher, Matthew Wilcox, Jan Kara, Andrew Morton, Chris Li, Kairui Song, Nilay Shroff, cgroups, linux-doc, linux-kernel, linux-block, linux-bcache, dm-devel, linux-raid, nvdimm, virtualization, gfs2, linux-fsdevel, linux-mm Hi Kuai,Christoph, 在 2026/8/4 23:30, yu kuai 写道: > 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? >> yes, I'll turn the reproducer into a blktests case and send it out. >>> 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. > Both of your approaches go further than my store-the-pointer idea; looking forward to the next version. Thanks, Tao >> ^ permalink raw reply [flat|nested] 12+ messages in thread
* [RFC PATCH v1 3/3] blk-cgroup: move async bio punt state to blkcg 2026-08-04 6:53 [RFC PATCH v1 0/3] blk-cgroup: store blkcg in bio before blkcg_mutex conversion Yu Kuai 2026-08-04 6:53 ` [RFC PATCH v1 1/3] blk-cgroup: add helpers for bio cgroup state Yu Kuai 2026-08-04 6:53 ` [RFC PATCH v1 2/3] blk-cgroup: store blkcg in bio instead of blkg Yu Kuai @ 2026-08-04 6:53 ` Yu Kuai 2 siblings, 0 replies; 12+ messages in thread From: Yu Kuai @ 2026-08-04 6:53 UTC (permalink / raw) To: Jens Axboe Cc: Tejun Heo, Johannes Weiner, Michal Koutný, Jonathan Corbet, Yu Kuai, Josef Bacik, Coly Li, Kent Overstreet, Alasdair Kergon, Mike Snitzer, Mikulas Patocka, Benjamin Marzinski, Song Liu, Dan Williams, Vishal Verma, Dave Jiang, Alison Schofield, Pankaj Gupta, Andreas Gruenbacher, Matthew Wilcox, Jan Kara, Andrew Morton, Chris Li, Kairui Song, Christoph Hellwig, Nilay Shroff, Tao Cui, cgroups, linux-doc, linux-kernel, linux-block, linux-bcache, dm-devel, linux-raid, nvdimm, virtualization, gfs2, linux-fsdevel, linux-mm From: Yu Kuai <yukuai@fygo.io> blkcg_punt_bio_submit() currently queues punted bios on blkg->async_bios, so it has to call bio_blkg() to find or create a queue-local blkg. Bios now carry and pin the blkcg css, so punted bio lifetime no longer needs to be anchored by a blkg. Keeping the punt state in blkg can instantiate a blkg even when no blkcg policy is enabled, just to bounce submission from a shared kthread. Move async_bio_lock, async_bios and async_bio_work to struct blkcg, and queue punted bios on bio_blkcg() for non-root cgroups. Root or unassociated bios are submitted directly. This preserves the priority-inversion avoidance while preventing blkcg_punt_bio_submit() from creating blkgs that are not needed by any policy. Signed-off-by: Yu Kuai <yukuai@fygo.io> --- block/blk-cgroup.c | 44 +++++++++++++++++++++----------------------- block/blk-cgroup.h | 14 ++++++-------- 2 files changed, 27 insertions(+), 31 deletions(-) diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c index 93ab57e0a9f1..236220db8578 100644 --- a/block/blk-cgroup.c +++ b/block/blk-cgroup.c @@ -170,10 +170,6 @@ static void __blkg_release(struct rcu_head *rcu) { struct blkcg_gq *blkg = container_of(rcu, struct blkcg_gq, rcu_head); -#ifdef CONFIG_BLK_CGROUP_PUNT_BIO - WARN_ON(!bio_list_empty(&blkg->async_bios)); -#endif - blkg_free(blkg); } @@ -206,19 +202,18 @@ static void blkg_release(struct percpu_ref *ref) #ifdef CONFIG_BLK_CGROUP_PUNT_BIO static struct workqueue_struct *blkcg_punt_bio_wq; -static void blkg_async_bio_workfn(struct work_struct *work) +static void blkcg_async_bio_workfn(struct work_struct *work) { - struct blkcg_gq *blkg = container_of(work, struct blkcg_gq, - async_bio_work); + struct blkcg *blkcg = container_of(work, struct blkcg, async_bio_work); struct bio_list bios = BIO_EMPTY_LIST; struct bio *bio; struct blk_plug plug; bool need_plug = false; - /* as long as there are pending bios, @blkg can't go away */ - spin_lock(&blkg->async_bio_lock); - bio_list_merge_init(&bios, &blkg->async_bios); - spin_unlock(&blkg->async_bio_lock); + /* as long as there are pending bios, @blkcg can't go away */ + spin_lock(&blkcg->async_bio_lock); + bio_list_merge_init(&bios, &blkcg->async_bios); + spin_unlock(&blkcg->async_bio_lock); /* start plug only when bio_list contains at least 2 bios */ if (bios.head && bios.head->bi_next) { @@ -239,15 +234,15 @@ static void blkg_async_bio_workfn(struct work_struct *work) */ void blkcg_punt_bio_submit(struct bio *bio) { - struct blkcg_gq *blkg = bio_blkg(bio); + struct blkcg *blkcg = bio_blkcg(bio); - if (blkg && blkg->parent) { - spin_lock(&blkg->async_bio_lock); - bio_list_add(&blkg->async_bios, bio); - spin_unlock(&blkg->async_bio_lock); - queue_work(blkcg_punt_bio_wq, &blkg->async_bio_work); + if (blkcg && cgroup_parent(blkcg->css.cgroup)) { + spin_lock(&blkcg->async_bio_lock); + bio_list_add(&blkcg->async_bios, bio); + spin_unlock(&blkcg->async_bio_lock); + queue_work(blkcg_punt_bio_wq, &blkcg->async_bio_work); } else { - /* Never bounce if there is no non-root blkg to queue on. */ + /* Never bounce if there is no non-root blkcg to queue on. */ submit_bio(bio); } } @@ -325,11 +320,6 @@ static struct blkcg_gq *blkg_alloc(struct blkcg *blkcg, struct gendisk *disk, INIT_LIST_HEAD(&blkg->q_node); blkg->blkcg = blkcg; blkg->iostat.blkg = blkg; -#ifdef CONFIG_BLK_CGROUP_PUNT_BIO - spin_lock_init(&blkg->async_bio_lock); - bio_list_init(&blkg->async_bios); - INIT_WORK(&blkg->async_bio_work, blkg_async_bio_workfn); -#endif u64_stats_init(&blkg->iostat.sync); for_each_possible_cpu(cpu) { @@ -1360,6 +1350,9 @@ static void blkcg_css_free(struct cgroup_subsys_state *css) mutex_unlock(&blkcg_pol_mutex); +#ifdef CONFIG_BLK_CGROUP_PUNT_BIO + WARN_ON(!bio_list_empty(&blkcg->async_bios)); +#endif free_percpu(blkcg->lhead); kfree(blkcg); } @@ -1409,6 +1402,11 @@ blkcg_css_alloc(struct cgroup_subsys_state *parent_css) refcount_set(&blkcg->online_pin, 1); INIT_RADIX_TREE(&blkcg->blkg_tree, GFP_NOWAIT); INIT_HLIST_HEAD(&blkcg->blkg_list); +#ifdef CONFIG_BLK_CGROUP_PUNT_BIO + spin_lock_init(&blkcg->async_bio_lock); + bio_list_init(&blkcg->async_bios); + INIT_WORK(&blkcg->async_bio_work, blkcg_async_bio_workfn); +#endif #ifdef CONFIG_CGROUP_WRITEBACK INIT_LIST_HEAD(&blkcg->cgwb_list); #endif diff --git a/block/blk-cgroup.h b/block/blk-cgroup.h index b6fb85db4d3d..b457c2b4ac7c 100644 --- a/block/blk-cgroup.h +++ b/block/blk-cgroup.h @@ -73,14 +73,7 @@ struct blkcg_gq { struct blkg_iostat_set iostat; struct blkg_policy_data *pd[BLKCG_MAX_POLS]; -#ifdef CONFIG_BLK_CGROUP_PUNT_BIO - spinlock_t async_bio_lock; - struct bio_list async_bios; -#endif - union { - struct work_struct async_bio_work; - struct work_struct free_work; - }; + struct work_struct free_work; atomic_t use_delay; atomic64_t delay_nsec; @@ -111,6 +104,11 @@ struct blkcg { */ struct llist_head __percpu *lhead; +#ifdef CONFIG_BLK_CGROUP_PUNT_BIO + spinlock_t async_bio_lock; + struct bio_list async_bios; + struct work_struct async_bio_work; +#endif #ifdef CONFIG_BLK_CGROUP_FC_APPID char fc_app_id[FC_APPID_LEN]; #endif -- 2.51.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
end of thread, other threads:[~2026-08-05 0:59 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-08-04 6:53 [RFC PATCH v1 0/3] blk-cgroup: store blkcg in bio before blkcg_mutex conversion Yu Kuai 2026-08-04 6:53 ` [RFC PATCH v1 1/3] blk-cgroup: add helpers for bio cgroup state Yu Kuai 2026-08-04 10:52 ` Jan Kara 2026-08-04 13:25 ` Christoph Hellwig 2026-08-04 15:07 ` yu kuai 2026-08-04 6:53 ` [RFC PATCH v1 2/3] blk-cgroup: store blkcg in bio instead of blkg Yu Kuai 2026-08-04 9:19 ` Tao Cui 2026-08-04 13:32 ` Christoph Hellwig 2026-08-04 15:30 ` yu kuai 2026-08-04 15:47 ` Christoph Hellwig 2026-08-05 0:58 ` Tao Cui 2026-08-04 6:53 ` [RFC PATCH v1 3/3] blk-cgroup: move async bio punt state to blkcg Yu Kuai
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).