[Devel] [PATCH vz10 24/24] dm-ploop: fail BAT-writeback waiters instead of retrying forever on I/O error

[Konstantin Khorenko]

ploop_advance_local_after_bat_wb() unconditionally re-dispatched
md->wait_llist (pios postponed by ploop_delay_if_md_busy() while this BAT
page was already dirty/under writeback) for retry, regardless of whether the
writeback that just completed succeeded or failed - the exact same class of
bug just fixed for dm-qcow2 in commit db5b5d2d30d8 ("dm-qcow2: fail waiters
instead of retrying forever after metadata I/O error"), found by auditing
dm-ploop for the analogous pattern after that live repro.

On a persistently failing backing device, a retried pio re-triggers the same
BAT-page write, fails the same way, and lands back on md->wait_llist again -
an unbounded retry storm that leaves the pio's originating I/O blocked in an
uninterruptible wait forever (unkillable D state), wedging `ploop
umount`/`dmsetup remove` and eventually `reboot` on the stuck task.

A failed BAT writeback means the page's on-disk state was not persisted, so
retrying it as if nothing happened is not safe. Thread the real bi_status
through (instead of a plain success/failure bool) and fail md->wait_llist
waiters with it via ploop_pio_endio() on failure, exactly like
piwb->llready_data_pios (the data bios belonging to this same BAT update) are
already correctly handled a few lines below. The success path is unchanged.

Fixes: a9cd5e6dc646 ("dm-ploop: Add ploop target driver")
Feature: dm-ploop: ploop target driver
https://virtuozzo.atlassian.net/browse/VSTOR-137234
Signed-off-by: Konstantin Khorenko <khorenko at virtuozzo.com>
---
 drivers/md/dm-ploop-map.c | 28 ++++++++++++++++++++++++----
 1 file changed, 24 insertions(+), 4 deletions(-)

diff --git a/drivers/md/dm-ploop-map.c b/drivers/md/dm-ploop-map.c
index 48466435dd19..af6b95241564 100644
--- a/drivers/md/dm-ploop-map.c
+++ b/drivers/md/dm-ploop-map.c
@@ -810,9 +810,10 @@ static void ploop_complete_cow(struct ploop_cow *cow, blk_status_t bi_status)
  */
 static void ploop_advance_local_after_bat_wb(struct ploop *ploop,
 					     struct ploop_index_wb *piwb,
-					     bool success)
+					     blk_status_t bi_status)
 {
 	struct md_page *md = piwb->md;
+	bool success = !bi_status;
 	u32 i, last, *bat_entries;
 	map_index_t *dst_clu, off;
 	unsigned long flags;
@@ -890,7 +891,26 @@ static void ploop_advance_local_after_bat_wb(struct ploop *ploop,
 		llist_for_each_safe(pos, t, wait_llist_pending) {
 			pio = list_entry((struct list_head *)pos, typeof(*pio), list);
 			INIT_LIST_HEAD(&pio->list);
-			list_add(&pio->list, &list);
+			/*
+			 * These pios arrived while this BAT page's writeback
+			 * was already in flight and were postponed
+			 * (ploop_delay_if_md_busy()) to retry their own BAT
+			 * update once it finished. If it failed, this page's
+			 * on-disk state was not persisted, so retrying is not
+			 * safe: on a persistently failing backing device the
+			 * retried pio would immediately hit the same
+			 * disk-level failure and land back on this list
+			 * again - an unbounded retry storm that leaves the
+			 * pio's (uninterruptible) originator blocked forever.
+			 * Fail them, mirroring how piwb->llready_data_pios
+			 * is already handled above.
+			 */
+			if (success) {
+				list_add(&pio->list, &list);
+			} else {
+				pio->bi_status = bi_status;
+				ploop_pio_endio(pio);
+			}
 		}
 	}
 
@@ -956,13 +976,13 @@ static void ploop_bat_write_finish(struct pio *pio, void *piwb_ptr,
 		 * a bio, subsequent read wants to see written data
 		 * (ploop_map() wants to see not zero bat_entries[.]).
 		 */
-		ploop_advance_local_after_bat_wb(ploop, piwb, true);
+		ploop_advance_local_after_bat_wb(ploop, piwb, bi_status);
 	} else {
 		/*
 		 * Index wb failed. Mark clusters as unallocated again.
 		 * piwb->count is zero, so all data writers compeleted.
 		 */
-		ploop_advance_local_after_bat_wb(ploop, piwb, false);
+		ploop_advance_local_after_bat_wb(ploop, piwb, bi_status);
 	}
 
 	spin_lock_irqsave(&piwb->lock, flags);
-- 
2.47.1