Re: [PATCH] block: don't use submit_bio_noacct_nocheck in blk_zone_wplug_bio_work

On 6/19/25 02:13, Bart Van Assche wrote:

On 6/17/25 10:56 PM, Damien Le Moal wrote:

quoted

Can you check exactly the path that is being followed ? (your

 > backtrace does not seem to have everything)

Hmm ... it is not clear to me why this information is required? My 
understanding is that the root cause is the same as for the deadlock
fixed by Christoph:
1. A bio is queued onto zwplug->bio_list. Before this happens, the
    queue reference count is increased by one.
2. A value is written into a block device sysfs attribute and queue
    freezing starts. The queue freezing code waits for completion of
    all bios on zwplug->bio_list because the reference count owned by
    these bios is only released when these bios complete.
3. blk_zone_wplug_bio_work() dequeues a bio from zwplug->bio_list,
    calls dm_submit_bio() through a function pointer, dm_submit_bio()
    calls submit_bio_noacct() indirectly and submit_bio_noacct() calls
    bio_queue_enter() indirectly. bio_queue_enter() sees that queue
    freezing has started and waits until the queue is unfrozen.
4. A deadlock occurs because (2) and (3) wait for each other
    indefinitely.

Then we need to split DM BIOs immediately on submission, always.
So something like this totally untested patch should solve the issue.
Care to test ?

diff --git a/block/blk-zoned.c b/block/blk-zoned.c
index 55e64ca869d7..12aa56352176 100644
--- a/block/blk-zoned.c
+++ b/block/blk-zoned.c

@@ -1129,6 +1129,44 @@ static void

blk_zone_wplug_handle_native_zone_append(struct bio *bio)
        disk_put_zone_wplug(zwplug);
 }

+/**
+ * bio_needs_zone_write_plugging - Check if a BIO needs to be handled with zone
+ *                                write plugging
+ * @bio: The BIO being submitted
+ *
+ * Return true whenever @bio execution needs to be handled through zone
+ * write plugging. Return false otherwise.
+ */
+bool bio_needs_zone_write_plugging(struct bio *bio)
+{
+       if (!bdev_is_zoned(bio->bi_bdev))
+               return false;
+
+       /* Already handled ? */
+       if (bio_flagged(bio, BIO_ZONE_WRITE_PLUGGING))
+               return false;
+
+       /* Ignore empty flush */
+       if (op_is_flush(bio->bi_opf) && !bio_sectors(bio))
+               return false;
+
+       /*
+        * Regular writes and write zeroes need to be handled through zone
+        * write plugging. Zone append operations only need zone write plugging
+        * if they are emulated.
+        */
+       switch (bio_op(bio)) {
+       case REQ_OP_ZONE_APPEND:
+               return bdev_emulates_zone_append(bio->bi_bdev);
+       case REQ_OP_WRITE:
+       case REQ_OP_WRITE_ZEROES:
+               return true;
+       default:
+               return false;
+       }
+}
+EXPORT_SYMBOL_GPL(bio_needs_zone_write_plugging);
+
 /**
  * blk_zone_plug_bio - Handle a zone write BIO with zone write plugging
  * @bio: The BIO being submitted

diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index 21920991ab28..5c2d98a5fdf8 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c

@@ -1771,16 +1771,6 @@ static void init_clone_info(struct clone_info *ci, struct

dm_io *io,
 }

 #ifdef CONFIG_BLK_DEV_ZONED
-static inline bool dm_zone_bio_needs_split(struct mapped_device *md,
-                                          struct bio *bio)
-{
-       /*
-        * For mapped device that need zone append emulation, we must
-        * split any large BIO that straddles zone boundaries.
-        */
-       return dm_emulate_zone_append(md) && bio_straddles_zones(bio) &&
-               !bio_flagged(bio, BIO_ZONE_WRITE_PLUGGING);
-}
 static inline bool dm_zone_plug_bio(struct mapped_device *md, struct bio *bio)
 {
        return dm_emulate_zone_append(md) && blk_zone_plug_bio(bio, 0);

@@ -1899,11 +1889,6 @@ static blk_status_t __send_zone_reset_all(struct

clone_info *ci)
 }

 #else
-static inline bool dm_zone_bio_needs_split(struct mapped_device *md,
-                                          struct bio *bio)
-{
-       return false;
-}
 static inline bool dm_zone_plug_bio(struct mapped_device *md, struct bio *bio)
 {
        return false;

@@ -1928,8 +1913,7 @@ static void dm_split_and_process_bio(struct mapped_device *md,
        is_abnormal = is_abnormal_io(bio);
        if (static_branch_unlikely(&zoned_enabled)) {
                /* Special case REQ_OP_ZONE_RESET_ALL as it cannot be split. */
-               need_split = (bio_op(bio) != REQ_OP_ZONE_RESET_ALL) &&
-                       (is_abnormal || dm_zone_bio_needs_split(md, bio));
+               need_split = is_abnormal || bio_needs_zone_write_plugging(bio);
        } else {
                need_split = is_abnormal;
        }

diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index c2b3ddea8b6d..c3cab300e4c2 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h

@@ -984,6 +984,8 @@ static inline bool bio_straddles_zones(struct bio *bio)
                disk_zone_no(bio->bi_bdev->bd_disk, bio_end_sector(bio) - 1);
 }

+bool bio_needs_zone_write_plugging(struct bio *bio);
+
 /*
  * Return how much within the boundary is left to be used for I/O at a given
  * offset.

-- 
Damien Le Moal
Western Digital Research