| Message ID | 20240618032753.3502528-1-yukuai1@huaweicloud.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [-next] block, bfq: remove blkg_path() | expand |
On Tue 18-06-24 11:27:53, Yu Kuai wrote: > From: Yu Kuai <yukuai3@huawei.com> > > After commit 35fe6d763229 ("block: use standard blktrace API to output > cgroup info for debug notes"), the field 'bfqg->blkg_path' is not used > and hence can be removed, and therefor blkg_path() is not used anymore > and can be removed. > > Signed-off-by: Yu Kuai <yukuai3@huawei.com> Yay, nice to see code removed! Feel free to add: Reviewed-by: Jan Kara <jack@suse.cz> Honza > --- > block/bfq-cgroup.c | 51 --------------------------------------------- > block/bfq-iosched.h | 3 --- > block/blk-cgroup.h | 13 ------------ > 3 files changed, 67 deletions(-) > > diff --git a/block/bfq-cgroup.c b/block/bfq-cgroup.c > index d442ee358fc2..b758693697c0 100644 > --- a/block/bfq-cgroup.c > +++ b/block/bfq-cgroup.c > @@ -797,57 +797,6 @@ void bfq_bic_update_cgroup(struct bfq_io_cq *bic, struct bio *bio) > */ > bfq_link_bfqg(bfqd, bfqg); > __bfq_bic_change_cgroup(bfqd, bic, bfqg); > - /* > - * Update blkg_path for bfq_log_* functions. We cache this > - * path, and update it here, for the following > - * reasons. Operations on blkg objects in blk-cgroup are > - * protected with the request_queue lock, and not with the > - * lock that protects the instances of this scheduler > - * (bfqd->lock). This exposes BFQ to the following sort of > - * race. > - * > - * The blkg_lookup performed in bfq_get_queue, protected > - * through rcu, may happen to return the address of a copy of > - * the original blkg. If this is the case, then the > - * bfqg_and_blkg_get performed in bfq_get_queue, to pin down > - * the blkg, is useless: it does not prevent blk-cgroup code > - * from destroying both the original blkg and all objects > - * directly or indirectly referred by the copy of the > - * blkg. > - * > - * On the bright side, destroy operations on a blkg invoke, as > - * a first step, hooks of the scheduler associated with the > - * blkg. And these hooks are executed with bfqd->lock held for > - * BFQ. As a consequence, for any blkg associated with the > - * request queue this instance of the scheduler is attached > - * to, we are guaranteed that such a blkg is not destroyed, and > - * that all the pointers it contains are consistent, while we > - * are holding bfqd->lock. A blkg_lookup performed with > - * bfqd->lock held then returns a fully consistent blkg, which > - * remains consistent until this lock is held. > - * > - * Thanks to the last fact, and to the fact that: (1) bfqg has > - * been obtained through a blkg_lookup in the above > - * assignment, and (2) bfqd->lock is being held, here we can > - * safely use the policy data for the involved blkg (i.e., the > - * field bfqg->pd) to get to the blkg associated with bfqg, > - * and then we can safely use any field of blkg. After we > - * release bfqd->lock, even just getting blkg through this > - * bfqg may cause dangling references to be traversed, as > - * bfqg->pd may not exist any more. > - * > - * In view of the above facts, here we cache, in the bfqg, any > - * blkg data we may need for this bic, and for its associated > - * bfq_queue. As of now, we need to cache only the path of the > - * blkg, which is used in the bfq_log_* functions. > - * > - * Finally, note that bfqg itself needs to be protected from > - * destruction on the blkg_free of the original blkg (which > - * invokes bfq_pd_free). We use an additional private > - * refcounter for bfqg, to let it disappear only after no > - * bfq_queue refers to it any longer. > - */ > - blkg_path(bfqg_to_blkg(bfqg), bfqg->blkg_path, sizeof(bfqg->blkg_path)); > bic->blkcg_serial_nr = serial_nr; > } > > diff --git a/block/bfq-iosched.h b/block/bfq-iosched.h > index 467e8cfc41a2..08ddf2cfae5b 100644 > --- a/block/bfq-iosched.h > +++ b/block/bfq-iosched.h > @@ -1003,9 +1003,6 @@ struct bfq_group { > /* must be the first member */ > struct blkg_policy_data pd; > > - /* cached path for this blkg (see comments in bfq_bic_update_cgroup) */ > - char blkg_path[128]; > - > /* reference counter (see comments in bfq_bic_update_cgroup) */ > refcount_t ref; > > diff --git a/block/blk-cgroup.h b/block/blk-cgroup.h > index 25833221a12b..6dcaf63c560a 100644 > --- a/block/blk-cgroup.h > +++ b/block/blk-cgroup.h > @@ -301,19 +301,6 @@ static inline struct blkcg *cpd_to_blkcg(struct blkcg_policy_data *cpd) > return cpd ? cpd->blkcg : NULL; > } > > -/** > - * blkg_path - format cgroup path of blkg > - * @blkg: blkg of interest > - * @buf: target buffer > - * @buflen: target buffer length > - * > - * Format the path of the cgroup of @blkg into @buf. > - */ > -static inline int blkg_path(struct blkcg_gq *blkg, char *buf, int buflen) > -{ > - return cgroup_path(blkg->blkcg->css.cgroup, buf, buflen); > -} > - > /** > * blkg_get - get a blkg reference > * @blkg: blkg to get > -- > 2.39.2 >
On Tue, 18 Jun 2024 11:27:53 +0800, Yu Kuai wrote: > After commit 35fe6d763229 ("block: use standard blktrace API to output > cgroup info for debug notes"), the field 'bfqg->blkg_path' is not used > and hence can be removed, and therefor blkg_path() is not used anymore > and can be removed. > > Applied, thanks! [1/1] block, bfq: remove blkg_path() commit: bb7e5a193d8becf3920e3848287f1b23c5fc9b24 Best regards,
diff --git a/block/bfq-cgroup.c b/block/bfq-cgroup.c index d442ee358fc2..b758693697c0 100644 --- a/block/bfq-cgroup.c +++ b/block/bfq-cgroup.c @@ -797,57 +797,6 @@ void bfq_bic_update_cgroup(struct bfq_io_cq *bic, struct bio *bio) */ bfq_link_bfqg(bfqd, bfqg); __bfq_bic_change_cgroup(bfqd, bic, bfqg); - /* - * Update blkg_path for bfq_log_* functions. We cache this - * path, and update it here, for the following - * reasons. Operations on blkg objects in blk-cgroup are - * protected with the request_queue lock, and not with the - * lock that protects the instances of this scheduler - * (bfqd->lock). This exposes BFQ to the following sort of - * race. - * - * The blkg_lookup performed in bfq_get_queue, protected - * through rcu, may happen to return the address of a copy of - * the original blkg. If this is the case, then the - * bfqg_and_blkg_get performed in bfq_get_queue, to pin down - * the blkg, is useless: it does not prevent blk-cgroup code - * from destroying both the original blkg and all objects - * directly or indirectly referred by the copy of the - * blkg. - * - * On the bright side, destroy operations on a blkg invoke, as - * a first step, hooks of the scheduler associated with the - * blkg. And these hooks are executed with bfqd->lock held for - * BFQ. As a consequence, for any blkg associated with the - * request queue this instance of the scheduler is attached - * to, we are guaranteed that such a blkg is not destroyed, and - * that all the pointers it contains are consistent, while we - * are holding bfqd->lock. A blkg_lookup performed with - * bfqd->lock held then returns a fully consistent blkg, which - * remains consistent until this lock is held. - * - * Thanks to the last fact, and to the fact that: (1) bfqg has - * been obtained through a blkg_lookup in the above - * assignment, and (2) bfqd->lock is being held, here we can - * safely use the policy data for the involved blkg (i.e., the - * field bfqg->pd) to get to the blkg associated with bfqg, - * and then we can safely use any field of blkg. After we - * release bfqd->lock, even just getting blkg through this - * bfqg may cause dangling references to be traversed, as - * bfqg->pd may not exist any more. - * - * In view of the above facts, here we cache, in the bfqg, any - * blkg data we may need for this bic, and for its associated - * bfq_queue. As of now, we need to cache only the path of the - * blkg, which is used in the bfq_log_* functions. - * - * Finally, note that bfqg itself needs to be protected from - * destruction on the blkg_free of the original blkg (which - * invokes bfq_pd_free). We use an additional private - * refcounter for bfqg, to let it disappear only after no - * bfq_queue refers to it any longer. - */ - blkg_path(bfqg_to_blkg(bfqg), bfqg->blkg_path, sizeof(bfqg->blkg_path)); bic->blkcg_serial_nr = serial_nr; } diff --git a/block/bfq-iosched.h b/block/bfq-iosched.h index 467e8cfc41a2..08ddf2cfae5b 100644 --- a/block/bfq-iosched.h +++ b/block/bfq-iosched.h @@ -1003,9 +1003,6 @@ struct bfq_group { /* must be the first member */ struct blkg_policy_data pd; - /* cached path for this blkg (see comments in bfq_bic_update_cgroup) */ - char blkg_path[128]; - /* reference counter (see comments in bfq_bic_update_cgroup) */ refcount_t ref; diff --git a/block/blk-cgroup.h b/block/blk-cgroup.h index 25833221a12b..6dcaf63c560a 100644 --- a/block/blk-cgroup.h +++ b/block/blk-cgroup.h @@ -301,19 +301,6 @@ static inline struct blkcg *cpd_to_blkcg(struct blkcg_policy_data *cpd) return cpd ? cpd->blkcg : NULL; } -/** - * blkg_path - format cgroup path of blkg - * @blkg: blkg of interest - * @buf: target buffer - * @buflen: target buffer length - * - * Format the path of the cgroup of @blkg into @buf. - */ -static inline int blkg_path(struct blkcg_gq *blkg, char *buf, int buflen) -{ - return cgroup_path(blkg->blkcg->css.cgroup, buf, buflen); -} - /** * blkg_get - get a blkg reference * @blkg: blkg to get