Re: [PATCH] fstests: generic/563: use fs blocksize to do the writes

[Boris Burkov <boris@bur.io>]

On Mon, Sep 30, 2024 at 09:20:38AM +0930, Qu Wenruo wrote:
> [FALSE ALERTS]
> If the system has a page size larger than 4K, and the fs block size
> matches the page size, test case generic/563 will fail:
> 
>     --- tests/generic/563.out	2024-04-25 18:13:45.178550333 +0930
>     +++ /home/adam/xfstests-dev/results//generic/563.out.bad	2024-09-30 09:09:16.155312379 +0930
>     @@ -3,7 +3,8 @@
>      read is in range
>      write is in range
>      write -> read/write
>     -read is in range
>     +read has value of 8388608
>     +read is NOT in range -33792 .. 33792
>      write is in range
>     ...
> 
> Both Ext4 and btrfs fail with 64K block size and 64K page size
> 
> [CAUSE]
> The test case writes the 8MiB file using the default block size xfs_io
> pwrite, which is 4KiB.
> 
> Since the fs block size is 64K, such 4KiB write is unaligned inside a
> block, causing the fs to read out the full page.
> 
> Thus the pwrite will cause the fs to read out every page, resulting the
> above 8MiB+ read value.
> 
> [FIX]
> Fix the test case by using the fs block size to avoid such unaligned
> buffered write.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>

The rationale and change look good to me. I also tested it on my system
with blocksize=pagesize=4k, though I don't have access to one with a 64k
page size for the more interesting confirmation.

Reviewed-by: Boris Burkov <boris@bur.io>

> ---
>  tests/generic/563 | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/tests/generic/563 b/tests/generic/563
> index 0a8129a6..e8db8acf 100755
> --- a/tests/generic/563
> +++ b/tests/generic/563
> @@ -94,6 +94,8 @@ sminor=$((0x`stat -L -c %T $LOOP_DEV`))
>  _mkfs_dev $LOOP_DEV >> $seqres.full 2>&1
>  _mount $LOOP_DEV $SCRATCH_MNT || _fail "mount failed"
>  
> +blksize=$(_get_block_size "$SCRATCH_MNT")
> +
>  drop_io_cgroup=
>  grep -q -w io $cgdir/cgroup.subtree_control || drop_io_cgroup=1
>  
> @@ -103,7 +105,7 @@ echo "+io" > $cgdir/cgroup.subtree_control || _fail "subtree control"
>  echo "read/write"
>  reset
>  switch_cg $cgdir/$seq-cg
> -$XFS_IO_PROG -c "pread 0 $iosize" -c "pwrite 0 $iosize" -c fsync \
> +$XFS_IO_PROG -c "pread 0 $iosize" -c "pwrite -b $blksize 0 $iosize" -c fsync \
>  	$SCRATCH_MNT/file >> $seqres.full 2>&1
>  switch_cg $cgdir
>  $XFS_IO_PROG -c fsync $SCRATCH_MNT/file
> @@ -114,9 +116,9 @@ check_cg $cgdir/$seq-cg $iosize $iosize 5% 5%
>  echo "write -> read/write"
>  reset
>  switch_cg $cgdir/$seq-cg
> -$XFS_IO_PROG -c "pwrite 0 $iosize" $SCRATCH_MNT/file >> $seqres.full 2>&1
> +$XFS_IO_PROG -c "pwrite -b $blksize 0 $iosize" $SCRATCH_MNT/file >> $seqres.full 2>&1
>  switch_cg $cgdir/$seq-cg-2
> -$XFS_IO_PROG -c "pread 0 $iosize" -c "pwrite 0 $iosize" $SCRATCH_MNT/file \
> +$XFS_IO_PROG -c "pread 0 $iosize" -c "pwrite -b $blksize 0 $iosize" $SCRATCH_MNT/file \
>  	>> $seqres.full 2>&1
>  switch_cg $cgdir
>  $XFS_IO_PROG -c fsync $SCRATCH_MNT/file
> @@ -134,7 +136,7 @@ reset
>  switch_cg $cgdir/$seq-cg
>  $XFS_IO_PROG -c "pread 0 $iosize" $SCRATCH_MNT/file >> $seqres.full 2>&1
>  switch_cg $cgdir/$seq-cg-2
> -$XFS_IO_PROG -c "pread 0 $iosize" -c "pwrite 0 $iosize" $SCRATCH_MNT/file \
> +$XFS_IO_PROG -c "pread 0 $iosize" -c "pwrite -b $blksize 0 $iosize" $SCRATCH_MNT/file \
>  	>> $seqres.full 2>&1
>  switch_cg $cgdir
>  $XFS_IO_PROG -c fsync $SCRATCH_MNT/file
> -- 
> 2.46.0
>