[Devel] [PATCH vz9] Revert "dm-ploop: truncate preallocated space on ploop destruction"

[Pavel Tikhomirov]

On 7/9/25 22:06, Konstantin Khorenko wrote:
> This reverts commit 34451d746b817b69b5e9df07bfef622bd4e1a2df.
> 
> The patch being reverted causes image size to be non-align to cluster
> size (1Mb).
> 
> It's not yet clear why and if it really causes data loss/corruption,
> but until we understand and fix this let's revert the patch.
> 
> Results with the patch reverted:
> 
>    [root at localhost a1]# du -h .
>    0       .
>    [root at localhost a1]# ploop init -s 10G image
>    ...
>    [root at localhost a1]# du -h .
>    129M    .
>    [root at localhost a1]# ploop mount -m /mnt/fstrim/ DiskDescriptor.xml
>    ...
>    [root at localhost a1]# dd if=/dev/urandom of=/mnt/fstrim/file bs=1M count=1024 oflag=direct
>    ...
>    1073741824 bytes (1.1 GB, 1.0 GiB) copied, 3.39694 s, 316 MB/s
>    [root at localhost a1]# ploop umount DiskDescriptor.xml
>    ...
>    [root at localhost a1]# du -h .
>    1.2G    .
>    [root at localhost a1]# ploop mount -m /mnt/fstrim/ DiskDescriptor.xml
>    ...
>    [root at localhost a1]# rm -rf /mnt/fstrim/file
>    [root at localhost a1]# fstrim /mnt/fstrim
>    [root at localhost a1]# ploop umount DiskDescriptor.xml
>    ...
>    [root at localhost a1]# du -h .
>    129M    .
>    [root at localhost a1]# stat image
>      File: image
>      Size: 1207959552      Blocks: 262144     IO Block: 4096   regular file
>    ...
> 
> Results before the revert:
> 
>    [root at ptikh-hci fstrim]# du -h .
>    4.0K    .
>    [root at ptikh-hci fstrim]# ploop init -s 10G image
>    [root at ptikh-hci fstrim]# du -h .
>    3.2M    .
>    [root at ptikh-hci fstrim]# ploop mount -m /mnt/fstrim/ DiskDescriptor.xml
>    ...
>    [root at ptikh-hci fstrim]# dd if=/dev/urandom of=/mnt/fstrim/file bs=1M count=1024 oflag=direct
>    ...
>    [root at ptikh-hci fstrim]# ploop umount DiskDescriptor.xml
>    ...
>    [root at ptikh-hci fstrim]# du -h .
>    1.1G    .
>    [root at ptikh-hci fstrim]# ploop mount -m /mnt/fstrim/ DiskDescriptor.xml
>    ...
>    [root at ptikh-hci fstrim]# rm -rf /mnt/fstrim/file
>    [root at ptikh-hci fstrim]# fstrim /mnt/fstrim
>    [root at ptikh-hci fstrim]# ploop umount DiskDescriptor.xml
>    ...
>    [root at ptikh-hci fstrim]# du -h .
>    4.2M    .
> 
> 
> So as expected after revert we do not truncate preallocated but not used
> 128Mb, but this time we don’t preallocate indefinitely and lose blocks.
> 
> https://virtuozzo.atlassian.net/browse/VSTOR-110285
> 
> Signed-off-by: Konstantin Khorenko <khorenko at virtuozzo.com>
> 
> Feature: dm-ploop: ploop target driver
> ---
>   drivers/md/dm-ploop-map.c    | 2 --
>   drivers/md/dm-ploop-target.c | 6 ------
>   2 files changed, 8 deletions(-)
> 
> diff --git a/drivers/md/dm-ploop-map.c b/drivers/md/dm-ploop-map.c
> index 828ae0b9c033..f21a3ae5ba4a 100644
> --- a/drivers/md/dm-ploop-map.c
> +++ b/drivers/md/dm-ploop-map.c
> @@ -1332,8 +1332,6 @@ static int ploop_allocate_cluster(struct ploop *ploop, u32 *dst_clu, struct file
>   		}
>   	}
>   
> -	if (end > top->file_preallocated_area_start)
> -		top->file_preallocated_area_start = end;

We should probably leave this part.

First, we had it in 72.5 and in original patch (see d32286d80e91 
("dm-ploop: Add ploop target driver")).

Second, we use file_preallocated_area_start in ploop_allocate_cluster(), 
to determine if the area may contain dirty data and need to be cleared 
on allocation. Dropping it may lead to data corruption (seeing non-zero 
data where it should be zero).

>   	spin_unlock_irqrestore(&ploop->bat_lock, flags);
>   
>   	return 0;
> diff --git a/drivers/md/dm-ploop-target.c b/drivers/md/dm-ploop-target.c
> index d0c519fdd97c..af0658a455ce 100644
> --- a/drivers/md/dm-ploop-target.c
> +++ b/drivers/md/dm-ploop-target.c
> @@ -162,7 +162,6 @@ static bool ploop_empty_htable(struct hlist_head head[])
>   
>   static void ploop_destroy(struct ploop *ploop)
>   {
> -	struct ploop_delta *top = ploop_top_delta(ploop);
>   	int i;
>   
>   	if (ploop->kt_worker) {
> @@ -202,11 +201,6 @@ static void ploop_destroy(struct ploop *ploop)
>   
>   	for (i = 0; i < 2; i++)
>   		percpu_ref_exit(&ploop->inflight_bios_ref[i]);
> -
> -	if (top->file_preallocated_area_start < top->file_size &&
> -	    vfs_truncate2(&top->file->f_path, top->file_preallocated_area_start, top->file))
> -		PL_ERR("Unable to truncate preallocated area on destroy");
> -
>   	/* Nobody uses it after destroy_workqueue() */
>   	while (ploop->nr_deltas-- > 0) {
>   		if (ploop->deltas[ploop->nr_deltas].file) {

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