From 7a4dd281ec66224f802093962d1d903d86b09560 Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Mon, 5 Mar 2012 13:15:09 -0800 Subject: [PATCH] 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: Tejun Heo Cc: Vivek Goyal Signed-off-by: Jens Axboe --- block/blk-cgroup.c | 86 +++++++++++++++++++++++--------------------- block/blk-cgroup.h | 2 -- block/blk-throttle.c | 51 ++------------------------ block/cfq-iosched.c | 21 ----------- 4 files changed, 47 insertions(+), 113 deletions(-) diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c index adf61c99258c..8742af3be84b 100644 --- a/block/blk-cgroup.c +++ b/block/blk-cgroup.c @@ -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,18 +1049,17 @@ 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) { - if (BLKIOFILE_POLICY(cft->private) != blkg->plid) - continue; - if (pcpu) - cgroup_total += blkio_get_stat_cpu(blkg, cb, - blkg->dev, type); - else { - spin_lock_irq(&blkg->stats_lock); - cgroup_total += blkio_get_stat(blkg, cb, - blkg->dev, type); - spin_unlock_irq(&blkg->stats_lock); - } + 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, dname, + type); + else { + spin_lock_irq(&blkg->stats_lock); + cgroup_total += blkio_get_stat(blkg, cb, dname, type); + spin_unlock_irq(&blkg->stats_lock); } } if (show_total) diff --git a/block/blk-cgroup.h b/block/blk-cgroup.h index 9a5c68d7cc92..7ebecf6ea8f1 100644 --- a/block/blk-cgroup.h +++ b/block/blk-cgroup.h @@ -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; diff --git a/block/blk-throttle.c b/block/blk-throttle.c index 791b10719e43..52a429397d3b 100644 --- a/block/blk-throttle.c +++ b/block/blk-throttle.c @@ -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)); diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c index 08d4fdd188fa..f67d109eb974 100644 --- a/block/cfq-iosched.c +++ b/block/cfq-iosched.c @@ -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; }