diff options
| author | Nikanth Karthikesan <knikanth@suse.de> | 2009-10-06 14:16:55 -0400 | 
|---|---|---|
| committer | Jens Axboe <jens.axboe@oracle.com> | 2009-10-06 14:16:55 -0400 | 
| commit | 316d315bffa4026f28085f6b24ebcebede370ac7 (patch) | |
| tree | 10b6b057fec2382536371d2e14f9d4c0d6cf9eea | |
| parent | 23e018a1b083ecb4b8bb2fb43d58e7c19b5d7959 (diff) | |
block: Seperate read and write statistics of in_flight requests v2
Commit a9327cac440be4d8333bba975cbbf76045096275 added seperate read
and write statistics of in_flight requests. And exported the number
of read and write requests in progress seperately through sysfs.
But  Corrado Zoccolo <czoccolo@gmail.com> reported getting strange
output from "iostat -kx 2". Global values for service time and
utilization were garbage. For interval values, utilization was always
100%, and service time is higher than normal.
So this was reverted by commit 0f78ab9899e9d6acb09d5465def618704255963b
The problem was in part_round_stats_single(), I missed the following:
        if (now == part->stamp)
                return;
-       if (part->in_flight) {
+       if (part_in_flight(part)) {
                __part_stat_add(cpu, part, time_in_queue,
                                part_in_flight(part) * (now - part->stamp));
                __part_stat_add(cpu, part, io_ticks, (now - part->stamp));
With this chunk included, the reported regression gets fixed.
Signed-off-by: Nikanth Karthikesan <knikanth@suse.de>
--
Signed-off-by: Jens Axboe <jens.axboe@oracle.com>
| -rw-r--r-- | block/blk-core.c | 8 | ||||
| -rw-r--r-- | block/blk-merge.c | 2 | ||||
| -rw-r--r-- | block/genhd.c | 4 | ||||
| -rw-r--r-- | drivers/md/dm.c | 16 | ||||
| -rw-r--r-- | fs/partitions/check.c | 12 | ||||
| -rw-r--r-- | include/linux/genhd.h | 21 | 
6 files changed, 43 insertions, 20 deletions
diff --git a/block/blk-core.c b/block/blk-core.c index 73ecbed02605..ac0fa10f8fa5 100644 --- a/block/blk-core.c +++ b/block/blk-core.c  | |||
| @@ -70,7 +70,7 @@ static void drive_stat_acct(struct request *rq, int new_io) | |||
| 70 | part_stat_inc(cpu, part, merges[rw]); | 70 | part_stat_inc(cpu, part, merges[rw]); | 
| 71 | else { | 71 | else { | 
| 72 | part_round_stats(cpu, part); | 72 | part_round_stats(cpu, part); | 
| 73 | part_inc_in_flight(part); | 73 | part_inc_in_flight(part, rw); | 
| 74 | } | 74 | } | 
| 75 | 75 | ||
| 76 | part_stat_unlock(); | 76 | part_stat_unlock(); | 
| @@ -1030,9 +1030,9 @@ static void part_round_stats_single(int cpu, struct hd_struct *part, | |||
| 1030 | if (now == part->stamp) | 1030 | if (now == part->stamp) | 
| 1031 | return; | 1031 | return; | 
| 1032 | 1032 | ||
| 1033 | if (part->in_flight) { | 1033 | if (part_in_flight(part)) { | 
| 1034 | __part_stat_add(cpu, part, time_in_queue, | 1034 | __part_stat_add(cpu, part, time_in_queue, | 
| 1035 | part->in_flight * (now - part->stamp)); | 1035 | part_in_flight(part) * (now - part->stamp)); | 
| 1036 | __part_stat_add(cpu, part, io_ticks, (now - part->stamp)); | 1036 | __part_stat_add(cpu, part, io_ticks, (now - part->stamp)); | 
| 1037 | } | 1037 | } | 
| 1038 | part->stamp = now; | 1038 | part->stamp = now; | 
| @@ -1739,7 +1739,7 @@ static void blk_account_io_done(struct request *req) | |||
| 1739 | part_stat_inc(cpu, part, ios[rw]); | 1739 | part_stat_inc(cpu, part, ios[rw]); | 
| 1740 | part_stat_add(cpu, part, ticks[rw], duration); | 1740 | part_stat_add(cpu, part, ticks[rw], duration); | 
| 1741 | part_round_stats(cpu, part); | 1741 | part_round_stats(cpu, part); | 
| 1742 | part_dec_in_flight(part); | 1742 | part_dec_in_flight(part, rw); | 
| 1743 | 1743 | ||
| 1744 | part_stat_unlock(); | 1744 | part_stat_unlock(); | 
| 1745 | } | 1745 | } | 
diff --git a/block/blk-merge.c b/block/blk-merge.c index b0de8574fdc8..99cb5cf1f447 100644 --- a/block/blk-merge.c +++ b/block/blk-merge.c  | |||
| @@ -351,7 +351,7 @@ static void blk_account_io_merge(struct request *req) | |||
| 351 | part = disk_map_sector_rcu(req->rq_disk, blk_rq_pos(req)); | 351 | part = disk_map_sector_rcu(req->rq_disk, blk_rq_pos(req)); | 
| 352 | 352 | ||
| 353 | part_round_stats(cpu, part); | 353 | part_round_stats(cpu, part); | 
| 354 | part_dec_in_flight(part); | 354 | part_dec_in_flight(part, rq_data_dir(req)); | 
| 355 | 355 | ||
| 356 | part_stat_unlock(); | 356 | part_stat_unlock(); | 
| 357 | } | 357 | } | 
diff --git a/block/genhd.c b/block/genhd.c index 5a0861da324d..517e4332cb37 100644 --- a/block/genhd.c +++ b/block/genhd.c  | |||
| @@ -869,6 +869,7 @@ static DEVICE_ATTR(size, S_IRUGO, part_size_show, NULL); | |||
| 869 | static DEVICE_ATTR(alignment_offset, S_IRUGO, disk_alignment_offset_show, NULL); | 869 | static DEVICE_ATTR(alignment_offset, S_IRUGO, disk_alignment_offset_show, NULL); | 
| 870 | static DEVICE_ATTR(capability, S_IRUGO, disk_capability_show, NULL); | 870 | static DEVICE_ATTR(capability, S_IRUGO, disk_capability_show, NULL); | 
| 871 | static DEVICE_ATTR(stat, S_IRUGO, part_stat_show, NULL); | 871 | static DEVICE_ATTR(stat, S_IRUGO, part_stat_show, NULL); | 
| 872 | static DEVICE_ATTR(inflight, S_IRUGO, part_inflight_show, NULL); | ||
| 872 | #ifdef CONFIG_FAIL_MAKE_REQUEST | 873 | #ifdef CONFIG_FAIL_MAKE_REQUEST | 
| 873 | static struct device_attribute dev_attr_fail = | 874 | static struct device_attribute dev_attr_fail = | 
| 874 | __ATTR(make-it-fail, S_IRUGO|S_IWUSR, part_fail_show, part_fail_store); | 875 | __ATTR(make-it-fail, S_IRUGO|S_IWUSR, part_fail_show, part_fail_store); | 
| @@ -888,6 +889,7 @@ static struct attribute *disk_attrs[] = { | |||
| 888 | &dev_attr_alignment_offset.attr, | 889 | &dev_attr_alignment_offset.attr, | 
| 889 | &dev_attr_capability.attr, | 890 | &dev_attr_capability.attr, | 
| 890 | &dev_attr_stat.attr, | 891 | &dev_attr_stat.attr, | 
| 892 | &dev_attr_inflight.attr, | ||
| 891 | #ifdef CONFIG_FAIL_MAKE_REQUEST | 893 | #ifdef CONFIG_FAIL_MAKE_REQUEST | 
| 892 | &dev_attr_fail.attr, | 894 | &dev_attr_fail.attr, | 
| 893 | #endif | 895 | #endif | 
| @@ -1053,7 +1055,7 @@ static int diskstats_show(struct seq_file *seqf, void *v) | |||
| 1053 | part_stat_read(hd, merges[1]), | 1055 | part_stat_read(hd, merges[1]), | 
| 1054 | (unsigned long long)part_stat_read(hd, sectors[1]), | 1056 | (unsigned long long)part_stat_read(hd, sectors[1]), | 
| 1055 | jiffies_to_msecs(part_stat_read(hd, ticks[1])), | 1057 | jiffies_to_msecs(part_stat_read(hd, ticks[1])), | 
| 1056 | hd->in_flight, | 1058 | part_in_flight(hd), | 
| 1057 | jiffies_to_msecs(part_stat_read(hd, io_ticks)), | 1059 | jiffies_to_msecs(part_stat_read(hd, io_ticks)), | 
| 1058 | jiffies_to_msecs(part_stat_read(hd, time_in_queue)) | 1060 | jiffies_to_msecs(part_stat_read(hd, time_in_queue)) | 
| 1059 | ); | 1061 | ); | 
diff --git a/drivers/md/dm.c b/drivers/md/dm.c index 23e76fe0d359..376f1ab48a24 100644 --- a/drivers/md/dm.c +++ b/drivers/md/dm.c  | |||
| @@ -130,7 +130,7 @@ struct mapped_device { | |||
| 130 | /* | 130 | /* | 
| 131 | * A list of ios that arrived while we were suspended. | 131 | * A list of ios that arrived while we were suspended. | 
| 132 | */ | 132 | */ | 
| 133 | atomic_t pending; | 133 | atomic_t pending[2]; | 
| 134 | wait_queue_head_t wait; | 134 | wait_queue_head_t wait; | 
| 135 | struct work_struct work; | 135 | struct work_struct work; | 
| 136 | struct bio_list deferred; | 136 | struct bio_list deferred; | 
| @@ -453,13 +453,14 @@ static void start_io_acct(struct dm_io *io) | |||
| 453 | { | 453 | { | 
| 454 | struct mapped_device *md = io->md; | 454 | struct mapped_device *md = io->md; | 
| 455 | int cpu; | 455 | int cpu; | 
| 456 | int rw = bio_data_dir(io->bio); | ||
| 456 | 457 | ||
| 457 | io->start_time = jiffies; | 458 | io->start_time = jiffies; | 
| 458 | 459 | ||
| 459 | cpu = part_stat_lock(); | 460 | cpu = part_stat_lock(); | 
| 460 | part_round_stats(cpu, &dm_disk(md)->part0); | 461 | part_round_stats(cpu, &dm_disk(md)->part0); | 
| 461 | part_stat_unlock(); | 462 | part_stat_unlock(); | 
| 462 | dm_disk(md)->part0.in_flight = atomic_inc_return(&md->pending); | 463 | dm_disk(md)->part0.in_flight[rw] = atomic_inc_return(&md->pending[rw]); | 
| 463 | } | 464 | } | 
| 464 | 465 | ||
| 465 | static void end_io_acct(struct dm_io *io) | 466 | static void end_io_acct(struct dm_io *io) | 
| @@ -479,8 +480,9 @@ static void end_io_acct(struct dm_io *io) | |||
| 479 | * After this is decremented the bio must not be touched if it is | 480 | * After this is decremented the bio must not be touched if it is | 
| 480 | * a barrier. | 481 | * a barrier. | 
| 481 | */ | 482 | */ | 
| 482 | dm_disk(md)->part0.in_flight = pending = | 483 | dm_disk(md)->part0.in_flight[rw] = pending = | 
| 483 | atomic_dec_return(&md->pending); | 484 | atomic_dec_return(&md->pending[rw]); | 
| 485 | pending += atomic_read(&md->pending[rw^0x1]); | ||
| 484 | 486 | ||
| 485 | /* nudge anyone waiting on suspend queue */ | 487 | /* nudge anyone waiting on suspend queue */ | 
| 486 | if (!pending) | 488 | if (!pending) | 
| @@ -1785,7 +1787,8 @@ static struct mapped_device *alloc_dev(int minor) | |||
| 1785 | if (!md->disk) | 1787 | if (!md->disk) | 
| 1786 | goto bad_disk; | 1788 | goto bad_disk; | 
| 1787 | 1789 | ||
| 1788 | atomic_set(&md->pending, 0); | 1790 | atomic_set(&md->pending[0], 0); | 
| 1791 | atomic_set(&md->pending[1], 0); | ||
| 1789 | init_waitqueue_head(&md->wait); | 1792 | init_waitqueue_head(&md->wait); | 
| 1790 | INIT_WORK(&md->work, dm_wq_work); | 1793 | INIT_WORK(&md->work, dm_wq_work); | 
| 1791 | init_waitqueue_head(&md->eventq); | 1794 | init_waitqueue_head(&md->eventq); | 
| @@ -2088,7 +2091,8 @@ static int dm_wait_for_completion(struct mapped_device *md, int interruptible) | |||
| 2088 | break; | 2091 | break; | 
| 2089 | } | 2092 | } | 
| 2090 | spin_unlock_irqrestore(q->queue_lock, flags); | 2093 | spin_unlock_irqrestore(q->queue_lock, flags); | 
| 2091 | } else if (!atomic_read(&md->pending)) | 2094 | } else if (!atomic_read(&md->pending[0]) && | 
| 2095 | !atomic_read(&md->pending[1])) | ||
| 2092 | break; | 2096 | break; | 
| 2093 | 2097 | ||
| 2094 | if (interruptible == TASK_INTERRUPTIBLE && | 2098 | if (interruptible == TASK_INTERRUPTIBLE && | 
diff --git a/fs/partitions/check.c b/fs/partitions/check.c index f38fee0311a7..7b685e10cbad 100644 --- a/fs/partitions/check.c +++ b/fs/partitions/check.c  | |||
| @@ -248,11 +248,19 @@ ssize_t part_stat_show(struct device *dev, | |||
| 248 | part_stat_read(p, merges[WRITE]), | 248 | part_stat_read(p, merges[WRITE]), | 
| 249 | (unsigned long long)part_stat_read(p, sectors[WRITE]), | 249 | (unsigned long long)part_stat_read(p, sectors[WRITE]), | 
| 250 | jiffies_to_msecs(part_stat_read(p, ticks[WRITE])), | 250 | jiffies_to_msecs(part_stat_read(p, ticks[WRITE])), | 
| 251 | p->in_flight, | 251 | part_in_flight(p), | 
| 252 | jiffies_to_msecs(part_stat_read(p, io_ticks)), | 252 | jiffies_to_msecs(part_stat_read(p, io_ticks)), | 
| 253 | jiffies_to_msecs(part_stat_read(p, time_in_queue))); | 253 | jiffies_to_msecs(part_stat_read(p, time_in_queue))); | 
| 254 | } | 254 | } | 
| 255 | 255 | ||
| 256 | ssize_t part_inflight_show(struct device *dev, | ||
| 257 | struct device_attribute *attr, char *buf) | ||
| 258 | { | ||
| 259 | struct hd_struct *p = dev_to_part(dev); | ||
| 260 | |||
| 261 | return sprintf(buf, "%8u %8u\n", p->in_flight[0], p->in_flight[1]); | ||
| 262 | } | ||
| 263 | |||
| 256 | #ifdef CONFIG_FAIL_MAKE_REQUEST | 264 | #ifdef CONFIG_FAIL_MAKE_REQUEST | 
| 257 | ssize_t part_fail_show(struct device *dev, | 265 | ssize_t part_fail_show(struct device *dev, | 
| 258 | struct device_attribute *attr, char *buf) | 266 | struct device_attribute *attr, char *buf) | 
| @@ -281,6 +289,7 @@ static DEVICE_ATTR(start, S_IRUGO, part_start_show, NULL); | |||
| 281 | static DEVICE_ATTR(size, S_IRUGO, part_size_show, NULL); | 289 | static DEVICE_ATTR(size, S_IRUGO, part_size_show, NULL); | 
| 282 | static DEVICE_ATTR(alignment_offset, S_IRUGO, part_alignment_offset_show, NULL); | 290 | static DEVICE_ATTR(alignment_offset, S_IRUGO, part_alignment_offset_show, NULL); | 
| 283 | static DEVICE_ATTR(stat, S_IRUGO, part_stat_show, NULL); | 291 | static DEVICE_ATTR(stat, S_IRUGO, part_stat_show, NULL); | 
| 292 | static DEVICE_ATTR(inflight, S_IRUGO, part_inflight_show, NULL); | ||
| 284 | #ifdef CONFIG_FAIL_MAKE_REQUEST | 293 | #ifdef CONFIG_FAIL_MAKE_REQUEST | 
| 285 | static struct device_attribute dev_attr_fail = | 294 | static struct device_attribute dev_attr_fail = | 
| 286 | __ATTR(make-it-fail, S_IRUGO|S_IWUSR, part_fail_show, part_fail_store); | 295 | __ATTR(make-it-fail, S_IRUGO|S_IWUSR, part_fail_show, part_fail_store); | 
| @@ -292,6 +301,7 @@ static struct attribute *part_attrs[] = { | |||
| 292 | &dev_attr_size.attr, | 301 | &dev_attr_size.attr, | 
| 293 | &dev_attr_alignment_offset.attr, | 302 | &dev_attr_alignment_offset.attr, | 
| 294 | &dev_attr_stat.attr, | 303 | &dev_attr_stat.attr, | 
| 304 | &dev_attr_inflight.attr, | ||
| 295 | #ifdef CONFIG_FAIL_MAKE_REQUEST | 305 | #ifdef CONFIG_FAIL_MAKE_REQUEST | 
| 296 | &dev_attr_fail.attr, | 306 | &dev_attr_fail.attr, | 
| 297 | #endif | 307 | #endif | 
diff --git a/include/linux/genhd.h b/include/linux/genhd.h index 7beaa21b3880..297df45ffd0a 100644 --- a/include/linux/genhd.h +++ b/include/linux/genhd.h  | |||
| @@ -98,7 +98,7 @@ struct hd_struct { | |||
| 98 | int make_it_fail; | 98 | int make_it_fail; | 
| 99 | #endif | 99 | #endif | 
| 100 | unsigned long stamp; | 100 | unsigned long stamp; | 
| 101 | int in_flight; | 101 | int in_flight[2]; | 
| 102 | #ifdef CONFIG_SMP | 102 | #ifdef CONFIG_SMP | 
| 103 | struct disk_stats *dkstats; | 103 | struct disk_stats *dkstats; | 
| 104 | #else | 104 | #else | 
| @@ -322,18 +322,23 @@ static inline void free_part_stats(struct hd_struct *part) | |||
| 322 | #define part_stat_sub(cpu, gendiskp, field, subnd) \ | 322 | #define part_stat_sub(cpu, gendiskp, field, subnd) \ | 
| 323 | part_stat_add(cpu, gendiskp, field, -subnd) | 323 | part_stat_add(cpu, gendiskp, field, -subnd) | 
| 324 | 324 | ||
| 325 | static inline void part_inc_in_flight(struct hd_struct *part) | 325 | static inline void part_inc_in_flight(struct hd_struct *part, int rw) | 
| 326 | { | 326 | { | 
| 327 | part->in_flight++; | 327 | part->in_flight[rw]++; | 
| 328 | if (part->partno) | 328 | if (part->partno) | 
| 329 | part_to_disk(part)->part0.in_flight++; | 329 | part_to_disk(part)->part0.in_flight[rw]++; | 
| 330 | } | 330 | } | 
| 331 | 331 | ||
| 332 | static inline void part_dec_in_flight(struct hd_struct *part) | 332 | static inline void part_dec_in_flight(struct hd_struct *part, int rw) | 
| 333 | { | 333 | { | 
| 334 | part->in_flight--; | 334 | part->in_flight[rw]--; | 
| 335 | if (part->partno) | 335 | if (part->partno) | 
| 336 | part_to_disk(part)->part0.in_flight--; | 336 | part_to_disk(part)->part0.in_flight[rw]--; | 
| 337 | } | ||
| 338 | |||
| 339 | static inline int part_in_flight(struct hd_struct *part) | ||
| 340 | { | ||
| 341 | return part->in_flight[0] + part->in_flight[1]; | ||
| 337 | } | 342 | } | 
| 338 | 343 | ||
| 339 | /* block/blk-core.c */ | 344 | /* block/blk-core.c */ | 
| @@ -546,6 +551,8 @@ extern ssize_t part_size_show(struct device *dev, | |||
| 546 | struct device_attribute *attr, char *buf); | 551 | struct device_attribute *attr, char *buf); | 
| 547 | extern ssize_t part_stat_show(struct device *dev, | 552 | extern ssize_t part_stat_show(struct device *dev, | 
| 548 | struct device_attribute *attr, char *buf); | 553 | struct device_attribute *attr, char *buf); | 
| 554 | extern ssize_t part_inflight_show(struct device *dev, | ||
| 555 | struct device_attribute *attr, char *buf); | ||
| 549 | #ifdef CONFIG_FAIL_MAKE_REQUEST | 556 | #ifdef CONFIG_FAIL_MAKE_REQUEST | 
| 550 | extern ssize_t part_fail_show(struct device *dev, | 557 | extern ssize_t part_fail_show(struct device *dev, | 
| 551 | struct device_attribute *attr, char *buf); | 558 | struct device_attribute *attr, char *buf); | 
