Skip to contentSkip to conversation

xfsqa: new tests and fixes

PATCH In review Dave Chinner 3 participants 11 messages Mar 3, 2010 – Mar 4, 2010

lore
Share & tools
lore

Dave Chinner

author
#0 lore raw

cover xfsqa: new tests and fixes

Two new tests - an enospc exerciser for delalloc writes and a repost of the fiemap exerciser that Josef and Eric put together in July last year.

One fix - small scratch filesystems were not being correctly specified so they weren't being made small at all.

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

author
#1 lore raw

PATCH 1/3 xfsqa: add delayed allocation @ ENOSPC exerciser

From: Dave Chinner <[email protected]>

When we deplete the reserve block pool or receive an error during delayed allocation, we currently toss the page away. If that page has delayed allocation extents on it, we can fail to remove them and leave stale delayed llocation extents lying around on the inode. This can, in certain circumstances, trigger errors later on when the stale delalloc extent it found again, including tripping a BUG().

Exercise this failure path so that we get code coverage of the fix that prevents stale delalloc mappings from being left on the inode when pages are tossed.

This is based on a test case supplied by Lachlan McIlroy.

Signed-off-by Dave Chinner <[email protected]>
---
 224     |   88 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 224.out |    2 +
 group   |    1 +
 3 files changed, 91 insertions(+), 0 deletions(-)
 create mode 100644 224
 create mode 100644 224.out
224 new file +88 -0
new file mode 100644
index 0000000..ee01c41
@@ -0,0 +1,88 @@
+#! /bin/bash
+# FS QA Test No. 224
+#
+# Delayed allocation at ENOSPC test
+#
+# Derived from a test case from Lachlan McIlroy and improved to
+# reliably trigger a BUG in xfs_get_blocks(). Despite this XFS
+# focus, the test can to run on any filesystem to exercise ENOSPC
+# behaviour.
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2010 Dave Chinner.  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
+#
+#-----------------------------------------------------------------------
+#
+# creator
[email protected]
+
+seq=`basename $0`
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1	# failure is the default!
+
+_cleanup()
+{
+	killall -9 dd > /dev/null 2>&1
+	rm -f $tmp.*
+	rm -f $SCRATCH_MNT/testfile.*
+}
+
+trap "_cleanup ; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+
+# real QA test starts here
+_supported_fs generic
+_supported_os Linux
+
+_require_scratch
+
+# make a 1GB filesystem
+_scratch_mkfs_sized `expr 1024 \* 1024 \* 1024` > $seq.full 2>&1
+_scratch_mount >> $seq.full 2>&1
+
+# set the reserved block pool to almost empty for XFS
+if [ "$FSTYP" = "xfs" ]; then
+	xfs_io -x -c "resblks 4" /mnt/scratch/ >> $seq.full 2>&1
+fi
+
+FILES=1000
+for i in `seq 0 1 $FILES`; do
+
+	# set the file size to be 10MB - that way the direct IO will always try
+	# to read at least 10MB even if only 4k was written. This allows
+	# discarded delalloc pages that would have been beyond EOF to be
+	# tripped over.
+        (
+		sleep 5
+		xfs_io -f -c "truncate 10485760" $SCRATCH_MNT/testfile.$i
+		dd if=/dev/zero of=$SCRATCH_MNT/testfile.$i bs=4k conv=notrunc
+	) > /dev/null 2>&1 &
+done
+wait
+
+for i in `seq 0 1 $FILES`; do
+        dd of=/dev/null if=$SCRATCH_MNT/testfile.$i bs=512k iflag=direct > /dev/null 2>&1 &
+done
+wait
+echo "*** Silence is golden ***"
+status=0
+exit
224.out new file +2 -0
new file mode 100644
index 0000000..cf4fd9c
@@ -0,0 +1,2 @@
+QA output created by 224
+*** Silence is golden ***
group +1 -0
index c66d965..5d675f0 100644
@@ -337,3 +337,4 @@ deprecated
 221 auto metadata quick
 222 auto fsr ioctl quick
 223 auto quick
+224 auto
Signature · 6 lines
1.6.5

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
#2 lore raw

Re: [PATCH 1/3] xfsqa: add delayed allocation @ ENOSPC exerciser

224 line 41

Quoted 1 line · Dave Chinner
224 new file +1 -0
+	killall -9 dd > /dev/null 2>&1

This seems a bit dangerous. I have at least one old system that uses dd to continously read the kernel message buffer in the background, and there's tons of other potential dd uses.

Given that we now require bash and bash has array support you might be able to store the pids of "our" dd instances in an array and loop over them.

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

author
#3 lore raw

Re: [PATCH 1/3] xfsqa: add delayed allocation @ ENOSPC exerciser

Quoted 6 lines · Christoph Hellwig

224 line 41

Quoted 1 line · Dave Chinner
224 new file +1 -0
+	killall -9 dd > /dev/null 2>&1

This seems a bit dangerous. I have at least one old system that uses dd to continously read the kernel message buffer in the background, and there's tons of other potential dd uses.

I only put it there to make sure they all died when I ^C'd out of a test run while testing the test. I forgot to remove it - I'll drop it from the patch.

Cheers,

Dave.

Signature · 7 lines
Dave Chinner
[email protected]

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
#4 lore raw

Re: [PATCH 1/3] xfsqa: add delayed allocation @ ENOSPC exerciser

Quoted 11 lines · Dave Chinner
Quoted 6 lines · Christoph Hellwig

224 line 41

Quoted 1 line · Dave Chinner
224 new file +1 -0
+	killall -9 dd > /dev/null 2>&1

This seems a bit dangerous. I have at least one old system that uses dd to continously read the kernel message buffer in the background, and there's tons of other potential dd uses.

I only put it there to make sure they all died when I ^C'd out of a test run while testing the test. I forgot to remove it - I'll drop it from the patch.

Ok with that change. Although beeing able to Ctrl+C testcase is a nice gimmick to have.

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

author
#5 lore raw

PATCH 2/3 xfsqa: fix size specification for scratch mkfs

In reply to Dave Chinner

When making a specifically sized scratch filesystem, we need to calculate the size rather than assigning the caclculation expression to a variable and passing that into the _scratch_mkfs_sized function. Currently sized filesystems are failing to be made and the code is falling back to full sized filesystems so we are not testing enospc issues correctly.

Signed-off-by Dave Chinner <[email protected]>
---
 077 |    2 +-
 204 |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
077 +1 -1
index 72f1b2a..4e12163 100755
@@ -62,7 +62,7 @@ rm -f $seq.full
 umount $SCRATCH_DEV >/dev/null 2>&1
 echo "*** MKFS ***"                         >>$seq.full
 echo ""                                     >>$seq.full
-let SIZE=50*1024*1024
+SIZE=`expr 50 \* 1024 \* 1024`
 _scratch_mkfs_sized $SIZE                   >>$seq.full 2>&1 \
 	|| _fail "mkfs failed"
 _scratch_mount                              >>$seq.full 2>&1 \
204 +1 -1
index 14ebcdc..8b94f60 100755
@@ -40,7 +40,7 @@ _supported_os Linux
 
 _require_scratch
 
-SIZE=104*1024*1024
+SIZE=`expr 104 \* 1024 \* 1024`
 _scratch_mkfs_sized $SIZE  &> /dev/null
 _scratch_mount
Signature · 6 lines
1.6.5

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
#6 lore raw

Re: [PATCH 2/3] xfsqa: fix size specification for scratch mkfs

Quoted 6 lines · Dave Chinner

When making a specifically sized scratch filesystem, we need to calculate the size rather than assigning the caclculation expression to a variable and passing that into the _scratch_mkfs_sized function. Currently sized filesystems are failing to be made and the code is falling back to full sized filesystems so we are not testing enospc issues correctly.

This seems rather fragile. Maybe instead of calculating the bignums which I suspect are the problem could we pass symbolic values like 104M in and parse them in _scratch_mkfs_sized?

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

#7 lore raw

Re: [PATCH 2/3] xfsqa: fix size specification for scratch mkfs

Quoted 11 lines · Christoph Hellwig
Quoted 6 lines · Dave Chinner

When making a specifically sized scratch filesystem, we need to calculate the size rather than assigning the caclculation expression to a variable and passing that into the _scratch_mkfs_sized function. Currently sized filesystems are failing to be made and the code is falling back to full sized filesystems so we are not testing enospc issues correctly.

This seems rather fragile. Maybe instead of calculating the bignums which I suspect are the problem could we pass symbolic values like 104M in and parse them in _scratch_mkfs_sized?

How about making _scratch_mkfs_sized something like:

_scratch_mkfs_sized()

{
    fssize=`echo $1 | bc`
    ...

as a quick test this seems to work:

#!/bin/bash

my_function()

{
	SIZE=$1
	echo $SIZE
	SIZE2=`echo $SIZE | bc`
	echo $SIZE2
}

SIZE=24*1024*1024
my_function $SIZE
SIZE=2410241024
my_function $SIZE

yields the right answers for both numbers & expressions passed in:

#./testit.sh
24*1024*1024
25165824
2410241024
2410241024

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

author
#8 lore raw

Re: [PATCH 2/3] xfsqa: fix size specification for scratch mkfs

In reply to Christoph Hellwig
Quoted 11 lines · Christoph Hellwig
Quoted 6 lines · Dave Chinner

When making a specifically sized scratch filesystem, we need to calculate the size rather than assigning the caclculation expression to a variable and passing that into the _scratch_mkfs_sized function. Currently sized filesystems are failing to be made and the code is falling back to full sized filesystems so we are not testing enospc issues correctly.

This seems rather fragile. Maybe instead of calculating the bignums which I suspect are the problem could we pass symbolic values like 104M in and parse them in _scratch_mkfs_sized?

The problem was that "mkfs.xfs -d size=104*1024*1024 ..." fails because $fssize was not a numeric value.

There's no problem with 64 bit numbers with expr:

$ expr 5 \* 1024 \* 1024 \* 1024 \* 1024
5497558138880

So specifying large sizes by a byte count like this is not an issue. Lots of other tests use expr in this way.

If we change the calculations in _scratch_mkfs_sized() to use expr then there's no 32 bit overflow issues, either. This seems like a simpler way to go to introduce the complexity of symbolic values and parsing, esp. as we'd still have to handle 64 bit values correctly...

The updated patch below uses expr in _scratch_mkfs_sized as well.

Cheers,

Dave.

Signature · 18 lines
Dave Chinner
[email protected]

xfsqa: fix size specification for scratch mkfs

When making a specifically sized scratch filesystem, we need to
calculate the size rather than assigning the caclculation expression
to a variable and passing that into the _scratch_mkfs_sized
function. Currently sized filesystems are failing to be made and the
code is falling back to full sized filesystems so we are not testing
enospc issues correctly.

Signed-off-by: Dave Chinner <[email protected]>
---
 077       |    2 +-
 204       |    2 +-
 common.rc |    2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)
077 +1 -1
index 72f1b2a..4e12163 100755
@@ -62,7 +62,7 @@ rm -f $seq.full
 umount $SCRATCH_DEV >/dev/null 2>&1
 echo "*** MKFS ***"                         >>$seq.full
 echo ""                                     >>$seq.full
-let SIZE=50*1024*1024
+SIZE=`expr 50 \* 1024 \* 1024`
 _scratch_mkfs_sized $SIZE                   >>$seq.full 2>&1 \
 	|| _fail "mkfs failed"
 _scratch_mount                              >>$seq.full 2>&1 \
204 +1 -1
index 14ebcdc..8b94f60 100755
@@ -40,7 +40,7 @@ _supported_os Linux
 
 _require_scratch
 
-SIZE=104*1024*1024
+SIZE=`expr 104 \* 1024 \* 1024`
 _scratch_mkfs_sized $SIZE  &> /dev/null
 _scratch_mount
common.rc +1 -1
index ca2cd2c..d71e0a2 100644
@@ -320,7 +320,7 @@ _scratch_mkfs_sized()
     fssize=$1
     blocksize=$2
     [ -z "$blocksize" ] && blocksize=4096
-    let blocks=$fssize/$blocksize
+    blocks=`expr $fssize / $blocksize`
 
     case $FSTYP in
     xfs)

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

author
#9 lore raw

PATCH 3/3 xfsqa: Add fiemap exerciser

In reply to Dave Chinner

From: Eric Sandeen <[email protected]>

Preliminary fiemap testing support based on a test util written by Josef Bacik.

For now it's only run with preallocation disabled, because xfs has a tendency to fill in holes with data blocks (EOF prealloc stuff I think) and similar for explicit preallocation, so this is breaking the preallocation tests for now, when it finds a "data" block where it expects a preallocated block.

Signed-off-by Eric Sandeen <[email protected]>
Reviewed-by Dave Chinner <[email protected]>
---
 225                  |   62 +++++
 225.out              |    2 +
 aclocal.m4           |   19 ++
 configure.in         |    2 +
 group                |    1 +
 include/builddefs.in |    2 +
 src/Makefile         |    8 +
 src/fiemap-tester.c  |  638 ++++++++++++++++++++++++++++++++++++++++++++++++++
 8 files changed, 734 insertions(+), 0 deletions(-)
 create mode 100644 225
 create mode 100644 225.out
 create mode 100644 src/fiemap-tester.c
225 new file +62 -0
new file mode 100644
index 0000000..673e93e
@@ -0,0 +1,62 @@
+#! /bin/sh
+# FS QA Test No. 225
+#
+# Run the fiemap (file extent mapping) tester
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2009 Eric Sandeen.  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
+#-----------------------------------------------------------------------
+#
+# creator
[email protected]
+
+seq=`basename $0`
+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.*
+	_cleanup_testdir
+}
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+
+# real QA test starts here
+_supported_fs generic
+_supported_os Linux
+
+_setup_testdir
+
+fiemapfile=$TEST_DIR/fiemap.$$
+
+[ -x $here/src/fiemap-tester ] || _notrun "fiemap-tester not built"
+
+echo "fiemap run without preallocation"
+$here/src/fiemap-tester -q -p 0 -r 200 /mnt/test/fiemapfile
+
+rm -f $fiemapfile
+rm -f $seq.full
+
+status=0
+exit
225.out new file +2 -0
new file mode 100644
index 0000000..7bc9312
@@ -0,0 +1,2 @@
+QA output created by 225
+fiemap run without preallocation
aclocal.m4 +19 -0
index 02f56f2..6457d39 100644
@@ -11,6 +11,25 @@
 # even the implied warranty of MERCHANTABILITY or FITNESS FOR A
 # PARTICULAR PURPOSE.
 
+AC_DEFUN([AC_PACKAGE_WANT_LINUX_FIEMAP_H],
+  [ AC_CHECK_HEADERS([linux/fiemap.h], [ have_fiemap=true ], [ have_fiemap=false ])
+    AC_SUBST(have_fiemap)
+  ])
+
+AC_DEFUN([AC_PACKAGE_WANT_FALLOCATE],
+  [ AC_MSG_CHECKING([for fallocate])
+    AC_TRY_LINK([
+#define _GNU_SOURCE
+#define _FILE_OFFSET_BITS 64
+#include <fcntl.h>
+#include <linux/falloc.h>
+    ], [
+         fallocate(0, 0, 0, 0);
+    ], have_fallocate=yes
+       AC_MSG_RESULT(yes),
+       AC_MSG_RESULT(no))
+    AC_SUBST(have_fallocate)
+  ])
 m4_include([m4/multilib.m4])
 m4_include([m4/package_acldev.m4])
 m4_include([m4/package_aiodev.m4])
configure.in +2 -0
index 0d6f153..45b7fd1 100644
@@ -64,6 +64,8 @@ in
 		AC_PACKAGE_WANT_GDBM
 		AC_PACKAGE_WANT_AIO
 		AC_PACKAGE_WANT_DMAPI
+		AC_PACKAGE_WANT_LINUX_FIEMAP_H
+		AC_PACKAGE_WANT_FALLOCATE
 		;;
 esac
group +1 -0
index 5d675f0..8d4a83a 100644
@@ -338,3 +338,4 @@ deprecated
 222 auto fsr ioctl quick
 223 auto quick
 224 auto
+225 auto quick
include/builddefs.in +2 -0
index 23a4991..3bea050 100644
@@ -60,6 +60,8 @@ HAVE_DB = @have_db@
 HAVE_AIO = @have_aio@
 HAVE_DMAPI = @have_dmapi@
 HAVE_ATTR_LIST = @have_attr_list@
+HAVE_FIEMAP = @have_fiemap@
+HAVE_FALLOCATE = @have_fallocate@
 
 GCCFLAGS = -funsigned-char -fno-strict-aliasing -Wall
src/Makefile +8 -0
index d86d50a..2f95fe2 100644
@@ -25,6 +25,14 @@ ifeq ($(HAVE_XLOG_ASSIGN_LSN), true)
 LINUX_TARGETS += loggen
 endif
 
+ifeq ($(HAVE_FIEMAP), true)
+LINUX_TARGETS += fiemap-tester
+endif
+
+ifeq ($(HAVE_FALLOCATE),yes)
+LCFLAGS += -DHAVE_FALLOCATE
+endif
+
 IRIX_TARGETS = open_unlink
 
 ifeq ($(PKG_PLATFORM),linux)
src/fiemap-tester.c new file +638 -0

638 lines not on this page · raw message

Signature · 6 lines
1.6.5

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
Reviewed-by on v1 3/3
#10 lore raw

Re: [PATCH 3/3] xfsqa: Add fiemap exerciser

Quoted 5 lines · Dave Chinner

From: Eric Sandeen <[email protected]>

Preliminary fiemap testing support based on a test util written by Josef Bacik.

Quoted 8 lines

For now it's only run with preallocation disabled, because xfs has a tendency to fill in holes with data blocks (EOF prealloc stuff I think) and similar for explicit preallocation, so this is breaking the preallocation tests for now, when it finds a "data" block where it expects a preallocated block.

Signed-off-by Eric Sandeen <[email protected]>
Reviewed-by Dave Chinner <[email protected]>

Looks good,

Reviewed-by Christoph Hellwig <[email protected]>

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs