[PATCH] fsstress: consistently index the ops array by OP_ type

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



From: Darrick J. Wong <djwong@xxxxxxxxxx>

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@xxxxxxxxxx>
---
 ltp/fsstress.c |  140 ++++++++++++++++++++++++++------------------------------
 1 file changed, 66 insertions(+), 74 deletions(-)

diff --git a/ltp/fsstress.c b/ltp/fsstress.c
index 003e0e49..5f3126e6 100644
--- a/ltp/fsstress.c
+++ b/ltp/fsstress.c
@@ -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)



[Index of Archives]     [XFS Filesystem Development (older mail)]     [Linux Filesystem Development]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux RAID]     [Linux SCSI]


  Powered by Linux