diff mbox series

[09/14] dm: prep for following changes

Message ID 20220210223832.99412-10-snitzer@redhat.com (mailing list archive)
State New, archived
Headers show
Series dm: improve bio-based IO accounting | expand

Commit Message

Mike Snitzer Feb. 10, 2022, 10:38 p.m. UTC
Rename dm_io struct's 'endio_lock' member to 'lock' to reuse spinlock
to protect new member used to flag if IO accounting has been started.

Also move kicking of the suspend queue out to dm_io_dec_pending (the
only caller) since end_io_acct will soon only be called if IO
accounting was started.

Some comment tweaks and removal of local variables. No functional
change.

Signed-off-by: Mike Snitzer <snitzer@redhat.com>
---
 drivers/md/dm-core.h |  2 +-
 drivers/md/dm.c      | 32 ++++++++++++++------------------
 2 files changed, 15 insertions(+), 19 deletions(-)

Comments

Christoph Hellwig Feb. 11, 2022, 6:57 a.m. UTC | #1
On Thu, Feb 10, 2022 at 05:38:27PM -0500, Mike Snitzer wrote:
> Rename dm_io struct's 'endio_lock' member to 'lock' to reuse spinlock
> to protect new member used to flag if IO accounting has been started.
> 
> Also move kicking of the suspend queue out to dm_io_dec_pending (the
> only caller) since end_io_acct will soon only be called if IO
> accounting was started.
> 
> Some comment tweaks and removal of local variables. No functional
> change.

The changes looks fine to me, but for the reader it would be much nicer
if the lock rename, kicking changes and local variable tweaks were split
into separate patches.
diff mbox series

Patch

diff --git a/drivers/md/dm-core.h b/drivers/md/dm-core.h
index f40be01cca81..8dd196aec130 100644
--- a/drivers/md/dm-core.h
+++ b/drivers/md/dm-core.h
@@ -230,7 +230,7 @@  struct dm_io {
 	atomic_t io_count;
 	struct bio *orig_bio;
 	unsigned long start_time;
-	spinlock_t endio_lock;
+	spinlock_t lock;
 	struct dm_stats_aux stats_aux;
 	/* last member of dm_target_io is 'struct bio' */
 	struct dm_target_io tio;
diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index 3bd872b0e891..8c0e96b8e1a5 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -487,12 +487,12 @@  EXPORT_SYMBOL_GPL(dm_start_time_ns_from_clone);
 
 static void start_io_acct(struct dm_io *io)
 {
-	struct mapped_device *md = io->md;
 	struct bio *bio = io->orig_bio;
 
 	bio_start_io_acct_time(bio, io->start_time);
-	if (unlikely(dm_stats_used(&md->stats)))
-		dm_stats_account_io(&md->stats, bio_data_dir(bio),
+
+	if (unlikely(dm_stats_used(&io->md->stats)))
+		dm_stats_account_io(&io->md->stats, bio_data_dir(bio),
 				    bio->bi_iter.bi_sector, bio_sectors(bio),
 				    false, 0, &io->stats_aux);
 }
@@ -500,18 +500,12 @@  static void start_io_acct(struct dm_io *io)
 static void end_io_acct(struct mapped_device *md, struct bio *bio,
 			unsigned long start_time, struct dm_stats_aux *stats_aux)
 {
-	unsigned long duration = jiffies - start_time;
-
 	bio_end_io_acct(bio, start_time);
 
 	if (unlikely(dm_stats_used(&md->stats)))
 		dm_stats_account_io(&md->stats, bio_data_dir(bio),
 				    bio->bi_iter.bi_sector, bio_sectors(bio),
-				    true, duration, stats_aux);
-
-	/* nudge anyone waiting on suspend queue */
-	if (unlikely(wq_has_sleeper(&md->wait)))
-		wake_up(&md->wait);
+				    true, jiffies - start_time, stats_aux);
 }
 
 static struct dm_io *alloc_io(struct mapped_device *md, struct bio *bio)
@@ -532,7 +526,7 @@  static struct dm_io *alloc_io(struct mapped_device *md, struct bio *bio)
 	atomic_set(&io->io_count, 1);
 	io->orig_bio = bio;
 	io->md = md;
-	spin_lock_init(&io->endio_lock);
+	spin_lock_init(&io->lock);
 
 	io->start_time = jiffies;
 
@@ -796,10 +790,10 @@  void dm_io_dec_pending(struct dm_io *io, blk_status_t error)
 
 	/* Push-back supersedes any I/O errors */
 	if (unlikely(error)) {
-		spin_lock_irqsave(&io->endio_lock, flags);
+		spin_lock_irqsave(&io->lock, flags);
 		if (!(io->status == BLK_STS_DM_REQUEUE && __noflush_suspending(md)))
 			io->status = error;
-		spin_unlock_irqrestore(&io->endio_lock, flags);
+		spin_unlock_irqrestore(&io->lock, flags);
 	}
 
 	if (atomic_dec_and_test(&io->io_count)) {
@@ -829,6 +823,10 @@  void dm_io_dec_pending(struct dm_io *io, blk_status_t error)
 		free_io(io);
 		end_io_acct(md, bio, start_time, &stats_aux);
 
+		/* nudge anyone waiting on suspend queue */
+		if (unlikely(wq_has_sleeper(&md->wait)))
+			wake_up(&md->wait);
+
 		if (io_error == BLK_STS_DM_REQUEUE)
 			return;
 
@@ -1127,9 +1125,7 @@  static void __map_bio(struct bio *clone)
 	clone->bi_end_io = clone_endio;
 
 	/*
-	 * Map the clone.  If r == 0 we don't need to do
-	 * anything, the target has assumed ownership of
-	 * this io.
+	 * Map the clone.
 	 */
 	dm_io_inc_pending(io);
 	tio->old_sector = clone->bi_iter.bi_sector;
@@ -1154,6 +1150,7 @@  static void __map_bio(struct bio *clone)
 
 	switch (r) {
 	case DM_MAPIO_SUBMITTED:
+		/* target has assumed ownership of this io */
 		break;
 	case DM_MAPIO_REMAPPED:
 		/* the bio has been remapped so dispatch it */
@@ -1301,10 +1298,9 @@  static bool is_abnormal_io(struct bio *bio)
 static bool __process_abnormal_io(struct clone_info *ci, struct dm_target *ti,
 				  int *result)
 {
-	struct bio *bio = ci->bio;
 	unsigned num_bios = 0;
 
-	switch (bio_op(bio)) {
+	switch (bio_op(ci->bio)) {
 	case REQ_OP_DISCARD:
 		num_bios = ti->num_discard_bios;
 		break;