[Devel] [PATCH hci-8.1 v2 1/4] dm-ploop: fix pio submission racing with the suspend and resume of a cmd

[Denis V. Lunev]

From: Denis V. Lunev <den at openvz.org>

ploop_submit_embedded_pio() tests stop_submitting_pios and then takes a
ref on the current inflight_bios_ref index without deferred_lock. A cmd
sets the flag and flips the index under that lock and then waits only
for the refs of the old index, so:

- a pio that passed the check before the flag was set takes its ref on the
  new index or on the already killed old one. The cmd does not wait for
  it, and it runs while a merge, notify_merged, resize or delta index
  update rewrites the BAT levels and deltas[];

- a pio that saw the flag set is added to llsuspended_pios after resume
  has cleared the flag and drained the list. Nothing drains it until
  the next cmd, the request never completes and a dm suspend of the
  device hangs.

The submission path dropped deferred_lock on purpose when the lists
became lockless, so keep it lockless. Test the flag and take the ref in
one RCU read-side section, and wait for a grace period in the cmd
between setting the flag and flipping the index: a submitter that missed
the flag then holds its ref on the index the cmd waits for. Only a
submitter that saw the flag takes deferred_lock and tests it again
before parking the pio, and resume clears the flag under that lock
before it drains the list.

Fixes: b90109c12b1c ("dm-ploop: convert the rest of the lists to use llist variant")
Reported-by: Konstantin Khorenko <khorenko at virtuozzo.com>
https://virtuozzo.atlassian.net/browse/VSTOR-152985
Feature: dm-ploop: ploop target driver
Reviewed-by: Andrey Zhadchenko <andrey.zhadchenko at virtuozzo.com>
Reviewed-by: Konstantin Khorenko <khorenko at virtuozzo.com>
Signed-off-by: Denis V. Lunev <den at openvz.org>
---
 drivers/md/dm-ploop-cmd.c |  7 +++++--
 drivers/md/dm-ploop-map.c | 16 ++++++++++++----
 2 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/drivers/md/dm-ploop-cmd.c b/drivers/md/dm-ploop-cmd.c
index 645c5136e7ba..d85f4d803b61 100644
--- a/drivers/md/dm-ploop-cmd.c
+++ b/drivers/md/dm-ploop-cmd.c
@@ -132,7 +132,7 @@ static void ploop_resume_submitting_pios(struct ploop *ploop)
 	ploop_enable_writeback_delay(ploop);
 	spin_lock_irq(&ploop->deferred_lock);
 	WARN_ON_ONCE(!ploop->stop_submitting_pios);
-	ploop->stop_submitting_pios = false;
+	WRITE_ONCE(ploop->stop_submitting_pios, false);
 	spin_unlock_irq(&ploop->deferred_lock);
 
 	suspended_pending = llist_del_all(&ploop->llsuspended_pios);
@@ -147,9 +147,12 @@ static int ploop_suspend_submitting_pios(struct ploop *ploop)
 	ploop_disable_writeback_delay(ploop);
 	spin_lock_irq(&ploop->deferred_lock);
 	WARN_ON_ONCE(ploop->stop_submitting_pios);
-	ploop->stop_submitting_pios = true;
+	WRITE_ONCE(ploop->stop_submitting_pios, true);
 	spin_unlock_irq(&ploop->deferred_lock);
 
+	/* A submitter that missed the stop gets its ref on the old index */
+	synchronize_rcu();
+
 	ret = ploop_inflight_bios_ref_switch(ploop, true);
 	if (ret)
 		goto out;
diff --git a/drivers/md/dm-ploop-map.c b/drivers/md/dm-ploop-map.c
index 016be97caaa8..f0832fcefeb8 100644
--- a/drivers/md/dm-ploop-map.c
+++ b/drivers/md/dm-ploop-map.c
@@ -2528,6 +2528,7 @@ static void ploop_submit_embedded_pio(struct ploop *ploop, struct pio *pio, gfp_
 	struct ploop_rq *prq = pio->endio_cb_data;
 	struct request *rq = prq->rq;
 	LLIST_HEAD(deferred_pios);
+	unsigned long irqflags;
 	int ret = 0;
 	struct pio *spio;
 
@@ -2538,12 +2539,19 @@ static void ploop_submit_embedded_pio(struct ploop *ploop, struct pio *pio, gfp_
 		pio->queue_list_id = PLOOP_LIST_FLUSH;
 	}
 
-	if (unlikely(ploop->stop_submitting_pios)) {
-		llist_add((struct llist_node *)(&pio->list), &ploop->llsuspended_pios);
-		return;
+	rcu_read_lock();
+	if (unlikely(READ_ONCE(ploop->stop_submitting_pios))) {
+		spin_lock_irqsave(&ploop->deferred_lock, irqflags);
+		if (ploop->stop_submitting_pios) {
+			llist_add((struct llist_node *)(&pio->list), &ploop->llsuspended_pios);
+			spin_unlock_irqrestore(&ploop->deferred_lock, irqflags);
+			rcu_read_unlock();
+			return;
+		}
+		spin_unlock_irqrestore(&ploop->deferred_lock, irqflags);
 	}
-
 	ploop_inc_nr_inflight(ploop, pio);
+	rcu_read_unlock();
 
 	if ((pio->bi_op & REQ_OP_MASK) == REQ_OP_FLUSH || pio->bi_op & REQ_PREFLUSH) {
 		ret = ploop_prepare_flush(ploop, pio);
-- 
2.53.0