Re: [PATCH v2] fstests: generic/353 should accomodate other pwrite behaviors

[Date Prev] [Date Next] [Thread Prev] [Thread Next] [Date Index] [Thread Index]



On Fri, Sep 01, 2023 at 11:18:16AM -0500, Bill O'Donnell wrote:
> xfs_io pwrite issues a series of block size writes, but there is no
> guarantee that the resulting extent(s) will be singular or contiguous.
> This behavior is acceptable, but the test is flawed in that it expects
> a single extent for a pwrite.
> 
> Modify test to use actual blocksize for pwrite and reflink. Also
> modify it to accommodate pwrite and reflink that produce different
> mapping results.
> 
> Signed-off-by: Bill O'Donnell <bodonnel@xxxxxxxxxx>
> ---

This patch makes sense to me, but this case looks like a regression test
test for a known btrfs issue. But this test case doesn't point out which
bug/fix does it test for. So I don't know if the 64k blocksize is a
necessary condition to reproduce the bug.

If we can prove it still can reproduce that bug with this patch at least,
then it's good to me to merge it.

Thanks,
Zorro

>  tests/generic/353     | 29 ++++++++++++++++-------------
>  tests/generic/353.out | 15 +--------------
>  2 files changed, 17 insertions(+), 27 deletions(-)
> 
> diff --git a/tests/generic/353 b/tests/generic/353
> index 9a1471bd..c5639725 100755
> --- a/tests/generic/353
> +++ b/tests/generic/353
> @@ -29,31 +29,34 @@ _require_xfs_io_command "fiemap"
>  _scratch_mkfs > /dev/null 2>&1
>  _scratch_mount
>  
> -blocksize=64k
> +blocksize=$(_get_file_block_size $SCRATCH_MNT)
> +
>  file1="$SCRATCH_MNT/file1"
>  file2="$SCRATCH_MNT/file2"
> +extmap1="$SCRATCH_MNT/extmap1"
> +extmap2="$SCRATCH_MNT/extmap2"
>  
>  # write the initial file
> -_pwrite_byte 0xcdcdcdcd 0 $blocksize $file1 | _filter_xfs_io
> +_pwrite_byte 0xcdcdcdcd 0 $blocksize $file1 > /dev/null
>  
>  # reflink initial file
> -_reflink_range $file1 0 $file2 0 $blocksize | _filter_xfs_io
> +_reflink_range $file1 0 $file2 0 $blocksize > /dev/null
>  
>  # check their fiemap to make sure it's correct
> -echo "before sync:"
> -echo "$file1" | _filter_scratch
> -$XFS_IO_PROG -c "fiemap -v" $file1 | _filter_fiemap_flags
> -echo "$file2" | _filter_scratch
> -$XFS_IO_PROG -c "fiemap -v" $file2 | _filter_fiemap_flags
> +$XFS_IO_PROG -c "fiemap -v" $file1 | _filter_fiemap_flags > $extmap1
> +$XFS_IO_PROG -c "fiemap -v" $file2 | _filter_fiemap_flags > $extmap2
> +
> +cmp -s $extmap1 $extmap2 || echo "mismatched extent maps before sync"
>  
>  # sync and recheck, to make sure the fiemap doesn't change just
>  # due to sync
>  sync
> -echo "after sync:"
> -echo "$file1" | _filter_scratch
> -$XFS_IO_PROG -c "fiemap -v" $file1 | _filter_fiemap_flags
> -echo "$file2" | _filter_scratch
> -$XFS_IO_PROG -c "fiemap -v" $file2 | _filter_fiemap_flags
> +$XFS_IO_PROG -c "fiemap -v" $file1 | _filter_fiemap_flags > $extmap1
> +$XFS_IO_PROG -c "fiemap -v" $file2 | _filter_fiemap_flags > $extmap2
> +
> +cmp -s $extmap1 $extmap2 || echo "mismatched extent maps after sync"
> +
> +echo "Silence is golden"
>  
>  # success, all done
>  status=0
> diff --git a/tests/generic/353.out b/tests/generic/353.out
> index 4f6e0b92..16ba4f1f 100644
> --- a/tests/generic/353.out
> +++ b/tests/generic/353.out
> @@ -1,15 +1,2 @@
>  QA output created by 353
> -wrote 65536/65536 bytes at offset 0
> -XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> -linked 65536/65536 bytes at offset 0
> -XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> -before sync:
> -SCRATCH_MNT/file1
> -0: [0..127]: shared|last
> -SCRATCH_MNT/file2
> -0: [0..127]: shared|last
> -after sync:
> -SCRATCH_MNT/file1
> -0: [0..127]: shared|last
> -SCRATCH_MNT/file2
> -0: [0..127]: shared|last
> +Silence is golden
> -- 
> 2.41.0
> 




[Index of Archives]     [Linux Filesystems Development]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux