Re: [PATCH v2 2/4] generic/274: Make the pwrite block sizes and offsets to 64k

[Qu Wenruo <wqu@suse.com>]

在 2025/8/19 21:30, Nirjhar Roy (IBM) 写道:
> This test was written with 4k block size in mind and it fails with
> 64k block size when tested with btrfs.
> The test first does pre-allocation, then fills up the
> filesystem. After that it tries to fragment and fill holes at offsets
> of 4k(i.e, 1 fsblock) - which works fine with 4k block size, but with
> 64k block size, the test tries to fragment and fill holes within
> 1 fsblock(of size 64k). This results in overwrite of 64k fsblocks
> and the write fails. The reason for this failure is that during
> overwrite, there is no more space available for COW.
> Fix this by changing the pwrite block size and offsets to 64k
> so that the test never tries to punch holes or overwrite within 1 fsblock
> and the test becomes compatible with all block sizes.
> 
> For non-COW filesystems/files, this test should work even if the
> underlying filesytem block size > 64k.
> 
> Reported-by: Disha Goel <disgoel@linux.ibm.com>
> Signed-off-by: Nirjhar Roy (IBM) <nirjhar.roy.lists@gmail.com>

Overall looks good to me.

Although still a minor concern inlined below.

[...]>
>   # Fill the rest of the fs completely
>   # Note, this will show ENOSPC errors in $seqres.full, that's ok.
>   echo "Fill fs with 1M IOs; ENOSPC expected" >> $seqres.full
>   dd if=/dev/zero of=$SCRATCH_MNT/tmp1 bs=1M >>$seqres.full 2>&1
> -echo "Fill fs with 4K IOs; ENOSPC expected" >> $seqres.full
> -dd if=/dev/zero of=$SCRATCH_MNT/tmp2 bs=4K >>$seqres.full 2>&1
> +echo "Fill fs with 64K IOs; ENOSPC expected" >> $seqres.full
> +dd if=/dev/zero of=$SCRATCH_MNT/tmp2 bs=64K >>$seqres.full 2>&1

Not sure if using 64K block size to fill the fs is the correct way.

For example on a fs with 4K block size, but at end of filling there are 
only 60K left.

This will fail the filling as we can not reserve 64K data space anymore.
But it's not 100% filling the data space either.
This may not matter that much as in the preallocated filling stage, 
every operation is still in 64K block size though.

I'd prefer to keep the old 4K as block size (as it's the minimal support 
one), or use the fs block size for filling.
This will ensure we really use up all the data space.

Thanks,
Qu

>   _scratch_sync
>   # Last effort, use O_SYNC
> -echo "Fill fs with 4K DIOs; ENOSPC expected" >> $seqres.full
> -dd if=/dev/zero of=$SCRATCH_MNT/tmp3 bs=4K oflag=sync >>$seqres.full 2>&1
> +echo "Fill fs with 64K DIOs; ENOSPC expected" >> $seqres.full
> +dd if=/dev/zero of=$SCRATCH_MNT/tmp3 bs=64K oflag=sync >>$seqres.full 2>&1
>   # Save space usage info
>   echo "Post-fill space:" >> $seqres.full
>   df $SCRATCH_MNT >>$seqres.full 2>&1
> @@ -63,7 +63,7 @@ df $SCRATCH_MNT >>$seqres.full 2>&1
>   echo "Fill in prealloc space; fragment at offsets:" >> $seqres.full
>   for i in `seq 1 2 1023`; do
>   	echo -n "$i " >> $seqres.full
> -	dd if=/dev/zero of=$SCRATCH_MNT/test seek=$i bs=4K count=1 conv=notrunc \
> +	dd if=/dev/zero of=$SCRATCH_MNT/test seek=$i bs=64K count=1 conv=notrunc \
>   		>>$seqres.full 2>/dev/null || _fail "failed to write to test file"
>   done
>   _scratch_sync
> @@ -71,7 +71,7 @@ echo >> $seqres.full
>   echo "Fill in prealloc space; fill holes at offsets:" >> $seqres.full
>   for i in `seq 2 2 1023`; do
>   	echo -n "$i " >> $seqres.full
> -	dd if=/dev/zero of=$SCRATCH_MNT/test seek=$i bs=4K count=1 conv=notrunc \
> +	dd if=/dev/zero of=$SCRATCH_MNT/test seek=$i bs=64K count=1 conv=notrunc \
>   		>>$seqres.full 2>/dev/null || _fail "failed to fill test file"
>   done
>   _scratch_sync