[Devel] [PATCH vz9 v1 20/63] dm-ploop: make new allocations immediately visible in BAT

[Pavel Tikhomirov]

On 1/24/25 23:35, Alexander Atanasov wrote:
> @@ -1124,8 +1124,13 @@ static int ploop_alloc_cluster(struct ploop *ploop, struct ploop_index_wb *piwb,
>   		goto out;
>   
>   	to = kmap_local_page(piwb->bat_page);
> +	spin_lock_irqsave(&piwb->md->md_lock, flags);
>   	WRITE_ONCE(to[clu], *dst_clu);
> +	WRITE_ONCE(piwb->md->bat_levels[clu], ploop_top_level(ploop));
> +	spin_unlock_irqrestore(&piwb->md->md_lock, flags);
>   	kunmap_local(to);
> +	to = piwb->md->kmpage;
> +	WRITE_ONCE(to[clu], *dst_clu);
>   out:
>   	return ret;
>   }

> @@ -1409,6 +1415,12 @@ static void ploop_submit_cow_index_wb(struct ploop_cow *cow)
>   	WRITE_ONCE(to[clu], cow->dst_clu);
>   	kunmap_local(to);
>   
> +	spin_lock_irqsave(&md->md_lock, flags);
> +	to = md->kmpage;
> +	WRITE_ONCE(to[clu], cow->dst_clu);
> +	WRITE_ONCE(md->bat_levels[clu], ploop_top_level(ploop));
> +	spin_unlock_irqrestore(&md->md_lock, flags);
> +
>   	/* Prevent double clearing of holes_bitmap bit on complete_cow() */
>   	cow->dst_clu = BAT_ENTRY_NONE;
>   	spin_lock_irq(&ploop->deferred_lock);

If we just consider basic symmetry principles (without understanding 
what the code actually does) the above two hunks added by this patch 
does not seem to be "good".

One spinlock critical section works with:

   to = kmap_local_page(piwb->bat_page);
   WRITE_ONCE(to[clu], *dst_clu);

and another works with:

   to = md->kmpage;
   WRITE_ONCE(to[clu], cow->dst_clu);

Is this asymmetry intentional? (if so please explain it a bit) Shouldn't 
we work with md->kmpage in both cases? Please check.

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