| Message ID | 20221217030527.1250083-3-yukuai1@huaweicloud.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | blk-iocost: make sure parent iocg is exited before child | expand |
On Sat, Dec 17, 2022 at 11:05:25AM +0800, Yu Kuai wrote: > From: Yu Kuai <yukuai3@huawei.com> > > bio will grab blkg reference, however, blkcg->online_pin is not grabbed, > hence cgroup can be removed after thread exit while bio is still in > progress. Bypass io in this case since it doesn't make sense to > throttle bio while cgroup is removed. I don't get it. Why wouldn't that make sense? ISTR some occasions where we clear the config to mitigate exits stalling for too long but in general a policy being active on a draining cgroup shouldn't be a problem. Thanks.
Hi, 在 2022/12/20 5:28, Tejun Heo 写道: > On Sat, Dec 17, 2022 at 11:05:25AM +0800, Yu Kuai wrote: >> From: Yu Kuai <yukuai3@huawei.com> >> >> bio will grab blkg reference, however, blkcg->online_pin is not grabbed, >> hence cgroup can be removed after thread exit while bio is still in >> progress. Bypass io in this case since it doesn't make sense to >> throttle bio while cgroup is removed. > > I don't get it. Why wouldn't that make sense? ISTR some occasions where we > clear the config to mitigate exits stalling for too long but in general a > policy being active on a draining cgroup shouldn't be a problem. The main reason here for patch 2/3 is for patch 4, since bio can still reach rq_qos after pd_offline_fn is called. Currently, it's not consistent and seems messy how different policies implement pd_alloc/free_fn, pd_online/offline_fn, and pd_init_fn. For iocost, iocg is exited in pd_free_fn, and parent iocg can exits before child, which will cause many problems. Patch 2/3 are not necessary if we don't choose to fix such problems by exiting iocg in ioc_pd_offline() in patch 4. I'll try to think about how to use refcnting, either from blkg layer or add refcnting for iocg. Thanks, Kuai > > Thanks. >
diff --git a/block/blk-iocost.c b/block/blk-iocost.c index 1498879c4a52..23cc734dbe43 100644 --- a/block/blk-iocost.c +++ b/block/blk-iocost.c @@ -695,6 +695,20 @@ static struct ioc_cgrp *blkcg_to_iocc(struct blkcg *blkcg) struct ioc_cgrp, cpd); } +static struct ioc_gq *ioc_bio_iocg(struct bio *bio) +{ + struct blkcg_gq *blkg = bio->bi_blkg; + + if (blkg && blkg->online) { + struct ioc_gq *iocg = blkg_to_iocg(blkg); + + if (iocg && iocg->online) + return iocg; + } + + return NULL; +} + /* * Scale @abs_cost to the inverse of @hw_inuse. The lower the hierarchical * weight, the more expensive each IO. Must round up. @@ -1262,6 +1276,9 @@ static bool iocg_activate(struct ioc_gq *iocg, struct ioc_now *now) spin_lock_irq(&ioc->lock); + if (!iocg->online) + goto fail_unlock; + ioc_now(ioc, now); /* update period */ @@ -2561,9 +2578,8 @@ static u64 calc_size_vtime_cost(struct request *rq, struct ioc *ioc) static void ioc_rqos_throttle(struct rq_qos *rqos, struct bio *bio) { - struct blkcg_gq *blkg = bio->bi_blkg; struct ioc *ioc = rqos_to_ioc(rqos); - struct ioc_gq *iocg = blkg_to_iocg(blkg); + struct ioc_gq *iocg = ioc_bio_iocg(bio); struct ioc_now now; struct iocg_wait wait; u64 abs_cost, cost, vtime; @@ -2697,7 +2713,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 = ioc_bio_iocg(bio); struct ioc *ioc = rqos_to_ioc(rqos); sector_t bio_end = bio_end_sector(bio); struct ioc_now now; @@ -2755,7 +2771,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 = ioc_bio_iocg(bio); if (iocg && bio->bi_iocost_cost) atomic64_add(bio->bi_iocost_cost, &iocg->done_vtime);