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 <[email protected]>
---
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
_______________________________________________
Devel mailing list
[email protected]
https://lists.openvz.org/mailman/listinfo/devel
