diff mbox

[fstests] hardcoded values in common/rc for xfs (and probably others)

Message ID 20171020170558.GY4755@magnolia (mailing list archive)
State Not Applicable
Headers show

Commit Message

Darrick J. Wong Oct. 20, 2017, 5:05 p.m. UTC 
On Fri, Oct 20, 2017 at 07:52:31PM +0300, Anatoly Pugachev wrote:
> Hello!
> 
> While running generic/012 on xfs filesystem created on sparc64 linux
> (pagesize 8k) with "mkfs.xfs -b size=8192 /dev/vdiskb1" (but not with
> the default mkfs.xfs which will use 4k block size) , I'm getting the
> following output:
> 
> root@ttip:xfstests-dev# cat local.config
> export TEST_DIR=/testvol
> export TEST_DEV=/dev/vdiskb1
> export SCRATCH_DEV_POOL="/dev/loop0 /dev/loop1 /dev/loop2"
> export SCRATCH_MNT=/1/scratch
> export MKFS_OPTIONS="-m reflink=1"
> 
> root@ttip:xfstests-dev# mkfs.xfs -b size=8192 -m reflink=1 -f /dev/vdiskb1
> meta-data=/dev/vdiskb1           isize=512    agcount=4, agsize=983008 blks
>          =                       sectsz=8192  attr=2, projid32bit=1
>          =                       crc=1        finobt=1, sparse=0,
> rmapbt=0, reflink=1
> data     =                       bsize=8192   blocks=3932032, imaxpct=25
>          =                       sunit=0      swidth=0 blks
> naming   =version 2              bsize=8192   ascii-ci=0 ftype=1
> log      =internal log           bsize=8192   blocks=2160, version=2
>          =                       sectsz=8192  sunit=1 blks, lazy-count=1
> realtime =none                   extsz=8192   blocks=0, rtextents=0
> 
> root@ttip:xfstests-dev# ./check generic/012
> FSTYP         -- xfs (debug)
> PLATFORM      -- Linux/sparc64 ttip 4.14.0-rc5
> MKFS_OPTIONS  -- -f -m reflink=1 /dev/loop0
> MOUNT_OPTIONS -- /dev/loop0 /1/scratch
> 
> generic/012 2s ... [not run] xfs_io fcollapse failed (old kernel/wrong
> fs/bad args?)
> Not run: generic/012
> Passed all 0 tests
> 
> root@ttip:xfstests-dev# xfs_io -V
> xfs_io version 4.13.1
> 
> 
> debugging it, comes from the following xfs_io command:
> 
> root@ttip:xfstests-dev# mount /dev/vdiskb1 /testvol/
> root@ttip:xfstests-dev# /opt/xfsprogs/sbin/xfs_io -i -F -f -c "pwrite
> 0 20k" -c fsync -c "fcollapse 4k 8k" /testvol/248329.xfs_io 2>&1
> wrote 20480/20480 bytes at offset 0
> 20 KiB, 3 ops; 0.0000 sec (95.741 MiB/sec and 14705.8824 ops/sec)
> fallocate: Invalid argument
> 
> 
> While chatting in #xfs irc channel, Darrick told that there is should
> not be hardcoded values (like 4k for fcollapse) in generic/012, but it
> comes from _require_xfs_io_command() from common/rc, quote:
> 
> 19:19 < djwong> anyway... fcollapse (and finsert) both requires that
> the offset/length arguments are block-aligned
> 19:19 < djwong> hence you can't fcollapse starting at 4k on an fs with 8k blocks
> 
> And it's not only generic/012, it fails generic/0{16,17,21,22} (and
> probably more) as well.
> 
> Can someone look into this issue?

Does the following xfstests patch help?

--D

--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Anatoly Pugachev Oct. 20, 2017, 7:56 p.m. UTC | #1 
On Fri, Oct 20, 2017 at 8:05 PM, Darrick J. Wong
<darrick.wong@oracle.com> wrote:
> On Fri, Oct 20, 2017 at 07:52:31PM +0300, Anatoly Pugachev wrote:
>> Can someone look into this issue?
>
> Does the following xfstests patch help?
>
> --D
>
> diff --git a/common/punch b/common/punch
> index c4ed261..5648bd8 100644
> --- a/common/punch
> +++ b/common/punch
> @@ -341,13 +341,26 @@ _test_generic_punch()
>         testfile=$6
>         multiple=1
>
> +       # This routine was originally written for fallocate modes that
> +       # don't have alignment requirements so the (sort of) hardcoded
> +       # 4k offsets didn't matter.  fcollapse and finsert require
> +       # block-aligned arguments, so increase $multiple until we get
> +       # to the file's minimum data block size.
> +       case "$zero_cmd" in
> +       "fcollapse"|"finsert")
> +               touch $testfile
> +               bs=$(_get_file_block_size $testfile)
> +               test "$bs" -gt 4096 && multiple=$((bs / 4096))
> +               ;;
> +       esac
> +
>         #
>         # If we are testing collapse range, we increare all the offsets of this
>         # test by a factor of 4. We do this because unlike punch, collapse
>         # range also decreases the size of file hence require bigger offsets.
>         #
>         if [ "$zero_cmd" == "fcollapse" ]; then
> -               multiple=4
> +               multiple=$((multiple * 4))
>         fi
>
>         _4k="$((multiple * 4))k"
> diff --git a/common/rc b/common/rc
> index fe68d67..b585016 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -2063,8 +2063,8 @@ _require_xfs_io_command()
>                 param_checked=1
>                 ;;
>         "fpunch" | "fcollapse" | "zero" | "fzero" | "finsert" | "funshare")
> -               testio=`$XFS_IO_PROG -F -f -c "pwrite 0 20k" -c "fsync" \
> -                       -c "$command 4k 8k" $testfile 2>&1`
> +               testio=`$XFS_IO_PROG -F -f -c "pwrite 0 256k" -c "fsync" \
> +                       -c "$command 64k 128k" $testfile 2>&1`
>                 ;;
>         "fiemap")
>                 testio=`$XFS_IO_PROG -F -f -c "pwrite 0 20k" -c "fsync" \

Darrick,

I had to change from
bs=$(_get_file_block_size $testfile)
to
bs=$(_get_file_block_size $testdir)

since _get_file_block_size gets directory as a parameter (and i'm
puzzled why it's called _get_file_block_size, if it really wants a
directory as a parameter, and there's _get_block_size as well).

the patch makes test as passed, but with error.

For example, with the default mkfs.xfs (4k block size), it is executed
as follows:

root@ttip:xfstests-dev# mkfs.xfs -f /dev/vdiskb1
specified blocksize 4096 is less than device physical sector size 8192
switching to logical sector size 512
meta-data=/dev/vdiskb1           isize=512    agcount=4, agsize=1966016 blks
         =                       sectsz=512   attr=2, projid32bit=1
         =                       crc=1        finobt=1, sparse=0,
rmapbt=0, reflink=0
data     =                       bsize=4096   blocks=7864064, imaxpct=25
         =                       sunit=0      swidth=0 blks
naming   =version 2              bsize=4096   ascii-ci=0 ftype=1
log      =internal log           bsize=4096   blocks=3839, version=2
         =                       sectsz=512   sunit=0 blks, lazy-count=1
realtime =none                   extsz=4096   blocks=0, rtextents=0
root@ttip:xfstests-dev# ./check  generic/012
FSTYP         -- xfs (debug)
PLATFORM      -- Linux/sparc64 ttip 4.14.0-rc5
MKFS_OPTIONS  -- -f -m reflink=1 /dev/loop0
MOUNT_OPTIONS -- /dev/loop0 /1/scratch

generic/012 2s ... 1s
Ran: generic/012
Passed all 1 tests


but if I change block size, it ends with an error:


root@ttip:xfstests-dev# mkfs.xfs -f -b size=8192 /dev/vdiskb1
meta-data=/dev/vdiskb1           isize=512    agcount=4, agsize=983008 blks
         =                       sectsz=8192  attr=2, projid32bit=1
         =                       crc=1        finobt=1, sparse=0,
rmapbt=0, reflink=0
data     =                       bsize=8192   blocks=3932032, imaxpct=25
         =                       sunit=0      swidth=0 blks
naming   =version 2              bsize=8192   ascii-ci=0 ftype=1
log      =internal log           bsize=8192   blocks=1919, version=2
         =                       sectsz=8192  sunit=1 blks, lazy-count=1
realtime =none                   extsz=8192   blocks=0, rtextents=0
root@ttip:xfstests-dev# ./check  generic/012
FSTYP         -- xfs (debug)
PLATFORM      -- Linux/sparc64 ttip 4.14.0-rc5
MKFS_OPTIONS  -- -f -m reflink=1 /dev/loop0
MOUNT_OPTIONS -- /dev/loop0 /1/scratch

generic/012 1s ... - output mismatch (see
/1/mator/xfstests-dev/results/generic/012.out.bad)
    --- tests/generic/012.out   2016-09-24 11:51:48.238890811 +0300
    +++ /1/mator/xfstests-dev/results/generic/012.out.bad
2017-10-20 21:54:51.594814663 +0300
    @@ -1,49 +1,49 @@
     QA output created by 012
        1. into a hole
    -f4f35d60b3cc18aaa6d8d92f0cd3708a
    +0a9156c4e3c48ef827980639c4d1e263
        2. into allocated space
    -0: [0..95]: extent
    -f1894a71ac539f6f90426d98a4990a47
    ...
    (Run 'diff -u tests/generic/012.out
/1/mator/xfstests-dev/results/generic/012.out.bad'  to see the entire
diff)
Ran: generic/012
Failures: generic/012
Failed 1 of 1 tests

root@ttip:xfstests-dev# cat results/generic/012.out.bad
QA output created by 012
        1. into a hole
0a9156c4e3c48ef827980639c4d1e263
        2. into allocated space
0: [0..191]: extent
6f1f8907250bdbf673e1e55212b56b82
        3. into unwritten space
0: [0..191]: extent
39bf78f7c676fdcf11558ad3a1a23aef
        4. hole -> data
0: [0..127]: extent
1: [128..191]: hole
c9a5ea7ad1648eee6202850420432dfc
        5. hole -> unwritten
0: [0..127]: extent
1: [128..191]: hole
39bf78f7c676fdcf11558ad3a1a23aef
        6. data -> hole
0: [0..63]: extent
1: [64..191]: hole
39bf78f7c676fdcf11558ad3a1a23aef
        7. data -> unwritten
0: [0..127]: extent
1: [128..191]: hole
39bf78f7c676fdcf11558ad3a1a23aef
        8. unwritten -> hole
0: [0..63]: extent
1: [64..191]: hole
39bf78f7c676fdcf11558ad3a1a23aef
        9. unwritten -> data
0: [0..127]: extent
1: [128..191]: hole
c9a5ea7ad1648eee6202850420432dfc
        10. hole -> data -> hole
0: [0..63]: extent
1: [64..127]: hole
d5755b0153a84fc6789af22ab8d1bb04
        11. data -> hole -> data
0: [0..127]: extent
27c9068d1b51da575a53ad34c57ca5cc
        12. unwritten -> data -> unwritten
0: [0..127]: extent
d5755b0153a84fc6789af22ab8d1bb04
        13. data -> unwritten -> data
0: [0..127]: extent
27c9068d1b51da575a53ad34c57ca5cc
        14. data -> hole @ 0
0: [0..191]: extent
6f1f8907250bdbf673e1e55212b56b82
root@ttip:xfstests-dev#
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Darrick J. Wong Oct. 21, 2017, 12:32 a.m. UTC | #2 
On Fri, Oct 20, 2017 at 10:56:56PM +0300, Anatoly Pugachev wrote:
> On Fri, Oct 20, 2017 at 8:05 PM, Darrick J. Wong
> <darrick.wong@oracle.com> wrote:
> > On Fri, Oct 20, 2017 at 07:52:31PM +0300, Anatoly Pugachev wrote:
> >> Can someone look into this issue?
> >
> > Does the following xfstests patch help?
> >
> > --D
> >
> > diff --git a/common/punch b/common/punch
> > index c4ed261..5648bd8 100644
> > --- a/common/punch
> > +++ b/common/punch
> > @@ -341,13 +341,26 @@ _test_generic_punch()
> >         testfile=$6
> >         multiple=1
> >
> > +       # This routine was originally written for fallocate modes that
> > +       # don't have alignment requirements so the (sort of) hardcoded
> > +       # 4k offsets didn't matter.  fcollapse and finsert require
> > +       # block-aligned arguments, so increase $multiple until we get
> > +       # to the file's minimum data block size.
> > +       case "$zero_cmd" in
> > +       "fcollapse"|"finsert")
> > +               touch $testfile
> > +               bs=$(_get_file_block_size $testfile)
> > +               test "$bs" -gt 4096 && multiple=$((bs / 4096))
> > +               ;;
> > +       esac
> > +
> >         #
> >         # If we are testing collapse range, we increare all the offsets of this
> >         # test by a factor of 4. We do this because unlike punch, collapse
> >         # range also decreases the size of file hence require bigger offsets.
> >         #
> >         if [ "$zero_cmd" == "fcollapse" ]; then
> > -               multiple=4
> > +               multiple=$((multiple * 4))
> >         fi
> >
> >         _4k="$((multiple * 4))k"
> > diff --git a/common/rc b/common/rc
> > index fe68d67..b585016 100644
> > --- a/common/rc
> > +++ b/common/rc
> > @@ -2063,8 +2063,8 @@ _require_xfs_io_command()
> >                 param_checked=1
> >                 ;;
> >         "fpunch" | "fcollapse" | "zero" | "fzero" | "finsert" | "funshare")
> > -               testio=`$XFS_IO_PROG -F -f -c "pwrite 0 20k" -c "fsync" \
> > -                       -c "$command 4k 8k" $testfile 2>&1`
> > +               testio=`$XFS_IO_PROG -F -f -c "pwrite 0 256k" -c "fsync" \
> > +                       -c "$command 64k 128k" $testfile 2>&1`
> >                 ;;
> >         "fiemap")
> >                 testio=`$XFS_IO_PROG -F -f -c "pwrite 0 20k" -c "fsync" \
> 
> Darrick,
> 
> I had to change from
> bs=$(_get_file_block_size $testfile)
> to
> bs=$(_get_file_block_size $testdir)
> 
> since _get_file_block_size gets directory as a parameter (and i'm
> puzzled why it's called _get_file_block_size, if it really wants a
> directory as a parameter, and there's _get_block_size as well).
> 
> the patch makes test as passed, but with error.
> 
> For example, with the default mkfs.xfs (4k block size), it is executed
> as follows:
> 
> root@ttip:xfstests-dev# mkfs.xfs -f /dev/vdiskb1
> specified blocksize 4096 is less than device physical sector size 8192
> switching to logical sector size 512
> meta-data=/dev/vdiskb1           isize=512    agcount=4, agsize=1966016 blks
>          =                       sectsz=512   attr=2, projid32bit=1
>          =                       crc=1        finobt=1, sparse=0,
> rmapbt=0, reflink=0
> data     =                       bsize=4096   blocks=7864064, imaxpct=25
>          =                       sunit=0      swidth=0 blks
> naming   =version 2              bsize=4096   ascii-ci=0 ftype=1
> log      =internal log           bsize=4096   blocks=3839, version=2
>          =                       sectsz=512   sunit=0 blks, lazy-count=1
> realtime =none                   extsz=4096   blocks=0, rtextents=0
> root@ttip:xfstests-dev# ./check  generic/012
> FSTYP         -- xfs (debug)
> PLATFORM      -- Linux/sparc64 ttip 4.14.0-rc5
> MKFS_OPTIONS  -- -f -m reflink=1 /dev/loop0
> MOUNT_OPTIONS -- /dev/loop0 /1/scratch
> 
> generic/012 2s ... 1s
> Ran: generic/012
> Passed all 1 tests
> 
> 
> but if I change block size, it ends with an error:
> 
> 
> root@ttip:xfstests-dev# mkfs.xfs -f -b size=8192 /dev/vdiskb1
> meta-data=/dev/vdiskb1           isize=512    agcount=4, agsize=983008 blks
>          =                       sectsz=8192  attr=2, projid32bit=1
>          =                       crc=1        finobt=1, sparse=0,
> rmapbt=0, reflink=0
> data     =                       bsize=8192   blocks=3932032, imaxpct=25
>          =                       sunit=0      swidth=0 blks
> naming   =version 2              bsize=8192   ascii-ci=0 ftype=1
> log      =internal log           bsize=8192   blocks=1919, version=2
>          =                       sectsz=8192  sunit=1 blks, lazy-count=1
> realtime =none                   extsz=8192   blocks=0, rtextents=0
> root@ttip:xfstests-dev# ./check  generic/012
> FSTYP         -- xfs (debug)
> PLATFORM      -- Linux/sparc64 ttip 4.14.0-rc5
> MKFS_OPTIONS  -- -f -m reflink=1 /dev/loop0
> MOUNT_OPTIONS -- /dev/loop0 /1/scratch
> 
> generic/012 1s ... - output mismatch (see
> /1/mator/xfstests-dev/results/generic/012.out.bad)
>     --- tests/generic/012.out   2016-09-24 11:51:48.238890811 +0300
>     +++ /1/mator/xfstests-dev/results/generic/012.out.bad
> 2017-10-20 21:54:51.594814663 +0300
>     @@ -1,49 +1,49 @@
>      QA output created by 012
>         1. into a hole
>     -f4f35d60b3cc18aaa6d8d92f0cd3708a
>     +0a9156c4e3c48ef827980639c4d1e263
>         2. into allocated space
>     -0: [0..95]: extent
>     -f1894a71ac539f6f90426d98a4990a47

Oh, drat, md5 hashes.  I guess we'll have to increase the hardcoded
blocksize to 64k and fix all the resulting test failures. :(

Will try to do that next week.

--D

>     ...
>     (Run 'diff -u tests/generic/012.out
> /1/mator/xfstests-dev/results/generic/012.out.bad'  to see the entire
> diff)
> Ran: generic/012
> Failures: generic/012
> Failed 1 of 1 tests
> 
> root@ttip:xfstests-dev# cat results/generic/012.out.bad
> QA output created by 012
>         1. into a hole
> 0a9156c4e3c48ef827980639c4d1e263
>         2. into allocated space
> 0: [0..191]: extent
> 6f1f8907250bdbf673e1e55212b56b82
>         3. into unwritten space
> 0: [0..191]: extent
> 39bf78f7c676fdcf11558ad3a1a23aef
>         4. hole -> data
> 0: [0..127]: extent
> 1: [128..191]: hole
> c9a5ea7ad1648eee6202850420432dfc
>         5. hole -> unwritten
> 0: [0..127]: extent
> 1: [128..191]: hole
> 39bf78f7c676fdcf11558ad3a1a23aef
>         6. data -> hole
> 0: [0..63]: extent
> 1: [64..191]: hole
> 39bf78f7c676fdcf11558ad3a1a23aef
>         7. data -> unwritten
> 0: [0..127]: extent
> 1: [128..191]: hole
> 39bf78f7c676fdcf11558ad3a1a23aef
>         8. unwritten -> hole
> 0: [0..63]: extent
> 1: [64..191]: hole
> 39bf78f7c676fdcf11558ad3a1a23aef
>         9. unwritten -> data
> 0: [0..127]: extent
> 1: [128..191]: hole
> c9a5ea7ad1648eee6202850420432dfc
>         10. hole -> data -> hole
> 0: [0..63]: extent
> 1: [64..127]: hole
> d5755b0153a84fc6789af22ab8d1bb04
>         11. data -> hole -> data
> 0: [0..127]: extent
> 27c9068d1b51da575a53ad34c57ca5cc
>         12. unwritten -> data -> unwritten
> 0: [0..127]: extent
> d5755b0153a84fc6789af22ab8d1bb04
>         13. data -> unwritten -> data
> 0: [0..127]: extent
> 27c9068d1b51da575a53ad34c57ca5cc
>         14. data -> hole @ 0
> 0: [0..191]: extent
> 6f1f8907250bdbf673e1e55212b56b82
> root@ttip:xfstests-dev#
> --
> To unsubscribe from this list: send the line "unsubscribe fstests" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/common/punch b/common/punch
index c4ed261..5648bd8 100644
--- a/common/punch
+++ b/common/punch
@@ -341,13 +341,26 @@  _test_generic_punch()
 	testfile=$6
 	multiple=1
 
+	# This routine was originally written for fallocate modes that
+	# don't have alignment requirements so the (sort of) hardcoded
+	# 4k offsets didn't matter.  fcollapse and finsert require
+	# block-aligned arguments, so increase $multiple until we get
+	# to the file's minimum data block size.
+	case "$zero_cmd" in
+	"fcollapse"|"finsert")
+		touch $testfile
+		bs=$(_get_file_block_size $testfile)
+		test "$bs" -gt 4096 && multiple=$((bs / 4096))
+		;;
+	esac
+
 	#
 	# If we are testing collapse range, we increare all the offsets of this
 	# test by a factor of 4. We do this because unlike punch, collapse
 	# range also decreases the size of file hence require bigger offsets.
 	#
 	if [ "$zero_cmd" == "fcollapse" ]; then
-		multiple=4
+		multiple=$((multiple * 4))
 	fi
 
 	_4k="$((multiple * 4))k"
diff --git a/common/rc b/common/rc
index fe68d67..b585016 100644
--- a/common/rc
+++ b/common/rc
@@ -2063,8 +2063,8 @@  _require_xfs_io_command()
 		param_checked=1
 		;;
 	"fpunch" | "fcollapse" | "zero" | "fzero" | "finsert" | "funshare")
-		testio=`$XFS_IO_PROG -F -f -c "pwrite 0 20k" -c "fsync" \
-			-c "$command 4k 8k" $testfile 2>&1`
+		testio=`$XFS_IO_PROG -F -f -c "pwrite 0 256k" -c "fsync" \
+			-c "$command 64k 128k" $testfile 2>&1`
 		;;
 	"fiemap")
 		testio=`$XFS_IO_PROG -F -f -c "pwrite 0 20k" -c "fsync" \