[Devel] [RFC PATCH vz9 v6 21/62] dm-ploop: introduce per-md page locking

[Pavel Tikhomirov]

On 1/14/25 16:39, Alexander Atanasov wrote:
> On 14.01.25 7:14, Pavel Tikhomirov wrote:
>>
>>
>> On 1/14/25 12:03, Pavel Tikhomirov wrote:
>>> On 12/6/24 05:55, Alexander Atanasov wrote:
>>>> @@ -411,7 +415,6 @@ static void ploop_apply_delta_mappings(struct 
>>>> ploop *ploop,
>>>>       if (!is_raw)
>>>>           d_md = ploop_md_first_entry(md_root);
>>>> -    write_lock_irq(&ploop->bat_rwlock);
>>>>       ploop_for_each_md_page(ploop, md, node) {
>>>>           bat_entries = md->kmpage;
>>>>           if (!is_raw)
>>>> @@ -455,7 +458,6 @@ static void ploop_apply_delta_mappings(struct 
>>>> ploop *ploop,
>>>>           if (!is_raw)
>>>>               d_md = ploop_md_next_entry(d_md);
>>>>       }
>>>> -    write_unlock_irq(&ploop->bat_rwlock);
>>>>   }
>>>>   int ploop_check_delta_length(struct ploop *ploop, struct file 
>>>> *file, loff_t *file_size)
>>>
>>> This is not possible to remove this lock, as ploop->bat_entries 
>>> rbtree is protected by it. In other words ploop_for_each_md_page 
>>> should be protected by critical section against concurrent 
>>> ploop_add_md_pages.
> 
> ploop_apply_delta_mappings is only used when device is created or it is 
> suspended - so concurency is not possible.

For history, I see it used only in this stack:

   +-< ploop_apply_delta_mappings
     +-< ploop_add_delta
       +-< ploop_add_deltas_stack
         +-< ploop_ctr

And ploop_ctr does `spin_lock_init(&ploop->bat_lock);` so it should be 
at ploop initialization at this point.

> 
>>
>> Also, in case we have some other ploop design concept that orders 
>> those places and concurrency between them is not possible (e.g.: like 
>> suspended ploop) it must be carefully checked for all places and 
>> described in commit message.
> 
> Agree, it needs to be documented not only in commit message but it must 
> be put into tech design document along with how the whole ploop works, 
> as none exists at the moment.

Totally agree.

> 
> 

-- 
Best regards, Tikhomirov Pavel
Senior Software Developer, Virtuozzo.