diff mbox series

[2/3] block/diskstats: remove redundant and inaccurate time_in_queue counter

Message ID 155413438636.3201.8688215933219744911.stgit@buzz (mailing list archive)
State New, archived
Headers show
Series [1/3] block/diskstats: accumulate all per-cpu counters in one pass | expand

Commit Message

Konstantin Khlebnikov April 1, 2019, 3:59 p.m. UTC
Diskstat's column "time_in_queue" must be equal to sum "read tisks",
"write ticks" and "discard ticks". But it is accounted separately and
in jiffies rather than nanoseconds as other times. As a result total
time is not even close to the sum, especially for fast ssd disks.

In Documentation/block/stat.txt these fields are defined as product of
time when disk had such requests and count of them. But that is equal to
sum of running time of completed requests plus running time of in-flight
requests. Latter part is small or even zero if disk is idle.

This patch removes field time_in_queue and replaces with simple sum.

Fixes: 5b18b5a73760 ("block: delete part_round_stats and switch to less precise counting")
Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
---
 block/bio.c               |    1 -
 block/blk-core.c          |    1 -
 block/genhd.c             |    6 ++++--
 block/partition-generic.c |    5 ++++-
 include/linux/genhd.h     |    1 -
 5 files changed, 8 insertions(+), 6 deletions(-)
diff mbox series

Patch

diff --git a/block/bio.c b/block/bio.c
index b64cedc7f87c..c0a60f3e9b7b 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -1772,7 +1772,6 @@  void generic_end_io_acct(struct request_queue *q, int req_op,
 
 	update_io_ticks(part, now);
 	part_stat_add(part, nsecs[sgrp], jiffies_to_nsecs(duration));
-	part_stat_add(part, time_in_queue, duration);
 	part_dec_in_flight(q, part, op_is_write(req_op));
 
 	part_stat_unlock();
diff --git a/block/blk-core.c b/block/blk-core.c
index 4673ebe42255..d89168b167e9 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -1337,7 +1337,6 @@  void blk_account_io_done(struct request *req, u64 now)
 		update_io_ticks(part, jiffies);
 		part_stat_inc(part, ios[sgrp]);
 		part_stat_add(part, nsecs[sgrp], now - req->start_time_ns);
-		part_stat_add(part, time_in_queue, nsecs_to_jiffies64(now - req->start_time_ns));
 		part_dec_in_flight(req->q, part, rq_data_dir(req));
 
 		hd_struct_put(part);
diff --git a/block/genhd.c b/block/genhd.c
index 2f986af81ba1..7e53d6af08fb 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -63,7 +63,6 @@  void part_stat_read_all(struct hd_struct *part, struct disk_stats *stat)
 		}
 
 		stat->io_ticks += ptr->io_ticks;
-		stat->time_in_queue += ptr->time_in_queue;
 	}
 }
 #else /* CONFIG_SMP */
@@ -1403,7 +1402,10 @@  static int diskstats_show(struct seq_file *seqf, void *v)
 							NSEC_PER_MSEC),
 			   inflight,
 			   jiffies_to_msecs(stat.io_ticks),
-			   jiffies_to_msecs(stat.time_in_queue),
+			   (unsigned int)div_u64(stat.nsecs[STAT_READ] +
+						 stat.nsecs[STAT_WRITE] +
+						 stat.nsecs[STAT_DISCARD],
+							NSEC_PER_MSEC),
 			   stat.ios[STAT_DISCARD],
 			   stat.merges[STAT_DISCARD],
 			   stat.sectors[STAT_DISCARD],
diff --git a/block/partition-generic.c b/block/partition-generic.c
index 621e0d9f3c92..935df55c0c91 100644
--- a/block/partition-generic.c
+++ b/block/partition-generic.c
@@ -142,7 +142,10 @@  ssize_t part_stat_show(struct device *dev,
 		(unsigned int)div_u64(stat.nsecs[STAT_WRITE], NSEC_PER_MSEC),
 		inflight,
 		jiffies_to_msecs(stat.io_ticks),
-		jiffies_to_msecs(stat.time_in_queue),
+		(unsigned int)div_u64(stat.nsecs[STAT_READ] +
+				      stat.nsecs[STAT_WRITE] +
+				      stat.nsecs[STAT_DISCARD],
+						NSEC_PER_MSEC),
 		stat.ios[STAT_DISCARD],
 		stat.merges[STAT_DISCARD],
 		(unsigned long long)stat.sectors[STAT_DISCARD],
diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index 7f981be190b4..2f5a9ed7e86e 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -89,7 +89,6 @@  struct disk_stats {
 	unsigned long ios[NR_STAT_GROUPS];
 	unsigned long merges[NR_STAT_GROUPS];
 	unsigned long io_ticks;
-	unsigned long time_in_queue;
 	local_t in_flight[2];
 };