Andrey, can you please review the patch?

Thank you.

--
Best regards,

Konstantin Khorenko,
Virtuozzo Linux Kernel Team

On 21.03.2025 16:37, Alexander Atanasov wrote:
On 21.03.25 17:33, Konstantin Khorenko wrote:
Please, add reviewer to CC.


Whomever is available, please, review.

--
Best regards,

Konstantin Khorenko,
Virtuozzo Linux Kernel Team

On 21.03.2025 16:17, Alexander Atanasov wrote:
We missed the fact that resize/grow touch md0 page to update ploop
parameters, other pages are not linked so there is no issue.
But in case there is a concurrent update to md0 piwb is not handled
correctly, also using sync updates in parallel is not okay.
To fix this update code to use the new mechanism with MD_UPDATING
flag and instead of using sync operations pass the updates to
runner threads.

https://virtuozzo.atlassian.net/browse/VSTOR-101871
Signed-off-by: Alexander Atanasov <alexander.atana...@virtuozzo.com>
---
   drivers/md/dm-ploop-map.c | 37 ++++++++++++++++++++++++++++---------
   drivers/md/dm-ploop.h     |  7 ++++++-
   2 files changed, 34 insertions(+), 10 deletions(-)

diff --git a/drivers/md/dm-ploop-map.c b/drivers/md/dm-ploop-map.c
index ef280a8b0f90..04e71c851b64 100644
--- a/drivers/md/dm-ploop-map.c
+++ b/drivers/md/dm-ploop-map.c
@@ -613,11 +613,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;
@@ -1031,6 +1026,7 @@ static int ploop_prepare_bat_update(struct ploop
*ploop, struct md_page *md,
       struct pio *pio;
       map_index_t *to;
       u8 level;
+    int ret = -ENOMEM;
       lockdep_assert_held(&ploop->bat_lock);
@@ -1050,6 +1046,9 @@ static int ploop_prepare_bat_update(struct ploop
*ploop, struct md_page *md,
       if (WARN_ON(md->piwb)) {
           PL_ERR("md %p has piwb: %p type:%d ourtype:%d\n",
               md, md->piwb, md->piwb->type, type);
+        spin_unlock(&md->md_lock);
+        ret = -EBUSY;
+        goto err;
       }
       md->piwb = piwb;
       piwb->md = md;
@@ -1094,7 +1093,7 @@ static int ploop_prepare_bat_update(struct ploop
*ploop, struct md_page *md,
       return 0;
   err:
       ploop_free_piwb(piwb);
-    return -ENOMEM;
+    return ret;
   }
   void ploop_break_bat_update(struct ploop *ploop, struct md_page *md,
@@ -2692,31 +2691,45 @@ static void ploop_handle_cleanup(struct ploop
*ploop, struct pio *pio)
    * another index instead of existing. If so, management of
    * old bat_entries[@clu] and of related holes_bitmap bit
    * is caller duty.
+ * Caller must clear MD_UPDATING and comply to add_for_wb
    */
   int ploop_prepare_reloc_index_wb(struct ploop *ploop,
                    struct md_page **ret_md,
                    u32 clu, u32 *dst_clu,
-                 struct file *file)
+                 struct file *file,
+                 int *add_for_wb)
   {
       enum piwb_type type = PIWB_TYPE_ALLOC;
       u32 page_id = ploop_bat_clu_to_page_nr(clu);
       struct md_page *md = ploop_md_page_find(ploop, page_id);
       struct ploop_index_wb *piwb;
       int err;
+    bool add_to_wblist;
       if (dst_clu)
           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);
+    spin_lock(&md->md_lock);
+    if (test_bit(MD_DIRTY, &md->status) || test_bit(MD_WRITEBACK,
&md->status)
+        || test_bit(MD_UPDATING, &md->status)) {
+        err = -EBUSY;
+        spin_unlock(&md->md_lock);
           goto out_error;
+    } else {
+        set_bit(MD_UPDATING, &md->status);
       }
+    spin_unlock(&md->md_lock);
+
       err = ploop_prepare_bat_update(ploop, md, type);
       if (err)
           goto out_error;
+    add_to_wblist = ploop_md_make_dirty(ploop, md);
+
       piwb = md->piwb;
       if (dst_clu) {
@@ -2734,12 +2747,18 @@ int ploop_prepare_reloc_index_wb(struct ploop
*ploop,
       spin_unlock_irq(&ploop->bat_lock);
       *ret_md = md;
+    *add_for_wb = add_to_wblist ? 1 : 0;
+
       return 0;
   out_reset:
       ploop_break_bat_update(ploop, md, piwb);
   out_error:
+    if (add_to_wblist)
+        clear_bit(MD_DIRTY, &md->status);
+    clear_bit(MD_UPDATING, &md->status);
       spin_unlock_irq(&ploop->bat_lock);
+
       return err;
   }
   ALLOW_ERROR_INJECTION(ploop_prepare_reloc_index_wb, ERRNO);
diff --git a/drivers/md/dm-ploop.h b/drivers/md/dm-ploop.h
index 46450cac8c7d..db4d92c9679a 100644
--- a/drivers/md/dm-ploop.h
+++ b/drivers/md/dm-ploop.h
@@ -578,6 +578,11 @@ static inline const char
*ploop_device_name(struct ploop *ploop)
       return ploop->ti->table->md->disk->disk_name;
   }
+static inline 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);
+}
+
   #define PL_FMT(fmt) "ploop: %s: " fmt "\n"
   #define PL_ERR(fmt, ...) pr_err(PL_FMT(fmt),
ploop_device_name(ploop), ##__VA_ARGS__)
   #define PL_ERR_ONCE(fmt, ...) pr_err_once(PL_FMT(fmt),
ploop_device_name(ploop), ##__VA_ARGS__)
@@ -612,7 +617,7 @@ extern void ploop_map_and_submit_rw(struct ploop
*ploop, u32 dst_clu,
                       struct pio *pio, u8 level);
   extern int ploop_prepare_reloc_index_wb(struct ploop *ploop,
                       struct md_page **ret_md, u32 clu, u32 *dst_clu,
-                    struct file *file);
+                    struct file *file, int *add_for_wb);
   extern void ploop_break_bat_update(struct ploop *ploop, struct
md_page *md,
                      struct ploop_index_wb *piwb);
   extern void ploop_index_wb_submit(struct ploop *, struct
ploop_index_wb *);



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

Reply via email to