[Devel] [RFC PATCH vz9 v6 43/62] dm-ploop: make ploop_submit_metadata_writeback return number of requests sent

[Alexander Atanasov]

On 20.01.25 12:56, Pavel Tikhomirov wrote:
> 
> 
> On 1/20/25 18:51, Alexander Atanasov wrote:
>> On 20.01.25 11:43, Pavel Tikhomirov wrote:
>>>
>>>
>>> On 12/6/24 05:56, Alexander Atanasov wrote:
>>>> We need to wait for write back to complete to issue pending flushes.
>>>> To know if we have to wait return numer of pios submitted.
>>>>
>>>> https://virtuozzo.atlassian.net/browse/VSTOR-91821
>>>> Signed-off-by: Alexander Atanasov <alexander.atanasov at virtuozzo.com>
>>>> ---
>>>>   drivers/md/dm-ploop-map.c | 15 ++++++++++++---
>>>>   1 file changed, 12 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/md/dm-ploop-map.c b/drivers/md/dm-ploop-map.c
>>>> index 276e8cf3b178..b3e8b934ff13 100644
>>>> --- a/drivers/md/dm-ploop-map.c
>>>> +++ b/drivers/md/dm-ploop-map.c
>>>> @@ -1889,7 +1889,7 @@ static inline int 
>>>> ploop_runners_have_pending(struct ploop *ploop)
>>>>   }
>>>>   #endif
>>>> -static void ploop_submit_metadata_writeback(struct ploop *ploop, 
>>>> int force)
>>>> +static int ploop_submit_metadata_writeback(struct ploop *ploop, int 
>>>> force)
>>>>   {
>>>>       unsigned long flags;
>>>>       LIST_HEAD(ll_skipped);
>>>> @@ -1897,19 +1897,25 @@ static void 
>>>> ploop_submit_metadata_writeback(struct ploop *ploop, int force)
>>>>       struct llist_node *pos, *t;
>>>>       struct llist_node *ll_wb_batch;
>>>>       unsigned long timeout = jiffies;
>>>> +    int ret;
>>>>       /* Lock here to protect against md_inflight counting */
>>>>       spin_lock_irqsave(&ploop->bat_lock, flags);
>>>>       ll_wb_batch = llist_del_all(&ploop->wb_batch_llist);
>>>>       spin_unlock_irqrestore(&ploop->bat_lock, flags);
>>>> +    if (!ll_wb_batch)
>>>> +        return 0;
>>>> +
>>>> +    ret = 0;
>>>> +
>>>>       /*
>>>>        * Pages are set dirty so no one must touch lists
>>>>        * if new md entries are dirtied they are added at the start 
>>>> of the list
>>>>        */
>>>>       llist_for_each_safe(pos, t, ll_wb_batch) {
>>>>           md = list_entry((struct list_head *)pos, typeof(*md), 
>>>> wb_link);
>>>> -        if (test_bit(MD_HIGHPRIO, &md->status) || time_before(md- 
>>>> >dirty_timeout, timeout)
>>>> -            || ploop->force_md_writeback) {
>>>> +        if (!llist_empty(&md->wait_llist) || force || 
>>>> test_bit(MD_HIGHPRIO, &md->status) ||
>>>> +            time_before(md->dirty_timeout, timeout)) {
>>>
>>> Why do we add "!llist_empty(&md->wait_llist)" condition here? If it's 
>>> intentional, it should probably be explained and be a separate patch, 
>>> because it seems unrelated to what the commit message says.
>>
>>
>> It is intentional - If we have pios in &md->wait_llist we can not 
>> delay since it will delay pio execution. Updated commit message .
> 
> It should definitely be a separate patch, as it's unrelated to adding 
> return value to ploop_submit_metadata_writeback.

Moved to dm-ploop: add interface to disable bat writeback delay
picked as best candidate

> 
>>
>>>
>>>>               /* 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));
>>>> @@ -1917,10 +1923,13 @@ static void 
>>>> ploop_submit_metadata_writeback(struct ploop *ploop, int force)
>>>>               clear_bit(MD_DIRTY, &md->status);
>>>>               clear_bit(MD_HIGHPRIO, &md->status);
>>>>               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);
>>>>           }
>>>>       }
>>>> +    return ret;
>>>>   }
>>>>   static void process_ploop_fsync_work(struct ploop *ploop, struct 
>>>> llist_node *llflush_pios)
>>>
>>
> 

-- 
Regards,
Alexander Atanasov