diff mbox series

[2/3] block: factor out common code for part_stat_show() and diskstats_show()

Message ID 20220317112653.1019490-3-yukuai3@huawei.com (mailing list archive)
State New, archived
Headers show
Series optimizations for io accounting | expand

Commit Message

Yu Kuai March 17, 2022, 11:26 a.m. UTC 
part_stat_show() and diskstats_show() are very similar, just factor out
common code.

Signed-off-by: Yu Kuai <yukuai3@huawei.com>
---
 block/genhd.c | 130 +++++++++++++++++++-------------------------------
 1 file changed, 49 insertions(+), 81 deletions(-)

Comments

Christoph Hellwig March 25, 2022, 8:46 a.m. UTC | #1 
On Thu, Mar 17, 2022 at 07:26:52PM +0800, Yu Kuai wrote:
> part_stat_show() and diskstats_show() are very similar, just factor out
> common code.

Well, it doesn't really "factor" much, but creates a big and pretty
unmaintainble macro.  I don't really see the benefit here.
Yu Kuai March 26, 2022, 1:11 a.m. UTC | #2 
在 2022/03/25 16:46, Christoph Hellwig 写道:
> On Thu, Mar 17, 2022 at 07:26:52PM +0800, Yu Kuai wrote:
>> part_stat_show() and diskstats_show() are very similar, just factor out
>> common code.
> 
> Well, it doesn't really "factor" much, but creates a big and pretty
> unmaintainble macro.  I don't really see the benefit here.

Hi,

Thanks for your advice, I'll remove this patch.

BTW, do you have any suggestion about patch 3 ?

Thanks,
Kuai
> .
>
diff mbox series

Patch

diff --git a/block/genhd.c b/block/genhd.c
index e5307f512185..f2c7de2e7ca9 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -53,6 +53,30 @@  static atomic64_t diskseq;
 #define NR_EXT_DEVT		(1 << MINORBITS)
 static DEFINE_IDA(ext_devt_ida);
 
+#define part_stat_info(inflight, stat) \
+	"%lu %lu %llu %u " \
+	"%lu %lu %llu %u " \
+	"%u %u %u " \
+	"%lu %lu %llu %u " \
+	"%lu %u" \
+	"\n", \
+	(stat).ios[STAT_READ], (stat).merges[STAT_READ], \
+	(unsigned long long)(stat).sectors[STAT_READ], \
+	(unsigned int)div_u64((stat).nsecs[STAT_READ], NSEC_PER_MSEC), \
+	(stat).ios[STAT_WRITE], (stat).merges[STAT_WRITE], \
+	(unsigned long long)(stat).sectors[STAT_WRITE], \
+	(unsigned int)div_u64((stat).nsecs[STAT_WRITE], NSEC_PER_MSEC), \
+	(inflight), jiffies_to_msecs((stat).io_ticks), \
+	(unsigned int)div_u64((stat).nsecs[STAT_READ] + \
+			      (stat).nsecs[STAT_WRITE] + \
+			      (stat).nsecs[STAT_DISCARD] + \
+			      (stat).nsecs[STAT_FLUSH], NSEC_PER_MSEC), \
+	(stat).ios[STAT_DISCARD], (stat).merges[STAT_DISCARD], \
+	(unsigned long long)(stat).sectors[STAT_DISCARD], \
+	(unsigned int)div_u64((stat).nsecs[STAT_DISCARD], NSEC_PER_MSEC), \
+	(stat).ios[STAT_FLUSH], \
+	(unsigned int)div_u64((stat).nsecs[STAT_FLUSH], NSEC_PER_MSEC)
+
 void set_capacity(struct gendisk *disk, sector_t sectors)
 {
 	struct block_device *bdev = disk->part0;
@@ -929,17 +953,13 @@  ssize_t part_size_show(struct device *dev,
 	return sprintf(buf, "%llu\n", bdev_nr_sectors(dev_to_bdev(dev)));
 }
 
-ssize_t part_stat_show(struct device *dev,
-		       struct device_attribute *attr, char *buf)
+static unsigned int part_get_stat(struct block_device *bdev,
+				  struct disk_stats *stat)
+
 {
-	struct block_device *bdev = dev_to_bdev(dev);
 	struct request_queue *q = bdev_get_queue(bdev);
-	struct disk_stats stat;
 	unsigned int inflight;
 
-	if (!blk_queue_io_stat(q))
-		return sprintf(buf, "io accounting is disabled\n");
-
 	if (queue_is_mq(q))
 		inflight = blk_mq_in_flight(q, bdev);
 	else
@@ -950,35 +970,23 @@  ssize_t part_stat_show(struct device *dev,
 		update_io_ticks(bdev, jiffies, true);
 		part_stat_unlock();
 	}
-	part_stat_read_all(bdev, &stat);
-	return sprintf(buf,
-		"%8lu %8lu %8llu %8u "
-		"%8lu %8lu %8llu %8u "
-		"%8u %8u %8u "
-		"%8lu %8lu %8llu %8u "
-		"%8lu %8u"
-		"\n",
-		stat.ios[STAT_READ],
-		stat.merges[STAT_READ],
-		(unsigned long long)stat.sectors[STAT_READ],
-		(unsigned int)div_u64(stat.nsecs[STAT_READ], NSEC_PER_MSEC),
-		stat.ios[STAT_WRITE],
-		stat.merges[STAT_WRITE],
-		(unsigned long long)stat.sectors[STAT_WRITE],
-		(unsigned int)div_u64(stat.nsecs[STAT_WRITE], NSEC_PER_MSEC),
-		inflight,
-		jiffies_to_msecs(stat.io_ticks),
-		(unsigned int)div_u64(stat.nsecs[STAT_READ] +
-				      stat.nsecs[STAT_WRITE] +
-				      stat.nsecs[STAT_DISCARD] +
-				      stat.nsecs[STAT_FLUSH],
-						NSEC_PER_MSEC),
-		stat.ios[STAT_DISCARD],
-		stat.merges[STAT_DISCARD],
-		(unsigned long long)stat.sectors[STAT_DISCARD],
-		(unsigned int)div_u64(stat.nsecs[STAT_DISCARD], NSEC_PER_MSEC),
-		stat.ios[STAT_FLUSH],
-		(unsigned int)div_u64(stat.nsecs[STAT_FLUSH], NSEC_PER_MSEC));
+	part_stat_read_all(bdev, stat);
+
+	return inflight;
+}
+
+ssize_t part_stat_show(struct device *dev,
+		       struct device_attribute *attr, char *buf)
+{
+	struct block_device *bdev = dev_to_bdev(dev);
+	struct disk_stats stat;
+	unsigned int inflight;
+
+	if (!blk_queue_io_stat(bdev_get_queue(bdev)))
+		return sprintf(buf, "io accounting is disabled\n");
+
+	inflight = part_get_stat(bdev, &stat);
+	return sprintf(buf, part_stat_info(inflight, stat));
 }
 
 ssize_t part_inflight_show(struct device *dev, struct device_attribute *attr,
@@ -1212,51 +1220,11 @@  static int diskstats_show(struct seq_file *seqf, void *v)
 			continue;
 		if (!blk_queue_io_stat(gp->queue))
 			continue;
-		if (queue_is_mq(gp->queue))
-			inflight = blk_mq_in_flight(gp->queue, hd);
-		else
-			inflight = part_in_flight(hd);
-
-		if (inflight) {
-			part_stat_lock();
-			update_io_ticks(hd, jiffies, true);
-			part_stat_unlock();
-		}
-		part_stat_read_all(hd, &stat);
-		seq_printf(seqf, "%4d %7d %pg "
-			   "%lu %lu %lu %u "
-			   "%lu %lu %lu %u "
-			   "%u %u %u "
-			   "%lu %lu %lu %u "
-			   "%lu %u"
-			   "\n",
-			   MAJOR(hd->bd_dev), MINOR(hd->bd_dev), hd,
-			   stat.ios[STAT_READ],
-			   stat.merges[STAT_READ],
-			   stat.sectors[STAT_READ],
-			   (unsigned int)div_u64(stat.nsecs[STAT_READ],
-							NSEC_PER_MSEC),
-			   stat.ios[STAT_WRITE],
-			   stat.merges[STAT_WRITE],
-			   stat.sectors[STAT_WRITE],
-			   (unsigned int)div_u64(stat.nsecs[STAT_WRITE],
-							NSEC_PER_MSEC),
-			   inflight,
-			   jiffies_to_msecs(stat.io_ticks),
-			   (unsigned int)div_u64(stat.nsecs[STAT_READ] +
-						 stat.nsecs[STAT_WRITE] +
-						 stat.nsecs[STAT_DISCARD] +
-						 stat.nsecs[STAT_FLUSH],
-							NSEC_PER_MSEC),
-			   stat.ios[STAT_DISCARD],
-			   stat.merges[STAT_DISCARD],
-			   stat.sectors[STAT_DISCARD],
-			   (unsigned int)div_u64(stat.nsecs[STAT_DISCARD],
-						 NSEC_PER_MSEC),
-			   stat.ios[STAT_FLUSH],
-			   (unsigned int)div_u64(stat.nsecs[STAT_FLUSH],
-						 NSEC_PER_MSEC)
-			);
+
+		inflight = part_get_stat(hd, &stat);
+		seq_printf(seqf, "%4d %7d %pg ",
+			   MAJOR(hd->bd_dev), MINOR(hd->bd_dev), hd);
+		seq_printf(seqf, part_stat_info(inflight, stat));
 	}
 	rcu_read_unlock();