[Devel] [PATCH vz9 v1 06/63] dm-ploop: convert enospc handling to use lockless lists

[Alexander Atanasov]

On 3.02.25 9:27, Pavel Tikhomirov wrote:
> 
> 
> On 2/3/25 14:45, Alexander Atanasov wrote:
>> On 3.02.25 8:01, Pavel Tikhomirov wrote:
>>>
>>>> @@ -166,7 +171,6 @@ static bool ploop_try_delay_enospc(struct 
>>>> ploop_rq *prq, struct pio *pio)
>>>>       bool delayed = true;
>>>>       unsigned long flags;
>>>> -    spin_lock_irqsave(&ploop->deferred_lock, flags);
>>>>       if (unlikely(ploop->wants_suspend)) {
>>>>           delayed = false;
>>>>           goto unlock;
>>>> @@ -176,10 +180,11 @@ static bool ploop_try_delay_enospc(struct 
>>>> ploop_rq *prq, struct pio *pio)
>>>>       pr_err_once(PL_FMT("underlying disk is almost full"),
>>>>           ploop_device_name(ploop));
>>>> +    spin_lock_irqsave(&ploop->deferred_lock, flags);
>>>>       ploop->event_enospc = true;
>>>> -    list_add_tail(&pio->list, &ploop->enospc_pios);
>>>> -unlock:
>>>>       spin_unlock_irqrestore(&ploop->deferred_lock, flags);
>>>> +    llist_add((struct llist_node *)(&pio->list), &ploop->enospc_pios);
>>>> +unlock:
>>>>       if (delayed)
>>>>           mod_timer(&ploop->enospc_timer, jiffies + 
>>>> PLOOP_ENOSPC_TIMEOUT);
>>>
>>> Can you please explain why we need to take defered_lock around ploop- 
>>> >event_enospc setting after your patch? (It looks that this lock does 
>>> not do anything now.)
>>>
>>
>> see static int ploop_get_event(...), without lock event can be missed
> 
> That is not an explanation. How exactly can it be missed?


         spin_lock_irq(&ploop->deferred_lock);
         if (ploop->event_enospc) {                    <- while emit 
other cpu can set
                 ret = (DMEMIT("event_ENOSPC\n")) ? 1 : 0;
                 if (ret)
                         ploop->event_enospc = false; <- next cleaars 
here -> second event lost
         }
         spin_unlock_irq(&ploop->deferred_lock);



-- 
Regards,
Alexander Atanasov