[Devel] [PATCH v5 vz9 4/7] dm-ploop: simplify "prealloc_more" loop in ploop_preallocate_cluster()

[Pavel Tikhomirov]

On 7/8/25 03:47, Konstantin Khorenko wrote:
> From: Pavel Tikhomirov <ptikhomirov at virtuozzo.com>
> 
> Just a cleanup/simplification, no logic changed.

I would argue with that no logic is changed, it is changed.

First we actually fix excess spin_lock/unlock contention. Now lock is 
only released when we need it to run ploop_truncate_prealloc_safe(), 
which can sleep.

Second, and most importantly, we fix possible race between prealloc_size 
check for zero and resetting prealloc_in_progress and prealloc_size. 
Previously we could've lost some preallocation requests due to this race 
as the bat_lock was released between the check and reset.

> 
> https://virtuozzo.atlassian.net/browse/VSTOR-108868
> Co-developed-by: Andrey Zhadchenko <andrey.zhadchenko at virtuozzo.com>
> Signed-off-by: Pavel Tikhomirov <ptikhomirov at virtuozzo.com>
> 
> Feature: dm-ploop: ploop target driver
> ---
>   drivers/md/dm-ploop-map.c | 12 +++---------
>   1 file changed, 3 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/md/dm-ploop-map.c b/drivers/md/dm-ploop-map.c
> index f7bb6bcf13feb..b59e0d514c95f 100644
> --- a/drivers/md/dm-ploop-map.c
> +++ b/drivers/md/dm-ploop-map.c
> @@ -1172,10 +1172,10 @@ static int ploop_preallocate_cluster(struct ploop *ploop, struct file *file)
>   	struct ploop_delta *top = ploop_top_delta(ploop);
>   	loff_t end;
>   	unsigned long flags;
> -	int ret, more = 0;
> +	int ret;
>   
> -prealloc_more:
>   	spin_lock_irqsave(&ploop->bat_lock, flags);
> +prealloc_more:
>   	ploop->prealloc_in_progress = ploop->prealloc_size;
>   	end = top->file_size + ploop->prealloc_in_progress;
>   	loff_t new_len = ALIGN(end, ploop->prealloc_in_progress);
> @@ -1190,6 +1190,7 @@ static int ploop_preallocate_cluster(struct ploop *ploop, struct file *file)
>   					   new_len, file, __func__);
>   	if (ret) {
>   		PL_ERR("Failed to preallocate space: %d\n", ret);
> +		spin_lock_irqsave(&ploop->bat_lock, flags);
>   		goto out;
>   	}
>   
> @@ -1201,15 +1202,8 @@ static int ploop_preallocate_cluster(struct ploop *ploop, struct file *file)
>   		PL_ERR("unexpected file size change\n");
>   	}
>   	if (ploop->prealloc_size)
> -		more = 1;
> -	spin_unlock_irqrestore(&ploop->bat_lock, flags);
> -	if (more) {
> -		more = 0;
>   		goto prealloc_more;
> -	}
> -
>   out:
> -	spin_lock_irqsave(&ploop->bat_lock, flags);
>   	ploop->prealloc_in_progress = 0;
>   	ploop->prealloc_size = 0;
>   	spin_unlock_irqrestore(&ploop->bat_lock, flags);

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