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

[Bill O'Donnell <bodonnel@redhat.com>]

On Fri, Sep 29, 2023 at 01:06:51PM +0800, Zorro Lang wrote:
> 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@redhat.com>
> > ---
> 
> 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.

I'd like Qu to weigh in on this from the btrfs standpoint.

Thanks-
Bill

> 
> 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
> > 
>