xfs
[Top] [All Lists]

[PATCH v3] xfstests:Make 225 compare map and fiemap at each block.

To: sandeen@xxxxxxxxxx
Subject: [PATCH v3] xfstests:Make 225 compare map and fiemap at each block.
From: Yongqiang Yang <xiaoqiangnk@xxxxxxxxx>
Date: Mon, 23 May 2011 15:07:03 +0800
Cc: josef@xxxxxxxxxx, david@xxxxxxxxxxxxx, linux-ext4@xxxxxxxxxxxxxxx, xfs@xxxxxxxxxxx, Yongqiang Yang <xiaoqiangnk@xxxxxxxxx>
Dkim-signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:from:to:cc:subject:date:message-id:x-mailer; bh=GSjA65UJYXQT8G4viSr3f74cwTnyfnhelkaKyKDRxtg=; b=rzvgTbf7QvbyPgUNRNhD7xinLx5XV3Z92qXiQISm0kex9WEMmJi6ltdIxORAtaScDK +bDSjznPsiDOFUvqVONl38vQ/5GcXh4XlR1qBcPQ3SMhhhKgJUyezLYEbXBWRIEERCsF jI+C3TC4jcuXnSSWulg/ztXWTs2lfMSyqaBbs=
Domainkey-signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=from:to:cc:subject:date:message-id:x-mailer; b=GesprjkdGJFt+O5l+KfRJ5HpXLtHojSnRXe5BB4/kLDsa2YspfBCSf9hZesMREXlKC oIWOtzrcwyVRWTuJBvZXKu57Epxn+iyjqNQWPMyRJj/YwKYnbbmFvsdRKWfdozIAZcms y8izoJ99PTEdPPoNTbBfCpVI14WpYO1h89EIo=
Due to my carelessness,  I induced a ugly patch to ext4's fiemap, as a result
delayed-extents that did not start at the head block of a page was ignored
in ext4 with 1K block, but 225 could not find it.  So I looked into the 225
and could not figure out logic in compare_map_and_fiemap(), which seemed to
mix extents with blocks.  Then I made 225 compare map and fiemap at each block,
the new 225 can find the bug I induced and another bug in ext4 with 1k block,
which ignored delayed-extents after a hole, which did not start at the head
block of a page.


The new 225 works well on ext3, ext4 and xfs with both 1K and 4K block.

Reviewed-by: josef@xxxxxxxxx
Signed-off-by: Yongqiang Yang <xiaoqiangnk@xxxxxxxxx>
---
v2->v3:
  Add much more specific changelog expalining the bug the old 225 could not 
find.

v1->v2:
Josef <josef@xxxxxxxxxx> reviewed the v1 patch and pointed out the bug which
made xfs not work and several coding styles.

According to reply from Josef, I
 fixed the bug which added ';' after an if statement.
 removed the trailing whitespace.

Apart from things above, I
 made check_weird_fs_hole() verify bytes read by pread().

 src/fiemap-tester.c |  251 ++++++++++++++++++++++++++++-----------------------
 1 files changed, 140 insertions(+), 111 deletions(-)

diff --git a/src/fiemap-tester.c b/src/fiemap-tester.c
index 1663f84..319a9bb 100644
--- a/src/fiemap-tester.c
+++ b/src/fiemap-tester.c
@@ -14,6 +14,9 @@
  * 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
+ *
+ * Compare map and fiemap at each block,
+ * Yongqiang Yang <xiaoqiangnk@xxxxxxxxx>, 2011
  */
 
 #include <stdio.h>
@@ -57,7 +60,7 @@ generate_file_mapping(int blocks, int prealloc)
        int num_types = 2, cur_block = 0;
        int i = 0;
 
-       map = malloc(sizeof(char) * blocks);
+       map = malloc(sizeof(char) * (blocks + 1));
        if (!map)
                return NULL;
 
@@ -81,6 +84,7 @@ generate_file_mapping(int blocks, int prealloc)
                cur_block++;
        }
 
+       map[blocks] = 0;
        return map;
 }
 
@@ -247,55 +251,36 @@ check_flags(struct fiemap *fiemap, int blocksize)
 }
 
 static int
-check_data(struct fiemap *fiemap, __u64 logical_offset, int blocksize,
+check_data(struct fiemap_extent *extent ,  __u64 logical_offset, int blocksize,
           int last, int prealloc)
 {
-       struct fiemap_extent *extent;
-       __u64 orig_offset = logical_offset;
-       int c, found = 0;
-
-       for (c = 0; c < fiemap->fm_mapped_extents; c++) {
-               __u64 start, end;
-               extent = &fiemap->fm_extents[c];
-
-               start = extent->fe_logical;
-               end = extent->fe_logical + extent->fe_length;
-
-               if (logical_offset > end)
-                       continue;
-
-               if (logical_offset + blocksize < start)
-                       break;
-
-               if (logical_offset >= start &&
-                   logical_offset < end) {
-                       if (prealloc &&
-                           !(extent->fe_flags & FIEMAP_EXTENT_UNWRITTEN)) {
-                               printf("ERROR: preallocated extent is not "
-                                      "marked with FIEMAP_EXTENT_UNWRITTEN: "
-                                      "%llu\n",
-                                      (unsigned long long)
-                                      (start / blocksize));
-                               return -1;
-                       }
-
-                       if (logical_offset + blocksize > end) {
-                               logical_offset = end+1;
-                               continue;
-                       } else {
-                               found = 1;
-                               break;
-                       }
+       int found = 0;
+       __u64 start, end;
+
+       start = extent->fe_logical;
+       end = extent->fe_logical + extent->fe_length;
+
+       if (logical_offset >= start &&
+           logical_offset < end) {
+               if (prealloc &&
+                   !(extent->fe_flags & FIEMAP_EXTENT_UNWRITTEN)) {
+                       printf("ERROR: preallocated extent is not "
+                              "marked with FIEMAP_EXTENT_UNWRITTEN: "
+                              "%llu\n",
+                              (unsigned long long)
+                              (start / blocksize));
+                       return -1;
                }
+               found = 1;
        }
 
        if (!found) {
                printf("ERROR: couldn't find extent at %llu\n",
-                      (unsigned long long)(orig_offset / blocksize));
+                      (unsigned long long)(logical_offset / blocksize));
        } else if (last &&
-                  !(fiemap->fm_extents[c].fe_flags & FIEMAP_EXTENT_LAST)) {
+                  !(extent->fe_flags & FIEMAP_EXTENT_LAST)) {
                printf("ERROR: last extent not marked as last: %llu\n",
-                      (unsigned long long)(orig_offset / blocksize));
+                      (unsigned long long)(logical_offset / blocksize));
                found = 0;
        }
 
@@ -306,7 +291,7 @@ static int
 check_weird_fs_hole(int fd, __u64 logical_offset, int blocksize)
 {
        static int warning_printed = 0;
-       int block, i;
+       int block, i, count;
        size_t buf_len = sizeof(char) * blocksize;
        char *buf;
 
@@ -330,15 +315,16 @@ check_weird_fs_hole(int fd, __u64 logical_offset, int 
blocksize)
                return -1;
        }
 
-       if (pread(fd, buf, buf_len, (off_t)logical_offset) < 0) {
+       count = pread(fd, buf, buf_len, (off_t)logical_offset);
+       if (count < 0) {
                perror("Error reading from file");
                free(buf);
                return -1;
        }
 
-       for (i = 0; i < buf_len; i++) {
+       for (i = 0; i < count; i++) {
                if (buf[i] != 0) {
-                       printf("ERROR: FIEMAP claimed there was data (%c) at "
+                       printf("ERROR: FIEMAP claimed there was data (%x) at "
                               "block %llu that should have been a hole, and "
                               "FIBMAP confirmed that it was allocated, but "
                               "it should be filled with 0's, but it was not "
@@ -370,35 +356,25 @@ check_weird_fs_hole(int fd, __u64 logical_offset, int 
blocksize)
 }
 
 static int
-check_hole(struct fiemap *fiemap, int fd, __u64 logical_offset, int blocksize)
+check_hole(struct fiemap_extent *extent, int fd, __u64 logical_offset,
+          int blocksize)
 {
-       struct fiemap_extent *extent;
-       int c;
+       __u64 start, end;
 
-       for (c = 0; c < fiemap->fm_mapped_extents; c++) {
-               __u64 start, end;
-               extent = &fiemap->fm_extents[c];
-
-               start = extent->fe_logical;
-               end = extent->fe_logical + extent->fe_length;
-
-               if (logical_offset > end)
-                       continue;
-               if (logical_offset + blocksize < start)
-                       break;
+       start = extent->fe_logical;
+       end = extent->fe_logical + extent->fe_length;
 
-               if (logical_offset >= start &&
-                   logical_offset < end) {
+       if (logical_offset >= start &&
+           logical_offset < end) {
 
-                       if (check_weird_fs_hole(fd, logical_offset,
-                                               blocksize) == 0)
-                               break;
+               if (check_weird_fs_hole(fd, logical_offset,
+                                       blocksize) == 0)
+                       return 0;
 
-                       printf("ERROR: found an allocated extent where a hole "
-                              "should be: %llu\n",
-                              (unsigned long long)(start / blocksize));
-                       return -1;
-               }
+               printf("ERROR: found an allocated extent where a hole "
+                      "should be: %llu\n",
+                      (unsigned long long)(logical_offset / blocksize));
+               return -1;
        }
 
        return 0;
@@ -423,9 +399,11 @@ compare_fiemap_and_map(int fd, char *map, int blocks, int 
blocksize, int syncfil
 {
        struct fiemap *fiemap;
        char *fiebuf;
-       int blocks_to_map, ret, cur_extent = 0, last_data;
+       int blocks_to_map, ret, last_data = -1;
        __u64 map_start, map_length;
        int i, c;
+       int cur_block = 0;
+       int last_found = 0;
 
        if (query_fiemap_count(fd, blocks, blocksize) < 0)
                return -1;
@@ -451,8 +429,11 @@ compare_fiemap_and_map(int fd, char *map, int blocks, int 
blocksize, int syncfil
        fiemap->fm_extent_count = blocks_to_map;
        fiemap->fm_mapped_extents = 0;
 
+       /* check fiemap by looking at each block. */
        do {
-               fiemap->fm_start = map_start;
+               int nr_extents;
+
+               fiemap->fm_start = cur_block * blocksize;
                fiemap->fm_length = map_length;
 
                ret = ioctl(fd, FS_IOC_FIEMAP, (unsigned long)fiemap);
@@ -465,45 +446,93 @@ compare_fiemap_and_map(int fd, char *map, int blocks, int 
blocksize, int syncfil
                if (check_flags(fiemap, blocksize))
                        goto error;
 
-               for (i = cur_extent, c = 1; i < blocks; i++, c++) {
-                       __u64 logical_offset = i * blocksize;
-
-                       if (c > fiemap->fm_mapped_extents) {
-                               i++;
-                               break;
+               nr_extents = fiemap->fm_mapped_extents;
+               if (nr_extents == 0) {
+                       int block = cur_block + (map_length - 1) / blocksize;
+                       for (; cur_block <= block &&
+                              cur_block < blocks; cur_block++) {
+                               /* check hole */
+                               if (map[cur_block] != 'H') {
+                                       printf("ERROR: map[%d] should not be "
+                                              "a hole\n", cur_block);
+                                       goto error;
+                               }
                        }
+                       continue;
+               }
 
-                       switch (map[i]) {
-                       case 'D':
-                               if (check_data(fiemap, logical_offset,
-                                              blocksize, last_data == i, 0))
+               for (c = 0; c < nr_extents; c++) {
+                       __u64 offset;
+                       int block;
+                       struct fiemap_extent *extent;
+
+
+                       extent = &fiemap->fm_extents[c];
+                       offset = extent->fe_logical;
+                       block = offset / blocksize;
+
+                       /* check hole. */
+                       for (; cur_block < block && cur_block < blocks;
+                            cur_block++) {
+                               if (map[cur_block] != 'H') {
+                                       printf("ERROR: map[%d] should not be "
+                                              "a hole\n", cur_block);
                                        goto error;
-                               break;
-                       case 'H':
-                               if (check_hole(fiemap, fd, logical_offset,
-                                              blocksize))
+                               }
+                       }
+
+                       offset = extent->fe_logical + extent->fe_length;
+                       block = offset / blocksize;
+                       /* check data */
+                       for (; cur_block < block && cur_block < blocks;
+                            cur_block++) {
+                               int last;
+                               offset = (__u64)cur_block * blocksize;
+                               last = (cur_block == last_data);
+                               last_found = last_found ? last_found : last;
+                               switch (map[cur_block]) {
+                               case 'D':
+                                       if (check_data(extent, offset,
+                                                      blocksize, last, 0))
+                                               goto error;
+                                       break;
+                               case 'H':
+                                       if (check_hole(extent, fd, offset,
+                                                      blocksize))
+                                               goto error;
+                                       break;
+
+                               case 'P':
+                                       if (check_data(extent, offset,
+                                                      blocksize, last, 1))
+                                               goto error;
+                                       break;
+                               default:
+                                       printf("ERROR: weird value in "
+                                              "map: %c\n", map[i]);
                                        goto error;
-                               break;
-                       case 'P':
-                               if (check_data(fiemap, logical_offset,
-                                              blocksize, last_data == i, 1))
+                               }
+                       }
+
+                       for (; cur_block < block; cur_block++) {
+                               offset = (__u64)cur_block * blocksize;
+                               if (check_hole(extent, fd, offset, blocksize))
                                        goto error;
-                               break;
-                       default:
-                               printf("ERROR: weird value in map: %c\n",
-                                      map[i]);
-                               goto error;
                        }
                }
-               cur_extent = i;
-               map_start = i * blocksize;
-       } while (cur_extent < blocks);
+       } while (cur_block < blocks);
 
-       ret = 0;
-       return ret;
+       if (!last_found && last_data != -1) {
+               printf("ERROR: find no last extent\n");
+               goto error;
+       }
+
+       free(fiebuf);
+       return 0;
 error:
        printf("map is '%s'\n", map);
        show_extents(fiemap, blocksize);
+       free(fiebuf);
        return -1;
 }
 
@@ -605,6 +634,18 @@ main(int argc, char **argv)
                printf("Starting infinite run, if you don't see any output "
                       "then its working properly.\n");
        do {
+               if (ftruncate(fd, 0)) {
+                       perror("Could not truncate file\n");
+                       close(fd);
+                       exit(1);
+               }
+
+               if (lseek(fd, 0, SEEK_SET)) {
+                       perror("Could not seek set\n");
+                       close(fd);
+                       exit(1);
+               }
+
                if (!map) {
                        blocks = random() % maxblocks;
                        if (blocks == 0) {
@@ -639,18 +680,6 @@ main(int argc, char **argv)
                free(map);
                map = NULL;
 
-               if (ftruncate(fd, 0)) {
-                       perror("Could not truncate file\n");
-                       close(fd);
-                       exit(1);
-               }
-
-               if (lseek(fd, 0, SEEK_SET)) {
-                       perror("Could not seek set\n");
-                       close(fd);
-                       exit(1);
-               }
-
                if (runs) runs--;
        } while (runs != 0);
 
-- 
1.7.5.1

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