[PATCH block/for-3.18] blkcg: remove blkcg->id

[PATCH block/for-3.18] blkcg: remove blkcg->id

[Tejun Heo]

blkcg->id is a unique id given to each blkcg; however, the
cgroup_subsys_state which each blkcg embeds already has ->serial_nr
which can be used for the same purpose.  Drop blkcg->id and replace
its uses with blkcg->css.serial_nr.  Rename cfq_cgroup->blkcg_id to
->blkcg_serial_nr and @id in check_blkcg_changed() to @serial_nr for
consistency.

Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Vivek Goyal <vgoyal@redhat.com>
---
 block/blk-cgroup.c  |    2 --
 block/blk-cgroup.h  |    3 ---
 block/cfq-iosched.c |   10 +++++-----
 3 files changed, 5 insertions(+), 10 deletions(-)

--- a/block/blk-cgroup.c
+++ b/block/blk-cgroup.c
@@ -822,7 +822,6 @@ static void blkcg_css_free(struct cgroup
 static struct cgroup_subsys_state *
 blkcg_css_alloc(struct cgroup_subsys_state *parent_css)
 {
-	static atomic64_t id_seq = ATOMIC64_INIT(0);
 	struct blkcg *blkcg;
 
 	if (!parent_css) {
@@ -836,7 +835,6 @@ blkcg_css_alloc(struct cgroup_subsys_sta
 
 	blkcg->cfq_weight = CFQ_WEIGHT_DEFAULT;
 	blkcg->cfq_leaf_weight = CFQ_WEIGHT_DEFAULT;
-	blkcg->id = atomic64_inc_return(&id_seq); /* root is 0, start from 1 */
 done:
 	spin_lock_init(&blkcg->lock);
 	INIT_RADIX_TREE(&blkcg->blkg_tree, GFP_ATOMIC);
--- a/block/blk-cgroup.h
+++ b/block/blk-cgroup.h
@@ -50,9 +50,6 @@ struct blkcg {
 	struct blkcg_gq			*blkg_hint;
 	struct hlist_head		blkg_list;
 
-	/* for policies to test whether associated blkcg has changed */
-	uint64_t			id;
-
 	/* TODO: per-policy storage in blkcg */
 	unsigned int			cfq_weight;	/* belongs to cfq */
 	unsigned int			cfq_leaf_weight;
--- a/block/cfq-iosched.c
+++ b/block/cfq-iosched.c
@@ -299,7 +299,7 @@ struct cfq_io_cq {
 	struct cfq_ttime	ttime;
 	int			ioprio;		/* the current ioprio */
 #ifdef CONFIG_CFQ_GROUP_IOSCHED
-	uint64_t		blkcg_id;	/* the current blkcg ID */
+	uint64_t		blkcg_serial_nr; /* the current blkcg serial */
 #endif
 };
 
@@ -3534,17 +3534,17 @@ static void check_blkcg_changed(struct c
 {
 	struct cfq_data *cfqd = cic_to_cfqd(cic);
 	struct cfq_queue *sync_cfqq;
-	uint64_t id;
+	uint64_t serial_nr;
 
 	rcu_read_lock();
-	id = bio_blkcg(bio)->id;
+	serial_nr = bio_blkcg(bio)->css.serial_nr;
 	rcu_read_unlock();
 
 	/*
 	 * Check whether blkcg has changed.  The condition may trigger
 	 * spuriously on a newly created cic but there's no harm.
 	 */
-	if (unlikely(!cfqd) || likely(cic->blkcg_id == id))
+	if (unlikely(!cfqd) || likely(cic->blkcg_serial_nr == serial_nr))
 		return;
 
 	sync_cfqq = cic_to_cfqq(cic, 1);
@@ -3558,7 +3558,7 @@ static void check_blkcg_changed(struct c
 		cfq_put_queue(sync_cfqq);
 	}
 
-	cic->blkcg_id = id;
+	cic->blkcg_serial_nr = serial_nr;
 }
 #else
 static inline void check_blkcg_changed(struct cfq_io_cq *cic, struct bio *bio) { }

Re: [PATCH block/for-3.18] blkcg: remove blkcg->id

[Vivek Goyal]

On Mon, Sep 08, 2014 at 08:15:20AM +0900, Tejun Heo wrote:
> blkcg->id is a unique id given to each blkcg; however, the
> cgroup_subsys_state which each blkcg embeds already has ->serial_nr
> which can be used for the same purpose.  Drop blkcg->id and replace
> its uses with blkcg->css.serial_nr.  Rename cfq_cgroup->blkcg_id to
> ->blkcg_serial_nr and @id in check_blkcg_changed() to @serial_nr for
> consistency.
> 
> Signed-off-by: Tejun Heo <tj@kernel.org>
> Cc: Vivek Goyal <vgoyal@redhat.com>

Looks good to me.

Acked-by: Vivek Goyal <vgoyal@redhat.com>

Vivek

> ---
>  block/blk-cgroup.c  |    2 --
>  block/blk-cgroup.h  |    3 ---
>  block/cfq-iosched.c |   10 +++++-----
>  3 files changed, 5 insertions(+), 10 deletions(-)
> 
> --- a/block/blk-cgroup.c
> +++ b/block/blk-cgroup.c
> @@ -822,7 +822,6 @@ static void blkcg_css_free(struct cgroup
>  static struct cgroup_subsys_state *
>  blkcg_css_alloc(struct cgroup_subsys_state *parent_css)
>  {
> -	static atomic64_t id_seq = ATOMIC64_INIT(0);
>  	struct blkcg *blkcg;
>  
>  	if (!parent_css) {
> @@ -836,7 +835,6 @@ blkcg_css_alloc(struct cgroup_subsys_sta
>  
>  	blkcg->cfq_weight = CFQ_WEIGHT_DEFAULT;
>  	blkcg->cfq_leaf_weight = CFQ_WEIGHT_DEFAULT;
> -	blkcg->id = atomic64_inc_return(&id_seq); /* root is 0, start from 1 */
>  done:
>  	spin_lock_init(&blkcg->lock);
>  	INIT_RADIX_TREE(&blkcg->blkg_tree, GFP_ATOMIC);
> --- a/block/blk-cgroup.h
> +++ b/block/blk-cgroup.h
> @@ -50,9 +50,6 @@ struct blkcg {
>  	struct blkcg_gq			*blkg_hint;
>  	struct hlist_head		blkg_list;
>  
> -	/* for policies to test whether associated blkcg has changed */
> -	uint64_t			id;
> -
>  	/* TODO: per-policy storage in blkcg */
>  	unsigned int			cfq_weight;	/* belongs to cfq */
>  	unsigned int			cfq_leaf_weight;
> --- a/block/cfq-iosched.c
> +++ b/block/cfq-iosched.c
> @@ -299,7 +299,7 @@ struct cfq_io_cq {
>  	struct cfq_ttime	ttime;
>  	int			ioprio;		/* the current ioprio */
>  #ifdef CONFIG_CFQ_GROUP_IOSCHED
> -	uint64_t		blkcg_id;	/* the current blkcg ID */
> +	uint64_t		blkcg_serial_nr; /* the current blkcg serial */
>  #endif
>  };
>  
> @@ -3534,17 +3534,17 @@ static void check_blkcg_changed(struct c
>  {
>  	struct cfq_data *cfqd = cic_to_cfqd(cic);
>  	struct cfq_queue *sync_cfqq;
> -	uint64_t id;
> +	uint64_t serial_nr;
>  
>  	rcu_read_lock();
> -	id = bio_blkcg(bio)->id;
> +	serial_nr = bio_blkcg(bio)->css.serial_nr;
>  	rcu_read_unlock();
>  
>  	/*
>  	 * Check whether blkcg has changed.  The condition may trigger
>  	 * spuriously on a newly created cic but there's no harm.
>  	 */
> -	if (unlikely(!cfqd) || likely(cic->blkcg_id == id))
> +	if (unlikely(!cfqd) || likely(cic->blkcg_serial_nr == serial_nr))
>  		return;
>  
>  	sync_cfqq = cic_to_cfqq(cic, 1);
> @@ -3558,7 +3558,7 @@ static void check_blkcg_changed(struct c
>  		cfq_put_queue(sync_cfqq);
>  	}
>  
> -	cic->blkcg_id = id;
> +	cic->blkcg_serial_nr = serial_nr;
>  }
>  #else
>  static inline void check_blkcg_changed(struct cfq_io_cq *cic, struct bio *bio) { }

Re: [PATCH block/for-3.18] blkcg: remove blkcg->id

[Jens Axboe]

On 09/07/2014 05:15 PM, Tejun Heo wrote:
> blkcg->id is a unique id given to each blkcg; however, the
> cgroup_subsys_state which each blkcg embeds already has ->serial_nr
> which can be used for the same purpose.  Drop blkcg->id and replace
> its uses with blkcg->css.serial_nr.  Rename cfq_cgroup->blkcg_id to
> ->blkcg_serial_nr and @id in check_blkcg_changed() to @serial_nr for
> consistency.

Thanks Tejun, applied.

-- 
Jens Axboe