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

[Alexander Atanasov]

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.

> 
> 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.


-- 
Regards,
Alexander Atanasov