Re: [PATCH] generic/501: count with PAGE_SIZE instead of KB

[Murphy Zhou <jencce.kernel@gmail.com>]

On Wed, Jul 22, 2020 at 12:52:16PM +0100, Filipe Manana wrote:
> On Wed, Jul 22, 2020 at 12:48 PM Zorro Lang <zlang@redhat.com> wrote:
> >
> > On Wed, Jul 22, 2020 at 02:38:32PM +0800, Murphy Zhou wrote:
> > > Hard coded reflink offset/length numbers are not working well on 64K page
> > > size arch like ppc64le, reporting:
> > >       +XFS_IOC_CLONE_RANGE: Invalid argument
> > >
> > > Because they are not aligned with the current PAGE_SIZE.
> > >
> > > Translate original numbers into multiples of PAGE_SIZE. Then we're writing
> > > different testfiles with different md5sums when PAGE_SIZE is different.
> > > So we need to link different output file based on the current PAGE_SIZE.
> > >
> > > This testcase is covering a btrfs regression which has been fixed in
> > > v4.18. I tried to reproduce it on 4.17 kernel but failed with and
> > > without this patch.
> > >
> > > Signed-off-by: Murphy Zhou <jencce.kernel@gmail.com>
> > > ---
> > >  .gitignore                            |  1 +
> > >  tests/generic/501                     | 17 +++++++++++++----
> > >  tests/generic/{501.out => 501.out.4K} |  0
> > >  tests/generic/501.out.64K             |  5 +++++
> > >  4 files changed, 19 insertions(+), 4 deletions(-)
> > >  rename tests/generic/{501.out => 501.out.4K} (100%)
> > >  create mode 100644 tests/generic/501.out.64K
> > >
> > > diff --git a/.gitignore b/.gitignore
> > > index 5f5c4a0f..39318615 100644
> > > --- a/.gitignore
> > > +++ b/.gitignore
> > > @@ -263,6 +263,7 @@
> > >  /tests/xfs/033.out
> > >  /tests/xfs/071.out
> > >  /tests/xfs/096.out
> > > +/tests/generic/501.out
> > >
> > >  # cscope files
> > >  cscope.*
> > > diff --git a/tests/generic/501 b/tests/generic/501
> > > index 0d1f6ffe..4e589cda 100755
> > > --- a/tests/generic/501
> > > +++ b/tests/generic/501
> > > @@ -9,6 +9,7 @@
> > >  # the file again and then power fail, after we mount again the filesystem, no
> > >  # file data was lost or corrupted.
> > >  #
> > > +seqfull=$0
> > >  seq=`basename $0`
> > >  seqres=$RESULT_DIR/$seq
> > >  echo "QA output created by $seq"
> > > @@ -42,14 +43,22 @@ _require_metadata_journaling $SCRATCH_DEV
> > >  _init_flakey
> > >  _mount_flakey
> > >
> > > -$XFS_IO_PROG -f -c "pwrite -S 0x18 9000K 6908K" $SCRATCH_MNT/foo >>$seqres.full
> > > -$XFS_IO_PROG -f -c "pwrite -S 0x20 2572K 156K" $SCRATCH_MNT/bar >>$seqres.full
> > > +pagesz=$(getconf PAGE_SIZE)
> > > +
> > > +if [ $pagesz -eq 65536 ] ; then
> > > +     ln -sf $seqfull.out.64K $seqfull.out
> > > +elif [ $pagesz -eq 4096 ] ; then
> > > +     ln -sf $seqfull.out.4K $seqfull.out
> > > +fi
> >
> > You can't sure there're only 4k and 64k system.
> > This case trys to make sure the md5 of $SCRATCH_MNT/bar isn't changed before and
> > after _flakey_drop_and_remount. So if you turn to use different pagesize, you
> > can silence the golden image, and compare the md5 quietly. Something likes(just
> > example):
> 
> This can be simpler. The test can be changed to use multiples of 64K,

Agree! Thanks Zorro very much for suggestion!

> so it will work everywhere (like other reflink tests).
> I'll try it out and test on 4.17 to confirm it still reproduces the
> original btrfs bug.

Ya, this is important. I'm wondering if the original bug can be
reproduced by using multiples of 64k.

Thanks!
> 
> Thanks.
> 
> >
> > diff --git a/tests/generic/501 b/tests/generic/501
> > index 0d1f6ffe..7e7f9be5 100755
> > --- a/tests/generic/501
> > +++ b/tests/generic/501
> > @@ -53,18 +53,21 @@ $XFS_IO_PROG -c "fsync" \
> >              -c "fsync" \
> >              $SCRATCH_MNT/bar >>$seqres.full
> >
> > -echo "File bar digest before power failure:"
> > -md5sum $SCRATCH_MNT/bar | _filter_scratch
> > +# File bar digest before power failure
> > +md5sum $SCRATCH_MNT/bar > $tmp.md5.org
> >
> >  # Simulate a power failure and mount the filesystem to check that no file data
> >  # was lost or corrupted.
> >  _flakey_drop_and_remount
> >
> > -echo "File bar digest after power failure:"
> > -md5sum $SCRATCH_MNT/bar | _filter_scratch
> > +# File bar digest after power failure
> > +md5sum $SCRATCH_MNT/bar > $tmp.md5.new
> > +diff -u $tmp.md5.org $tmp.md5.new
> >
> >  _unmount_flakey
> >  _cleanup_flakey
> >
> > +echo "Silence is golden"
> > +
> >  status=0
> >  exit
> > diff --git a/tests/generic/501.out b/tests/generic/501.out
> > index 5d7da017..00133b63 100644
> > --- a/tests/generic/501.out
> > +++ b/tests/generic/501.out
> > @@ -1,5 +1,2 @@
> >  QA output created by 501
> > -File bar digest before power failure:
> > -95a95813a8c2abc9aa75a6c2914a077e  SCRATCH_MNT/bar
> > -File bar digest after power failure:
> > -95a95813a8c2abc9aa75a6c2914a077e  SCRATCH_MNT/bar
> > +Silence is golden
> >
> > Thanks,
> > Zorro
> >
> > > +
> > > +$XFS_IO_PROG -f -c "pwrite -S 0x18 $((2250*pagesz)) $((1727*pagesz))" $SCRATCH_MNT/foo >>$seqres.full
> > > +$XFS_IO_PROG -f -c "pwrite -S 0x20 $((643*pagesz)) $((39*pagesz))" $SCRATCH_MNT/bar >>$seqres.full
> > >
> > >  # We clone from file foo into a range of file bar that overlaps the existing
> > >  # extent at file bar. The destination offset of the reflink operation matches
> > > -# the eof position of file bar minus 4Kb.
> > > +# the eof position of file bar minus 1 PAGE_SIZE.
> > >  $XFS_IO_PROG -c "fsync" \
> > > -          -c "reflink ${SCRATCH_MNT}/foo 0 2724K 15908K" \
> > > +          -c "reflink ${SCRATCH_MNT}/foo 0 $((681*pagesz)) $((3977*pagesz))" \
> > >            -c "fsync" \
> > >            $SCRATCH_MNT/bar >>$seqres.full
> > >
> > > diff --git a/tests/generic/501.out b/tests/generic/501.out.4K
> > > similarity index 100%
> > > rename from tests/generic/501.out
> > > rename to tests/generic/501.out.4K
> > > diff --git a/tests/generic/501.out.64K b/tests/generic/501.out.64K
> > > new file mode 100644
> > > index 00000000..7e1013b2
> > > --- /dev/null
> > > +++ b/tests/generic/501.out.64K
> > > @@ -0,0 +1,5 @@
> > > +QA output created by 501
> > > +File bar digest before power failure:
> > > +3058797b969076e91c518cb206b21163  SCRATCH_MNT/bar
> > > +File bar digest after power failure:
> > > +3058797b969076e91c518cb206b21163  SCRATCH_MNT/bar
> > > --
> > > 2.20.1
> > >
> >
> 
> 
> -- 
> Filipe David Manana,
> 
> “Whether you think you can, or you think you can't — you're right.”

-- 
Murphy