diff mbox

xfs: Test infinite loop while searching for a free inode slot

Message ID 20170815125541.12243-1-cmaiolino@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Carlos Maiolino Aug. 15, 2017, 12:55 p.m. UTC
Tests the search algorithm for a free inode slot in a specific AG, done
in xfs_dialloc_ag_inobt().

When finobt is not used, and agi->freecount is not 0, XFS will scan the AG inode
tree looking for a free inode slot, but if agi->freecount is corrupted,
and there is no free slot at all, it will end up in an infinite loop.

This test checks for the infinite loop fix.

Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---
 tests/xfs/057     | 80 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/xfs/057.out |  2 ++
 tests/xfs/group   |  1 +
 3 files changed, 83 insertions(+)
 create mode 100755 tests/xfs/057
 create mode 100644 tests/xfs/057.out

Comments

Eryu Guan Aug. 16, 2017, 7:02 a.m. UTC | #1
Hi Carlos,

On Tue, Aug 15, 2017 at 02:55:41PM +0200, Carlos Maiolino wrote:
> Tests the search algorithm for a free inode slot in a specific AG, done
> in xfs_dialloc_ag_inobt().
> 
> When finobt is not used, and agi->freecount is not 0, XFS will scan the AG inode
> tree looking for a free inode slot, but if agi->freecount is corrupted,
> and there is no free slot at all, it will end up in an infinite loop.
> 
> This test checks for the infinite loop fix.

The test passed for me with 4.13-rc kernel with latest upstream
xfsprogs, but I expected it to fail (hang). Does it hang for you?

Some comments inline.

> 
> Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
> ---
>  tests/xfs/057     | 80 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/xfs/057.out |  2 ++
>  tests/xfs/group   |  1 +
>  3 files changed, 83 insertions(+)
>  create mode 100755 tests/xfs/057
>  create mode 100644 tests/xfs/057.out
> 
> diff --git a/tests/xfs/057 b/tests/xfs/057
> new file mode 100755
> index 00000000..446ea1eb
> --- /dev/null
> +++ b/tests/xfs/057
> @@ -0,0 +1,80 @@
> +#! /bin/bash
> +# FS QA Test 057
> +#
> +# Check if the filesystem will lockup when trying to allocate a new inode in
> +# an AG with no free inodes but with a corrupted agi->freecount showing free inodes. 

Trailing whitespace in above line :)

> +#
> +# At the end of the test, the scratch device will purposely be in a corrupted
> +# state, so there is no need for checking that.
> +#-----------------------------------------------------------------------
> +# Copyright (c) 2017 Red Hat, Inc.  All Rights Reserved.
> +#
> +# This program is free software; you can redistribute it and/or
> +# modify it under the terms of the GNU General Public License as
> +# published by the Free Software Foundation.
> +#
> +# This program is distributed in the hope that it would be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program; if not, write the Free Software Foundation,
> +# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
> +#-----------------------------------------------------------------------
> +#
> +
> +seq=`basename $0`
> +seqres=$RESULT_DIR/$seq
> +echo "QA output created by $seq"
> +
> +here=`pwd`
> +tmp=/tmp/$$
> +status=1	# failure is the default!
> +trap "_cleanup; exit \$status" 0 1 2 3 15
> +
> +_cleanup()
> +{
> +	cd /
> +	rm -f $tmp.*
> +}
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/filter
> +
> +# remove previous $seqres.full before test
> +rm -f $seqres.full
> +
> +# real QA test starts here
> +
> +# Modify as appropriate.
> +_supported_fs generic
> +_supported_os Linux
> +#_require_scratch

Can be removed.

> +_require_scratch_nocheck
> +
> +_scratch_mkfs_xfs -mfinobt=0 >/dev/null 2>&1 \
> +	|| _notrun "Failed to initialize filesystem."

Hmm, I'd use _require_xfs_finobt to _notrun on old kernel/userspace
where finobt feature is not supported, i.e.

_require_scratch_nocheck
_require_xfs_finobt

_scratch_mkfs -mfinobt=0 >>$seqres.full 2>&1

> +_scratch_mount >/dev/null 2>&1

This mount should succeed, so I'd drop the I/O redirection, just let the
error message fail the test if there's a mount failure.

> +
> +# At the end of filesystem's initialization, AG 0 will still have 61 free
> +# inodes in the agi->freecount, create 61 extra dummy files to fill it up
> +for i in `seq 1 61`; do
> +	touch $SCRATCH_MNT/dummy_file$1
                                    ^^^ $i not $1
so I know why I can't reproduce the hang now, fixing this typo works for
me :)

> +done

How about getting the freecount number via xfs_db (before
_scratch_mount) instead of assmuing it always be 61?

freecount=`_scratch_xfs_db -c "agi 0" -c "p freecount" | cut -d' ' -f 3`

> +
> +_scratch_unmount
> +
> +# agi->freecount is 0 here, corrupt it to show extra free inodes
> +$XFS_DB_PROG -x -c "agi 0" -c "write -d freecount 10" $SCRATCH_DEV >/dev/null 2>&1

redirect stdout and stderr to $seqres.full for debug purpose

> +
> +_scratch_mount >/dev/null 2>&1

No need to drop stdout and stderr either.

> +
> +# Lock up a buggy kernel
> +touch $SCRATCH_MNT/lockupfile

Would this touch return any error message if the kernel bug is fixed? If
so, I'd drop stdout/stderr of touch.

> +
> +# If we reach this point, the filesystem is fixed
> +echo "Silence is golden"
> +status=0
> +exit
> diff --git a/tests/xfs/057.out b/tests/xfs/057.out
> new file mode 100644
> index 00000000..185023c7
> --- /dev/null
> +++ b/tests/xfs/057.out
> @@ -0,0 +1,2 @@
> +QA output created by 057
> +Silence is golden
> diff --git a/tests/xfs/group b/tests/xfs/group
> index cf876a29..3ca8e75a 100644
> --- a/tests/xfs/group
> +++ b/tests/xfs/group
> @@ -54,6 +54,7 @@
>  054 auto quick
>  055 dump ioctl remote tape
>  056 dump ioctl auto quick
> +057 dangerous

I think it's a good fit to auto and quick group too, and maybe fuzzers
group.

Thanks,
Eryu

>  059 dump ioctl auto quick
>  060 dump ioctl auto quick
>  061 dump ioctl auto quick
> -- 
> 2.13.5
> 
> --
> 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 fstests" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Eryu Guan Aug. 16, 2017, 7:31 a.m. UTC | #2
On Wed, Aug 16, 2017 at 03:02:14PM +0800, Eryu Guan wrote:
> > +
> > +_scratch_unmount
> > +
> > +# agi->freecount is 0 here, corrupt it to show extra free inodes
> > +$XFS_DB_PROG -x -c "agi 0" -c "write -d freecount 10" $SCRATCH_DEV >/dev/null 2>&1
> 
> redirect stdout and stderr to $seqres.full for debug purpose

And you can use _scratch_xfs_db wrapper instead of open coded xfs_db
command.

And I found that "write -d" failed on a crc=0 XFS, though "write -c"
allows corrupted data and bad CRC, but touch couldn't reproduce the hang
in this case, because kernel detected bad crc first and aborted inode
creation.

I think we can force a CRC enabled XFS by

_scratch_mkfs -m crc=1,finobt=0 >>$seqres.full 2>&1

Thanks,
Eryu
--
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
Darrick J. Wong Aug. 16, 2017, 3:53 p.m. UTC | #3
On Wed, Aug 16, 2017 at 03:31:59PM +0800, Eryu Guan wrote:
> On Wed, Aug 16, 2017 at 03:02:14PM +0800, Eryu Guan wrote:
> > > +
> > > +_scratch_unmount
> > > +
> > > +# agi->freecount is 0 here, corrupt it to show extra free inodes
> > > +$XFS_DB_PROG -x -c "agi 0" -c "write -d freecount 10" $SCRATCH_DEV >/dev/null 2>&1
> > 
> > redirect stdout and stderr to $seqres.full for debug purpose
> 
> And you can use _scratch_xfs_db wrapper instead of open coded xfs_db
> command.
> 
> And I found that "write -d" failed on a crc=0 XFS, though "write -c"
> allows corrupted data and bad CRC, but touch couldn't reproduce the hang
> in this case, because kernel detected bad crc first and aborted inode
> creation.

What error did you get, specifically?

Sounds more like a bug in write/fuzz -d to me. :)

--D

> 
> I think we can force a CRC enabled XFS by
> 
> _scratch_mkfs -m crc=1,finobt=0 >>$seqres.full 2>&1
> 
> Thanks,
> Eryu
> --
> 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 fstests" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Eryu Guan Aug. 17, 2017, 3:19 a.m. UTC | #4
On Wed, Aug 16, 2017 at 08:53:07AM -0700, Darrick J. Wong wrote:
> On Wed, Aug 16, 2017 at 03:31:59PM +0800, Eryu Guan wrote:
> > On Wed, Aug 16, 2017 at 03:02:14PM +0800, Eryu Guan wrote:
> > > > +
> > > > +_scratch_unmount
> > > > +
> > > > +# agi->freecount is 0 here, corrupt it to show extra free inodes
> > > > +$XFS_DB_PROG -x -c "agi 0" -c "write -d freecount 10" $SCRATCH_DEV >/dev/null 2>&1
> > > 
> > > redirect stdout and stderr to $seqres.full for debug purpose
> > 
> > And you can use _scratch_xfs_db wrapper instead of open coded xfs_db
> > command.
> > 
> > And I found that "write -d" failed on a crc=0 XFS, though "write -c"
> > allows corrupted data and bad CRC, but touch couldn't reproduce the hang
> > in this case, because kernel detected bad crc first and aborted inode
> > creation.
> 
> What error did you get, specifically?
> 
> Sounds more like a bug in write/fuzz -d to me. :)

"Cannot recalculate CRCs on this type of object"

Though there's no crc enabled..

[root@dhcp-66-86-11 xfstests]# mkfs -t xfs -m crc=0 -f /dev/sda5
meta-data=/dev/sda5              isize=256    agcount=4, agsize=983040 blks
         =                       sectsz=512   attr=2, projid32bit=1
         =                       crc=0        finobt=0, sparse=0, rmapbt=0, reflink=0
data     =                       bsize=4096   blocks=3932160, imaxpct=25
         =                       sunit=0      swidth=0 blks
naming   =version 2              bsize=4096   ascii-ci=0 ftype=1
log      =internal log           bsize=4096   blocks=2560, version=2
         =                       sectsz=512   sunit=0 blks, lazy-count=1
realtime =none                   extsz=4096   blocks=0, rtextents=0
[root@dhcp-66-86-11 xfstests]# xfs_db -c "agi 0" -c "p freecount" /dev/sda5
freecount = 61
[root@dhcp-66-86-11 xfstests]# xfs_db -x -c "agi 0" -c "write -d freecount 10" /dev/sda5
Cannot recalculate CRCs on this type of object
[root@dhcp-66-86-11 xfstests]# xfs_db -c "agi 0" -c "p freecount" /dev/sda5
freecount = 61

Thanks,
Eryu

> 
> --D
> 
> > 
> > I think we can force a CRC enabled XFS by
> > 
> > _scratch_mkfs -m crc=1,finobt=0 >>$seqres.full 2>&1
> > 
> > Thanks,
> > Eryu
> > --
> > 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 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 fstests" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Carlos Maiolino Aug. 17, 2017, 8:38 a.m. UTC | #5
On Wed, Aug 16, 2017 at 03:02:14PM +0800, Eryu Guan wrote:
> Hi Carlos,
> 
> On Tue, Aug 15, 2017 at 02:55:41PM +0200, Carlos Maiolino wrote:
> > Tests the search algorithm for a free inode slot in a specific AG, done
> > in xfs_dialloc_ag_inobt().
> > 
> > When finobt is not used, and agi->freecount is not 0, XFS will scan the AG inode
> > tree looking for a free inode slot, but if agi->freecount is corrupted,
> > and there is no free slot at all, it will end up in an infinite loop.
> > 
> > This test checks for the infinite loop fix.
> 
> The test passed for me with 4.13-rc kernel with latest upstream
> xfsprogs, but I expected it to fail (hang). Does it hang for you?
> 

Hmm, yes, it passes to me too, weird, the bug is still there, I'll figure it out
and send a V2


> Some comments inline.
> 
> > 
> > Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
> > ---
> >  tests/xfs/057     | 80 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  tests/xfs/057.out |  2 ++
> >  tests/xfs/group   |  1 +
> >  3 files changed, 83 insertions(+)
> >  create mode 100755 tests/xfs/057
> >  create mode 100644 tests/xfs/057.out
> > 
> > diff --git a/tests/xfs/057 b/tests/xfs/057
> > new file mode 100755
> > index 00000000..446ea1eb
> > --- /dev/null
> > +++ b/tests/xfs/057
> > @@ -0,0 +1,80 @@
> > +#! /bin/bash
> > +# FS QA Test 057
> > +#
> > +# Check if the filesystem will lockup when trying to allocate a new inode in
> > +# an AG with no free inodes but with a corrupted agi->freecount showing free inodes. 
> 
> Trailing whitespace in above line :)
> 
> > +#
> > +# At the end of the test, the scratch device will purposely be in a corrupted
> > +# state, so there is no need for checking that.
> > +#-----------------------------------------------------------------------
> > +# Copyright (c) 2017 Red Hat, Inc.  All Rights Reserved.
> > +#
> > +# This program is free software; you can redistribute it and/or
> > +# modify it under the terms of the GNU General Public License as
> > +# published by the Free Software Foundation.
> > +#
> > +# This program is distributed in the hope that it would be useful,
> > +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > +# GNU General Public License for more details.
> > +#
> > +# You should have received a copy of the GNU General Public License
> > +# along with this program; if not, write the Free Software Foundation,
> > +# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
> > +#-----------------------------------------------------------------------
> > +#
> > +
> > +seq=`basename $0`
> > +seqres=$RESULT_DIR/$seq
> > +echo "QA output created by $seq"
> > +
> > +here=`pwd`
> > +tmp=/tmp/$$
> > +status=1	# failure is the default!
> > +trap "_cleanup; exit \$status" 0 1 2 3 15
> > +
> > +_cleanup()
> > +{
> > +	cd /
> > +	rm -f $tmp.*
> > +}
> > +
> > +# get standard environment, filters and checks
> > +. ./common/rc
> > +. ./common/filter
> > +
> > +# remove previous $seqres.full before test
> > +rm -f $seqres.full
> > +
> > +# real QA test starts here
> > +
> > +# Modify as appropriate.
> > +_supported_fs generic
> > +_supported_os Linux
> > +#_require_scratch
> 
> Can be removed.
> 
> > +_require_scratch_nocheck
> > +
> > +_scratch_mkfs_xfs -mfinobt=0 >/dev/null 2>&1 \
> > +	|| _notrun "Failed to initialize filesystem."
> 
> Hmm, I'd use _require_xfs_finobt to _notrun on old kernel/userspace
> where finobt feature is not supported, i.e.
> 
> _require_scratch_nocheck
> _require_xfs_finobt
> 
> _scratch_mkfs -mfinobt=0 >>$seqres.full 2>&1
> 
> > +_scratch_mount >/dev/null 2>&1
> 
> This mount should succeed, so I'd drop the I/O redirection, just let the
> error message fail the test if there's a mount failure.
> 
> > +
> > +# At the end of filesystem's initialization, AG 0 will still have 61 free
> > +# inodes in the agi->freecount, create 61 extra dummy files to fill it up
> > +for i in `seq 1 61`; do
> > +	touch $SCRATCH_MNT/dummy_file$1
>                                     ^^^ $i not $1
> so I know why I can't reproduce the hang now, fixing this typo works for
> me :)
> 
> > +done
> 
> How about getting the freecount number via xfs_db (before
> _scratch_mount) instead of assmuing it always be 61?
> 
> freecount=`_scratch_xfs_db -c "agi 0" -c "p freecount" | cut -d' ' -f 3`
> 
> > +
> > +_scratch_unmount
> > +
> > +# agi->freecount is 0 here, corrupt it to show extra free inodes
> > +$XFS_DB_PROG -x -c "agi 0" -c "write -d freecount 10" $SCRATCH_DEV >/dev/null 2>&1
> 
> redirect stdout and stderr to $seqres.full for debug purpose
> 
> > +
> > +_scratch_mount >/dev/null 2>&1
> 
> No need to drop stdout and stderr either.
> 
> > +
> > +# Lock up a buggy kernel
> > +touch $SCRATCH_MNT/lockupfile
> 
> Would this touch return any error message if the kernel bug is fixed? If
> so, I'd drop stdout/stderr of touch.
> 
> > +
> > +# If we reach this point, the filesystem is fixed
> > +echo "Silence is golden"
> > +status=0
> > +exit
> > diff --git a/tests/xfs/057.out b/tests/xfs/057.out
> > new file mode 100644
> > index 00000000..185023c7
> > --- /dev/null
> > +++ b/tests/xfs/057.out
> > @@ -0,0 +1,2 @@
> > +QA output created by 057
> > +Silence is golden
> > diff --git a/tests/xfs/group b/tests/xfs/group
> > index cf876a29..3ca8e75a 100644
> > --- a/tests/xfs/group
> > +++ b/tests/xfs/group
> > @@ -54,6 +54,7 @@
> >  054 auto quick
> >  055 dump ioctl remote tape
> >  056 dump ioctl auto quick
> > +057 dangerous
> 
> I think it's a good fit to auto and quick group too, and maybe fuzzers
> group.
> 
> Thanks,
> Eryu
> 
> >  059 dump ioctl auto quick
> >  060 dump ioctl auto quick
> >  061 dump ioctl auto quick
> > -- 
> > 2.13.5
> > 
> > --
> > 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
Carlos Maiolino Aug. 17, 2017, 8:52 a.m. UTC | #6
> The test passed for me with 4.13-rc kernel with latest upstream
> xfsprogs, but I expected it to fail (hang). Does it hang for you?
> 

Found the problem.

> > +seq=`basename $0`
> > +seqres=$RESULT_DIR/$seq
> > +echo "QA output created by $seq"
> > +
>                                     ^^^ $i not $1
> so I know why I can't reproduce the hang now, fixing this typo works for
> me :)
> 

Yeah, pretty much what I found too :P When rebasing my test agains the updated
tree and using a more meaningful name to these dummy files, I messed up with it
:P

I'll fix up the remaining things and send a V2
> > +done
> 
> How about getting the freecount number via xfs_db (before
> _scratch_mount) instead of assmuing it always be 61?
> 
> freecount=`_scratch_xfs_db -c "agi 0" -c "p freecount" | cut -d' ' -f 3`
> 

> > +
> > +_scratch_unmount
> > +
> > +# agi->freecount is 0 here, corrupt it to show extra free inodes
> > +$XFS_DB_PROG -x -c "agi 0" -c "write -d freecount 10" $SCRATCH_DEV >/dev/null 2>&1
> 
> redirect stdout and stderr to $seqres.full for debug purpose
> 
> > +
> > +_scratch_mount >/dev/null 2>&1
> 
> No need to drop stdout and stderr either.
> 
> > +
> > +# Lock up a buggy kernel
> > +touch $SCRATCH_MNT/lockupfile
> 
> Would this touch return any error message if the kernel bug is fixed? If
> so, I'd drop stdout/stderr of touch.
> 

This should shut down the filesystem because xfs will catch the corrupted
freecount.

> > +
> > +# If we reach this point, the filesystem is fixed
> > +echo "Silence is golden"
> > +status=0
> > +exit
> > diff --git a/tests/xfs/057.out b/tests/xfs/057.out
> > new file mode 100644
> > index 00000000..185023c7
> > --- /dev/null
> > +++ b/tests/xfs/057.out
> > @@ -0,0 +1,2 @@
> > +QA output created by 057
> > +Silence is golden
> > diff --git a/tests/xfs/group b/tests/xfs/group
> > index cf876a29..3ca8e75a 100644
> > --- a/tests/xfs/group
> > +++ b/tests/xfs/group
> > @@ -54,6 +54,7 @@
> >  054 auto quick
> >  055 dump ioctl remote tape
> >  056 dump ioctl auto quick
> > +057 dangerous
> 
> I think it's a good fit to auto and quick group too, and maybe fuzzers
> group.
> 

May I ask what's the 'official' purpose of auto group? I agree of adding it to
fuzzers, but 'auto' group sounds to me like a group of tests being run by
default, which wouldn't be a good idea in this case.


> Thanks,
> Eryu
> 
> >  059 dump ioctl auto quick
> >  060 dump ioctl auto quick
> >  061 dump ioctl auto quick
> > -- 
> > 2.13.5
> > 
> > --
> > 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
Eryu Guan Aug. 17, 2017, 9:19 a.m. UTC | #7
On Thu, Aug 17, 2017 at 10:52:32AM +0200, Carlos Maiolino wrote:
> > The test passed for me with 4.13-rc kernel with latest upstream
> > xfsprogs, but I expected it to fail (hang). Does it hang for you?
> > 
> 
> Found the problem.
> 
> > > +seq=`basename $0`
> > > +seqres=$RESULT_DIR/$seq
> > > +echo "QA output created by $seq"
> > > +
> >                                     ^^^ $i not $1
> > so I know why I can't reproduce the hang now, fixing this typo works for
> > me :)
> > 
> 
> Yeah, pretty much what I found too :P When rebasing my test agains the updated
> tree and using a more meaningful name to these dummy files, I messed up with it
> :P
> 
> I'll fix up the remaining things and send a V2
> > > +done
> > 
> > How about getting the freecount number via xfs_db (before
> > _scratch_mount) instead of assmuing it always be 61?
> > 
> > freecount=`_scratch_xfs_db -c "agi 0" -c "p freecount" | cut -d' ' -f 3`
> > 
> 
> > > +
> > > +_scratch_unmount
> > > +
> > > +# agi->freecount is 0 here, corrupt it to show extra free inodes
> > > +$XFS_DB_PROG -x -c "agi 0" -c "write -d freecount 10" $SCRATCH_DEV >/dev/null 2>&1
> > 
> > redirect stdout and stderr to $seqres.full for debug purpose
> > 
> > > +
> > > +_scratch_mount >/dev/null 2>&1
> > 
> > No need to drop stdout and stderr either.
> > 
> > > +
> > > +# Lock up a buggy kernel
> > > +touch $SCRATCH_MNT/lockupfile
> > 
> > Would this touch return any error message if the kernel bug is fixed? If
> > so, I'd drop stdout/stderr of touch.
> > 
> 
> This should shut down the filesystem because xfs will catch the corrupted
> freecount.
> 
> > > +
> > > +# If we reach this point, the filesystem is fixed
> > > +echo "Silence is golden"
> > > +status=0
> > > +exit
> > > diff --git a/tests/xfs/057.out b/tests/xfs/057.out
> > > new file mode 100644
> > > index 00000000..185023c7
> > > --- /dev/null
> > > +++ b/tests/xfs/057.out
> > > @@ -0,0 +1,2 @@
> > > +QA output created by 057
> > > +Silence is golden
> > > diff --git a/tests/xfs/group b/tests/xfs/group
> > > index cf876a29..3ca8e75a 100644
> > > --- a/tests/xfs/group
> > > +++ b/tests/xfs/group
> > > @@ -54,6 +54,7 @@
> > >  054 auto quick
> > >  055 dump ioctl remote tape
> > >  056 dump ioctl auto quick
> > > +057 dangerous
> > 
> > I think it's a good fit to auto and quick group too, and maybe fuzzers
> > group.
> > 
> 
> May I ask what's the 'official' purpose of auto group? I agree of adding it to
> fuzzers, but 'auto' group sounds to me like a group of tests being run by
> default, which wouldn't be a good idea in this case.

I found one email I sent before that explained the auto group
definition, I copied & pasted it here:

"
I searched for Dave's explainations on 'auto' group in his reviews, and
got the following definitions:

- it should be a valid & reliable test (it's finished and have
  deterministic output) [1]
- it passes on current upstream kernels, if it fails, it's likely to be
  resolved in forseeable future [2]
- it should take no longer than 5 minutes to finish [3]

[1] http://www.spinics.net/lists/fstests/msg00938.html
[2] http://www.spinics.net/lists/fstests/msg01548.html
[3] http://www.spinics.net/lists/fstests/msg01719.html
"

So I think it's a good 'auto' test :)

Thanks,
Eryu
--
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
Carlos Maiolino Aug. 17, 2017, 9:33 a.m. UTC | #8
> > 
> > May I ask what's the 'official' purpose of auto group? I agree of adding it to
> > fuzzers, but 'auto' group sounds to me like a group of tests being run by
> > default, which wouldn't be a good idea in this case.
> 
> I found one email I sent before that explained the auto group
> definition, I copied & pasted it here:
> 
> "
> I searched for Dave's explainations on 'auto' group in his reviews, and
> got the following definitions:
> 
> - it should be a valid & reliable test (it's finished and have
>   deterministic output) [1]
> - it passes on current upstream kernels, if it fails, it's likely to be
>   resolved in forseeable future [2]
> - it should take no longer than 5 minutes to finish [3]
> 
> [1] http://www.spinics.net/lists/fstests/msg00938.html
> [2] http://www.spinics.net/lists/fstests/msg01548.html
> [3] http://www.spinics.net/lists/fstests/msg01719.html
> "
> 
> So I think it's a good 'auto' test :)
> 

Sounds fair :) thanks I don't remember to have seen it before

> Thanks,
> Eryu
diff mbox

Patch

diff --git a/tests/xfs/057 b/tests/xfs/057
new file mode 100755
index 00000000..446ea1eb
--- /dev/null
+++ b/tests/xfs/057
@@ -0,0 +1,80 @@ 
+#! /bin/bash
+# FS QA Test 057
+#
+# Check if the filesystem will lockup when trying to allocate a new inode in
+# an AG with no free inodes but with a corrupted agi->freecount showing free inodes. 
+#
+# At the end of the test, the scratch device will purposely be in a corrupted
+# state, so there is no need for checking that.
+#-----------------------------------------------------------------------
+# Copyright (c) 2017 Red Hat, Inc.  All Rights Reserved.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write the Free Software Foundation,
+# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+#-----------------------------------------------------------------------
+#
+
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1	# failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	cd /
+	rm -f $tmp.*
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+
+# remove previous $seqres.full before test
+rm -f $seqres.full
+
+# real QA test starts here
+
+# Modify as appropriate.
+_supported_fs generic
+_supported_os Linux
+#_require_scratch
+_require_scratch_nocheck
+
+_scratch_mkfs_xfs -mfinobt=0 >/dev/null 2>&1 \
+	|| _notrun "Failed to initialize filesystem."
+_scratch_mount >/dev/null 2>&1
+
+# At the end of filesystem's initialization, AG 0 will still have 61 free
+# inodes in the agi->freecount, create 61 extra dummy files to fill it up
+for i in `seq 1 61`; do
+	touch $SCRATCH_MNT/dummy_file$1
+done
+
+_scratch_unmount
+
+# agi->freecount is 0 here, corrupt it to show extra free inodes
+$XFS_DB_PROG -x -c "agi 0" -c "write -d freecount 10" $SCRATCH_DEV >/dev/null 2>&1
+
+_scratch_mount >/dev/null 2>&1
+
+# Lock up a buggy kernel
+touch $SCRATCH_MNT/lockupfile
+
+# If we reach this point, the filesystem is fixed
+echo "Silence is golden"
+status=0
+exit
diff --git a/tests/xfs/057.out b/tests/xfs/057.out
new file mode 100644
index 00000000..185023c7
--- /dev/null
+++ b/tests/xfs/057.out
@@ -0,0 +1,2 @@ 
+QA output created by 057
+Silence is golden
diff --git a/tests/xfs/group b/tests/xfs/group
index cf876a29..3ca8e75a 100644
--- a/tests/xfs/group
+++ b/tests/xfs/group
@@ -54,6 +54,7 @@ 
 054 auto quick
 055 dump ioctl remote tape
 056 dump ioctl auto quick
+057 dangerous
 059 dump ioctl auto quick
 060 dump ioctl auto quick
 061 dump ioctl auto quick