[Devel] [PATCH vz9] dm-ploop: fix resize and grow to use the new way of updating md pages

[Konstantin Khorenko]

Please, add reviewer to CC.

--
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.atanasov at 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 *);