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>
---
 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,
-- 
Jens Axboe

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
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR