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

Alexander Atanasov alexander.atanasov at virtuozzo.com
Fri Mar 21 18:37:46 MSK 2025


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


-- 
Regards,
Alexander Atanasov


More information about the Devel mailing list