diff mbox series

[09/14] dm: prep for following changes

Message ID 20220210223832.99412-10-snitzer@redhat.com (mailing list archive)
State Superseded, archived
Delegated to: Mike Snitzer
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.

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel
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;