FAILED: patch "[PATCH] dm: fix bio splitting and its bio completion order for" failed to apply to 5.4-stable tree

PATCH In review [email protected] 2 participants 6 messages Sep 28, 2020

Subsystems (2)

Stable branch Device mapper

lore

Share & tools

lore

[email protected]

author maintainer

2020-09-28 13:56 UTC #0 lore raw

The patch below does not apply to the 5.4-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

From ee1dfad5325ff1cfb2239e564cd411b3bfe8667a Mon Sep 17 00:00:00 2001
From: Mike Snitzer <[email protected]>
Date: Mon, 14 Sep 2020 13:04:19 -0400
Subject: [PATCH] dm: fix bio splitting and its bio completion order for
regular IO

dm_queue_split() is removed because __split_and_process_bio() _must_ handle splitting bios to ensure proper bio submission and completion ordering as a bio is split.

Otherwise, multiple recursive calls to ->submit_bio will cause multiple split bios to be allocated from the same ->bio_split mempool at the same time. This would result in deadlock in low memory conditions because no progress could be made (only one bio is available in ->bio_split mempool).

This fix has been verified to still fix the loss of performance, due to excess splitting, that commit 120c9257f5f1 provided.

Fixes 120c9257f5f1 ("Revert "dm: always call blk_queue_split() in dm_process_bio()"")

Cc [email protected] # 5.0+, requires custom backport due to 5.9 changes

Reported-by Ming Lei <[email protected]>

Signed-off-by Mike Snitzer <[email protected]>

drivers/md/dm.c +2 ~~-21~~

index 4a40df8af7d3..d948cd522431 100644
@@ -1724,23 +1724,6 @@ static blk_qc_t __process_bio(struct mapped_device *md, struct dm_table *map,
 	return ret;
 }
 
-static void dm_queue_split(struct mapped_device *md, struct dm_target *ti, struct bio **bio)
-{
-	unsigned len, sector_count;
-
-	sector_count = bio_sectors(*bio);
-	len = min_t(sector_t, max_io_len((*bio)->bi_iter.bi_sector, ti), sector_count);
-
-	if (sector_count > len) {
-		struct bio *split = bio_split(*bio, len, GFP_NOIO, &md->queue->bio_split);
-
-		bio_chain(split, *bio);
-		trace_block_split(md->queue, split, (*bio)->bi_iter.bi_sector);
-		submit_bio_noacct(*bio);
-		*bio = split;
-	}
-}
-
 static blk_qc_t dm_process_bio(struct mapped_device *md,
 			       struct dm_table *map, struct bio *bio)
 {

@@ -1768,14 +1751,12 @@ static blk_qc_t dm_process_bio(struct mapped_device *md,
 	if (current->bio_list) {
 		if (is_abnormal_io(bio))
 			blk_queue_split(&bio);
-		else
-			dm_queue_split(md, ti, &bio);
+		/* regular IO is split by __split_and_process_bio */
 	}
 
 	if (dm_get_md_type(md) == DM_TYPE_NVME_BIO_BASED)
 		return __process_bio(md, map, bio, ti);
-	else
-		return __split_and_process_bio(md, map, bio);
+	return __split_and_process_bio(md, map, bio);
 }
 
 static blk_qc_t dm_submit_bio(struct bio *bio)

Mike Snitzer

2020-09-28 15:29 UTC #1 lore raw

PATCH 1/1 dm: fix bio splitting and its bio completion order for regular IO

This backport applies (with mild offset) to both v5.4.67 and v5.8.11:

dm_queue_split() is removed because __split_and_process_bio() _must_ handle splitting bios to ensure proper bio submission and completion ordering as a bio is split.

This fix has been verified to still fix the loss of performance, due to excess splitting, that commit 120c9257f5f1 provided.

Fixes 120c9257f5f1 ("Revert "dm: always call blk_queue_split() in dm_process_bio()"")

Cc [email protected] # 5.0+, requires custom backport due to 5.9 changes

Reported-by Ming Lei <[email protected]>

Signed-off-by Mike Snitzer <[email protected]>

drivers/md/dm.c +2 ~~-21~~

@@ -1727,23 +1727,6 @@
 	return ret;
 }
 
-static void dm_queue_split(struct mapped_device *md, struct dm_target *ti, struct bio **bio)
-{
-	unsigned len, sector_count;
-
-	sector_count = bio_sectors(*bio);
-	len = min_t(sector_t, max_io_len((*bio)->bi_iter.bi_sector, ti), sector_count);
-
-	if (sector_count > len) {
-		struct bio *split = bio_split(*bio, len, GFP_NOIO, &md->queue->bio_split);
-
-		bio_chain(split, *bio);
-		trace_block_split(md->queue, split, (*bio)->bi_iter.bi_sector);
-		generic_make_request(*bio);
-		*bio = split;
-	}
-}
-
 static blk_qc_t dm_process_bio(struct mapped_device *md,
 			       struct dm_table *map, struct bio *bio)
 {

@@ -1773,14 +1756,12 @@
 	if (current->bio_list) {
 		if (is_abnormal_io(bio))
 			blk_queue_split(md->queue, &bio);
-		else
-			dm_queue_split(md, ti, &bio);
+		/* regular IO is split by __split_and_process_bio */
 	}
 
 	if (dm_get_md_type(md) == DM_TYPE_NVME_BIO_BASED)
 		return __process_bio(md, map, bio, ti);
-	else
-		return __split_and_process_bio(md, map, bio);
+	return __split_and_process_bio(md, map, bio);
 }
 
 static blk_qc_t dm_make_request(struct request_queue *q, struct bio *bio)

Greg KH

author maintainer

2020-09-28 17:31 UTC #2 lore raw

Re: [PATCH 5.4, 5.8] dm: fix bio splitting and its bio completion order for regular IO

Quoted 17 lines · Mike Snitzer

This backport applies (with mild offset) to both v5.4.67 and v5.8.11:
Quoted 1 line
From ee1dfad5325ff1cfb2239e564cd411b3bfe8667a Mon Sep 17 00:00:00 2001
From: Mike Snitzer <[email protected]>
Date: Mon, 14 Sep 2020 13:04:19 -0400
Subject: [PATCH] dm: fix bio splitting and its bio completion order for regular IO
dm_queue_split() is removed because __split_and_process_bio() _must_ handle splitting bios to ensure proper bio submission and completion ordering as a bio is split.
Otherwise, multiple recursive calls to ->submit_bio will cause multiple split bios to be allocated from the same ->bio_split mempool at the same time. This would result in deadlock in low memory conditions because no progress could be made (only one bio is available in ->bio_split mempool).

Quoted 7 lines

This fix has been verified to still fix the loss of performance, due to excess splitting, that commit 120c9257f5f1 provided.
Fixes 120c9257f5f1 ("Revert "dm: always call blk_queue_split() in dm_process_bio()"")
Cc [email protected] # 5.0+, requires custom backport due to 5.9 changes
Reported-by Ming Lei <[email protected]>
Signed-off-by Mike Snitzer <[email protected]>

What is the git id of this patch in Linus's tree?

thanks,

greg k-h

Greg KH

author maintainer

2020-09-28 17:52 UTC #3 lore raw

Re: [PATCH 5.4, 5.8] dm: fix bio splitting and its bio completion order for regular IO

Quoted 21 lines · Greg KH

Quoted 20 lines · Mike Snitzer
This backport applies (with mild offset) to both v5.4.67 and v5.8.11:
Quoted 1 line
From ee1dfad5325ff1cfb2239e564cd411b3bfe8667a Mon Sep 17 00:00:00 2001
From: Mike Snitzer <[email protected]>
Date: Mon, 14 Sep 2020 13:04:19 -0400
Subject: [PATCH] dm: fix bio splitting and its bio completion order for regular IO
dm_queue_split() is removed because __split_and_process_bio() _must_ handle splitting bios to ensure proper bio submission and completion ordering as a bio is split.
Otherwise, multiple recursive calls to ->submit_bio will cause multiple split bios to be allocated from the same ->bio_split mempool at the same time. This would result in deadlock in low memory conditions because no progress could be made (only one bio is available in ->bio_split mempool).
This fix has been verified to still fix the loss of performance, due to excess splitting, that commit 120c9257f5f1 provided.

Quoted 6 lines

Quoted 4 lines
Fixes 120c9257f5f1 ("Revert "dm: always call blk_queue_split() in dm_process_bio()"")
Cc [email protected] # 5.0+, requires custom backport due to 5.9 changes
Reported-by Ming Lei <[email protected]>
Signed-off-by Mike Snitzer <[email protected]>
What is the git id of this patch in Linus's tree?

I dug it up:
ee1dfad5325f ("dm: fix bio splitting and its bio completion order for regular IO")

Mike Snitzer

2020-09-28 18:03 UTC #4 lore raw

Re: [PATCH 5.4, 5.8] dm: fix bio splitting and its bio completion order for regular IO

Quoted 27 lines · Greg KH

Quoted 26 lines · Greg KH
Quoted 24 lines · Mike Snitzer
This backport applies (with mild offset) to both v5.4.67 and v5.8.11:
Quoted 1 line
From ee1dfad5325ff1cfb2239e564cd411b3bfe8667a Mon Sep 17 00:00:00 2001
From: Mike Snitzer <[email protected]>
Date: Mon, 14 Sep 2020 13:04:19 -0400
Subject: [PATCH] dm: fix bio splitting and its bio completion order for regular IO
dm_queue_split() is removed because __split_and_process_bio() _must_ handle splitting bios to ensure proper bio submission and completion ordering as a bio is split.
Otherwise, multiple recursive calls to ->submit_bio will cause multiple split bios to be allocated from the same ->bio_split mempool at the same time. This would result in deadlock in low memory conditions because no progress could be made (only one bio is available in ->bio_split mempool).
This fix has been verified to still fix the loss of performance, due to excess splitting, that commit 120c9257f5f1 provided.
Fixes 120c9257f5f1 ("Revert "dm: always call blk_queue_split() in dm_process_bio()"")
Cc [email protected] # 5.0+, requires custom backport due to 5.9 changes
Reported-by Ming Lei <[email protected]>
Signed-off-by Mike Snitzer <[email protected]>

Quoted 4 lines

Quoted 1 line
What is the git id of this patch in Linus's tree?
I dug it up:
ee1dfad5325f ("dm: fix bio splitting and its bio completion order for regular IO")

I thought this line was adequate in my original email:
"From ee1dfad5325ff1cfb2239e564cd411b3bfe8667a Mon Sep 17 00:00:00 2001"

Maybe it was too subtle? For future knowledge, how would you like to see the git id of Linus's tree referenced?

Thanks,
Mike

Greg KH

author maintainer

2020-09-28 18:43 UTC #5 lore raw

Re: [PATCH 5.4, 5.8] dm: fix bio splitting and its bio completion order for regular IO

Quoted 32 lines · Mike Snitzer

Quoted 29 lines · Greg KH
Quoted 27 lines · Greg KH
Quoted 17 lines · Mike Snitzer
This backport applies (with mild offset) to both v5.4.67 and v5.8.11:
Quoted 1 line
From ee1dfad5325ff1cfb2239e564cd411b3bfe8667a Mon Sep 17 00:00:00 2001
From: Mike Snitzer <[email protected]>
Date: Mon, 14 Sep 2020 13:04:19 -0400
Subject: [PATCH] dm: fix bio splitting and its bio completion order for regular IO
dm_queue_split() is removed because __split_and_process_bio() _must_ handle splitting bios to ensure proper bio submission and completion ordering as a bio is split.
Otherwise, multiple recursive calls to ->submit_bio will cause multiple split bios to be allocated from the same ->bio_split mempool at the same time. This would result in deadlock in low memory conditions because no progress could be made (only one bio is available in ->bio_split mempool).
Quoted 7 lines
This fix has been verified to still fix the loss of performance, due to excess splitting, that commit 120c9257f5f1 provided.
Fixes 120c9257f5f1 ("Revert "dm: always call blk_queue_split() in dm_process_bio()"")
Cc [email protected] # 5.0+, requires custom backport due to 5.9 changes
Reported-by Ming Lei <[email protected]>
Signed-off-by Mike Snitzer <[email protected]>
What is the git id of this patch in Linus's tree?

Quoted 8 lines

Quoted 2 lines
I dug it up:
ee1dfad5325f ("dm: fix bio splitting and its bio completion order for regular IO")
I thought this line was adequate in my original email:
"From ee1dfad5325ff1cfb2239e564cd411b3bfe8667a Mon Sep 17 00:00:00 2001"
Maybe it was too subtle? For future knowledge, how would you like to see the git id of Linus's tree referenced?

Oh, that was subtle, sorry, missed it :)

Better ways can be in that sentence that said what trees to backport it to, right? Explicit is good, especially when having to deal with as many patches as we deal with, right?

thanks,

greg k-h