Commit 7a4dd281 authored by Tejun Heo's avatar Tejun Heo Committed by Jens Axboe
Browse files

blkcg: kill the mind-bending blkg->dev 



blkg->dev is dev_t recording the device number of the block device for
the associated request_queue.  It is used to identify the associated
block device when printing out configuration or stats.

This is redundant to begin with.  A blkg is an association between a
cgroup and a request_queue and it of course is possible to reach
request_queue from blkg and synchronization conventions are in place
for safe q dereferencing, so this shouldn't be necessary from the
beginning.  Furthermore, it's initialized by sscanf()ing the device
name of backing_dev_info.  The mind boggles.

Anyways, if blkg is visible under rcu lock, we *know* that the
associated request_queue hasn't gone away yet and its bdi is
registered and alive - blkg can't be created for request_queue which
hasn't been fully initialized and it can't go away before blkg is
removed.

Let stat and conf read functions get device name from
blkg->q->backing_dev_info.dev and pass it down to printing functions
and remove blkg->dev.

Signed-off-by: default avatarTejun Heo <tj@kernel.org>
Cc: Vivek Goyal <vgoyal@redhat.com>
Signed-off-by: default avatarJens Axboe <axboe@kernel.dk>
parent 4bfd482e

block/blk-cgroup.c

+45 −41
Original line number Diff line number Diff line
@@ -662,10 +662,10 @@ blkiocg_reset_stats(struct cgroup *cgroup, struct cftype *cftype, u64 val) 
	return 0;

}



static void blkio_get_key_name(enum stat_sub_type type, dev_t dev, char *str,

				int chars_left, bool diskname_only)

static void blkio_get_key_name(enum stat_sub_type type, const char *dname,

			       char *str, int chars_left, bool diskname_only)

{

	snprintf(str, chars_left, "%d:%d", MAJOR(dev), MINOR(dev));

	snprintf(str, chars_left, "%s", dname);

	chars_left -= strlen(str);

	if (chars_left <= 0) {

		printk(KERN_WARNING
@@ -696,9 +696,9 @@ static void blkio_get_key_name(enum stat_sub_type type, dev_t dev, char *str, 
}



static uint64_t blkio_fill_stat(char *str, int chars_left, uint64_t val,

				struct cgroup_map_cb *cb, dev_t dev)

				struct cgroup_map_cb *cb, const char *dname)

{

	blkio_get_key_name(0, dev, str, chars_left, true);

	blkio_get_key_name(0, dname, str, chars_left, true);

	cb->fill(cb, str, val);

	return val;

}
@@ -730,7 +730,8 @@ static uint64_t blkio_read_stat_cpu(struct blkio_group *blkg, 
}



static uint64_t blkio_get_stat_cpu(struct blkio_group *blkg,

		struct cgroup_map_cb *cb, dev_t dev, enum stat_type_cpu type)

				   struct cgroup_map_cb *cb, const char *dname,

				   enum stat_type_cpu type)

{

	uint64_t disk_total, val;

	char key_str[MAX_KEY_LEN];
@@ -738,12 +739,14 @@ static uint64_t blkio_get_stat_cpu(struct blkio_group *blkg, 


	if (type == BLKIO_STAT_CPU_SECTORS) {

		val = blkio_read_stat_cpu(blkg, type, 0);

		return blkio_fill_stat(key_str, MAX_KEY_LEN - 1, val, cb, dev);

		return blkio_fill_stat(key_str, MAX_KEY_LEN - 1, val, cb,

				       dname);

	}



	for (sub_type = BLKIO_STAT_READ; sub_type < BLKIO_STAT_TOTAL;

			sub_type++) {

		blkio_get_key_name(sub_type, dev, key_str, MAX_KEY_LEN, false);

		blkio_get_key_name(sub_type, dname, key_str, MAX_KEY_LEN,

				   false);

		val = blkio_read_stat_cpu(blkg, type, sub_type);

		cb->fill(cb, key_str, val);

	}
@@ -751,14 +754,16 @@ static uint64_t blkio_get_stat_cpu(struct blkio_group *blkg, 
	disk_total = blkio_read_stat_cpu(blkg, type, BLKIO_STAT_READ) +

			blkio_read_stat_cpu(blkg, type, BLKIO_STAT_WRITE);



	blkio_get_key_name(BLKIO_STAT_TOTAL, dev, key_str, MAX_KEY_LEN, false);

	blkio_get_key_name(BLKIO_STAT_TOTAL, dname, key_str, MAX_KEY_LEN,

			   false);

	cb->fill(cb, key_str, disk_total);

	return disk_total;

}



/* This should be called with blkg->stats_lock held */

static uint64_t blkio_get_stat(struct blkio_group *blkg,

		struct cgroup_map_cb *cb, dev_t dev, enum stat_type type)

			       struct cgroup_map_cb *cb, const char *dname,

			       enum stat_type type)

{

	uint64_t disk_total;

	char key_str[MAX_KEY_LEN];
@@ -766,11 +771,11 @@ static uint64_t blkio_get_stat(struct blkio_group *blkg, 


	if (type == BLKIO_STAT_TIME)

		return blkio_fill_stat(key_str, MAX_KEY_LEN - 1,

					blkg->stats.time, cb, dev);

					blkg->stats.time, cb, dname);

#ifdef CONFIG_DEBUG_BLK_CGROUP

	if (type == BLKIO_STAT_UNACCOUNTED_TIME)

		return blkio_fill_stat(key_str, MAX_KEY_LEN - 1,

					blkg->stats.unaccounted_time, cb, dev);

				       blkg->stats.unaccounted_time, cb, dname);

	if (type == BLKIO_STAT_AVG_QUEUE_SIZE) {

		uint64_t sum = blkg->stats.avg_queue_size_sum;

		uint64_t samples = blkg->stats.avg_queue_size_samples;
@@ -778,30 +783,33 @@ static uint64_t blkio_get_stat(struct blkio_group *blkg, 
			do_div(sum, samples);

		else

			sum = 0;

		return blkio_fill_stat(key_str, MAX_KEY_LEN - 1, sum, cb, dev);

		return blkio_fill_stat(key_str, MAX_KEY_LEN - 1,

				       sum, cb, dname);

	}

	if (type == BLKIO_STAT_GROUP_WAIT_TIME)

		return blkio_fill_stat(key_str, MAX_KEY_LEN - 1,

					blkg->stats.group_wait_time, cb, dev);

				       blkg->stats.group_wait_time, cb, dname);

	if (type == BLKIO_STAT_IDLE_TIME)

		return blkio_fill_stat(key_str, MAX_KEY_LEN - 1,

					blkg->stats.idle_time, cb, dev);

				       blkg->stats.idle_time, cb, dname);

	if (type == BLKIO_STAT_EMPTY_TIME)

		return blkio_fill_stat(key_str, MAX_KEY_LEN - 1,

					blkg->stats.empty_time, cb, dev);

				       blkg->stats.empty_time, cb, dname);

	if (type == BLKIO_STAT_DEQUEUE)

		return blkio_fill_stat(key_str, MAX_KEY_LEN - 1,

					blkg->stats.dequeue, cb, dev);

				       blkg->stats.dequeue, cb, dname);

#endif



	for (sub_type = BLKIO_STAT_READ; sub_type < BLKIO_STAT_TOTAL;

			sub_type++) {

		blkio_get_key_name(sub_type, dev, key_str, MAX_KEY_LEN, false);

		blkio_get_key_name(sub_type, dname, key_str, MAX_KEY_LEN,

				   false);

		cb->fill(cb, key_str, blkg->stats.stat_arr[type][sub_type]);

	}

	disk_total = blkg->stats.stat_arr[type][BLKIO_STAT_READ] +

			blkg->stats.stat_arr[type][BLKIO_STAT_WRITE];

	blkio_get_key_name(BLKIO_STAT_TOTAL, dev, key_str, MAX_KEY_LEN, false);

	blkio_get_key_name(BLKIO_STAT_TOTAL, dname, key_str, MAX_KEY_LEN,

			   false);

	cb->fill(cb, key_str, disk_total);

	return disk_total;

}
@@ -946,14 +954,15 @@ static int blkiocg_file_write(struct cgroup *cgrp, struct cftype *cft, 
static void blkio_print_group_conf(struct cftype *cft, struct blkio_group *blkg,

				   struct seq_file *m)

{

	const char *dname = dev_name(blkg->q->backing_dev_info.dev);

	int fileid = BLKIOFILE_ATTR(cft->private);

	int rw = WRITE;



	switch (blkg->plid) {

		case BLKIO_POLICY_PROP:

			if (blkg->conf.weight)

				seq_printf(m, "%u:%u\t%u\n", MAJOR(blkg->dev),

					MINOR(blkg->dev), blkg->conf.weight);

				seq_printf(m, "%s\t%u\n",

					   dname, blkg->conf.weight);

			break;

		case BLKIO_POLICY_THROTL:

			switch (fileid) {
@@ -961,19 +970,15 @@ static void blkio_print_group_conf(struct cftype *cft, struct blkio_group *blkg, 
				rw = READ;

			case BLKIO_THROTL_write_bps_device:

				if (blkg->conf.bps[rw])

					seq_printf(m, "%u:%u\t%llu\n",

						   MAJOR(blkg->dev),

						   MINOR(blkg->dev),

						   blkg->conf.bps[rw]);

					seq_printf(m, "%s\t%llu\n",

						   dname, blkg->conf.bps[rw]);

				break;

			case BLKIO_THROTL_read_iops_device:

				rw = READ;

			case BLKIO_THROTL_write_iops_device:

				if (blkg->conf.iops[rw])

					seq_printf(m, "%u:%u\t%u\n",

						   MAJOR(blkg->dev),

						   MINOR(blkg->dev),

						   blkg->conf.iops[rw]);

					seq_printf(m, "%s\t%u\n",

						   dname, blkg->conf.iops[rw]);

				break;

			}

			break;
@@ -1044,20 +1049,19 @@ static int blkio_read_blkg_stats(struct blkio_cgroup *blkcg, 


	rcu_read_lock();

	hlist_for_each_entry_rcu(blkg, n, &blkcg->blkg_list, blkcg_node) {

		if (blkg->dev) {

		const char *dname = dev_name(blkg->q->backing_dev_info.dev);



		if (BLKIOFILE_POLICY(cft->private) != blkg->plid)

			continue;

		if (pcpu)

				cgroup_total += blkio_get_stat_cpu(blkg, cb,

						blkg->dev, type);

			cgroup_total += blkio_get_stat_cpu(blkg, cb, dname,

							   type);

		else {

			spin_lock_irq(&blkg->stats_lock);

				cgroup_total += blkio_get_stat(blkg, cb,

						blkg->dev, type);

			cgroup_total += blkio_get_stat(blkg, cb, dname, type);

			spin_unlock_irq(&blkg->stats_lock);

		}

	}

	}

	if (show_total)

		cb->fill(cb, "Total", cgroup_total);

	rcu_read_unlock();

block/blk-cgroup.h

+0 −2
Original line number Diff line number Diff line
@@ -166,8 +166,6 @@ struct blkio_group { 
	unsigned short blkcg_id;

	/* Store cgroup path */

	char path[128];

	/* The device MKDEV(major, minor), this group has been created for */

	dev_t dev;

	/* policy which owns this blk group */

	enum blkio_policy_id plid;

block/blk-throttle.c

+2 −49
Original line number Diff line number Diff line
@@ -212,50 +212,12 @@ static struct blkio_group *throtl_alloc_blkio_group(struct request_queue *q, 
	return &tg->blkg;

}



static void

__throtl_tg_fill_dev_details(struct throtl_data *td, struct throtl_grp *tg)

{

	struct backing_dev_info *bdi = &td->queue->backing_dev_info;

	unsigned int major, minor;



	if (!tg || tg->blkg.dev)

		return;



	/*

	 * Fill in device details for a group which might not have been

	 * filled at group creation time as queue was being instantiated

	 * and driver had not attached a device yet

	 */

	if (bdi->dev && dev_name(bdi->dev)) {

		sscanf(dev_name(bdi->dev), "%u:%u", &major, &minor);

		tg->blkg.dev = MKDEV(major, minor);

	}

}



/*

 * Should be called with without queue lock held. Here queue lock will be

 * taken rarely. It will be taken only once during life time of a group

 * if need be

 */

static void

throtl_tg_fill_dev_details(struct throtl_data *td, struct throtl_grp *tg)

{

	if (!tg || tg->blkg.dev)

		return;



	spin_lock_irq(td->queue->queue_lock);

	__throtl_tg_fill_dev_details(td, tg);

	spin_unlock_irq(td->queue->queue_lock);

}



static void throtl_link_blkio_group(struct request_queue *q,

				    struct blkio_group *blkg)

{

	struct throtl_data *td = q->td;

	struct throtl_grp *tg = tg_of_blkg(blkg);



	__throtl_tg_fill_dev_details(td, tg);



	hlist_add_head(&tg->tg_node, &td->tg_list);

	td->nr_undestroyed_grps++;

}
@@ -263,20 +225,14 @@ static void throtl_link_blkio_group(struct request_queue *q, 
static struct

throtl_grp *throtl_lookup_tg(struct throtl_data *td, struct blkio_cgroup *blkcg)

{

	struct throtl_grp *tg = NULL;



	/*

	 * This is the common case when there are no blkio cgroups.

	 * Avoid lookup in this case

	 */

	if (blkcg == &blkio_root_cgroup)

		tg = td->root_tg;

	else

		tg = tg_of_blkg(blkg_lookup(blkcg, td->queue,

					    BLKIO_POLICY_THROTL));

		return td->root_tg;



	__throtl_tg_fill_dev_details(td, tg);

	return tg;

	return tg_of_blkg(blkg_lookup(blkcg, td->queue, BLKIO_POLICY_THROTL));

}



static struct throtl_grp *throtl_lookup_create_tg(struct throtl_data *td,
@@ -303,7 +259,6 @@ static struct throtl_grp *throtl_lookup_create_tg(struct throtl_data *td, 
			tg = td->root_tg;

	}



	__throtl_tg_fill_dev_details(td, tg);

	return tg;

}
@@ -1090,8 +1045,6 @@ bool blk_throtl_bio(struct request_queue *q, struct bio *bio) 
	blkcg = task_blkio_cgroup(current);

	tg = throtl_lookup_tg(td, blkcg);

	if (tg) {

		throtl_tg_fill_dev_details(td, tg);



		if (tg_no_rule_group(tg, rw)) {

			blkiocg_update_dispatch_stats(&tg->blkg, bio->bi_size,

					rw, rw_is_sync(bio->bi_rw));

block/cfq-iosched.c

+0 −21
Original line number Diff line number Diff line
@@ -1052,20 +1052,7 @@ static void cfq_link_blkio_group(struct request_queue *q, 
				 struct blkio_group *blkg)

{

	struct cfq_data *cfqd = q->elevator->elevator_data;

	struct backing_dev_info *bdi = &q->backing_dev_info;

	struct cfq_group *cfqg = cfqg_of_blkg(blkg);

	unsigned int major, minor;



	/*

	 * Add group onto cgroup list. It might happen that bdi->dev is

	 * not initialized yet. Initialize this new group without major

	 * and minor info and this info will be filled in once a new thread

	 * comes for IO.

	 */

	if (bdi->dev) {

		sscanf(dev_name(bdi->dev), "%u:%u", &major, &minor);

		blkg->dev = MKDEV(major, minor);

	}



	cfqd->nr_blkcg_linked_grps++;
@@ -1104,7 +1091,6 @@ static struct cfq_group *cfq_lookup_create_cfqg(struct cfq_data *cfqd, 
						struct blkio_cgroup *blkcg)

{

	struct request_queue *q = cfqd->queue;

	struct backing_dev_info *bdi = &q->backing_dev_info;

	struct cfq_group *cfqg = NULL;



	/* avoid lookup for the common case where there's no blkio cgroup */
@@ -1118,13 +1104,6 @@ static struct cfq_group *cfq_lookup_create_cfqg(struct cfq_data *cfqd, 
			cfqg = cfqg_of_blkg(blkg);

	}



	if (cfqg && !cfqg->blkg.dev && bdi->dev && dev_name(bdi->dev)) {

		unsigned int major, minor;



		sscanf(dev_name(bdi->dev), "%u:%u", &major, &minor);

		cfqg->blkg.dev = MKDEV(major, minor);

	}



	return cfqg;

}

CRA Git | Maintained and supported by SUSTech CRA and CCSE