[Devel] [PATCH RHEL9 COMMIT] Revert "dm-ploop: fix and rework md updates"

Konstantin Khorenko khorenko at virtuozzo.com
Fri Feb 21 14:47:38 MSK 2025


The commit is pushed to "branch-rh9-5.14.0-427.44.1.vz9.80.x-ovz" and will appear at git at bitbucket.org:openvz/vzkernel.git
after rh9-5.14.0-427.44.1.vz9.80.17
------>
commit c1bd7684a3a8e6df9df7cd65f12bd9482db7cc09
Author: Konstantin Khorenko <khorenko at virtuozzo.com>
Date:   Fri Feb 21 14:05:22 2025 +0300

    Revert "dm-ploop: fix and rework md updates"
    
    This reverts commit 4b8249e2da1a7d28882bd70844fc4d64db952f08.
    
    The new version of the patch is to be applied.
    https://virtuozzo.atlassian.net/browse/VSTOR-98807
    
    Signed-off-by: Konstantin Khorenko <khorenko at virtuozzo.com>
---
 drivers/md/dm-ploop-cmd.c |   4 +-
 drivers/md/dm-ploop-map.c | 153 +++++++++++++---------------------------------
 drivers/md/dm-ploop.h     |   8 +--
 3 files changed, 48 insertions(+), 117 deletions(-)

diff --git a/drivers/md/dm-ploop-cmd.c b/drivers/md/dm-ploop-cmd.c
index daec9827b42c..e032af52b64e 100644
--- a/drivers/md/dm-ploop-cmd.c
+++ b/drivers/md/dm-ploop-cmd.c
@@ -315,8 +315,10 @@ static int ploop_grow_relocate_cluster(struct ploop *ploop,
 		goto out;
 	}
 
+	spin_lock_irq(&ploop->bat_lock);
 	ret = ploop_prepare_reloc_index_wb(ploop, &md, clu, &new_dst,
 					   ploop_top_delta(ploop)->file);
+	spin_unlock_irq(&ploop->bat_lock);
 	if (ret < 0) {
 		PL_ERR("reloc: can't prepare it: %d", ret);
 		goto out;
@@ -327,7 +329,7 @@ static int ploop_grow_relocate_cluster(struct ploop *ploop,
 	ret = ploop_write_cluster_sync(ploop, pio, new_dst);
 	if (ret) {
 		PL_ERR("reloc: failed write: %d", ret);
-		ploop_break_bat_update(ploop, md, piwb);
+		ploop_break_bat_update(ploop, md);
 		goto out;
 	}
 
diff --git a/drivers/md/dm-ploop-map.c b/drivers/md/dm-ploop-map.c
index 0db3a0ff5bb3..f6fc453f7464 100644
--- a/drivers/md/dm-ploop-map.c
+++ b/drivers/md/dm-ploop-map.c
@@ -421,6 +421,7 @@ void ploop_dispatch_pios(struct ploop *ploop, struct pio *pio,
 static bool ploop_delay_if_md_busy(struct ploop *ploop, struct md_page *md,
 				   enum piwb_type type, struct pio *pio)
 {
+	struct ploop_index_wb *piwb;
 	unsigned long flags;
 	bool busy = false;
 
@@ -428,7 +429,8 @@ static bool ploop_delay_if_md_busy(struct ploop *ploop, struct md_page *md,
 
 	/* lock protects piwb */
 	spin_lock_irqsave(&md->md_lock, flags); /* read */
-	if (test_bit(MD_WRITEBACK, &md->status) || test_bit(MD_UPDATING, &md->status)) {
+	piwb = md->piwb;
+	if (piwb && (piwb->type != type || test_bit(MD_WRITEBACK, &md->status))) {
 		llist_add((struct llist_node *)(&pio->list), &md->wait_llist);
 		busy = true;
 	}
@@ -614,11 +616,6 @@ static void ploop_unlink_completed_pio(struct ploop *ploop, struct pio *pio)
 		ploop_dispatch_pios(ploop, NULL, &pio_list);
 }
 
-static void ploop_add_dirty_for_wb(struct ploop *ploop, struct md_page *md)
-{
-	llist_add((struct llist_node *)&md->wb_link, &ploop->wb_batch_llist);
-}
-
 static bool ploop_md_make_dirty(struct ploop *ploop, struct md_page *md)
 {
 	bool new = false;
@@ -631,8 +628,10 @@ static bool ploop_md_make_dirty(struct ploop *ploop, struct md_page *md)
 	lockdep_assert_held(&ploop->bat_lock);
 
 	WARN_ON_ONCE(test_bit(MD_WRITEBACK, &md->status));
-	if (!test_and_set_bit(MD_DIRTY, &md->status))
+	if (!test_and_set_bit(MD_DIRTY, &md->status)) {
+		llist_add((struct llist_node *)&md->wb_link, &ploop->wb_batch_llist);
 		new = true;
+	}
 	md->dirty_timeout = jiffies + ploop->md_submit_delay_ms*HZ/1000;
 
 	return new;
@@ -890,6 +889,9 @@ static void ploop_advance_local_after_bat_wb(struct ploop *ploop,
 #endif
 
 	WARN_ON_ONCE(!test_bit(MD_WRITEBACK, &md->status));
+	/* protect piwb */
+	clear_bit(MD_WRITEBACK, &md->status);
+	md->piwb = NULL;
 	spin_unlock(&md->md_lock);
 	spin_unlock_irqrestore(&ploop->bat_lock, flags);
 	kunmap_local(dst_clu);
@@ -905,11 +907,6 @@ static void ploop_advance_local_after_bat_wb(struct ploop *ploop,
 
 	if (!list_empty(&list))
 		ploop_dispatch_pios(ploop, NULL, &list);
-
-	spin_lock_irqsave(&ploop->bat_lock, flags);
-	/* Clear flag at the end when all processing related is done */
-	clear_bit(MD_WRITEBACK, &md->status);
-	spin_unlock_irqrestore(&ploop->bat_lock, flags);
 }
 
 static void ploop_free_piwb(struct ploop_index_wb *piwb)
@@ -1048,10 +1045,7 @@ static int ploop_prepare_bat_update(struct ploop *ploop, struct md_page *md,
 	bat_entries = md->kmpage;
 
 	spin_lock(&md->md_lock); /* write */
-	if (WARN_ON(md->piwb)) {
-		PL_ERR("md %p has piwb: %p type:%d ourtype:%d\n",
-			md, md->piwb, md->piwb->type, type);
-	}
+	WARN_ON_ONCE(md->piwb);
 	md->piwb = piwb;
 	piwb->md = md;
 
@@ -1098,18 +1092,15 @@ static int ploop_prepare_bat_update(struct ploop *ploop, struct md_page *md,
 	return -ENOMEM;
 }
 
-void ploop_break_bat_update(struct ploop *ploop, struct md_page *md,
-			    struct ploop_index_wb *piwb)
+void ploop_break_bat_update(struct ploop *ploop, struct md_page *md)
 {
+	struct ploop_index_wb *piwb;
 	unsigned long flags;
 
-	spin_lock_irqsave(&ploop->bat_lock, flags);
-	spin_lock(&md->md_lock); /* write */
-	/* Only break if piwb is the same that failed to prepare */
-	if (md->piwb == piwb)
-		md->piwb = NULL;
-	spin_unlock(&md->md_lock);
-	spin_unlock_irqrestore(&ploop->bat_lock, flags);
+	spin_lock_irqsave(&md->md_lock, flags); /* write */
+	piwb = md->piwb;
+	md->piwb = NULL;
+	spin_unlock_irqrestore(&md->md_lock, flags);
 
 	ploop_free_piwb(piwb);
 }
@@ -1359,9 +1350,7 @@ static int ploop_alloc_cluster(struct ploop *ploop, struct ploop_index_wb *piwb,
 	map_index_t *to;
 	int ret = 0;
 	u32 tmp_clu;
-#ifdef PLOOP_DELAYWB
 	unsigned long flags;
-#endif
 
 	/* Cluster index related to the page[page_id] start */
 	clu -= piwb->page_id * PAGE_SIZE / sizeof(map_index_t) - PLOOP_MAP_OFFSET;
@@ -1373,6 +1362,7 @@ static int ploop_alloc_cluster(struct ploop *ploop, struct ploop_index_wb *piwb,
 		*dst_clu = tmp_clu;
 		already_alloced = true;
 	}
+	kunmap_local(to);
 
 	if (already_alloced)
 		goto out;
@@ -1381,16 +1371,17 @@ static int ploop_alloc_cluster(struct ploop *ploop, struct ploop_index_wb *piwb,
 	if (unlikely(ret < 0))
 		goto out;
 
-	WRITE_ONCE(to[clu], *dst_clu);
-#ifdef PLOOP_DELAYWB
+	to = kmap_local_page(piwb->bat_page);
 	spin_lock_irqsave(&piwb->md->md_lock, flags);
-	to = piwb->md->kmpage;
 	WRITE_ONCE(to[clu], *dst_clu);
 	WRITE_ONCE(piwb->md->bat_levels[clu], ploop_top_level(ploop));
 	spin_unlock_irqrestore(&piwb->md->md_lock, flags);
+	kunmap_local(to);
+#ifdef PLOOP_DELAYWB
+	to = piwb->md->kmpage;
+	WRITE_ONCE(to[clu], *dst_clu);
 #endif
 out:
-	kunmap_local(to);
 	return ret;
 }
 
@@ -1659,7 +1650,6 @@ static void ploop_submit_cow_index_wb(struct ploop_cow *cow)
 	struct md_page *md;
 	unsigned long flags;
 	map_index_t *to;
-	bool add_to_wblist = false;
 
 	WARN_ON_ONCE(cow->aux_pio->queue_list_id != PLOOP_LIST_COW);
 	page_id = ploop_bat_clu_to_page_nr(clu);
@@ -1677,15 +1667,12 @@ static void ploop_submit_cow_index_wb(struct ploop_cow *cow)
 			spin_unlock_irqrestore(&ploop->bat_lock, flags);
 			goto err_resource;
 		}
-		add_to_wblist = ploop_md_make_dirty(ploop, md);
+		ploop_md_make_dirty(ploop, md);
 	}
-	piwb = md->piwb;
-	if (WARN_ON(!piwb))
-		PL_ERR("cow no piwb:%d\n", add_to_wblist);
-
-	set_bit(MD_UPDATING, &md->status);
 	spin_unlock_irqrestore(&ploop->bat_lock, flags);
 
+	piwb = md->piwb;
+
 	clu -= page_id * PAGE_SIZE / sizeof(map_index_t) - PLOOP_MAP_OFFSET;
 
 	to = kmap_local_page(piwb->bat_page);
@@ -1706,10 +1693,6 @@ static void ploop_submit_cow_index_wb(struct ploop_cow *cow)
 	cow->dst_clu = BAT_ENTRY_NONE;
 
 	llist_add((struct llist_node *)(&cow->aux_pio->list), &piwb->cow_llist);
-
-	if (add_to_wblist)
-		ploop_add_dirty_for_wb(ploop, md);
-	clear_bit(MD_UPDATING, &md->status);
 out:
 	return;
 err_resource:
@@ -1772,7 +1755,6 @@ static bool ploop_locate_new_cluster_and_attach_pio(struct ploop *ploop,
 	int err;
 	unsigned long flags;
 	struct file *file;
-	bool add_to_wblist = false;
 
 	WARN_ON_ONCE(pio->queue_list_id != PLOOP_LIST_DEFERRED);
 	spin_lock_irqsave(&ploop->bat_lock, flags);
@@ -1782,12 +1764,6 @@ static bool ploop_locate_new_cluster_and_attach_pio(struct ploop *ploop,
 	}
 
 
-	/*
-	 * Flag page as updating, so if there is already a pending update it
-	 * waits for the new update to be ready before processing it
-	 */
-	set_bit(MD_UPDATING, &md->status);
-
 	if (!test_bit(MD_DIRTY, &md->status)) {
 		 /* Locked since MD_DIRTY is set and cleared concurrently  */
 		page_id = ploop_bat_clu_to_page_nr(clu);
@@ -1796,18 +1772,19 @@ static bool ploop_locate_new_cluster_and_attach_pio(struct ploop *ploop,
 			spin_unlock_irqrestore(&ploop->bat_lock, flags);
 			goto error;
 		}
-		add_to_wblist = ploop_md_make_dirty(ploop, md);
 		bat_update_prepared = true;
+		ploop_md_make_dirty(ploop, md);
 	}
-	piwb = md->piwb;
-	WARN_ON_ONCE(!piwb);
 	spin_unlock_irqrestore(&ploop->bat_lock, flags);
 
+	piwb = md->piwb;
+
 	file = ploop_top_delta(ploop)->mtfile[pio->runner_id];
 
 	err = ploop_alloc_cluster(ploop, piwb, clu, dst_clu, file);
 	if (err) {
 		pio->bi_status = errno_to_blk_status(err);
+		clear_bit(MD_DIRTY, &md->status);
 		goto error;
 	}
 
@@ -1825,22 +1802,13 @@ static bool ploop_locate_new_cluster_and_attach_pio(struct ploop *ploop,
 	ploop_attach_end_action(pio, piwb);
 #endif
 	attached = true;
-	if (add_to_wblist)
-		ploop_add_dirty_for_wb(ploop, md);
-	clear_bit(MD_UPDATING, &md->status);
 out:
 	return attached;
 error:
 	/* Uninit piwb */
 	if (bat_update_prepared)
-		ploop_break_bat_update(ploop, md, piwb);
+		ploop_break_bat_update(ploop, md);
 	ploop_pio_endio(pio);
-
-	spin_lock_irqsave(&ploop->bat_lock, flags);
-	if (add_to_wblist)
-		clear_bit(MD_DIRTY, &md->status);
-	clear_bit(MD_UPDATING, &md->status);
-	spin_unlock_irqrestore(&ploop->bat_lock, flags);
 	return false;
 }
 
@@ -2039,7 +2007,6 @@ static void ploop_process_one_discard_pio(struct ploop *ploop, struct pio *pio)
 	struct md_page *md;
 	map_index_t *to;
 	unsigned long flags;
-	bool add_to_wblist = false;
 
 	WARN_ON(ploop->nr_deltas != 1 ||
 		pio->queue_list_id != PLOOP_LIST_DISCARD);
@@ -2052,18 +2019,17 @@ static void ploop_process_one_discard_pio(struct ploop *ploop, struct pio *pio)
 		return;
 	}
 
-	set_bit(MD_UPDATING, &md->status);
 	if (!test_bit(MD_DIRTY, &md->status)) {
 		if (ploop_prepare_bat_update(ploop, md, PIWB_TYPE_DISCARD) < 0) {
 			pio->bi_status = BLK_STS_RESOURCE;
 			goto err_unlck;
 		}
-		add_to_wblist = ploop_md_make_dirty(ploop, md);
 		bat_update_prepared = true;
+		ploop_md_make_dirty(ploop, md);
 	}
+	spin_unlock_irqrestore(&ploop->bat_lock, flags);
 
 	piwb = md->piwb;
-	spin_unlock_irqrestore(&ploop->bat_lock, flags);
 
 	/* Cluster index related to the page[page_id] start */
 	clu -= piwb->page_id * PAGE_SIZE / sizeof(map_index_t) - PLOOP_MAP_OFFSET;
@@ -2071,7 +2037,7 @@ static void ploop_process_one_discard_pio(struct ploop *ploop, struct pio *pio)
 	to = kmap_local_page(piwb->bat_page);
 	if (WARN_ON_ONCE(!to[clu])) {
 		pio->bi_status = BLK_STS_IOERR;
-		kunmap_local(to);
+		clear_bit(MD_DIRTY, &md->status);
 		goto err;
 	} else {
 		WRITE_ONCE(to[clu], 0);
@@ -2080,22 +2046,14 @@ static void ploop_process_one_discard_pio(struct ploop *ploop, struct pio *pio)
 	kunmap_local(to);
 
 	ploop_md_up_prio(ploop, md);
-	if (add_to_wblist)
-		ploop_add_dirty_for_wb(ploop, md);
-	clear_bit(MD_UPDATING, &md->status);
 
 	return;
 err_unlck:
 	spin_unlock_irqrestore(&ploop->bat_lock, flags);
 err:
 	if (bat_update_prepared)
-		ploop_break_bat_update(ploop, md, piwb);
+		ploop_break_bat_update(ploop, md);
 	ploop_pio_endio(pio);
-	spin_lock_irqsave(&ploop->bat_lock, flags);
-	if (add_to_wblist)
-		clear_bit(MD_DIRTY, &md->status);
-	clear_bit(MD_UPDATING, &md->status);
-	spin_unlock_irqrestore(&ploop->bat_lock, flags);
 }
 
 static inline int ploop_runners_have_pending(struct ploop *ploop)
@@ -2124,46 +2082,21 @@ static inline int ploop_submit_metadata_writeback(struct ploop *ploop, int force
 	 */
 	llist_for_each_safe(pos, t, ll_wb_batch) {
 		md = list_entry((struct list_head *)pos, typeof(*md), wb_link);
-		INIT_LIST_HEAD(&md->wb_link);
 		/* XXX: fixme delay results in a hang - TBD */
 		if (1 || !llist_empty(&md->wait_llist) || force ||
 		    test_bit(MD_HIGHPRIO, &md->status) ||
 		    time_before(md->dirty_timeout, timeout) ||
 		    ploop->force_md_writeback) {
-			spin_lock_irqsave(&ploop->bat_lock, flags);
-			if (test_bit(MD_UPDATING, &md->status) ||
-			    test_bit(MD_WRITEBACK, &md->status)) {
-				/* if updating or there is a writeback then delay */
-				spin_unlock_irqrestore(&ploop->bat_lock, flags);
-				llist_add((struct llist_node *)&md->wb_link,
-					  &ploop->wb_batch_llist);
-				continue;
-			}
-
 			/* L1L2 mustn't be redirtyed, when wb in-flight! */
-			if (WARN_ON_ONCE(!test_bit(MD_DIRTY, &md->status)))
-				PL_ERR("wb but dirty not set\n");
-
-			if (WARN_ON_ONCE(test_bit(MD_WRITEBACK, &md->status)))
-				PL_ERR("wb but writeback already set\n");
-
+			WARN_ON_ONCE(!test_bit(MD_DIRTY, &md->status));
+			WARN_ON_ONCE(test_bit(MD_WRITEBACK, &md->status));
+			set_bit(MD_WRITEBACK, &md->status);
+			clear_bit(MD_DIRTY, &md->status);
 			clear_bit(MD_HIGHPRIO, &md->status);
-			if (md->piwb) {
-				struct ploop_index_wb *piwb;
-				/* New updates will go in wait list */
-				set_bit(MD_WRITEBACK, &md->status);
-				clear_bit(MD_DIRTY, &md->status);
-				/* at this point we can clear md->piwb */
-				piwb = md->piwb;
-				md->piwb = NULL;
-				/* hand off to threads */
-				ploop_index_wb_submit(ploop, piwb);
-				ret++;
-			} else {
-				PL_ERR("Unexpected md piwb is null");
-			}
-			spin_unlock_irqrestore(&ploop->bat_lock, flags);
+			ploop_index_wb_submit(ploop, md->piwb);
+			ret++;
 		} else {
+			INIT_LIST_HEAD(&md->wb_link);
 			llist_add((struct llist_node *)&md->wb_link, &ploop->wb_batch_llist);
 		}
 	}
@@ -2704,7 +2637,6 @@ int ploop_prepare_reloc_index_wb(struct ploop *ploop,
 		type = PIWB_TYPE_RELOC;
 
 	err = -EIO;
-	spin_lock_irq(&ploop->bat_lock);
 	if (test_bit(MD_DIRTY, &md->status) || test_bit(MD_WRITEBACK, &md->status)) {
 		PL_ERR("Unexpected md status: %lx", md->status);
 		goto out_error;
@@ -2727,13 +2659,12 @@ int ploop_prepare_reloc_index_wb(struct ploop *ploop,
 		if (err)
 			goto out_reset;
 	}
-	spin_unlock_irq(&ploop->bat_lock);
 
 	*ret_md = md;
 	return 0;
 
 out_reset:
-	ploop_break_bat_update(ploop, md, piwb);
+	ploop_break_bat_update(ploop, md);
 out_error:
 	return err;
 }
diff --git a/drivers/md/dm-ploop.h b/drivers/md/dm-ploop.h
index 46450cac8c7d..06d95c525c90 100644
--- a/drivers/md/dm-ploop.h
+++ b/drivers/md/dm-ploop.h
@@ -116,10 +116,9 @@ struct ploop_index_wb {
 struct md_page {
 	struct rb_node node;
 	u32 id; /* Number of this page starting from hdr */
-#define MD_DIRTY	(1U << 1) /* Page contains changes and awaits writeback */
-#define MD_WRITEBACK	(1U << 2) /* Writeback in progress */
+#define MD_DIRTY	(1U << 1) /* Page contains changes and wants writeback */
+#define MD_WRITEBACK	(1U << 2) /* Writeback was submitted */
 #define MD_HIGHPRIO	(1U << 3) /* High priority writeback  */
-#define MD_UPDATING	(1U << 4) /* Preparing an update */
 	unsigned long status;
 	struct page *page;
 	void *kmpage;
@@ -613,8 +612,7 @@ extern void ploop_map_and_submit_rw(struct ploop *ploop, u32 dst_clu,
 extern int ploop_prepare_reloc_index_wb(struct ploop *ploop,
 					struct md_page **ret_md, u32 clu, u32 *dst_clu,
 					struct file *file);
-extern void ploop_break_bat_update(struct ploop *ploop, struct md_page *md,
-				   struct ploop_index_wb *piwb);
+extern void ploop_break_bat_update(struct ploop *ploop, struct md_page *md);
 extern void ploop_index_wb_submit(struct ploop *, struct ploop_index_wb *);
 extern int ploop_message(struct dm_target *ti, unsigned int argc, char **argv,
 			 char *result, unsigned int maxlen);


More information about the Devel mailing list