[PATCH v6 1/2] fsstress: add mwrite/mread into test operation list

[PATCH v6 1/2] fsstress: add mwrite/mread into test operation list

[Zorro Lang]

mmap as a popular and basic operation, most of softwares use it to
access files. More and more customers report bugs related with
mmap/munmap and other stress conditions.

So add mmap read/write test into fsstress to increase mmap related
stress to reproduce or find more bugs easily.

Signed-off-by: Zorro Lang <zlang@redhat.com>
---

Hi,

V6 do below changes:

1) pick all common things from mwrite and mread into new do_mmap() function.

Before V6, mwrite try to extend the file size by truncate and mwrite on new
extended place. But the reviewer suggest removing the truncate operation,
so mwrite and mread will be nearly same, except one does memset(), and the
other does memcpy().

2) Add sys/mman.h into configure.ac, and include it in global.h

Thanks,
Zorro

 configure.ac   |   1 +
 ltp/fsstress.c | 110 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 src/global.h   |   4 +++
 3 files changed, 115 insertions(+)

diff --git a/configure.ac b/configure.ac
index fa48d2f..246f92e 100644
--- a/configure.ac
+++ b/configure.ac
@@ -32,6 +32,7 @@ AC_HEADER_STDC
 			xfs/platform_defs.h	\
 			btrfs/ioctl.h		\
 			cifs/ioctl.h		\
+			sys/mman.h		\
     ])
 
 AC_CHECK_HEADERS([xfs/xfs_log_format.h],,,[
diff --git a/ltp/fsstress.c b/ltp/fsstress.c
index 7e7cf60..2f65034 100644
--- a/ltp/fsstress.c
+++ b/ltp/fsstress.c
@@ -69,6 +69,8 @@ typedef enum {
 	OP_LINK,
 	OP_MKDIR,
 	OP_MKNOD,
+	OP_MREAD,
+	OP_MWRITE,
 	OP_PUNCH,
 	OP_ZERO,
 	OP_COLLAPSE,
@@ -168,6 +170,8 @@ void	getdents_f(int, long);
 void	link_f(int, long);
 void	mkdir_f(int, long);
 void	mknod_f(int, long);
+void	mread_f(int, long);
+void	mwrite_f(int, long);
 void	punch_f(int, long);
 void	zero_f(int, long);
 void	collapse_f(int, long);
@@ -208,6 +212,8 @@ opdesc_t	ops[] = {
 	{ OP_LINK, "link", link_f, 1, 1 },
 	{ 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 },
@@ -2656,6 +2662,110 @@ mknod_f(int opno, long r)
 }
 
 void
+do_mmap(int opno, long r, int prot)
+{
+#ifdef HAVE_SYS_MMAN_H
+	char		*addr;
+	int		e;
+	pathname_t	f;
+	int		fd;
+	size_t		len;
+	__int64_t	lr;
+	off64_t		off;
+	int		flags;
+	struct stat64	stb;
+	int		v;
+	char		st[1024];
+
+	init_pathname(&f);
+	if (!get_fname(FT_REGFILE, r, &f, NULL, NULL, &v)) {
+		if (v)
+			printf("%d/%d: do_mmap - no filename\n", procid, opno);
+		free_pathname(&f);
+		return;
+	}
+	fd = open_path(&f, O_RDWR);
+	e = fd < 0 ? errno : 0;
+	check_cwd();
+	if (fd < 0) {
+		if (v)
+			printf("%d/%d: do_mmap - open %s failed %d\n",
+			       procid, opno, f.path, e);
+		free_pathname(&f);
+		return;
+	}
+	if (fstat64(fd, &stb) < 0) {
+		if (v)
+			printf("%d/%d: do_mmap - fstat64 %s failed %d\n",
+			       procid, opno, f.path, errno);
+		free_pathname(&f);
+		close(fd);
+		return;
+	}
+	if (stb.st_size == 0) {
+		if (v)
+			printf("%d/%d: do_mmap - %s%s zero size\n", procid, opno,
+			       f.path, st);
+		free_pathname(&f);
+		close(fd);
+		return;
+	}
+
+	inode_info(st, sizeof(st), &stb, v);
+	lr = ((__int64_t)random() << 32) + random();
+	off = (off64_t)(lr % stb.st_size);
+	off &= (off64_t)(~(sysconf(_SC_PAGE_SIZE) - 1));
+	len = (size_t)(random() % MIN(stb.st_size - off, FILELEN_MAX)) + 1;
+
+	flags = (random() % 2) ? MAP_SHARED : MAP_PRIVATE;
+	addr = mmap(NULL, len, prot, flags, fd, off);
+	e = (addr == MAP_FAILED) ? errno : 0;
+	if (e && v)
+		printf("%d/%d: do_mmap - mmap failed %s%s [%lld,%d,%s] %d\n",
+		       procid, opno, f.path, st, (long long)off, (int)len,
+		       (flags & MAP_PRIVATE) ? "MAP_PRIVATE" : "MAP_SHARED", e);
+
+	if (addr != MAP_FAILED) {
+		if (prot & PROT_WRITE) {
+		/* PROT_READ maybe set, if PROT_WRITE is set. Not vice versa */
+			memset(addr, nameseq & 0xff, len);
+		} else {
+			char *buf;
+			if ((buf = malloc(len)) != NULL) {
+				memcpy(buf, addr, len);
+				free(buf);
+			}
+		}
+		e = munmap(addr, len) < 0 ? errno : 0;
+		if (e && v)
+			printf("%d/%d: do_mmap - munmap failed %s%s [%lld,%d] %d\n",
+			       procid, opno, f.path, st, (long long)off,
+			       (int)len, e);
+	}
+	if (v)
+		printf("%d/%d: %s %s%s [%lld,%d,%s] %d\n",
+		       procid, opno, (prot & PROT_WRITE) ? "mwrite" : "mread",
+		       f.path, st, (long long)off, (int)len,
+		       (flags & MAP_PRIVATE) ? "MAP_PRIVATE" : "MAP_SHARED", e);
+
+	free_pathname(&f);
+	close(fd);
+#endif
+}
+
+void
+mread_f(int opno, long r)
+{
+	do_mmap(opno, r, PROT_READ);
+}
+
+void
+mwrite_f(int opno, long r)
+{
+	do_mmap(opno, r, PROT_WRITE);
+}
+
+void
 punch_f(int opno, long r)
 {
 #ifdef HAVE_LINUX_FALLOC_H
diff --git a/src/global.h b/src/global.h
index f63246b..3920c0d 100644
--- a/src/global.h
+++ b/src/global.h
@@ -178,4 +178,8 @@
 
 #endif /* HAVE_LINUX_FALLOC_H */
 
+#ifdef HAVE_SYS_MMAN_H
+#include <sys/mman.h>
+#endif
+
 #endif /* GLOBAL_H */
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe fstests" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  https://urldefense.proofpoint.com/v2/url?u=http-3A__vger.kernel.org_majordomo-2Dinfo.html&d=DwIBAg&c=RoP1YumCXCgaWHvlZYR8PQcxBKCX5YTpkKY057SbK10&r=-yMrTV4jriXR7ieyzzjV-QgHBD0UDw8ixoR77aMeAHE&m=5dhlwSx3gHCj3C-sr-gyfsJuz9OpNM7N7n3izgfTTzA&s=4ThK-lEJc0wE8Uo9Lx23qBeSch1B6f-KxfeCJtvwFuU&e= 

[PATCH v6 2/2] xfs/068: update golden output due to new operations in fsstress

[Zorro Lang]

xfs/068 use a fixed seed (-s) and number of operations (-n) to run
fsstress, to get fixed number of files and directories. But new
operations of fsstress will break this "fixed number". So update
it, after fsstress get new operations.

Signed-off-by: Zorro Lang <zlang@redhat.com>
---
 tests/xfs/068     | 2 --
 tests/xfs/068.out | 2 +-
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/tests/xfs/068 b/tests/xfs/068
index 4dac95e..7151e28 100755
--- a/tests/xfs/068
+++ b/tests/xfs/068
@@ -43,8 +43,6 @@ trap "rm -rf $tmp.*; exit \$status" 0 1 2 3 15
 _supported_fs xfs
 _supported_os Linux
 
-# need to ensure new fsstress operations don't perturb expected output
-FSSTRESS_AVOID="-f insert=0 $FSSTRESS_AVOID"
 _create_dumpdir_stress_num 4096
 _do_dump_restore
 
diff --git a/tests/xfs/068.out b/tests/xfs/068.out
index 2196eee..b20604f 100644
--- a/tests/xfs/068.out
+++ b/tests/xfs/068.out
@@ -22,7 +22,7 @@ xfsrestore: session id: ID
 xfsrestore: media ID: ID
 xfsrestore: searching media for directory dump
 xfsrestore: reading directories
-xfsrestore: 495 directories and 1593 entries processed
+xfsrestore: 474 directories and 1592 entries processed
 xfsrestore: directory post-processing
 xfsrestore: restoring non-directory files
 xfsrestore: restore complete: SECS seconds elapsed
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe fstests" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  https://urldefense.proofpoint.com/v2/url?u=http-3A__vger.kernel.org_majordomo-2Dinfo.html&d=DwIBAg&c=RoP1YumCXCgaWHvlZYR8PQcxBKCX5YTpkKY057SbK10&r=-yMrTV4jriXR7ieyzzjV-QgHBD0UDw8ixoR77aMeAHE&m=kzNTuTigKys48jFXO4in6REQqHS9KbThymLMrujZC7w&s=rKu1LU3HRu6yHBn5dzn9Qq0Cq-IsaQZdImuK_40Em60&e= 

Re: [PATCH v6 1/2] fsstress: add mwrite/mread into test operation list

[Eryu Guan]

On Thu, Mar 23, 2017 at 12:37:17AM +0800, Zorro Lang wrote:
> mmap as a popular and basic operation, most of softwares use it to
> access files. More and more customers report bugs related with
> mmap/munmap and other stress conditions.
> 
> So add mmap read/write test into fsstress to increase mmap related
> stress to reproduce or find more bugs easily.
> 
> Signed-off-by: Zorro Lang <zlang@redhat.com>
> ---
> 
> Hi,
> 
> V6 do below changes:
> 
> 1) pick all common things from mwrite and mread into new do_mmap() function.
> 
> Before V6, mwrite try to extend the file size by truncate and mwrite on new
> extended place. But the reviewer suggest removing the truncate operation,
> so mwrite and mread will be nearly same, except one does memset(), and the
> other does memcpy().
> 
> 2) Add sys/mman.h into configure.ac, and include it in global.h
> 
> Thanks,
> Zorro
> 
>  configure.ac   |   1 +
>  ltp/fsstress.c | 110 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  src/global.h   |   4 +++
>  3 files changed, 115 insertions(+)
> 
> diff --git a/configure.ac b/configure.ac
> index fa48d2f..246f92e 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -32,6 +32,7 @@ AC_HEADER_STDC
>  			xfs/platform_defs.h	\
>  			btrfs/ioctl.h		\
>  			cifs/ioctl.h		\
> +			sys/mman.h		\
>      ])
>  
>  AC_CHECK_HEADERS([xfs/xfs_log_format.h],,,[
> diff --git a/ltp/fsstress.c b/ltp/fsstress.c
> index 7e7cf60..2f65034 100644
> --- a/ltp/fsstress.c
> +++ b/ltp/fsstress.c
> @@ -69,6 +69,8 @@ typedef enum {
>  	OP_LINK,
>  	OP_MKDIR,
>  	OP_MKNOD,
> +	OP_MREAD,
> +	OP_MWRITE,
>  	OP_PUNCH,
>  	OP_ZERO,
>  	OP_COLLAPSE,
> @@ -168,6 +170,8 @@ void	getdents_f(int, long);
>  void	link_f(int, long);
>  void	mkdir_f(int, long);
>  void	mknod_f(int, long);
> +void	mread_f(int, long);
> +void	mwrite_f(int, long);
>  void	punch_f(int, long);
>  void	zero_f(int, long);
>  void	collapse_f(int, long);
> @@ -208,6 +212,8 @@ opdesc_t	ops[] = {
>  	{ OP_LINK, "link", link_f, 1, 1 },
>  	{ 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 },
> @@ -2656,6 +2662,110 @@ mknod_f(int opno, long r)
>  }
>  
>  void
> +do_mmap(int opno, long r, int prot)
> +{
> +#ifdef HAVE_SYS_MMAN_H
> +	char		*addr;
> +	int		e;
> +	pathname_t	f;
> +	int		fd;
> +	size_t		len;
> +	__int64_t	lr;
> +	off64_t		off;
> +	int		flags;
> +	struct stat64	stb;
> +	int		v;
> +	char		st[1024];
> +
> +	init_pathname(&f);
> +	if (!get_fname(FT_REGFILE, r, &f, NULL, NULL, &v)) {
> +		if (v)
> +			printf("%d/%d: do_mmap - no filename\n", procid, opno);
> +		free_pathname(&f);
> +		return;
> +	}
> +	fd = open_path(&f, O_RDWR);
> +	e = fd < 0 ? errno : 0;
> +	check_cwd();
> +	if (fd < 0) {
> +		if (v)
> +			printf("%d/%d: do_mmap - open %s failed %d\n",
> +			       procid, opno, f.path, e);
> +		free_pathname(&f);
> +		return;
> +	}
> +	if (fstat64(fd, &stb) < 0) {
> +		if (v)
> +			printf("%d/%d: do_mmap - fstat64 %s failed %d\n",
> +			       procid, opno, f.path, errno);
> +		free_pathname(&f);
> +		close(fd);
> +		return;
> +	}
> +	if (stb.st_size == 0) {
> +		if (v)
> +			printf("%d/%d: do_mmap - %s%s zero size\n", procid, opno,
> +			       f.path, st);
                                      ^^^^
st is used with uninitialized content.

> +		free_pathname(&f);
> +		close(fd);
> +		return;
> +	}
> +
> +	inode_info(st, sizeof(st), &stb, v);
> +	lr = ((__int64_t)random() << 32) + random();
> +	off = (off64_t)(lr % stb.st_size);
> +	off &= (off64_t)(~(sysconf(_SC_PAGE_SIZE) - 1));
> +	len = (size_t)(random() % MIN(stb.st_size - off, FILELEN_MAX)) + 1;
> +
> +	flags = (random() % 2) ? MAP_SHARED : MAP_PRIVATE;
> +	addr = mmap(NULL, len, prot, flags, fd, off);
> +	e = (addr == MAP_FAILED) ? errno : 0;
> +	if (e && v)
> +		printf("%d/%d: do_mmap - mmap failed %s%s [%lld,%d,%s] %d\n",
> +		       procid, opno, f.path, st, (long long)off, (int)len,
> +		       (flags & MAP_PRIVATE) ? "MAP_PRIVATE" : "MAP_SHARED", e);

I think we can return here if mmap failed.

> +
> +	if (addr != MAP_FAILED) {

then no need to do this check here.

> +		if (prot & PROT_WRITE) {
> +		/* PROT_READ maybe set, if PROT_WRITE is set. Not vice versa */
> +			memset(addr, nameseq & 0xff, len);
> +		} else {
> +			char *buf;
> +			if ((buf = malloc(len)) != NULL) {
> +				memcpy(buf, addr, len);
> +				free(buf);
> +			}
> +		}
> +		e = munmap(addr, len) < 0 ? errno : 0;
> +		if (e && v)
> +			printf("%d/%d: do_mmap - munmap failed %s%s [%lld,%d] %d\n",
> +			       procid, opno, f.path, st, (long long)off,
> +			       (int)len, e);
> +	}
> +	if (v)
> +		printf("%d/%d: %s %s%s [%lld,%d,%s] %d\n",
> +		       procid, opno, (prot & PROT_WRITE) ? "mwrite" : "mread",
> +		       f.path, st, (long long)off, (int)len,
> +		       (flags & MAP_PRIVATE) ? "MAP_PRIVATE" : "MAP_SHARED", e);
> +
> +	free_pathname(&f);
> +	close(fd);
> +#endif
> +}
> +
> +void
> +mread_f(int opno, long r)
> +{
> +	do_mmap(opno, r, PROT_READ);
> +}
> +
> +void
> +mwrite_f(int opno, long r)
> +{
> +	do_mmap(opno, r, PROT_WRITE);

These do_mmap calls should be protected by #ifdef HAVE_SYS_MMAN_H too,
otherwise PROT_READ|WRITE are not defined.

But I'm not sure if this HAVE_SYS_MMAN_H detection is really needed, I'm
fine with either way.

Thanks,
Eryu

> +}
> +
> +void
>  punch_f(int opno, long r)
>  {
>  #ifdef HAVE_LINUX_FALLOC_H
> diff --git a/src/global.h b/src/global.h
> index f63246b..3920c0d 100644
> --- a/src/global.h
> +++ b/src/global.h
> @@ -178,4 +178,8 @@
>  
>  #endif /* HAVE_LINUX_FALLOC_H */
>  
> +#ifdef HAVE_SYS_MMAN_H
> +#include <sys/mman.h>
> +#endif
> +
>  #endif /* GLOBAL_H */
> -- 
> 2.7.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe fstests" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  https://urldefense.proofpoint.com/v2/url?u=http-3A__vger.kernel.org_majordomo-2Dinfo.html&d=DwIBAg&c=RoP1YumCXCgaWHvlZYR8PQcxBKCX5YTpkKY057SbK10&r=-yMrTV4jriXR7ieyzzjV-QgHBD0UDw8ixoR77aMeAHE&m=XTfybgF_n-yJ8dN8EZft6eTnnHsUyt2edQItx7QAMqM&s=4_58NNRPLtl2UcHoM89bRFSL8rVcKI3I3WKHJfO6htw&e= 
--
To unsubscribe from this list: send the line "unsubscribe fstests" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  https://urldefense.proofpoint.com/v2/url?u=http-3A__vger.kernel.org_majordomo-2Dinfo.html&d=DwIBAg&c=RoP1YumCXCgaWHvlZYR8PQcxBKCX5YTpkKY057SbK10&r=-yMrTV4jriXR7ieyzzjV-QgHBD0UDw8ixoR77aMeAHE&m=XTfybgF_n-yJ8dN8EZft6eTnnHsUyt2edQItx7QAMqM&s=4_58NNRPLtl2UcHoM89bRFSL8rVcKI3I3WKHJfO6htw&e=