[Devel] [PATCH vz9] dm-ploop: fix and rework md updates

[Alexander Atanasov]

On 12.02.25 11:50, Pavel Tikhomirov wrote:
> 
> 
> On 2/11/25 22:25, Alexander Atanasov wrote:
>> @@ -2067,21 +2109,46 @@ 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! */
>> -            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);
>> +            if (WARN_ON_ONCE(!test_bit(MD_DIRTY, &md->status)))
>> +                PL_ERR("wb but dirty not set\n");
> 
> Maybe:
> 
> WARN_ONCE(!test_bit(MD_DIRTY, &md->status), PL_FMT("wb but dirty not 
> set"), ploop_device_name(ploop));

That may work, i will check.

> 
>> +
>> +            if (WARN_ON_ONCE(test_bit(MD_WRITEBACK, &md->status)))
>> +                PL_ERR("wb but writeback already set\n");
> 
> Having both WARN and PL_ERR is a bit ugly.

Problem is they are on consecutive lines and it is hard to tell which 
one triggered after compiler optimistations.


> 
>> +
>>               clear_bit(MD_HIGHPRIO, &md->status);
>> -            ploop_index_wb_submit(ploop, md->piwb);
>> -            ret++;
>> +            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);
>>           } else {
>> -            INIT_LIST_HEAD(&md->wb_link);
>>               llist_add((struct llist_node *)&md->wb_link, 
>> &ploop->wb_batch_llist);
>>           }
>>       }
> 

-- 
Regards,
Alexander Atanasov