***** SUSPECTED SPAM ***** [PATCH 0/4] xfstests: fsx is fallocate challenged.

***** SUSPECTED SPAM ***** [PATCH 0/4] xfstests: fsx is fallocate challenged.

[Dave Chinner]

The first three patches make fsx use fallocate/fpunch correctly.
Hole punching is not detected correctly, falloc/fpunch operations
can't actually be executed properly, and the logdump code will segv
on the first falloc operation it finds in the log.

The last patch widens the fsx coverage to mix direct IO reads/writes
with mmapped reads/writes as that was the test case that it was
executing (instead of using falloc/fpunch!) when it triggered the
error (a real bug) that lead to me finding this mess....

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 1/4] xfstests: fix fsx fpunch test to actually test for fpunch

[Dave Chinner]

From: Dave Chinner <dchinner@redhat.com>

The operation flags parameter to fallocate is the second parameter,
not the last. Hence the fpunch test is actually testing for falloc
support, not fpunch. Somebody needs a brown paper bag.

Also, add a ftruncate call whenthe fpunch succeeds just in case the
file was not already zero sized. Failing to ensure we start with a
zero length file can cause read ops to fail size checks if they
occur before the file is written to be the main test loop.

While there, observe the quiet flag the same as the falloc test
does and have them both emit the warning at the same error level.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
 ltp/fsx.c |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/ltp/fsx.c b/ltp/fsx.c
index 0683853..a37e223 100644
--- a/ltp/fsx.c
+++ b/ltp/fsx.c
@@ -1243,7 +1243,7 @@ test_fallocate()
 	if (!lite && fallocate_calls) {
 		if (fallocate(fd, 0, 0, 1) && errno == EOPNOTSUPP) {
 			if(!quiet)
-				prt("fsx: main: filesystem does not support fallocate, disabling\n");
+				warn("main: filesystem does not support fallocate, disabling\n");
 			fallocate_calls = 0;
 		} else {
 			ftruncate(fd, 0);
@@ -1260,13 +1260,13 @@ test_punch_hole()
 {
 #ifdef FALLOC_FL_PUNCH_HOLE
 	if (!lite && punch_hole_calls) {
-		if (fallocate(fd, 0, 0,
-			FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE) &&
-			errno == EOPNOTSUPP) {
-
-			warn("main: filesystem does not support fallocate punch hole, disabling");
+		if (fallocate(fd, FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE,
+				0, 1) && errno == EOPNOTSUPP) {
+			if(!quiet)
+				warn("main: filesystem does not support fallocate punch hole, disabling");
 			punch_hole_calls = 0;
-		}
+		} else
+			ftruncate(fd, 0);
 	}
 #else /* ! PUNCH HOLE */
 	punch_hole_calls = 0;
-- 
1.7.5.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 2/4] xfstests: fsx fallocate support is b0rked

[Dave Chinner]

From: Dave Chinner <dchinner@redhat.com>

The recent fallocate/fpunch additions to fsx have not actually be
executing fallocate/fpunch operations. The logic to select what
operation to run is broken in such a way that fsx has been executing
mapped writes and truncates instead of fallocate and fpunch
operations.

Remove all the (b0rken) smarty-pants selection logic from the test()
function. Replace it with a clearly defined set of operations for
each mode and use understandable fallback logic when various
operation types have been disabled. Then use a simple switch
statement to execute each of the different operations, removing the
tortured nesting of if/else statements that only serve to obfuscate
the code.

NAs a result, fsx uses fallocate/fpunch appropriately during
operations, and uses/disableѕ the operations as defined on the
command line correctly.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
 ltp/fsx.c |  162 +++++++++++++++++++++++++++++++++++++++----------------------
 1 files changed, 103 insertions(+), 59 deletions(-)

diff --git a/ltp/fsx.c b/ltp/fsx.c
index a37e223..66daefe 100644
--- a/ltp/fsx.c
+++ b/ltp/fsx.c
@@ -955,18 +955,42 @@ docloseopen(void)
 	}
 }
 
+#define TRIM_OFF_LEN(off, len, size)	\
+do {					\
+	if (file_size)			\
+		offset %= size;		\
+	else				\
+		offset = 0;		\
+	if (offset + len > size)	\
+		len = size - offset;	\
+} while (0)
 
+/*
+ * The operation matrix is complex due to conditional variables.
+ * We calculate how many different operations we can use, then
+ * map them appropriately according to how many options are enabled.
+ * The mapping is:
+ *
+ *		lite	!lite
+ * READ:	0	0
+ * WRITE:	1	1
+ * MAPREAD:	2	2
+ * MAPWRITE:	3	3
+ * TRUNCATE:	-	4
+ * FALLOCATE:	-	5
+ * PUNCH HOLE:	-	6
+ *
+ * When mapped read/writes are disabled, they are simply converted to normal
+ * reads and writes. When fallocate/fpunch calls are disabled, they are
+ * converted to OP_SKIPPED.
+ */
 void
 test(void)
 {
 	unsigned long	offset;
 	unsigned long	size = maxoplen;
 	unsigned long	rv = random();
-	unsigned long	op = rv % (3 + !lite + mapped_writes + fallocate_calls + punch_hole_calls);
-        /* turn off the map read if necessary */
-
-        if (op == 2 && !mapped_reads)
-            op = 0;
+	unsigned long	op;
 
 	if (simulatedopcount > 0 && testcalls == simulatedopcount)
 		writefileimage();
@@ -982,62 +1006,82 @@ test(void)
 	if (!quiet && testcalls < simulatedopcount && testcalls % 100000 == 0)
 		prt("%lu...\n", testcalls);
 
-	/*
-	 *                 lite  !lite
-	 * READ:	op = 0	   0
-	 * WRITE:	op = 1     1
-	 * MAPREAD:     op = 2     2
-	 * TRUNCATE:	op = -     3
-	 * MAPWRITE:    op = 3     4
-	 * FALLOCATE:   op = -     5
-	 * PUNCH HOLE:  op = -     6
-	 */
-	if (lite ? 0 : op == 3 && (style & 1) == 0) /* vanilla truncate? */
-		dotruncate(random() % maxfilelen);
-	else {
-		if (randomoplen)
-			size = random() % (maxoplen+1);
-
-		if (lite ? 0 : op == 3) {
-			/* truncate */
-			dotruncate(size);
-		} else {
-			offset = random();
-			if (op == 5) {
-				/* fallocate */
-				offset %= maxfilelen;
-				if (offset + size > maxfilelen)
-					size = maxfilelen - offset;
-				do_preallocate(offset, size);
-			} else if (op == 6) {
-				offset %= maxfilelen;
-				if (offset + size > maxfilelen)
-					size = maxfilelen - offset;
-				do_punch_hole(offset, size);
-			} else if (op == 1 || op == (lite ? 3 : 4)) {
-				/* write / mapwrite */
-				offset %= maxfilelen;
-				if (offset + size > maxfilelen)
-					size = maxfilelen - offset;
-				if (op != 1)
-					domapwrite(offset, size);
-				else
-					dowrite(offset, size);
-			} else {
-				/* read / mapread */
-				if (file_size)
-					offset %= file_size;
-				else
-					offset = 0;
-				if (offset + size > file_size)
-					size = file_size - offset;
-				if (op != 0)
-					domapread(offset, size);
-				else
-					doread(offset, size);
-			}
+	offset = random();
+	if (randomoplen)
+		size = random() % (maxoplen + 1);
+
+	/* calculate appropriate op to run */
+	if (lite)
+		op = rv % 4;
+	else
+		op = rv % 7;
+
+	switch (op) {
+	case 2:
+		if (!mapped_reads)
+			op = 0;
+		break;
+	case 3:
+		if (!mapped_writes)
+			op = 1;
+		break;
+	case 5:
+		if (!fallocate_calls) {
+			log4(OP_SKIPPED, OP_FALLOCATE, offset, size);
+			goto out;
+		}
+		break;
+	case 6:
+		if (!punch_hole_calls) {
+			log4(OP_SKIPPED, OP_PUNCH_HOLE, offset, size);
+			goto out;
 		}
+		break;
 	}
+
+	switch (op) {
+	case 0:	/* read */
+		TRIM_OFF_LEN(offset, size, file_size);
+		doread(offset, size);
+		break;
+
+	case 1: /* write */
+		TRIM_OFF_LEN(offset, size, maxfilelen);
+		dowrite(offset, size);
+		break;
+
+	case 2:	/* mapread */
+		TRIM_OFF_LEN(offset, size, maxfilelen);
+		domapread(offset, size);
+		break;
+
+	case 3:	/* mapwrite */
+		TRIM_OFF_LEN(offset, size, maxfilelen);
+		domapwrite(offset, size);
+		break;
+
+	case 4: /* truncate */
+		if (!style)
+			size = random() % maxfilelen;
+		dotruncate(size);
+		break;
+
+	case 5:	/* fallocate */
+		TRIM_OFF_LEN(offset, size, maxfilelen);
+		do_preallocate(offset, size);
+		break;
+
+	case 6:	/* punch */
+		TRIM_OFF_LEN(offset, size, maxfilelen);
+		do_punch_hole(offset, size);
+		break;
+	default:
+		prterr("test: unknown operation");
+		report_failure(42);
+		break;
+	}
+
+out:
 	if (sizechecks && testcalls > simulatedopcount)
 		check_size();
 	if (closeopen)
-- 
1.7.5.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 3/4] xfstests: fix brain-o in fallocate log dump

[Dave Chinner]

From: Dave Chinner <dchinner@redhat.com>

fsx segvs when dumping fallocate log entries. Fix magic string
array index parameters to be zero based rather than one based.

While touching log string related stuff, make the format consistent
with read and write operations so the log dump is easier to look at
with the human eye.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
 ltp/fsx.c |   28 +++++++++++++++-------------
 1 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/ltp/fsx.c b/ltp/fsx.c
index 66daefe..38d5278 100644
--- a/ltp/fsx.c
+++ b/ltp/fsx.c
@@ -223,14 +223,14 @@ logdump(void)
 		int opnum;
 
 		opnum = i+1 + (logcount/LOGSIZE)*LOGSIZE;
-		prt("%d(%d mod 256): ", opnum, opnum%256);
+		prt("%d(%3d mod 256): ", opnum, opnum%256);
 		lp = &oplog[i];
 		if ((closeopen = lp->operation < 0))
 			lp->operation = ~ lp->operation;
 			
 		switch (lp->operation) {
 		case OP_MAPREAD:
-			prt("MAPREAD\t0x%x thru 0x%x\t(0x%x bytes)",
+			prt("MAPREAD  0x%x thru 0x%x\t(0x%x bytes)",
 			    lp->args[0], lp->args[0] + lp->args[1] - 1,
 			    lp->args[1]);
 			if (badoff >= lp->args[0] && badoff <
@@ -246,7 +246,7 @@ logdump(void)
 				prt("\t******WWWW");
 			break;
 		case OP_READ:
-			prt("READ\t0x%x thru 0x%x\t(0x%x bytes)",
+			prt("READ     0x%x thru 0x%x\t(0x%x bytes)",
 			    lp->args[0], lp->args[0] + lp->args[1] - 1,
 			    lp->args[1]);
 			if (badoff >= lp->args[0] &&
@@ -254,7 +254,7 @@ logdump(void)
 				prt("\t***RRRR***");
 			break;
 		case OP_WRITE:
-			prt("WRITE\t0x%x thru 0x%x\t(0x%x bytes)",
+			prt("WRITE    0x%x thru 0x%x\t(0x%x bytes)",
 			    lp->args[0], lp->args[0] + lp->args[1] - 1,
 			    lp->args[1]);
 			if (lp->args[0] > lp->args[2])
@@ -275,14 +275,15 @@ logdump(void)
 			break;
 		case OP_FALLOCATE:
 			/* 0: offset 1: length 2: where alloced */
-			prt("FALLOCATE %s\tfrom 0x%x to 0x%x",
-			    falloc_type[lp->args[2]], lp->args[0], lp->args[0] + lp->args[1]);
+			prt("FALLOC   0x%x thru 0x%x\t(0x%x bytes) %s",
+				lp->args[0], lp->args[0] + lp->args[1],
+				lp->args[1], falloc_type[lp->args[2]]);
 			if (badoff >= lp->args[0] &&
 			    badoff < lp->args[0] + lp->args[1])
 				prt("\t******FFFF");
 			break;
 		case OP_PUNCH_HOLE:
-			prt("PUNCH HOLE\t0x%x thru 0x%x\t(0x%x bytes)",
+			prt("PUNCH    0x%x thru 0x%x\t(0x%x bytes)",
 			    lp->args[0], lp->args[0] + lp->args[1] - 1,
 			    lp->args[1]);
 			if (badoff >= lp->args[0] && badoff <
@@ -877,12 +878,12 @@ do_preallocate(unsigned offset, unsigned length)
 	}
 
 	/*
-	 * last arg:
-	 * 	1: allocate past EOF
-	 * 	2: extending prealloc
-	 * 	3: interior prealloc
+	 * last arg matches fallocate string array index in logdump:
+	 * 	0: allocate past EOF
+	 * 	1: extending prealloc
+	 * 	2: interior prealloc
 	 */
-	log4(OP_FALLOCATE, offset, length, (end_offset > file_size) ? (keep_size ? 1 : 2) : 3);
+	log4(OP_FALLOCATE, offset, length, (end_offset > file_size) ? (keep_size ? 0 : 1) : 2);
 
 	if (end_offset > file_size) {
 		memset(good_buf + file_size, '\0', end_offset - file_size);
@@ -895,7 +896,8 @@ do_preallocate(unsigned offset, unsigned length)
 	if ((progressinterval && testcalls % progressinterval == 0) ||
 	    (debug && (monitorstart == -1 || monitorend == -1 ||
 		      end_offset <= monitorend)))
-		prt("%lu falloc\tfrom 0x%x to 0x%x\n", testcalls, offset, length);
+		prt("%lu falloc\tfrom 0x%x to 0x%x (0x%x bytes)\n", testcalls,
+				offset, offset + length, length);
 	if (fallocate(fd, keep_size ? FALLOC_FL_KEEP_SIZE : 0, (loff_t)offset, (loff_t)length) == -1) {
 	        prt("fallocate: %x to %x\n", offset, length);
 		prterr("do_preallocate: fallocate");
-- 
1.7.5.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 4/4] xfstests: add mapped write fsx operations to 091

[Dave Chinner]

From: Dave Chinner <dchinner@redhat.com>

The recent busted fsx updates caused fsx to execute fsx with direct
IO and mmapped reads and writes on an XFS filesystem. The result
uncovered a direct-IO write vs mmap read bug to do with EOF
sub-block zeroing on the direct IO write.

Hence whiel we do not recommend that pepole mix DIO with mmap on the
same file, we should at least have tests that exercise it as they
often show up other problems like this.


Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
 091     |    3 +++
 091.out |    2 ++
 2 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/091 b/091
index a13d979..11b599e 100755
--- a/091
+++ b/091
@@ -88,6 +88,9 @@ kernel=`uname -r  | sed -e 's/\(2\..\).*/\1/'`
 #run_fsx -N 10000  -o 128000 -l 500000 -r PSIZE -t PSIZE -w PSIZE -Z -W
  run_fsx -N 10000  -o 128000 -l 500000 -r PSIZE -t BSIZE -w BSIZE -Z -W
 
+ run_fsx -N 10000  -o 8192   -l 500000 -r PSIZE -t BSIZE -w BSIZE -Z
+ run_fsx -N 10000  -o 128000 -l 500000 -r PSIZE -t BSIZE -w BSIZE -Z
+
 # Commented out calls above are less likely to pick up issues, so
 # save time by commenting them out (leave 'em for manual testing).
 
diff --git a/091.out b/091.out
index 31bd25d..27ed1e3 100644
--- a/091.out
+++ b/091.out
@@ -5,3 +5,5 @@ fsx -N 10000 -o 32768 -l 500000 -r PSIZE -t BSIZE -w BSIZE -Z -R -W
 fsx -N 10000 -o 8192 -l 500000 -r PSIZE -t BSIZE -w BSIZE -Z -R -W
 fsx -N 10000 -o 32768 -l 500000 -r PSIZE -t BSIZE -w BSIZE -Z -R -W
 fsx -N 10000 -o 128000 -l 500000 -r PSIZE -t BSIZE -w BSIZE -Z -W
+fsx -N 10000 -o 8192 -l 500000 -r PSIZE -t BSIZE -w BSIZE -Z
+fsx -N 10000 -o 128000 -l 500000 -r PSIZE -t BSIZE -w BSIZE -Z
-- 
1.7.5.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 1/4] xfstests: fix fsx fpunch test to actually test for fpunch

[Allison Henderson]

On 06/26/2011 10:48 PM, Dave Chinner wrote:
> From: Dave Chinner<dchinner@redhat.com>
>
> The operation flags parameter to fallocate is the second parameter,
> not the last. Hence the fpunch test is actually testing for falloc
> support, not fpunch. Somebody needs a brown paper bag.
>
> Also, add a ftruncate call whenthe fpunch succeeds just in case the
> file was not already zero sized. Failing to ensure we start with a
> zero length file can cause read ops to fail size checks if they
> occur before the file is written to be the main test loop.
>
> While there, observe the quiet flag the same as the falloc test
> does and have them both emit the warning at the same error level.

Hi there,

Sorry about that, I think this bug was mine.  I have tried your patch 
set on my box, and it appears to run with out problems for me. Thx!

Allison Henderson

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 1/4] xfstests: fix fsx fpunch test to actually test for fpunch

[Eric Sandeen]

On 6/27/11 12:48 AM, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> The operation flags parameter to fallocate is the second parameter,
> not the last. Hence the fpunch test is actually testing for falloc
> support, not fpunch. Somebody needs a brown paper bag.
> 
> Also, add a ftruncate call whenthe fpunch succeeds just in case the
> file was not already zero sized. Failing to ensure we start with a
> zero length file can cause read ops to fail size checks if they
> occur before the file is written to be the main test loop.
> 
> While there, observe the quiet flag the same as the falloc test
> does and have them both emit the warning at the same error level.
> 
> Signed-off-by: Dave Chinner <dchinner@redhat.com>

Reviewed-by: Eric Sandeen <sandeen@redhat.com>

> ---
>  ltp/fsx.c |   14 +++++++-------
>  1 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/ltp/fsx.c b/ltp/fsx.c
> index 0683853..a37e223 100644
> --- a/ltp/fsx.c
> +++ b/ltp/fsx.c
> @@ -1243,7 +1243,7 @@ test_fallocate()
>  	if (!lite && fallocate_calls) {
>  		if (fallocate(fd, 0, 0, 1) && errno == EOPNOTSUPP) {
>  			if(!quiet)
> -				prt("fsx: main: filesystem does not support fallocate, disabling\n");
> +				warn("main: filesystem does not support fallocate, disabling\n");
>  			fallocate_calls = 0;
>  		} else {
>  			ftruncate(fd, 0);
> @@ -1260,13 +1260,13 @@ test_punch_hole()
>  {
>  #ifdef FALLOC_FL_PUNCH_HOLE
>  	if (!lite && punch_hole_calls) {
> -		if (fallocate(fd, 0, 0,
> -			FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE) &&
> -			errno == EOPNOTSUPP) {
> -
> -			warn("main: filesystem does not support fallocate punch hole, disabling");
> +		if (fallocate(fd, FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE,
> +				0, 1) && errno == EOPNOTSUPP) {
> +			if(!quiet)
> +				warn("main: filesystem does not support fallocate punch hole, disabling");
>  			punch_hole_calls = 0;
> -		}
> +		} else
> +			ftruncate(fd, 0);
>  	}
>  #else /* ! PUNCH HOLE */
>  	punch_hole_calls = 0;

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 2/4] xfstests: fsx fallocate support is b0rked

[Eric Sandeen]

On 6/27/11 12:48 AM, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> The recent fallocate/fpunch additions to fsx have not actually be
> executing fallocate/fpunch operations. The logic to select what
> operation to run is broken in such a way that fsx has been executing
> mapped writes and truncates instead of fallocate and fpunch
> operations.
> 
> Remove all the (b0rken) smarty-pants selection logic from the test()

I hope I only extended that smarty-pants logic and didn't invent it.
I suppose maybe I broke it first though, damn.

> function. Replace it with a clearly defined set of operations for
> each mode and use understandable fallback logic when various
> operation types have been disabled. Then use a simple switch
> statement to execute each of the different operations, removing the
> tortured nesting of if/else statements that only serve to obfuscate
> the code.
> 
> NAs a result, fsx uses fallocate/fpunch appropriately during
> operations, and uses/disableѕ the operations as defined on the
> command line correctly.

Hm we're back in the fsx maintenance business I guess.

Would it be worth doing defines or an enum for the ops/cases, to make it
a little more readable?

Otherwise looks much better.

-Eric

> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> ---
>  ltp/fsx.c |  162 +++++++++++++++++++++++++++++++++++++++----------------------
>  1 files changed, 103 insertions(+), 59 deletions(-)
> 
> diff --git a/ltp/fsx.c b/ltp/fsx.c
> index a37e223..66daefe 100644
> --- a/ltp/fsx.c
> +++ b/ltp/fsx.c
> @@ -955,18 +955,42 @@ docloseopen(void)
>  	}
>  }
>  
> +#define TRIM_OFF_LEN(off, len, size)	\
> +do {					\
> +	if (file_size)			\
> +		offset %= size;		\
> +	else				\
> +		offset = 0;		\
> +	if (offset + len > size)	\
> +		len = size - offset;	\
> +} while (0)
>  
> +/*
> + * The operation matrix is complex due to conditional variables.
> + * We calculate how many different operations we can use, then
> + * map them appropriately according to how many options are enabled.
> + * The mapping is:
> + *
> + *		lite	!lite
> + * READ:	0	0
> + * WRITE:	1	1
> + * MAPREAD:	2	2
> + * MAPWRITE:	3	3
> + * TRUNCATE:	-	4
> + * FALLOCATE:	-	5
> + * PUNCH HOLE:	-	6
> + *
> + * When mapped read/writes are disabled, they are simply converted to normal
> + * reads and writes. When fallocate/fpunch calls are disabled, they are
> + * converted to OP_SKIPPED.
> + */
>  void
>  test(void)
>  {
>  	unsigned long	offset;
>  	unsigned long	size = maxoplen;
>  	unsigned long	rv = random();
> -	unsigned long	op = rv % (3 + !lite + mapped_writes + fallocate_calls + punch_hole_calls);
> -        /* turn off the map read if necessary */
> -
> -        if (op == 2 && !mapped_reads)
> -            op = 0;
> +	unsigned long	op;
>  
>  	if (simulatedopcount > 0 && testcalls == simulatedopcount)
>  		writefileimage();
> @@ -982,62 +1006,82 @@ test(void)
>  	if (!quiet && testcalls < simulatedopcount && testcalls % 100000 == 0)
>  		prt("%lu...\n", testcalls);
>  
> -	/*
> -	 *                 lite  !lite
> -	 * READ:	op = 0	   0
> -	 * WRITE:	op = 1     1
> -	 * MAPREAD:     op = 2     2
> -	 * TRUNCATE:	op = -     3
> -	 * MAPWRITE:    op = 3     4
> -	 * FALLOCATE:   op = -     5
> -	 * PUNCH HOLE:  op = -     6
> -	 */
> -	if (lite ? 0 : op == 3 && (style & 1) == 0) /* vanilla truncate? */
> -		dotruncate(random() % maxfilelen);
> -	else {
> -		if (randomoplen)
> -			size = random() % (maxoplen+1);
> -
> -		if (lite ? 0 : op == 3) {
> -			/* truncate */
> -			dotruncate(size);
> -		} else {
> -			offset = random();
> -			if (op == 5) {
> -				/* fallocate */
> -				offset %= maxfilelen;
> -				if (offset + size > maxfilelen)
> -					size = maxfilelen - offset;
> -				do_preallocate(offset, size);
> -			} else if (op == 6) {
> -				offset %= maxfilelen;
> -				if (offset + size > maxfilelen)
> -					size = maxfilelen - offset;
> -				do_punch_hole(offset, size);
> -			} else if (op == 1 || op == (lite ? 3 : 4)) {
> -				/* write / mapwrite */
> -				offset %= maxfilelen;
> -				if (offset + size > maxfilelen)
> -					size = maxfilelen - offset;
> -				if (op != 1)
> -					domapwrite(offset, size);
> -				else
> -					dowrite(offset, size);
> -			} else {
> -				/* read / mapread */
> -				if (file_size)
> -					offset %= file_size;
> -				else
> -					offset = 0;
> -				if (offset + size > file_size)
> -					size = file_size - offset;
> -				if (op != 0)
> -					domapread(offset, size);
> -				else
> -					doread(offset, size);
> -			}
> +	offset = random();
> +	if (randomoplen)
> +		size = random() % (maxoplen + 1);
> +
> +	/* calculate appropriate op to run */
> +	if (lite)
> +		op = rv % 4;
> +	else
> +		op = rv % 7;
> +
> +	switch (op) {
> +	case 2:
> +		if (!mapped_reads)
> +			op = 0;
> +		break;
> +	case 3:
> +		if (!mapped_writes)
> +			op = 1;
> +		break;
> +	case 5:
> +		if (!fallocate_calls) {
> +			log4(OP_SKIPPED, OP_FALLOCATE, offset, size);
> +			goto out;
> +		}
> +		break;
> +	case 6:
> +		if (!punch_hole_calls) {
> +			log4(OP_SKIPPED, OP_PUNCH_HOLE, offset, size);
> +			goto out;
>  		}
> +		break;
>  	}
> +
> +	switch (op) {
> +	case 0:	/* read */
> +		TRIM_OFF_LEN(offset, size, file_size);
> +		doread(offset, size);
> +		break;
> +
> +	case 1: /* write */
> +		TRIM_OFF_LEN(offset, size, maxfilelen);
> +		dowrite(offset, size);
> +		break;
> +
> +	case 2:	/* mapread */
> +		TRIM_OFF_LEN(offset, size, maxfilelen);
> +		domapread(offset, size);
> +		break;
> +
> +	case 3:	/* mapwrite */
> +		TRIM_OFF_LEN(offset, size, maxfilelen);
> +		domapwrite(offset, size);
> +		break;
> +
> +	case 4: /* truncate */
> +		if (!style)
> +			size = random() % maxfilelen;
> +		dotruncate(size);
> +		break;
> +
> +	case 5:	/* fallocate */
> +		TRIM_OFF_LEN(offset, size, maxfilelen);
> +		do_preallocate(offset, size);
> +		break;
> +
> +	case 6:	/* punch */
> +		TRIM_OFF_LEN(offset, size, maxfilelen);
> +		do_punch_hole(offset, size);
> +		break;
> +	default:
> +		prterr("test: unknown operation");
> +		report_failure(42);
> +		break;
> +	}
> +
> +out:
>  	if (sizechecks && testcalls > simulatedopcount)
>  		check_size();
>  	if (closeopen)

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 3/4] xfstests: fix brain-o in fallocate log dump

[Eric Sandeen]

On 6/27/11 12:48 AM, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> fsx segvs when dumping fallocate log entries. Fix magic string
> array index parameters to be zero based rather than one based.

Oh geez.  :(

> While touching log string related stuff, make the format consistent
> with read and write operations so the log dump is easier to look at
> with the human eye.
> 
> Signed-off-by: Dave Chinner <dchinner@redhat.com>

Reviewed-by: Eric Sandeen <sandeen@redhat.com>

> ---
>  ltp/fsx.c |   28 +++++++++++++++-------------
>  1 files changed, 15 insertions(+), 13 deletions(-)
> 
> diff --git a/ltp/fsx.c b/ltp/fsx.c
> index 66daefe..38d5278 100644
> --- a/ltp/fsx.c
> +++ b/ltp/fsx.c
> @@ -223,14 +223,14 @@ logdump(void)
>  		int opnum;
>  
>  		opnum = i+1 + (logcount/LOGSIZE)*LOGSIZE;
> -		prt("%d(%d mod 256): ", opnum, opnum%256);
> +		prt("%d(%3d mod 256): ", opnum, opnum%256);
>  		lp = &oplog[i];
>  		if ((closeopen = lp->operation < 0))
>  			lp->operation = ~ lp->operation;
>  			
>  		switch (lp->operation) {
>  		case OP_MAPREAD:
> -			prt("MAPREAD\t0x%x thru 0x%x\t(0x%x bytes)",
> +			prt("MAPREAD  0x%x thru 0x%x\t(0x%x bytes)",
>  			    lp->args[0], lp->args[0] + lp->args[1] - 1,
>  			    lp->args[1]);
>  			if (badoff >= lp->args[0] && badoff <
> @@ -246,7 +246,7 @@ logdump(void)
>  				prt("\t******WWWW");
>  			break;
>  		case OP_READ:
> -			prt("READ\t0x%x thru 0x%x\t(0x%x bytes)",
> +			prt("READ     0x%x thru 0x%x\t(0x%x bytes)",
>  			    lp->args[0], lp->args[0] + lp->args[1] - 1,
>  			    lp->args[1]);
>  			if (badoff >= lp->args[0] &&
> @@ -254,7 +254,7 @@ logdump(void)
>  				prt("\t***RRRR***");
>  			break;
>  		case OP_WRITE:
> -			prt("WRITE\t0x%x thru 0x%x\t(0x%x bytes)",
> +			prt("WRITE    0x%x thru 0x%x\t(0x%x bytes)",
>  			    lp->args[0], lp->args[0] + lp->args[1] - 1,
>  			    lp->args[1]);
>  			if (lp->args[0] > lp->args[2])
> @@ -275,14 +275,15 @@ logdump(void)
>  			break;
>  		case OP_FALLOCATE:
>  			/* 0: offset 1: length 2: where alloced */
> -			prt("FALLOCATE %s\tfrom 0x%x to 0x%x",
> -			    falloc_type[lp->args[2]], lp->args[0], lp->args[0] + lp->args[1]);
> +			prt("FALLOC   0x%x thru 0x%x\t(0x%x bytes) %s",
> +				lp->args[0], lp->args[0] + lp->args[1],
> +				lp->args[1], falloc_type[lp->args[2]]);
>  			if (badoff >= lp->args[0] &&
>  			    badoff < lp->args[0] + lp->args[1])
>  				prt("\t******FFFF");
>  			break;
>  		case OP_PUNCH_HOLE:
> -			prt("PUNCH HOLE\t0x%x thru 0x%x\t(0x%x bytes)",
> +			prt("PUNCH    0x%x thru 0x%x\t(0x%x bytes)",
>  			    lp->args[0], lp->args[0] + lp->args[1] - 1,
>  			    lp->args[1]);
>  			if (badoff >= lp->args[0] && badoff <
> @@ -877,12 +878,12 @@ do_preallocate(unsigned offset, unsigned length)
>  	}
>  
>  	/*
> -	 * last arg:
> -	 * 	1: allocate past EOF
> -	 * 	2: extending prealloc
> -	 * 	3: interior prealloc
> +	 * last arg matches fallocate string array index in logdump:
> +	 * 	0: allocate past EOF
> +	 * 	1: extending prealloc
> +	 * 	2: interior prealloc
>  	 */
> -	log4(OP_FALLOCATE, offset, length, (end_offset > file_size) ? (keep_size ? 1 : 2) : 3);
> +	log4(OP_FALLOCATE, offset, length, (end_offset > file_size) ? (keep_size ? 0 : 1) : 2);
>  
>  	if (end_offset > file_size) {
>  		memset(good_buf + file_size, '\0', end_offset - file_size);
> @@ -895,7 +896,8 @@ do_preallocate(unsigned offset, unsigned length)
>  	if ((progressinterval && testcalls % progressinterval == 0) ||
>  	    (debug && (monitorstart == -1 || monitorend == -1 ||
>  		      end_offset <= monitorend)))
> -		prt("%lu falloc\tfrom 0x%x to 0x%x\n", testcalls, offset, length);
> +		prt("%lu falloc\tfrom 0x%x to 0x%x (0x%x bytes)\n", testcalls,
> +				offset, offset + length, length);
>  	if (fallocate(fd, keep_size ? FALLOC_FL_KEEP_SIZE : 0, (loff_t)offset, (loff_t)length) == -1) {
>  	        prt("fallocate: %x to %x\n", offset, length);
>  		prterr("do_preallocate: fallocate");

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 4/4] xfstests: add mapped write fsx operations to 091

[Eric Sandeen]

On 6/27/11 12:48 AM, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> The recent busted fsx updates caused fsx to execute fsx with direct
> IO and mmapped reads and writes on an XFS filesystem. The result
> uncovered a direct-IO write vs mmap read bug to do with EOF
> sub-block zeroing on the direct IO write.
> 
> Hence whiel we do not recommend that pepole mix DIO with mmap on the
> same file, we should at least have tests that exercise it as they
> often show up other problems like this.
> 
> 
> Signed-off-by: Dave Chinner <dchinner@redhat.com>

Reviewed-by: Eric Sandeen <sandeen@redhat.com>

> ---
>  091     |    3 +++
>  091.out |    2 ++
>  2 files changed, 5 insertions(+), 0 deletions(-)
> 
> diff --git a/091 b/091
> index a13d979..11b599e 100755
> --- a/091
> +++ b/091
> @@ -88,6 +88,9 @@ kernel=`uname -r  | sed -e 's/\(2\..\).*/\1/'`
>  #run_fsx -N 10000  -o 128000 -l 500000 -r PSIZE -t PSIZE -w PSIZE -Z -W
>   run_fsx -N 10000  -o 128000 -l 500000 -r PSIZE -t BSIZE -w BSIZE -Z -W
>  
> + run_fsx -N 10000  -o 8192   -l 500000 -r PSIZE -t BSIZE -w BSIZE -Z
> + run_fsx -N 10000  -o 128000 -l 500000 -r PSIZE -t BSIZE -w BSIZE -Z
> +
>  # Commented out calls above are less likely to pick up issues, so
>  # save time by commenting them out (leave 'em for manual testing).
>  
> diff --git a/091.out b/091.out
> index 31bd25d..27ed1e3 100644
> --- a/091.out
> +++ b/091.out
> @@ -5,3 +5,5 @@ fsx -N 10000 -o 32768 -l 500000 -r PSIZE -t BSIZE -w BSIZE -Z -R -W
>  fsx -N 10000 -o 8192 -l 500000 -r PSIZE -t BSIZE -w BSIZE -Z -R -W
>  fsx -N 10000 -o 32768 -l 500000 -r PSIZE -t BSIZE -w BSIZE -Z -R -W
>  fsx -N 10000 -o 128000 -l 500000 -r PSIZE -t BSIZE -w BSIZE -Z -W
> +fsx -N 10000 -o 8192 -l 500000 -r PSIZE -t BSIZE -w BSIZE -Z
> +fsx -N 10000 -o 128000 -l 500000 -r PSIZE -t BSIZE -w BSIZE -Z

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 2/4] xfstests: fsx fallocate support is b0rked

[Dave Chinner]

On Mon, Jun 27, 2011 at 04:16:20PM -0500, Eric Sandeen wrote:
> On 6/27/11 12:48 AM, Dave Chinner wrote:
> > From: Dave Chinner <dchinner@redhat.com>
> > 
> > The recent fallocate/fpunch additions to fsx have not actually be
> > executing fallocate/fpunch operations. The logic to select what
> > operation to run is broken in such a way that fsx has been executing
> > mapped writes and truncates instead of fallocate and fpunch
> > operations.
> > 
> > Remove all the (b0rken) smarty-pants selection logic from the test()
> 
> I hope I only extended that smarty-pants logic and didn't invent it.
> I suppose maybe I broke it first though, damn.

It was convoluted before the fallocate code was added. I can see why
it was easy to break....

> > function. Replace it with a clearly defined set of operations for
> > each mode and use understandable fallback logic when various
> > operation types have been disabled. Then use a simple switch
> > statement to execute each of the different operations, removing the
> > tortured nesting of if/else statements that only serve to obfuscate
> > the code.
> > 
> > NAs a result, fsx uses fallocate/fpunch appropriately during
> > operations, and uses/disableѕ the operations as defined on the
> > command line correctly.
> 
> Hm we're back in the fsx maintenance business I guess.

As soon as we start modifying it...

> Would it be worth doing defines or an enum for the ops/cases, to make it
> a little more readable?

I think better will be to redefine the OP_* numbers to match this
generation technique so we don't have two different sets of op
numbers.

I'll do this and post a new version.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 1/4] xfstests: fix fsx fpunch test to actually test for fpunch

[Dave Chinner]

From: Dave Chinner <dchinner@redhat.com>

The operation flags parameter to fallocate is the second parameter,
not the last. Hence the fpunch test is actually testing for falloc
support, not fpunch. Somebody needs a brown paper bag.

Also, add a ftruncate call whenthe fpunch succeeds just in case the
file was not already zero sized. Failing to ensure we start with a
zero length file can cause read ops to fail size checks if they
occur before the file is written to be the main test loop.

While there, observe the quiet flag the same as the falloc test
does and have them both emit the warning at the same error level.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
 ltp/fsx.c |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/ltp/fsx.c b/ltp/fsx.c
index 0683853..a37e223 100644
--- a/ltp/fsx.c
+++ b/ltp/fsx.c
@@ -1243,7 +1243,7 @@ test_fallocate()
 	if (!lite && fallocate_calls) {
 		if (fallocate(fd, 0, 0, 1) && errno == EOPNOTSUPP) {
 			if(!quiet)
-				prt("fsx: main: filesystem does not support fallocate, disabling\n");
+				warn("main: filesystem does not support fallocate, disabling\n");
 			fallocate_calls = 0;
 		} else {
 			ftruncate(fd, 0);
@@ -1260,13 +1260,13 @@ test_punch_hole()
 {
 #ifdef FALLOC_FL_PUNCH_HOLE
 	if (!lite && punch_hole_calls) {
-		if (fallocate(fd, 0, 0,
-			FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE) &&
-			errno == EOPNOTSUPP) {
-
-			warn("main: filesystem does not support fallocate punch hole, disabling");
+		if (fallocate(fd, FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE,
+				0, 1) && errno == EOPNOTSUPP) {
+			if(!quiet)
+				warn("main: filesystem does not support fallocate punch hole, disabling");
 			punch_hole_calls = 0;
-		}
+		} else
+			ftruncate(fd, 0);
 	}
 #else /* ! PUNCH HOLE */
 	punch_hole_calls = 0;
-- 
1.7.5.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 1/4] xfstests: fix fsx fpunch test to actually test for fpunch

[Alex Elder]

On Fri, 2011-07-08 at 10:53 +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> The operation flags parameter to fallocate is the second parameter,
> not the last. Hence the fpunch test is actually testing for falloc
> support, not fpunch. Somebody needs a brown paper bag.
> 
> Also, add a ftruncate call whenthe fpunch succeeds just in case the
> file was not already zero sized. Failing to ensure we start with a
> zero length file can cause read ops to fail size checks if they
> occur before the file is written to be the main test loop.
> 
> While there, observe the quiet flag the same as the falloc test
> does and have them both emit the warning at the same error level.
> 
> Signed-off-by: Dave Chinner <dchinner@redhat.com>

Looks good.  Even if the arguments were in the
right order, the length has to be greater than
zero also.

Reviewed-by: Alex Elder <aelder@sgi.com>



_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs