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.
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel
