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

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



On Tue, Mar 21, 2017 at 05:18:02PM +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@xxxxxxxxxx>
> ---
> 
> Nothing changed from V4. Send V5 for fixing xfs/068 failure together.
> 
> Thanks,
> Zorro
> 
>  ltp/fsstress.c | 187 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  src/global.h   |   4 ++
>  2 files changed, 191 insertions(+)
> 
> diff --git a/ltp/fsstress.c b/ltp/fsstress.c
> index 7e7cf60..e341aa1 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, 4, 0 },
> +	{ OP_MWRITE, "mwrite", mwrite_f, 4, 1 },

I don't think mread/mwrite are widely used as read/write, so freq 4
seems a bit high, freq 2 should be OK.

>  	{ OP_PUNCH, "punch", punch_f, 1, 1 },
>  	{ OP_ZERO, "zero", zero_f, 1, 1 },
>  	{ OP_COLLAPSE, "collapse", collapse_f, 1, 1 },
> @@ -2656,6 +2662,187 @@ mknod_f(int opno, long r)
>  }
>  
>  void
> +mread_f(int opno, long r)
> +{
> +	char		*addr;
> +	char		*buf;
> +	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: mread - no filename\n", procid, opno);
> +		free_pathname(&f);
> +		return;
> +	}
> +	fd = open_path(&f, O_RDONLY);
> +	e = fd < 0 ? errno : 0;
> +	check_cwd();
> +	if (fd < 0) {
> +		if (v)
> +			printf("%d/%d: mread - open %s failed %d\n",
> +			       procid, opno, f.path, e);
> +		free_pathname(&f);
> +		return;
> +	}
> +	if (fstat64(fd, &stb) < 0) {
> +		if (v)
> +			printf("%d/%d: mread - 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: mread - %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;
> +
> +	/* try private file mappings with 20% rate */
> +	flags = (random() % 20) ? MAP_SHARED : MAP_PRIVATE;

This is not 20%, it's 5%. But I think 50%:50% should be OK?

> +	do {
> +		addr = mmap(NULL, len, PROT_READ, flags, fd, off);
> +		e = (addr == MAP_FAILED) ? errno : 0;
> +		if (errno == ENOMEM && flags & MAP_PRIVATE) {
> +		/* turn to shared mapping if memeory is not enough for private mapping */
> +			flags = MAP_SHARED;
> +		} else if (errno == ENOMEM && len > sysconf(_SC_PAGE_SIZE)) {
> +		/* reduce mapping length, if memeory is not enough for shared mapping */
> +			len /= 2;
> +		}
> +	} while (errno == ENOMEM && len > sysconf(_SC_PAGE_SIZE));

No need to retry on mmap, just let it fail and print out failure
message.

> +	if (e && v)
> +		printf("%d/%d: mread - 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 ((buf = malloc(len)) != NULL) {
> +			memcpy(buf, addr, len);
> +			free(buf);
> +		}
> +		e = munmap(addr, len) < 0 ? errno : 0;
> +		if (e && v)
> +			printf("%d/%d: mread - munmap failed %s%s [%lld,%d] %d\n",
> +			       procid, opno, f.path, st, (long long)off,
> +			       (int)len, e);
> +	}
> +	if (v)
> +		printf("%d/%d: mread %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);
> +
> +	free_pathname(&f);
> +	close(fd);
> +}
> +
> +void
> +mwrite_f(int opno, long r)
> +{
> +	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: mwrite - no filename\n", procid, opno);
> +		free_pathname(&f);
> +		return;
> +	}
> +	fd = open_path(&f, O_WRONLY);
> +	e = fd < 0 ? errno : 0;
> +	check_cwd();
> +	if (fd < 0) {
> +		if (v)
> +			printf("%d/%d: mwrite - open %s failed %d\n",
> +			       procid, opno, f.path, e);
> +		free_pathname(&f);
> +		return;
> +	}
> +	if (fstat64(fd, &stb) < 0) {
> +		if (v)
> +			printf("%d/%d: mwrite - fstat64 %s failed %d\n",
> +			       procid, opno, f.path, errno);
> +		free_pathname(&f);
> +		close(fd);
> +		return;
> +	}
> +	inode_info(st, sizeof(st), &stb, v);
> +	lr = ((__int64_t)random() << 32) + random();
> +	off = (off64_t)(lr % MIN(stb.st_size + (1024 * 1024), MAXFSIZE));
> +	off %= maxfsize;
> +	off &= (off64_t)(~(sysconf(_SC_PAGE_SIZE) - 1));
> +	len = (size_t)(random() % MIN(maxfsize - off, FILELEN_MAX)) + 1;
> +
> +	/*
> +	 * truncate file to the size we need to map and access,
> +	 * keep away SIGBUS / SIGSEGV killing this process
> +	 */
> +	e = truncate64_path(&f, off + len) < 0 ? errno : 0;

This implies a truncate(2) operation in mwrite. I don't think we need
this. Just mwrite within file size, let other operations like truncate
and write to extend file size.

> +	/* try private file mappings with 20% rate */
> +	flags = (random() % 20) ? MAP_SHARED : MAP_PRIVATE;

Again, this is 5%.

> +	do {
> +		addr = mmap(NULL, len, PROT_WRITE, flags, fd, off);
> +		e = (addr == MAP_FAILED) ? errno : 0;
> +		if (errno == ENOMEM && flags & MAP_PRIVATE) {
> +		/* turn to shared mapping if memeory is not enough for private mapping */
> +			flags = MAP_SHARED;
> +		} else if (errno == ENOMEM && len > sysconf(_SC_PAGE_SIZE)) {
> +		/* reduce mapping length, if memeory is not enough for shared mapping */
> +			len /= 2;
> +		}
> +	} while (errno == ENOMEM && len > sysconf(_SC_PAGE_SIZE));

And again, no need to retry on mmap failure, IMO, just let it fail.

> +	if (e && v)
> +		printf("%d/%d: mwrite - 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);

Consider putting this flag translation into a "struct print_flags"? See
struct print_flags falloc_flags [].

Thanks,
Eryu

> +
> +	if (addr != MAP_FAILED) {
> +		memset(addr, nameseq & 0xff, len);
> +		e = munmap(addr, len) < 0 ? errno : 0;
> +		if (e && v)
> +			printf("%d/%d: mwrite - munmap failed %s%s [%lld,%d] %d\n",
> +			       procid, opno, f.path, st, (long long)off,
> +			       (int)len, e);
> +	}
> +	if (v)
> +		printf("%d/%d: mwrite %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);
> +
> +	free_pathname(&f);
> +	close(fd);
> +}
> +
> +void
>  punch_f(int opno, long r)
>  {
>  #ifdef HAVE_LINUX_FALLOC_H
> diff --git a/src/global.h b/src/global.h
> index f63246b..51d1e94 100644
> --- a/src/global.h
> +++ b/src/global.h
> @@ -178,4 +178,8 @@
>  
>  #endif /* HAVE_LINUX_FALLOC_H */
>  
> +#ifndef 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@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe fstests" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Filesystems Development]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux