fsstress: consistently index the ops array by OP_ type
authorDarrick J. Wong <djwong@kernel.org>
Wed, 8 Dec 2021 16:45:18 +0000 (08:45 -0800)
committerEryu Guan <guaneryu@gmail.com>
Sun, 12 Dec 2021 14:24:16 +0000 (22:24 +0800)
A mismerge during a git rebase some time ago broke fsstress in my
development tree, because it added OP_XCHGRANGE into the opt_y typedef
definition at a different offset than the actual entry in the ops array.
This broke the relationship ops[i].op == i.

Since most of fsstress.c blindly assumes that it's ok to index the ops
array by OP_ type, this off-by-one error meant that when I created an
fstest with "-f unlink=1", it actually set the frequency of the adjacent
operation (unresvsp) to 1.  I didn't notice this until I started to
investigate how a filesystem created with "-z -f creat=4 -f unlink=4"
could end up with 1.8 million files after 30 seconds.

Eliminate the possibility for future screwups like this by using indexed
array initializers.  This enables us to remove the separate op field in
struct opdesc, for a minor savings of memory footprint and reduction in
footgun opportunity.

While we're at it, reformat the ops table to be more pleasing to the
eye.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Eryu Guan <guaneryu@gmail.com>
Signed-off-by: Eryu Guan <guaneryu@gmail.com>
ltp/fsstress.c

index 003e0e49a173b8cc58bac086f71af0d5b2d99494..5f3126e6747114bc2581d2d8776a06a3f7313c71 100644 (file)
@@ -153,7 +153,6 @@ typedef long long opnum_t;
 typedef void (*opfnc_t)(opnum_t, long);
 
 typedef struct opdesc {
-       opty_t  op;
        char    *name;
        opfnc_t func;
        int     freq;
@@ -279,74 +278,74 @@ void      write_f(opnum_t, long);
 void   writev_f(opnum_t, long);
 char   *xattr_flag_to_string(int);
 
-opdesc_t       ops[] = {
-     /* { OP_ENUM, "name", function, freq, iswrite }, */
-       { OP_AFSYNC, "afsync", afsync_f, 0, 1 },
-       { OP_ALLOCSP, "allocsp", allocsp_f, 1, 1 },
-       { OP_AREAD, "aread", aread_f, 1, 0 },
-       { OP_ATTR_REMOVE, "attr_remove", attr_remove_f, /* 1 */ 0, 1 },
-       { OP_ATTR_SET, "attr_set", attr_set_f, /* 2 */ 0, 1 },
-       { OP_AWRITE, "awrite", awrite_f, 1, 1 },
-       { OP_BULKSTAT, "bulkstat", bulkstat_f, 1, 0 },
-       { OP_BULKSTAT1, "bulkstat1", bulkstat1_f, 1, 0 },
-       { OP_CHOWN, "chown", chown_f, 3, 1 },
-       { OP_CLONERANGE, "clonerange", clonerange_f, 4, 1 },
-       { OP_COPYRANGE, "copyrange", copyrange_f, 4, 1 },
-       { OP_CREAT, "creat", creat_f, 4, 1 },
-       { OP_DEDUPERANGE, "deduperange", deduperange_f, 4, 1},
-       { OP_DREAD, "dread", dread_f, 4, 0 },
-       { OP_DWRITE, "dwrite", dwrite_f, 4, 1 },
-       { OP_FALLOCATE, "fallocate", fallocate_f, 1, 1 },
-       { OP_FDATASYNC, "fdatasync", fdatasync_f, 1, 1 },
-       { OP_FIEMAP, "fiemap", fiemap_f, 1, 1 },
-       { OP_FREESP, "freesp", freesp_f, 1, 1 },
-       { OP_FSYNC, "fsync", fsync_f, 1, 1 },
-       { OP_GETATTR, "getattr", getattr_f, 1, 0 },
-       { OP_GETDENTS, "getdents", getdents_f, 1, 0 },
+struct opdesc  ops[OP_LAST]    = {
+     /* [OP_ENUM]         = {"name",          function,        freq, iswrite }, */
+       [OP_AFSYNC]        = {"afsync",        afsync_f,        0, 1 },
+       [OP_ALLOCSP]       = {"allocsp",       allocsp_f,       1, 1 },
+       [OP_AREAD]         = {"aread",         aread_f,         1, 0 },
+       [OP_ATTR_REMOVE]   = {"attr_remove",   attr_remove_f,   0, 1 },
+       [OP_ATTR_SET]      = {"attr_set",      attr_set_f,      0, 1 },
+       [OP_AWRITE]        = {"awrite",        awrite_f,        1, 1 },
+       [OP_BULKSTAT]      = {"bulkstat",      bulkstat_f,      1, 0 },
+       [OP_BULKSTAT1]     = {"bulkstat1",     bulkstat1_f,     1, 0 },
+       [OP_CHOWN]         = {"chown",         chown_f,         3, 1 },
+       [OP_CLONERANGE]    = {"clonerange",    clonerange_f,    4, 1 },
+       [OP_COPYRANGE]     = {"copyrange",     copyrange_f,     4, 1 },
+       [OP_CREAT]         = {"creat",         creat_f,         4, 1 },
+       [OP_DEDUPERANGE]   = {"deduperange",   deduperange_f,   4, 1 },
+       [OP_DREAD]         = {"dread",         dread_f,         4, 0 },
+       [OP_DWRITE]        = {"dwrite",        dwrite_f,        4, 1 },
+       [OP_FALLOCATE]     = {"fallocate",     fallocate_f,     1, 1 },
+       [OP_FDATASYNC]     = {"fdatasync",     fdatasync_f,     1, 1 },
+       [OP_FIEMAP]        = {"fiemap",        fiemap_f,        1, 1 },
+       [OP_FREESP]        = {"freesp",        freesp_f,        1, 1 },
+       [OP_FSYNC]         = {"fsync",         fsync_f,         1, 1 },
+       [OP_GETATTR]       = {"getattr",       getattr_f,       1, 0 },
+       [OP_GETDENTS]      = {"getdents",      getdents_f,      1, 0 },
        /* get extended attribute */
-       { OP_GETFATTR, "getfattr", getfattr_f, 1, 0 },
-       { OP_LINK, "link", link_f, 1, 1 },
+       [OP_GETFATTR]      = {"getfattr",      getfattr_f,      1, 0 },
+       [OP_LINK]          = {"link",          link_f,          1, 1 },
        /* list extent attributes */
-       { OP_LISTFATTR, "listfattr", listfattr_f, 1, 0 },
-       { OP_MKDIR, "mkdir", mkdir_f, 2, 1 },
-       { OP_MKNOD, "mknod", mknod_f, 2, 1 },
-       { OP_MREAD, "mread", mread_f, 2, 0 },
-       { OP_MWRITE, "mwrite", mwrite_f, 2, 1 },
-       { OP_PUNCH, "punch", punch_f, 1, 1 },
-       { OP_ZERO, "zero", zero_f, 1, 1 },
-       { OP_COLLAPSE, "collapse", collapse_f, 1, 1 },
-       { OP_INSERT, "insert", insert_f, 1, 1 },
-       { OP_READ, "read", read_f, 1, 0 },
-       { OP_READLINK, "readlink", readlink_f, 1, 0 },
-       { OP_READV, "readv", readv_f, 1, 0 },
+       [OP_LISTFATTR]     = {"listfattr",     listfattr_f,     1, 0 },
+       [OP_MKDIR]         = {"mkdir",         mkdir_f,         2, 1 },
+       [OP_MKNOD]         = {"mknod",         mknod_f,         2, 1 },
+       [OP_MREAD]         = {"mread",         mread_f,         2, 0 },
+       [OP_MWRITE]        = {"mwrite",        mwrite_f,        2, 1 },
+       [OP_PUNCH]         = {"punch",         punch_f,         1, 1 },
+       [OP_ZERO]          = {"zero",          zero_f,          1, 1 },
+       [OP_COLLAPSE]      = {"collapse",      collapse_f,      1, 1 },
+       [OP_INSERT]        = {"insert",        insert_f,        1, 1 },
+       [OP_READ]          = {"read",          read_f,          1, 0 },
+       [OP_READLINK]      = {"readlink",      readlink_f,      1, 0 },
+       [OP_READV]         = {"readv",         readv_f,         1, 0 },
        /* remove (delete) extended attribute */
-       { OP_REMOVEFATTR, "removefattr", removefattr_f, 1, 1 },
-       { OP_RENAME, "rename", rename_f, 2, 1 },
-       { OP_RNOREPLACE, "rnoreplace", rnoreplace_f, 2, 1 },
-       { OP_REXCHANGE, "rexchange", rexchange_f, 2, 1 },
-       { OP_RWHITEOUT, "rwhiteout", rwhiteout_f, 2, 1 },
-       { OP_RESVSP, "resvsp", resvsp_f, 1, 1 },
-       { OP_RMDIR, "rmdir", rmdir_f, 1, 1 },
+       [OP_REMOVEFATTR]   = {"removefattr",   removefattr_f,   1, 1 },
+       [OP_RENAME]        = {"rename",        rename_f,        2, 1 },
+       [OP_RNOREPLACE]    = {"rnoreplace",    rnoreplace_f,    2, 1 },
+       [OP_REXCHANGE]     = {"rexchange",     rexchange_f,     2, 1 },
+       [OP_RWHITEOUT]     = {"rwhiteout",     rwhiteout_f,     2, 1 },
+       [OP_RESVSP]        = {"resvsp",        resvsp_f,        1, 1 },
+       [OP_RMDIR]         = {"rmdir",         rmdir_f,         1, 1 },
        /* set attribute flag (FS_IOC_SETFLAGS ioctl) */
-       { OP_SETATTR, "setattr", setattr_f, 0, 1 },
+       [OP_SETATTR]       = {"setattr",       setattr_f,       0, 1 },
        /* set extended attribute */
-       { OP_SETFATTR, "setfattr", setfattr_f, 2, 1 },
+       [OP_SETFATTR]      = {"setfattr",      setfattr_f,      2, 1 },
        /* set project id (XFS_IOC_FSSETXATTR ioctl) */
-       { OP_SETXATTR, "setxattr", setxattr_f, 1, 1 },
-       { OP_SNAPSHOT, "snapshot", snapshot_f, 1, 1 },
-       { OP_SPLICE, "splice", splice_f, 1, 1 },
-       { OP_STAT, "stat", stat_f, 1, 0 },
-       { OP_SUBVOL_CREATE, "subvol_create", subvol_create_f, 1, 1},
-       { OP_SUBVOL_DELETE, "subvol_delete", subvol_delete_f, 1, 1},
-       { OP_SYMLINK, "symlink", symlink_f, 2, 1 },
-       { OP_SYNC, "sync", sync_f, 1, 1 },
-       { OP_TRUNCATE, "truncate", truncate_f, 2, 1 },
-       { OP_UNLINK, "unlink", unlink_f, 1, 1 },
-       { OP_UNRESVSP, "unresvsp", unresvsp_f, 1, 1 },
-       { OP_URING_READ, "uring_read", uring_read_f, 1, 0 },
-       { OP_URING_WRITE, "uring_write", uring_write_f, 1, 1 },
-       { OP_WRITE, "write", write_f, 4, 1 },
-       { OP_WRITEV, "writev", writev_f, 4, 1 },
+       [OP_SETXATTR]      = {"setxattr",      setxattr_f,      1, 1 },
+       [OP_SNAPSHOT]      = {"snapshot",      snapshot_f,      1, 1 },
+       [OP_SPLICE]        = {"splice",        splice_f,        1, 1 },
+       [OP_STAT]          = {"stat",          stat_f,          1, 0 },
+       [OP_SUBVOL_CREATE] = {"subvol_create", subvol_create_f, 1, 1 },
+       [OP_SUBVOL_DELETE] = {"subvol_delete", subvol_delete_f, 1, 1 },
+       [OP_SYMLINK]       = {"symlink",       symlink_f,       2, 1 },
+       [OP_SYNC]          = {"sync",          sync_f,          1, 1 },
+       [OP_TRUNCATE]      = {"truncate",      truncate_f,      2, 1 },
+       [OP_UNLINK]        = {"unlink",        unlink_f,        1, 1 },
+       [OP_UNRESVSP]      = {"unresvsp",      unresvsp_f,      1, 1 },
+       [OP_URING_READ]    = {"uring_read",    uring_read_f,    1, 0 },
+       [OP_URING_WRITE]   = {"uring_write",   uring_write_f,   1, 1 },
+       [OP_WRITE]         = {"write",         write_f,         4, 1 },
+       [OP_WRITEV]        = {"writev",        writev_f,        4, 1 },
 }, *ops_end;
 
 flist_t        flist[FT_nft] = {
@@ -1520,7 +1519,7 @@ make_freq_table(void)
        freq_table_size = f;
        for (p = ops, i = 0; p < ops_end; p++) {
                for (f = 0; f < p->freq; f++, i++)
-                       freq_table[i] = p->op;
+                       freq_table[i] = p - ops;
        }
 }
 
@@ -1966,7 +1965,7 @@ opty_t btrfs_ops[] = {
 void
 non_btrfs_freq(const char *path)
 {
-       opdesc_t                *p;
+       int                     i;
 #ifdef HAVE_BTRFSUTIL_H
        enum btrfs_util_error   e;
 
@@ -1974,15 +1973,8 @@ non_btrfs_freq(const char *path)
        if (e != BTRFS_UTIL_ERROR_NOT_BTRFS)
                return;
 #endif
-       for (p = ops; p < ops_end; p++) {
-               int i;
-               for (i = 0; i < ARRAY_SIZE(btrfs_ops); i++) {
-                       if (p->op == btrfs_ops[i]) {
-                               p->freq = 0;
-                               break;
-                       }
-               }
-       }
+       for (i = 0; i < ARRAY_SIZE(btrfs_ops); i++)
+               ops[btrfs_ops[i]].freq = 0;
 }
 
 void inode_info(char *str, size_t sz, struct stat64 *s, int verbose)