small block accounting cleanups

small block accounting cleanups

[Christoph Hellwig]

Hi Jens,

find a few small accounting cleanups attached, mostly cleaning up
the somewhat ad-hoc blk-mq vs bio split.

[PATCH 1/4] block: mark blk_account_io_completion static

[Christoph Hellwig]

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/blk-core.c | 2 +-
 block/blk.h      | 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index cf5b2163edfef..fe73e816dae36 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -1361,7 +1361,7 @@ unsigned int blk_rq_err_bytes(const struct request *rq)
 }
 EXPORT_SYMBOL_GPL(blk_rq_err_bytes);
 
-void blk_account_io_completion(struct request *req, unsigned int bytes)
+static void blk_account_io_completion(struct request *req, unsigned int bytes)
 {
 	if (req->part && blk_do_io_stat(req)) {
 		const int sgrp = op_stat_group(req_op(req));
diff --git a/block/blk.h b/block/blk.h
index e5cd350ca3798..a813c573a48d5 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -196,7 +196,6 @@ bool blk_attempt_plug_merge(struct request_queue *q, struct bio *bio,
 		unsigned int nr_segs, struct request **same_queue_rq);
 
 void blk_account_io_start(struct request *req, bool new_io);
-void blk_account_io_completion(struct request *req, unsigned int bytes);
 void blk_account_io_done(struct request *req, u64 now);
 
 /*
-- 
2.26.2

Re: [PATCH 1/4] block: mark blk_account_io_completion static

[Johannes Thumshirn]

Looks good,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>

[PATCH 2/4] block: move the blk-mq calls out of part_in_flight{,_rw}

[Christoph Hellwig]

Don't bother to call part_in_flight / part_in_flight_rw on blk-mq
devices, just call the blk-mq versions directly.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/genhd.c | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/block/genhd.c b/block/genhd.c
index afdb2c3e5b22a..56e0560738c49 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -142,14 +142,9 @@ void part_dec_in_flight(struct request_queue *q, struct hd_struct *part, int rw)
 static unsigned int part_in_flight(struct request_queue *q,
 		struct hd_struct *part)
 {
+	unsigned int inflight = 0;
 	int cpu;
-	unsigned int inflight;
-
-	if (queue_is_mq(q)) {
-		return blk_mq_in_flight(q, part);
-	}
 
-	inflight = 0;
 	for_each_possible_cpu(cpu) {
 		inflight += part_stat_local_read_cpu(part, in_flight[0], cpu) +
 			    part_stat_local_read_cpu(part, in_flight[1], cpu);
@@ -165,11 +160,6 @@ static void part_in_flight_rw(struct request_queue *q, struct hd_struct *part,
 {
 	int cpu;
 
-	if (queue_is_mq(q)) {
-		blk_mq_in_flight_rw(q, part, inflight);
-		return;
-	}
-
 	inflight[0] = 0;
 	inflight[1] = 0;
 	for_each_possible_cpu(cpu) {
@@ -1307,7 +1297,10 @@ ssize_t part_stat_show(struct device *dev,
 	unsigned int inflight;
 
 	part_stat_read_all(p, &stat);
-	inflight = part_in_flight(q, p);
+	if (queue_is_mq(q))
+		inflight = blk_mq_in_flight(q, p);
+	else
+		inflight = part_in_flight(q, p);
 
 	return sprintf(buf,
 		"%8lu %8lu %8llu %8u "
@@ -1346,7 +1339,11 @@ ssize_t part_inflight_show(struct device *dev, struct device_attribute *attr,
 	struct request_queue *q = part_to_disk(p)->queue;
 	unsigned int inflight[2];
 
-	part_in_flight_rw(q, p, inflight);
+	if (queue_is_mq(q))
+		blk_mq_in_flight_rw(q, p, inflight);
+	else
+		part_in_flight_rw(q, p, inflight);
+
 	return sprintf(buf, "%8u %8u\n", inflight[0], inflight[1]);
 }
 
@@ -1601,7 +1598,10 @@ static int diskstats_show(struct seq_file *seqf, void *v)
 	disk_part_iter_init(&piter, gp, DISK_PITER_INCL_EMPTY_PART0);
 	while ((hd = disk_part_iter_next(&piter))) {
 		part_stat_read_all(hd, &stat);
-		inflight = part_in_flight(gp->queue, hd);
+		if (queue_is_mq(gp->queue))
+			inflight = blk_mq_in_flight(gp->queue, hd);
+		else
+			inflight = part_in_flight(gp->queue, hd);
 
 		seq_printf(seqf, "%4d %7d %s "
 			   "%lu %lu %lu %u "
-- 
2.26.2

Re: [PATCH 2/4] block: move the blk-mq calls out of part_in_flight{,_rw}

[Johannes Thumshirn]

Looks good,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>

[PATCH 3/4] block: don't call part_{inc,dec}_in_flight for blk-mq devices

[Christoph Hellwig]

part_inc_in_flight and part_dec_in_flight are no-ops for blk-mq queues,
so remove the calls in purely blk-mq callers.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/blk-core.c  | 21 +++++----------------
 block/blk-merge.c |  2 --
 2 files changed, 5 insertions(+), 18 deletions(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index fe73e816dae36..c22d3148a146e 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -1392,7 +1392,6 @@ void blk_account_io_done(struct request *req, u64 now)
 		update_io_ticks(part, jiffies, true);
 		part_stat_inc(part, ios[sgrp]);
 		part_stat_add(part, nsecs[sgrp], now - req->start_time_ns);
-		part_dec_in_flight(req->q, part, rq_data_dir(req));
 
 		hd_struct_put(part);
 		part_stat_unlock();
@@ -1401,25 +1400,15 @@ void blk_account_io_done(struct request *req, u64 now)
 
 void blk_account_io_start(struct request *rq, bool new_io)
 {
-	struct hd_struct *part;
-	int rw = rq_data_dir(rq);
-
 	if (!blk_do_io_stat(rq))
 		return;
 
 	part_stat_lock();
-
-	if (!new_io) {
-		part = rq->part;
-		part_stat_inc(part, merges[rw]);
-	} else {
-		part = disk_map_sector_rcu(rq->rq_disk, blk_rq_pos(rq));
-		part_inc_in_flight(rq->q, part, rw);
-		rq->part = part;
-	}
-
-	update_io_ticks(part, jiffies, false);
-
+	if (!new_io)
+		part_stat_inc(rq->part, merges[rq_data_dir(rq)]);
+	else
+		rq->part = disk_map_sector_rcu(rq->rq_disk, blk_rq_pos(rq));
+	update_io_ticks(rq->part, jiffies, false);
 	part_stat_unlock();
 }
 
diff --git a/block/blk-merge.c b/block/blk-merge.c
index a04e991b5ded9..7588523106708 100644
--- a/block/blk-merge.c
+++ b/block/blk-merge.c
@@ -670,8 +670,6 @@ static void blk_account_io_merge(struct request *req)
 		part_stat_lock();
 		part = req->part;
 
-		part_dec_in_flight(req->q, part, rq_data_dir(req));
-
 		hd_struct_put(part);
 		part_stat_unlock();
 	}
-- 
2.26.2

Re: [PATCH 3/4] block: don't call part_{inc,dec}_in_flight for blk-mq devices

[Johannes Thumshirn]

Looks good,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>

[PATCH 4/4] block: merge part_{inc,dev}_in_flight into their only callers

[Christoph Hellwig]

part_inc_in_flight and part_dec_in_flight only have one caller each, and
those callers are purely for bio based drivers.  Merge each function into
the only caller, and remove the superflous blk-mq checks.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/bio.c   | 10 ++++++++--
 block/blk.h   |  4 ----
 block/genhd.c | 20 --------------------
 3 files changed, 8 insertions(+), 26 deletions(-)

diff --git a/block/bio.c b/block/bio.c
index e4c46e2bd5ba5..1d93d74bf7f30 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -1390,13 +1390,16 @@ void generic_start_io_acct(struct request_queue *q, int op,
 			   unsigned long sectors, struct hd_struct *part)
 {
 	const int sgrp = op_stat_group(op);
+	int rw = op_is_write(op);
 
 	part_stat_lock();
 
 	update_io_ticks(part, jiffies, false);
 	part_stat_inc(part, ios[sgrp]);
 	part_stat_add(part, sectors[sgrp], sectors);
-	part_inc_in_flight(q, part, op_is_write(op));
+	part_stat_local_inc(part, in_flight[rw]);
+	if (part->partno)
+		part_stat_local_inc(&part_to_disk(part)->part0, in_flight[rw]);
 
 	part_stat_unlock();
 }
@@ -1408,12 +1411,15 @@ void generic_end_io_acct(struct request_queue *q, int req_op,
 	unsigned long now = jiffies;
 	unsigned long duration = now - start_time;
 	const int sgrp = op_stat_group(req_op);
+	int rw = op_is_write(req_op);
 
 	part_stat_lock();
 
 	update_io_ticks(part, now, true);
 	part_stat_add(part, nsecs[sgrp], jiffies_to_nsecs(duration));
-	part_dec_in_flight(q, part, op_is_write(req_op));
+	part_stat_local_dec(part, in_flight[rw]);
+	if (part->partno)
+		part_stat_local_dec(&part_to_disk(part)->part0, in_flight[rw]);
 
 	part_stat_unlock();
 }
diff --git a/block/blk.h b/block/blk.h
index a813c573a48d5..7fe3d6ed22356 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -354,10 +354,6 @@ void blk_queue_free_zone_bitmaps(struct request_queue *q);
 static inline void blk_queue_free_zone_bitmaps(struct request_queue *q) {}
 #endif
 
-void part_dec_in_flight(struct request_queue *q, struct hd_struct *part,
-			int rw);
-void part_inc_in_flight(struct request_queue *q, struct hd_struct *part,
-			int rw);
 void update_io_ticks(struct hd_struct *part, unsigned long now, bool end);
 struct hd_struct *disk_map_sector_rcu(struct gendisk *disk, sector_t sector);
 
diff --git a/block/genhd.c b/block/genhd.c
index 56e0560738c49..094ed90964964 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -119,26 +119,6 @@ static void part_stat_read_all(struct hd_struct *part, struct disk_stats *stat)
 }
 #endif /* CONFIG_SMP */
 
-void part_inc_in_flight(struct request_queue *q, struct hd_struct *part, int rw)
-{
-	if (queue_is_mq(q))
-		return;
-
-	part_stat_local_inc(part, in_flight[rw]);
-	if (part->partno)
-		part_stat_local_inc(&part_to_disk(part)->part0, in_flight[rw]);
-}
-
-void part_dec_in_flight(struct request_queue *q, struct hd_struct *part, int rw)
-{
-	if (queue_is_mq(q))
-		return;
-
-	part_stat_local_dec(part, in_flight[rw]);
-	if (part->partno)
-		part_stat_local_dec(&part_to_disk(part)->part0, in_flight[rw]);
-}
-
 static unsigned int part_in_flight(struct request_queue *q,
 		struct hd_struct *part)
 {
-- 
2.26.2

Re: [PATCH 4/4] block: merge part_{inc,dev}_in_flight into their only callers

[Johannes Thumshirn]

Looks good,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>

Re: small block accounting cleanups

[Jens Axboe]

On 5/13/20 4:49 AM, Christoph Hellwig wrote:
> Hi Jens,
> 
> find a few small accounting cleanups attached, mostly cleaning up
> the somewhat ad-hoc blk-mq vs bio split.

Applied, thanks.

-- 
Jens Axboe