[Devel] [PATCH hci-8.1 v2 4/4] dm-ploop: fix lost BAT updates when alloc and discard share a writeback

["Denis V. Lunev" <den@openvz.org>]

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

Since "dm-ploop: fix and rework md updates" any update of a BAT page
joins the pending index writeback of that page, whatever type it was
prepared with. On completion ploop_advance_local_after_bat_wb() still
applied the written page by type: a DISCARD writeback copied back only
entries that became zero, an ALLOC one only non-zero entries.

A first write to a cluster that joined a pending DISCARD writeback was
acknowledged with its new mapping on disk but not in the local BAT. It
read back as zeroes, the next write to the cluster allocated another
one, and the next writeback of the page stored that, leaking the
cluster with the acknowledged data. A discard that joined an ALLOC
writeback left the local BAT mapped while disk had zero, so data
written to that cluster afterwards was lost on the next open.

Apply the changes of the pios attached to the writeback instead: the
allocating data pios and COW pios set their entries, the discard pios
clear theirs, other entries are left alone. The relocation on grow
changes an entry that no pio stands for, so its writeback records
that entry to be applied the same way. On failure the same rule as
before frees only what this writeback allocated: a data pio may find
its cluster already mapped by an earlier writeback.

Entries nobody changed no longer get their level reset to the top
one; nothing needs that once the merge of the latest snapshot
renumbers the levels itself.

Fixes: ddf38fa44df4 ("dm-ploop: fix and rework md updates")
https://virtuozzo.atlassian.net/browse/VSTOR-151979

Signed-off-by: Denis V. Lunev <den@openvz.org>
Reviewed-by: Andrey Zhadchenko <andrey.zhadchenko@virtuozzo.com>

Feature: dm-ploop: ploop target driver
---
 drivers/md/dm-ploop-map.c | 133 ++++++++++++++++++++------------------
 drivers/md/dm-ploop.h     |   1 +
 2 files changed, 71 insertions(+), 63 deletions(-)

diff --git a/drivers/md/dm-ploop-map.c b/drivers/md/dm-ploop-map.c
index 2f8abfa8905b..016be97caaa8 100644
--- a/drivers/md/dm-ploop-map.c
+++ b/drivers/md/dm-ploop-map.c
@@ -86,6 +86,7 @@ void ploop_index_wb_init(struct ploop_index_wb *piwb, struct ploop *ploop)
 	piwb->page_id = PAGE_NR_NONE;
 	piwb->type = PIWB_TYPE_ALLOC;
 	piwb->flush_pio = NULL;
+	piwb->reloc_clu = BAT_ENTRY_NONE;
 }
 
 void ploop_init_pio(struct ploop *ploop, unsigned int bi_op, struct pio *pio)
@@ -802,73 +803,85 @@ static void ploop_complete_cow(struct ploop_cow *cow, blk_status_t bi_status)
 	kmem_cache_free(cow_cache, cow);
 }
 
+static void ploop_advance_allocated(struct ploop *ploop, struct md_page *md,
+				    map_index_t *dst_clu, u32 clu, bool success)
+{
+	u32 i = ploop_bat_clu_idx_in_page(clu);
+	u32 *bat_entries = md->kmpage;
+	u32 new_clu = READ_ONCE(dst_clu[i]);
+
+	if (WARN_ON_ONCE(!new_clu))
+		return;
+
+	if (success) {
+		WRITE_ONCE(bat_entries[i], new_clu);
+		WRITE_ONCE(md->bat_levels[i], ploop_top_level(ploop));
+	} else if (new_clu != READ_ONCE(bat_entries[i]) ||
+		   md->bat_levels[i] < ploop_top_level(ploop)) {
+		ploop_hole_set_bit(new_clu, ploop);
+	}
+}
+
+static void ploop_advance_discarded(struct ploop *ploop, struct md_page *md,
+				    map_index_t *dst_clu, u32 clu, bool success)
+{
+	u32 i = ploop_bat_clu_idx_in_page(clu);
+	u32 *bat_entries = md->kmpage;
+	u8 level = md->bat_levels[i];
+	u32 d_clu = READ_ONCE(bat_entries[i]);
+
+	if (!success || dst_clu[i] || d_clu == BAT_ENTRY_NONE ||
+	    level < ploop_top_level(ploop))
+		return;
+
+	WARN_ON_ONCE(ploop->nr_deltas != 1);
+	WRITE_ONCE(bat_entries[i], BAT_ENTRY_NONE);
+	WRITE_ONCE(md->bat_levels[i], 0);
+	ploop_hole_set_bit(d_clu, ploop);
+}
+
 /*
- * Update local BAT copy with written indexes on success.
- * Mark allocate clusters as holes on failure.
+ * Apply to the local BAT copy the changes made by the pios attached
+ * to @piwb, whatever type @piwb was prepared with: allocations and
+ * discards of one page may share a writeback.
  * FIXME: a failure may mean some sectors are written, so
  * we have to reread BAT page to check that.
  */
 static void ploop_advance_local_after_bat_wb(struct ploop *ploop,
 					     struct ploop_index_wb *piwb,
+					     struct llist_node *ll_ready_pios,
+					     struct llist_node *ll_cow_pios,
 					     bool success)
 {
 	struct md_page *md = piwb->md;
-	u32 i, last, *bat_entries;
-	map_index_t *dst_clu, off;
+	map_index_t *dst_clu;
 	unsigned long flags;
 	LIST_HEAD(list);
 	struct llist_node *wait_llist_pending;
+	struct ploop_cow *cow;
 	struct pio *pio;
 	struct llist_node *pos, *t;
 	BUG_ON(!md);
-	bat_entries = md->kmpage;
-
-	/* Absolute number of first index in page (negative for page#0) */
-	off = piwb->page_id * PAGE_SIZE / sizeof(map_index_t);
-	off -= PLOOP_MAP_OFFSET;
 
 	spin_lock_irqsave(&ploop->bat_lock, flags);
-	/* Last and first index in copied page */
-	last = READ_ONCE(ploop->nr_bat_entries) - off;
-	if (last > PAGE_SIZE / sizeof(map_index_t))
-		last = PAGE_SIZE / sizeof(map_index_t);
-	i = 0;
-	if (!piwb->page_id)
-		i = PLOOP_MAP_OFFSET;
-
 	dst_clu = kmap_local_page(piwb->bat_page);
 
 	/* holes bit map requires bat_lock */
 	spin_lock(&md->md_lock);
-	for (; i < last; i++) {
-		if (piwb->type == PIWB_TYPE_DISCARD) {
-			/* discard completed */
-			u8 level = md->bat_levels[i];
-			u32 d_clu = READ_ONCE(bat_entries[i]);
-
-			if (success && !dst_clu[i] &&
-			    (!(d_clu == BAT_ENTRY_NONE ||
-			    level < ploop_top_level(ploop)))) {
-				WARN_ON_ONCE(ploop->nr_deltas != 1);
-				WRITE_ONCE(bat_entries[i], BAT_ENTRY_NONE);
-				WRITE_ONCE(md->bat_levels[i], 0);
-				ploop_hole_set_bit(d_clu, ploop);
-			}
-
-			continue;
-		}
-
-		if (!dst_clu[i])
-			continue;
-
-		if (success) {
-			WRITE_ONCE(bat_entries[i], READ_ONCE(dst_clu[i]));
-			WRITE_ONCE(md->bat_levels[i], ploop_top_level(ploop));
-		} else if (dst_clu[i] != bat_entries[i] ||
-			   md->bat_levels[i] < ploop_top_level(ploop)) {
-			ploop_hole_set_bit(dst_clu[i], ploop);
-		}
+	llist_for_each(pos, ll_ready_pios) {
+		pio = list_entry((struct list_head *)pos, typeof(*pio), list);
+		if (pio->wants_discard_index_cleanup)
+			ploop_advance_discarded(ploop, md, dst_clu, pio->clu, success);
+		else if (pio->is_data_alloc)
+			ploop_advance_allocated(ploop, md, dst_clu, pio->clu, success);
 	}
+	llist_for_each(pos, ll_cow_pios) {
+		pio = list_entry((struct list_head *)pos, typeof(*pio), list);
+		cow = pio->endio_cb_data;
+		ploop_advance_allocated(ploop, md, dst_clu, cow->cow_pio->clu, success);
+	}
+	if (piwb->reloc_clu != BAT_ENTRY_NONE)
+		ploop_advance_allocated(ploop, md, dst_clu, piwb->reloc_clu, success);
 
 	WARN_ON_ONCE(!test_bit(MD_WRITEBACK, &md->status));
 
@@ -941,24 +954,6 @@ static void ploop_bat_write_finish(struct pio *pio, void *piwb_ptr,
 	struct llist_node *ll_cow_pios;
 	struct llist_node *ll_ready_pios;
 
-	if (!bi_status) {
-		/*
-		 * Success: now update local BAT copy. We could do this
-		 * from our delayed work, but we want to publish new
-		 * mapping in the fastest way. This must be done before
-		 * data bios completion, since right after we complete
-		 * 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);
-	} 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);
-	}
-
 	spin_lock_irqsave(&piwb->lock, flags);
 	ll_ready_pios = llist_del_all(&piwb->llready_data_pios);
 	if (bi_status)
@@ -966,6 +961,17 @@ static void ploop_bat_write_finish(struct pio *pio, void *piwb_ptr,
 	spin_unlock_irqrestore(&piwb->lock, flags);
 	ll_cow_pios = llist_reverse_order(llist_del_all(&piwb->cow_llist));
 
+	/*
+	 * On success publish the new mapping before the data bios
+	 * complete, since right after we complete a bio, subsequent
+	 * read wants to see written data (ploop_map() wants to see
+	 * not zero bat_entries[.]). On failure mark the clusters
+	 * allocated for this writeback as holes again: piwb->count
+	 * is zero, so all data writers completed.
+	 */
+	ploop_advance_local_after_bat_wb(ploop, piwb, ll_ready_pios,
+					 ll_cow_pios, !bi_status);
+
 	/*
 	 * End pending data bios.
 	 */
@@ -2708,6 +2714,7 @@ int ploop_prepare_reloc_index_wb(struct ploop *ploop,
 		err = ploop_alloc_cluster(ploop, piwb, clu, dst_clu, file);
 		if (err)
 			goto out_reset;
+		piwb->reloc_clu = clu;
 	}
 
 	*ret_md = md;
diff --git a/drivers/md/dm-ploop.h b/drivers/md/dm-ploop.h
index 23662943d56a..ce834df6df55 100644
--- a/drivers/md/dm-ploop.h
+++ b/drivers/md/dm-ploop.h
@@ -108,6 +108,7 @@ struct ploop_index_wb {
 	blk_status_t bi_status;
 	u32 page_id;
 	struct bio_vec aux_bvec;
+	u32 reloc_clu;
 	struct pio *flush_pio;
 };
 
-- 
2.53.0

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel