[dm-devel] [PATCH] dm: remove unnecessary bio_list check when submitting split bio

Jeffle Xu jefflexu at linux.alibaba.com
Wed Nov 4 03:57:56 UTC 2020


The depth-first splitting is introduced in commit 18a25da84354 ("dm:
ensure bio submission follows a depth-first tree walk"), which is used
to fix the potential deadlock in case of the misordering handling of
bios caused by bio_list. There're two paths submitting split bios,
dm_wq_work() from worker thread and submit_bio() from application. Back
upon that time, dm_wq_work() thread calls __split_and_process_bio()
directly and thus will not trigger this issue since bio_list doesn't
exist here. So this issue will only be triggered from application
calling submit_bio(), and the fix has to check if current->bio_list is
non-NULL to distinguish this case.

However since commit 0c2915b8c6db1 ("dm: fix missing imposition of
queue_limits from dm_wq_work() thread"), dm_wq_work() thread calls
submit_bio_noacct() and thus also uses bio_list. Since then all entries
into __split_and_process_bio() are under protection of bio_list, and
thus the checking of current->bio_list when determinning if the
depth-first principle should be used, seems kind of nonsense. After all
the checking always succeeds now.

But the check should be there from the perspective of design, to make
it a complete fix. So all the details and notes are explained thoroughly
in comments.

Signed-off-by: Jeffle Xu <jefflexu at linux.alibaba.com>
---
 drivers/md/dm.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index c18fc2548518..56f65a5b90f4 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -1589,7 +1589,15 @@ static blk_qc_t __split_and_process_bio(struct mapped_device *md,
 		ci.sector_count = bio_sectors(bio);
 		while (ci.sector_count && !error) {
 			error = __split_and_process_non_flush(&ci);
-			if (current->bio_list && ci.sector_count && !error) {
+			/*
+			 * Depth-first splitting to fix the potential deadlock caused by
+			 * reordering of handling bios because of bio_list. We should enable
+			 * this feature only when bio_list enabled, but the check of
+			 * current->bio_list is not needed any more since all entries into
+			 * __split_and_process_bio (dm_wq_work() from thread and submit_bio()
+			 * from application) are under bio_list now.
+			 */
+			if (ci.sector_count && !error) {
 				/*
 				 * Remainder must be passed to submit_bio_noacct()
 				 * so that it gets handled *after* bios already submitted
-- 
2.27.0




More information about the dm-devel mailing list