[03/14] dm: refactor dm_split_and_process_bio a bit

Commit Message

Mike Snitzer Feb. 10, 2022, 10:38 p.m. UTC

Remove needless branching. Leaves code to catch malformed
op_is_zone_mgmt bios (they shouldn't have a payload).

Signed-off-by: Mike Snitzer <[e-mail]>
---
 drivers/md/dm.c | 51 ++++++++++++++++++++++++++-------------------------
 1 file changed, 26 insertions(+), 25 deletions(-)

Comments

Christoph Hellwig Feb. 11, 2022, 6:52 a.m. UTC | #1

> +	error = __split_and_process_bio(&ci);
> +	if (ci.sector_count && !error) {

Maybe turn thisin to

	if (error || !ci.sector_count)
		goto out;

	ci.io->orig_bio = bio_split(bio, bio_sectors(bio) - ci.sector_count,
				    GFP_NOIO, &md->queue->bio_split);
	bio_chain(ci.io->orig_bio, bio);
	trace_block_split(ci.io->orig_bio, bio->bi_iter.bi_sector);
	submit_bio_noacct(bio);

and remove another layer of indentation uing the existing label?

Otherwise looks good:

Reviewed-by: Christoph Hellwig <[e-mail]>

Patch

diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index 2f1942b61d48..56734aae718d 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -1375,7 +1375,13 @@  static void init_clone_info(struct clone_info *ci, struct mapped_device *md,
 {
 	ci->map = map;
 	ci->io = alloc_io(md, bio);
+	ci->bio = bio;
 	ci->sector = bio->bi_iter.bi_sector;
+	ci->sector_count = bio_sectors(bio);
+
+	/* Shouldn't happen but sector_count was being set to 0 so... */
+	if (WARN_ON_ONCE(op_is_zone_mgmt(bio_op(bio)) && ci->sector_count))
+		ci->sector_count = 0;
 }
 
 /*
@@ -1392,34 +1398,29 @@  static void dm_split_and_process_bio(struct mapped_device *md,
 	if (bio->bi_opf & REQ_PREFLUSH) {
 		error = __send_empty_flush(&ci);
 		/* dm_io_dec_pending submits any data associated with flush */
-	} else if (op_is_zone_mgmt(bio_op(bio))) {
-		ci.bio = bio;
-		ci.sector_count = 0;
-		error = __split_and_process_bio(&ci);
-	} else {
-		ci.bio = bio;
-		ci.sector_count = bio_sectors(bio);
-		error = __split_and_process_bio(&ci);
-		if (ci.sector_count && !error) {
-			/*
-			 * Remainder must be passed to submit_bio_noacct()
-			 * so that it gets handled *after* bios already submitted
-			 * have been completely processed.
-			 * We take a clone of the original to store in
-			 * ci.io->orig_bio to be used by end_io_acct() and for
-			 * dm_io_dec_pending() to use for completion handling.
-			 */
-			struct bio *b = bio_split(bio, bio_sectors(bio) - ci.sector_count,
-						  GFP_NOIO, &md->queue->bio_split);
-			ci.io->orig_bio = b;
+		goto out;
+	}
 
-			bio_chain(b, bio);
-			trace_block_split(b, bio->bi_iter.bi_sector);
-			submit_bio_noacct(bio);
-		}
+	error = __split_and_process_bio(&ci);
+	if (ci.sector_count && !error) {
+		/*
+		 * Remainder must be passed to submit_bio_noacct()
+		 * so that it gets handled *after* bios already submitted
+		 * have been completely processed.
+		 * We take a clone of the original to store in
+		 * ci.io->orig_bio to be used by end_io_acct() and for
+		 * dm_io_dec_pending() to use for completion handling.
+		 */
+		struct bio *b = bio_split(bio, bio_sectors(bio) - ci.sector_count,
+					  GFP_NOIO, &md->queue->bio_split);
+		ci.io->orig_bio = b;
+
+		bio_chain(b, bio);
+		trace_block_split(b, bio->bi_iter.bi_sector);
+		submit_bio_noacct(bio);
 	}
+out:
 	start_io_acct(ci.io);
-
 	/* drop the extra reference count */
 	dm_io_dec_pending(ci.io, errno_to_blk_status(error));
 }