xfstests 240: test non-aligned AIO hole-filling

PATCH In review Eric Sandeen 2 participants 6 messages Jul 22, 2010 – Jul 23, 2010

Share & tools

Eric Sandeen

author

2010-07-22 19:02 UTC #0 lore raw

PATCH 1/1 xfstests 240: test non-aligned AIO hole-filling

This replicates file corruption we've seen with qemu-kvm when we use if=virtio,cache=none,aio=native for IO to a sparse ext4- or xfs-hosted file, and the partitions/filesystems within that file image are not block-aligned. (think sector 63 here...) This results in AIO IOs not aligned to the filesystem blocks.

This test modifies aiodio_sparse2.c to add an option to start the file IO at an offset.

When we do 4k writes to a 16k file in 2 threads, starting at offset 512, we get 0s interspersed in the file where they should not be:

00000000 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................| *
00000200 01 01 01 01 01 01 01 01 01 01 01 01 01 01 01 01 |................| *
00001000 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................| *
00001200 01 01 01 01 01 01 01 01 01 01 01 01 01 01 01 01 |................|
....

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

---

240 new file +62 -0

new file mode 100644
index 0000000..b8a1024
@@ -0,0 +1,62 @@
+#! /bin/bash
+# FS QA Test No. 240
+#
+# Test that non-block-aligned aio+dio into holes does not leave
+# zero'd out portions of the file
+#
+# QEMU IO to a file-backed device with misaligned partitions
+# can send this sort of IO
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2010 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
+#-----------------------------------------------------------------------
+#
+# 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.*
+}
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+
+_supported_fs generic
+_supported_os Linux
+
+echo "Silence is golden."
+
+# real QA test starts here
+rm -f $seq.full
+
+rm -f "$TEST_DIR/aiodio_sparse*"
+
+# 2 threads, 4k writes, 16k filesize, stride throug file by 4k, start at 512 offset
+$here/src/aio-dio-regress/aiodio_sparse2 -i 2 -w 4k -s 16k -n 4k -o 512 "$TEST_DIR/aiodio_sparse"
+
+status=$?
+exit

240.out new file +2 -0

new file mode 100644
index 0000000..b6bdbce
@@ -0,0 +1,2 @@
+QA output created by 240
+Silence is golden.

group +1 -0

index 87b1230..afe7614 100644
@@ -353,3 +353,4 @@ deprecated
 237 auto quick acl
 238 auto quick metadata ioctl
 239 auto aio rw
+240 auto aio quick rw

src/aio-dio-regress/aiodio_sparse2.c +15 -7

index b56f91a..f1225dd 100644
@@ -66,7 +66,8 @@ sig_term_func(int i, siginfo_t *si, void *p)
 /*
  * do async DIO writes to a sparse file
  */
-void aiodio_sparse(char *filename, int align, int writesize, int filesize, int num_aio, int step, int sparse, int direct, int keep)
+void aiodio_sparse(char *filename, int align, int writesize, int startoffset, int filesize,
+			int num_aio, int step, int sparse, int direct, int keep)
 {
 	int fd;
 	void *bufptr;

@@ -110,7 +111,7 @@ void aiodio_sparse(char *filename, int align, int writesize, int filesize, int n
 	/*
 	 * allocate the iocbs array and iocbs with buffers
 	 */
-	offset = 0;
+	offset = startoffset;
 	for (i = 0; i < num_aio; i++) {
 		void *bufptr;

@@ -220,9 +221,11 @@ void aiodio_sparse(char *filename, int align, int writesize, int filesize, int n
 		exit(1);
 
 	bufptr = malloc(writesize);
-	for (offset = 0; offset < filesize; offset += step)  {
+	for (offset = startoffset; offset < filesize; offset += step)  {
 		unsigned char *badbuf;
 
+		if (debug)
+			fprintf(stderr, "seek to %ld and read %d\n", offset, writesize);
 		lseek(fd, offset, SEEK_SET);
 		if (read(fd, bufptr, writesize) < writesize) {
 			fprintf(stderr, "short read() at offset %lld\n",

@@ -274,7 +277,7 @@ void dirty_freeblocks(char *filename, int size)
 void usage()
 {
 	fprintf(stderr, "usage: dio_sparse [-n step] [-s filesize]"
-		" [-w writesize] [-r readsize] filename\n");
+		" [-w writesize] [-o startoffset] [-r readsize] filename\n");
 	exit(1);
 }

@@ -305,7 +308,7 @@ long long scale_by_kmg(long long value, char scale)
 
 /*
  *	usage:
- * aiodio_sparse [-r readsize] [-w writesize] [-n step] [-a align] [-i num_aio] filename
+ * aiodio_sparse [-r readsize] [-w writesize] [-o startoffset] [-n step] [-a align] [-i num_aio] filename
  */
 
 int main(int argc, char **argv)

@@ -314,6 +317,7 @@ int main(int argc, char **argv)
 	long alignment = 512;
 	int readsize = 65536;
 	int writesize = 65536;
+	int startoffset = 0;
 	int filesize = 100*1024*1024;
 	int num_aio = 16;
 	int step = 5*1024*1024;

@@ -321,7 +325,7 @@ int main(int argc, char **argv)
 	extern char *optarg;
 	extern int optind, optopt, opterr;
 
-	while ((c = getopt(argc, argv, "dr:w:n:a:s:i:DkS")) != -1) {
+	while ((c = getopt(argc, argv, "dr:w:n:o:a:s:i:DkS")) != -1) {
 		char *endp;
 		switch (c) {
 		case 'D':

@@ -360,6 +364,10 @@ int main(int argc, char **argv)
 			step = strtol(optarg, &endp, 0);
 			step = (int)scale_by_kmg((long long)step, *endp);
 			break;
+		case 'o':
+			startoffset = strtol(optarg, &endp, 0);
+			startoffset = (int)scale_by_kmg((long long)startoffset, *endp);
+			break;
 		case '?':
 			usage();
 			break;

@@ -378,7 +386,7 @@ int main(int argc, char **argv)
 	 * Parent write to a hole in a file using async direct i/o
 	 */
 
-	aiodio_sparse(filename, alignment, writesize, filesize, num_aio, step, sparse, direct, keep);
+	aiodio_sparse(filename, alignment, writesize, startoffset, filesize, num_aio, step, sparse, direct, keep);
 
 	return 0;
 }

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

Dave Chinner

Reviewed-by on v1 1/1

2010-07-23 02:46 UTC #1 lore raw

Quoted 23 lines · Eric Sandeen

This replicates file corruption we've seen with qemu-kvm when we use if=virtio,cache=none,aio=native for IO to a sparse ext4- or xfs-hosted file, and the partitions/filesystems within that file image are not block-aligned. (think sector 63 here...) This results in AIO IOs not aligned to the filesystem blocks.
This test modifies aiodio_sparse2.c to add an option to start the file IO at an offset.
When we do 4k writes to a 16k file in 2 threads, starting at offset 512, we get 0s interspersed in the file where they should not be:
00000000 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................| *
00000200 01 01 01 01 01 01 01 01 01 01 01 01 01 01 01 01 |................| *
00001000 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................| *
00001200 01 01 01 01 01 01 01 01 01 01 01 01 01 01 01 01 |................|
....

Quoted 1 line

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

looks good - it needs to remove the $TEST_DIR/aiodio_sparse file before the test starts, though, so that repeated tests fail correctly. Otherwise:

Reviewed-by Dave Chinner <[email protected]>

Also, Eric, can you run this proof-of-concept test branch (on top of the current XFS for-2.6.36 branch) and see if QEMU is happy with it? It passes this test on my machine....

The following changes since commit d2ec2c790b3994bf0c4381cacc489b514705887c:

xfs: clean up xfs_bmap_get_bp (2010-07-22 12:52:08 +1000)

are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/dgc/xfsdev aio-unaligned

Dave Chinner (4):
Merge branch 'master' into working
Merge branch 'for-2.6.36' into working
fs: get_blocks needs an unaligned mapping flag
xfs: serialise unaligned direct IO into unwritten extents

 fs/buffer.c                 |   17 ++++++++-------
 fs/direct-io.c              |   25 ++++++++++++++++++++--
 fs/ioctl.c                  |    2 +-
 fs/mpage.c                  |    6 +++-
 fs/xfs/linux-2.6/xfs_aops.c |   47 +++++++++++++++++++++++++++++++++---------
 include/linux/fs.h          |   10 ++++++++-
 6 files changed, 82 insertions(+), 25 deletions(-)

Cheers,

Dave.

Signature · 7 lines

Dave Chinner
[email protected]

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

Eric Sandeen

author

2010-07-23 03:29 UTC #2 lore raw

Quoted 25 lines · Dave Chinner

Quoted 23 lines · Eric Sandeen
This replicates file corruption we've seen with qemu-kvm when we use if=virtio,cache=none,aio=native for IO to a sparse ext4- or xfs-hosted file, and the partitions/filesystems within that file image are not block-aligned. (think sector 63 here...) This results in AIO IOs not aligned to the filesystem blocks.
This test modifies aiodio_sparse2.c to add an option to start the file IO at an offset.
When we do 4k writes to a 16k file in 2 threads, starting at offset 512, we get 0s interspersed in the file where they should not be:
00000000 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................| *
00000200 01 01 01 01 01 01 01 01 01 01 01 01 01 01 01 01 |................| *
00001000 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................| *
00001200 01 01 01 01 01 01 01 01 01 01 01 01 01 01 01 01 |................|
....

Quoted 8 lines

Quoted 2 lines
Signed-off-by Eric Sandeen <[email protected]>
looks good - it needs to remove the $TEST_DIR/aiodio_sparse file before the test starts, though, so that repeated tests fail correctly. Otherwise:

hm, but:

+rm -f "$TEST_DIR/aiodio_sparse*"
+
+# 2 threads, 4k writes, 16k filesize, stride throug file by 4k, start at 512 offset +$here/src/aio-dio-regress/aiodio_sparse2 -i 2 -w 4k -s 16k -n 4k -o 512 "$TEST_DIR/aiodio_sparse"

It does remove it ... no?

Quoted 3 lines

Reviewed-by Dave Chinner <[email protected]>

thanks

Quoted 5 lines

Also, Eric, can you run this proof-of-concept test branch (on top of the current XFS for-2.6.36 branch) and see if QEMU is happy with it? It passes this test on my machine....

Absolutely, thanks!

-Eric

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

Eric Sandeen

author

2010-07-23 03:34 UTC #3 lore raw

Quoted 1 line · Eric Sandeen

Dave Chinner wrote:

...

Quoted 12 lines

Quoted 5 lines
looks good - it needs to remove the $TEST_DIR/aiodio_sparse file before the test starts, though, so that repeated tests fail correctly. Otherwise:
hm, but:
240 new file +4 -0
+rm -f "$TEST_DIR/aiodio_sparse*"
+
+# 2 threads, 4k writes, 16k filesize, stride throug file by 4k, start at 512 offset
+$here/src/aio-dio-regress/aiodio_sparse2 -i 2 -w 4k -s 16k -n 4k -o 512 "$TEST_DIR/aiodio_sparse"

Quoted 1 line

It does remove it ... no?

hm, no, what are those quotes doing there...

Ok, will fix up thanks.

-Eric

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

Dave Chinner

2010-07-23 03:55 UTC #4 lore raw

In reply to Eric Sandeen

Quoted 42 lines · Eric Sandeen

Quoted 25 lines · Dave Chinner
Quoted 23 lines · Eric Sandeen
This replicates file corruption we've seen with qemu-kvm when we use if=virtio,cache=none,aio=native for IO to a sparse ext4- or xfs-hosted file, and the partitions/filesystems within that file image are not block-aligned. (think sector 63 here...) This results in AIO IOs not aligned to the filesystem blocks.
This test modifies aiodio_sparse2.c to add an option to start the file IO at an offset.
When we do 4k writes to a 16k file in 2 threads, starting at offset 512, we get 0s interspersed in the file where they should not be:
00000000 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................| *
00000200 01 01 01 01 01 01 01 01 01 01 01 01 01 01 01 01 |................| *
00001000 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................| *
00001200 01 01 01 01 01 01 01 01 01 01 01 01 01 01 01 01 |................|
....
Quoted 8 lines
Quoted 2 lines
Signed-off-by Eric Sandeen <[email protected]>
looks good - it needs to remove the $TEST_DIR/aiodio_sparse file before the test starts, though, so that repeated tests fail correctly. Otherwise:
hm, but:
240 new file +4 -0
+rm -f "$TEST_DIR/aiodio_sparse*"
+
+# 2 threads, 4k writes, 16k filesize, stride throug file by 4k, start at 512 offset
+$here/src/aio-dio-regress/aiodio_sparse2 -i 2 -w 4k -s 16k -n 4k -o 512 "$TEST_DIR/aiodio_sparse"

Quoted 1 line

It does remove it ... no?

Sorry, my bad - that went in one eye and out the other when I read through the whole patch.

When i was testing I was getting a repeatable fail - pass - fail - pass ... pattern occurring without any kernel patches applied, and the test warns about removing the file after failure, so I just connected the dots up in the obvious manner and got it completely wrong....

Cheers,

Dave.

Signature · 7 lines

Dave Chinner
[email protected]

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

Dave Chinner

2010-07-23 10:09 UTC #5 lore raw

In reply to Dave Chinner

Quoted 36 lines · Dave Chinner

Quoted 23 lines · Eric Sandeen
This replicates file corruption we've seen with qemu-kvm when we use if=virtio,cache=none,aio=native for IO to a sparse ext4- or xfs-hosted file, and the partitions/filesystems within that file image are not block-aligned. (think sector 63 here...) This results in AIO IOs not aligned to the filesystem blocks.
This test modifies aiodio_sparse2.c to add an option to start the file IO at an offset.
When we do 4k writes to a 16k file in 2 threads, starting at offset 512, we get 0s interspersed in the file where they should not be:
00000000 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................| *
00000200 01 01 01 01 01 01 01 01 01 01 01 01 01 01 01 01 |................| *
00001000 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................| *
00001200 01 01 01 01 01 01 01 01 01 01 01 01 01 01 01 01 |................|
....
Quoted 1 line
Signed-off-by Eric Sandeen <[email protected]>
looks good - it needs to remove the $TEST_DIR/aiodio_sparse file before the test starts, though, so that repeated tests fail correctly. Otherwise:
Reviewed-by Dave Chinner <[email protected]>
Also, Eric, can you run this proof-of-concept test branch (on top of the current XFS for-2.6.36 branch) and see if QEMU is happy with it? It passes this test on my machine....

Quoted 6 lines

The following changes since commit d2ec2c790b3994bf0c4381cacc489b514705887c:
xfs: clean up xfs_bmap_get_bp (2010-07-22 12:52:08 +1000)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/dgc/xfsdev aio-unaligned

I just pushed another patch onto this series that will makes test 240 pass reliably here rather than hanging. It's all a bit messy, but it does serve to illustrate the problem and a potential avenue to prevent it....

Dave Chinner (5):
Merge branch 'master' into working
Merge branch 'for-2.6.36' into working
fs: get_blocks needs an unaligned mapping flag
xfs: serialise unaligned direct IO into unwritten extents
xfs: wait on IO completion inside an IO context

 fs/buffer.c                 |   17 ++++----
 fs/direct-io.c              |   25 ++++++++++-
 fs/ioctl.c                  |    2 +-
 fs/mpage.c                  |    6 ++-
 fs/xfs/linux-2.6/xfs_aops.c |   95 ++++++++++++++++++++++++++++++++++++++-----
 fs/xfs/xfs_inode.h          |   13 +++---
 include/linux/fs.h          |   10 ++++-
 7 files changed, 136 insertions(+), 32 deletions(-)

Signature · 7 lines

Dave Chinner
[email protected]

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