xfs
[Top] [All Lists]

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

To: xfs@xxxxxxxxxxx
Subject: [PATCH 2/4] xfstests: fsx fallocate support is b0rked
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Mon, 27 Jun 2011 15:48:40 +1000
In-reply-to: <1309153722-1231-1-git-send-email-david@xxxxxxxxxxxxx>
References: <1309153722-1231-1-git-send-email-david@xxxxxxxxxxxxx>
From: Dave Chinner <dchinner@xxxxxxxxxx>

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@xxxxxxxxxx>
---
 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

<Prev in Thread] Current Thread [Next in Thread>