[dm-devel] [PATCH] dm: account statistics correctly in case of bio split

[Mikulas Patocka]

If a bio was split to multiple targets, only one target's sub-range was
counted. This patch changes it so that all the targets' ranges are
counted.

Note that calls to bio_start_io_acct_remapped and bio_end_io_acct must
match, so we maintain a counter how many times we have called
bio_start_io_acct_remapped. When the io finishes, we call end_io_acct
repeatedly.

Signed-off-by: Mikulas Patocka <mpatocka at redhat.com>

---
 drivers/md/dm-core.h |    2 +-
 drivers/md/dm.c      |   39 ++++++++++++++++++++-------------------
 2 files changed, 21 insertions(+), 20 deletions(-)

Index: linux-dm/drivers/md/dm-core.h
===================================================================
--- linux-dm.orig/drivers/md/dm-core.h	2022-02-11 15:44:33.000000000 +0100
+++ linux-dm/drivers/md/dm-core.h	2022-02-11 15:44:33.000000000 +0100
@@ -230,7 +230,7 @@ struct dm_io {
 	atomic_t io_count;
 	struct bio *orig_bio;
 	unsigned long start_time;
-	int was_accounted;
+	atomic_t n_accounted;
 	spinlock_t lock;
 	struct dm_stats_aux stats_aux;
 	/* last member of dm_target_io is 'struct bio' */
Index: linux-dm/drivers/md/dm.c
===================================================================
--- linux-dm.orig/drivers/md/dm.c	2022-02-11 15:44:33.000000000 +0100
+++ linux-dm/drivers/md/dm.c	2022-02-11 15:52:04.000000000 +0100
@@ -487,27 +487,30 @@ EXPORT_SYMBOL_GPL(dm_start_time_ns_from_
 
 static void start_io_acct(struct dm_io *io, struct bio *bio)
 {
-	struct bio *orig_bio;
-
-	/* Ensure IO accounting is only ever started once */
-	if (xchg(&io->was_accounted, 1) == 1)
-		return;
-
-	orig_bio = io->orig_bio;
+	struct bio *orig_bio = io->orig_bio;
 
 	bio_start_io_acct_remapped(bio, io->start_time,
 				   orig_bio->bi_bdev);
 
-	if (unlikely(dm_stats_used(&io->md->stats)))
+	if (unlikely(dm_stats_used(&io->md->stats))) {
+		if (unlikely(atomic_inc_return(&io->n_accounted) != 1))
+			return;
 		dm_stats_account_io(&io->md->stats, bio_data_dir(orig_bio),
 				    orig_bio->bi_iter.bi_sector, bio_sectors(orig_bio),
 				    false, 0, &io->stats_aux);
+	} else {
+		atomic_inc(&io->n_accounted);
+	}
 }
 
-static void end_io_acct(struct mapped_device *md, struct bio *bio,
+static void end_io_acct(struct mapped_device *md, unsigned n_accounted, struct bio *bio,
 			unsigned long start_time, struct dm_stats_aux *stats_aux)
 {
-	bio_end_io_acct(bio, start_time);
+	if (unlikely(!n_accounted))
+		return;
+	do {
+		bio_end_io_acct(bio, start_time);
+	} while (unlikely(--n_accounted));
 
 	if (unlikely(dm_stats_used(&md->stats)))
 		dm_stats_account_io(&md->stats, bio_data_dir(bio),
@@ -536,7 +539,7 @@ static struct dm_io *alloc_io(struct map
 	spin_lock_init(&io->lock);
 
 	io->start_time = jiffies;
-	io->was_accounted = 0;
+	io->n_accounted = (atomic_t)ATOMIC_INIT(0);
 
 	return io;
 }
@@ -793,7 +796,7 @@ void dm_io_dec_pending(struct dm_io *io,
 	blk_status_t io_error;
 	struct bio *bio;
 	struct mapped_device *md = io->md;
-	bool was_accounted = false;
+	int n_accounted;
 	unsigned long start_time = 0;
 	struct dm_stats_aux stats_aux;
 
@@ -827,14 +830,12 @@ void dm_io_dec_pending(struct dm_io *io,
 		}
 
 		io_error = io->status;
-		if (io->was_accounted) {
-			was_accounted = true;
-			start_time = io->start_time;
-			stats_aux = io->stats_aux;
-		}
+		n_accounted = atomic_read(&io->n_accounted);
+		start_time = io->start_time;
+		stats_aux = io->stats_aux;
+
 		free_io(io);
-		if (was_accounted)
-			end_io_acct(md, bio, start_time, &stats_aux);
+		end_io_acct(md, n_accounted, bio, start_time, &stats_aux);
 
 		/* nudge anyone waiting on suspend queue */
 		if (unlikely(wq_has_sleeper(&md->wait)))