diff mbox series

[v2] Refactoring mmap16.c test using new LTP API

Message ID 20220210151448.18394-1-andrea.cervesato@suse.de
State Accepted
Headers show
Series [v2] Refactoring mmap16.c test using new LTP API | expand

Commit Message

Andrea Cervesato Feb. 10, 2022, 3:14 p.m. UTC
Removed TST_CHECKPOINT_INIT and replaced it with the .needs_checkpoints
LTP test feature. Simplified source code.

Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.de>
---
 testcases/kernel/syscalls/mmap/mmap16.c | 328 ++++++++++--------------
 1 file changed, 134 insertions(+), 194 deletions(-)

Comments

Cyril Hrubis Feb. 16, 2022, 12:12 p.m. UTC | #1
Hi!
Pushed with a minor changes, thanks.

The biggest issue was that the original test did run the actuall test 10
times, my guess that it was to make sure that we are able to reproduce
the issue. The rest of the changes is cosmetic, such as better
description and code formatting.

Full diff:

diff --git a/testcases/kernel/syscalls/mmap/mmap16.c b/testcases/kernel/syscalls/mmap/mmap16.c
index a892051ac..cb146d120 100644
--- a/testcases/kernel/syscalls/mmap/mmap16.c
+++ b/testcases/kernel/syscalls/mmap/mmap16.c
@@ -7,9 +7,22 @@
 /*\
  * [Description]
  *
- * This is a regression test for commits:
- * https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=0572639ff66dcffe62d37adfe4c4576f9fc398f4
- * https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=d6320cbfc92910a3e5f10c42d98c231c98db4f60
+ * This is a regression test for a silent data corruption for a mmaped file
+ * when filesystem gets out of space.
+ *
+ * Fixed by commits:
+ *
+ *  commit 0572639ff66dcffe62d37adfe4c4576f9fc398f4
+ *  Author: Xiaoguang Wang <wangxg.fnst@cn.fujitsu.com>
+ *  Date:   Thu Feb 12 23:00:17 2015 -0500
+ *
+ *      ext4: fix mmap data corruption in nodelalloc mode when blocksize < pagesize
+ *
+ *  commit d6320cbfc92910a3e5f10c42d98c231c98db4f60
+ *  Author: Jan Kara <jack@suse.cz>
+ *  Date:   Wed Oct 1 21:49:46 2014 -0400
+ *
+ *      ext4: fix mmap data corruption when blocksize < pagesize
  */
 
 #define _GNU_SOURCE
@@ -25,8 +38,8 @@
 #define FILE_PARENT "mntpoint/testfilep"
 #define FILE_CHILD "mntpoint/testfilec"
 #define FS_BLOCKSIZE 1024
+#define LOOPS 10
 
-static int page_size;
 static int parentfd = -1;
 static int childfd = -1;
 
@@ -83,7 +96,7 @@ static void do_child(void)
 	exit(1);
 }
 
-static void run(void)
+static void run_single(void)
 {
 	int ret, status;
 	pid_t child;
@@ -138,6 +151,14 @@ static void run(void)
 		tst_res(TPASS, "bug is not reproduced");
 }
 
+static void run(void)
+{
+	int i;
+
+	for (i = 0; i < LOOPS; i++)
+		run_single();
+}
+
 static void cleanup(void)
 {
 	if (childfd >= 0)
@@ -154,32 +175,25 @@ static struct tst_test test = {
 	.needs_root = 1,
 	.needs_device = 1,
 	.needs_checkpoints = 1,
-	.needs_tmpdir = 1,
 	.mount_device = 1,
 	.mntpoint = MNTPOINT,
 	.dev_fs_type = "ext4",
-	.dev_fs_opts =
-		(const char *const[]){
-			"-b",
-			"1024",
-			NULL,
-		},
-	.dev_extra_opts =
-		(const char *const[]){
-			"10240",
-			NULL,
-		},
-	.needs_cmds =
-		(const char *const[]){
-			"mkfs.ext4",
-			NULL,
-		},
-	.tags =
-		(const struct tst_tag[]){
-			{ "linux-git",
-			  "d6320cbfc92910a3e5f10c42d98c231c98db4f60" },
-			{ "linux-git",
-			  "0572639ff66dcffe62d37adfe4c4576f9fc398f4" },
-			{},
-		},
+	.dev_fs_opts = (const char *const[]){
+		"-b",
+		"1024",
+		NULL,
+	},
+	.dev_extra_opts = (const char *const[]){
+		"10240",
+		NULL,
+	},
+	.needs_cmds = (const char *const[]){
+		"mkfs.ext4",
+		NULL,
+	},
+	.tags = (const struct tst_tag[]){
+		{"linux-git", "d6320cbfc929"},
+		{"linux-git", "0572639ff66d"},
+		{},
+	},
 };
diff mbox series

Patch

diff --git a/testcases/kernel/syscalls/mmap/mmap16.c b/testcases/kernel/syscalls/mmap/mmap16.c
index 0d1fc3e96..a892051ac 100644
--- a/testcases/kernel/syscalls/mmap/mmap16.c
+++ b/testcases/kernel/syscalls/mmap/mmap16.c
@@ -1,196 +1,48 @@ 
+// SPDX-License-Identifier: GPL-2.0-or-later
 /*
- *   Copyright (c) 2015 Fujitsu Ltd.
- *   Author: Xiaoguang Wang <wangxg.fnst@cn.fujitsu.com>
- *
- *   This program is free software;  you can redistribute it and/or modify
- *   it under the terms of the GNU General Public License as published by
- *   the Free Software Foundation; either version 2 of the License, or
- *   (at your option) any later version.
- *
- *   This program is distributed in the hope that it will be useful,
- *   but WITHOUT ANY WARRANTY;  without even the implied warranty of
- *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See
- *   the GNU General Public License for more details.
+ * Copyright (c) 2015 Fujitsu Ltd. Xiaoguang Wang <wangxg.fnst@cn.fujitsu.com>
+ * Copyright (C) 2021 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com>
  */
 
-/*
- * This is a regression test for commit:
- * http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/
- * commit/?id=90a8020
- * http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/
- * commit/?id=d6320cb
+/*\
+ * [Description]
+ *
+ * This is a regression test for commits:
+ * https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=0572639ff66dcffe62d37adfe4c4576f9fc398f4
+ * https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=d6320cbfc92910a3e5f10c42d98c231c98db4f60
  */
 
 #define _GNU_SOURCE
 
-#include <stdio.h>
-#include <errno.h>
-#include <fcntl.h>
-#include <sys/types.h>
-#include <sys/wait.h>
-#include <unistd.h>
-#include <sys/mount.h>
+#include <stdlib.h>
+#include <signal.h>
+#include <time.h>
 #include <sys/mman.h>
+#include <sys/wait.h>
+#include "tst_test.h"
 
-#include "test.h"
-#include "safe_macros.h"
-
-#define MNTPOINT	"mntpoint"
-#define FS_BLOCKSIZE	1024
-#define SUB_LOOPCOUNT	10
-
-static void setup(void);
-static void cleanup(void);
-static void do_child(void);
-static void do_test(void);
-
-static const char *device;
-static const char *fs_type = "ext4";
-static int mount_flag;
-static int chdir_flag;
-static int parentfd = -1;
+#define MNTPOINT "mntpoint"
+#define FILE_PARENT "mntpoint/testfilep"
+#define FILE_CHILD "mntpoint/testfilec"
+#define FS_BLOCKSIZE 1024
 
 static int page_size;
-static int bug_reproduced;
-
-char *TCID = "mmap16";
-int TST_TOTAL = 1;
-
-int main(int ac, char **av)
-{
-	int i, lc;
-
-	tst_parse_opts(ac, av, NULL, NULL);
-
-	setup();
-
-	/*
-	 * If child process was killed by SIGBUS, indeed that can not guarantee
-	 * this bug must have been fixed, If we're luckily enough, virtual
-	 * memory subsystem happen to decide that it is time to write dirty
-	 * pages, it will make mapped pages write-protect, so ->page_mkwrite()
-	 * still will be called, child process will be killed by SIGBUS, but
-	 * it's not because of above fixing patches. So here we run this test
-	 * 10 times, if once, child process exits normally, we can sure that
-	 * this bug is not fixed.
-	 */
-	for (lc = 0; TEST_LOOPING(lc); lc++) {
-		tst_count = 0;
-
-		for (i = 0; i < SUB_LOOPCOUNT; i++)
-			do_test();
-	}
-
-	if (bug_reproduced)
-		tst_resm(TFAIL, "Bug is reproduced!");
-	else
-		tst_resm(TPASS, "Bug is not reproduced!");
-
-	cleanup();
-	tst_exit();
-}
-
-static void do_test(void)
-{
-	int ret, status;
-	pid_t child;
-	char buf[FS_BLOCKSIZE];
-
-	SAFE_TOUCH(cleanup, "testfilep", 0644, NULL);
-	SAFE_TOUCH(cleanup, "testfilec", 0644, NULL);
-
-	child = tst_fork();
-	switch (child) {
-	case -1:
-		tst_brkm(TBROK | TERRNO, cleanup, "fork failed");
-	case 0:
-		do_child();
-	default:
-		parentfd = SAFE_OPEN(cleanup, "testfilep", O_RDWR);
-		memset(buf, 'a', FS_BLOCKSIZE);
-
-		TST_SAFE_CHECKPOINT_WAIT(cleanup, 0);
-		while (1) {
-			ret = write(parentfd, buf, FS_BLOCKSIZE);
-			if (ret < 0) {
-				if (errno == ENOSPC) {
-					break;
-				} else {
-					tst_brkm(TBROK | TERRNO, cleanup,
-						 "write failed unexpectedly");
-				}
-			}
-		}
-		SAFE_CLOSE(cleanup, parentfd);
-		TST_SAFE_CHECKPOINT_WAKE(cleanup, 0);
-	}
-
-	wait(&status);
-	if (WIFEXITED(status) && WEXITSTATUS(status) == 1) {
-		bug_reproduced = 1;
-	} else {
-		/*
-		 * If child process was killed by SIGBUS, bug is not reproduced.
-		 */
-		if (!WIFSIGNALED(status) || WTERMSIG(status) != SIGBUS) {
-			tst_brkm(TBROK | TERRNO, cleanup,
-				 "child process terminate unexpectedly");
-		}
-	}
-
-	SAFE_UNLINK(cleanup, "testfilep");
-	SAFE_UNLINK(cleanup, "testfilec");
-}
-
-static void setup(void)
-{
-	const char *fs_opts[3] = {"-b", "1024", NULL};
-	const char *extra_opts[] = {"10240", NULL};
-
-	tst_sig(FORK, DEF_HANDLER, NULL);
-	tst_require_root();
-
-	TEST_PAUSE;
-	tst_tmpdir();
-
-	TST_CHECKPOINT_INIT(tst_rmdir);
-
-	page_size = getpagesize();
-
-	device = tst_acquire_device(cleanup);
-	if (!device)
-		tst_brkm(TCONF, cleanup, "Failed to obtain block device");
-	tst_mkfs(cleanup, device, fs_type, fs_opts, extra_opts);
-
-	SAFE_MKDIR(cleanup, MNTPOINT, 0755);
-	/*
-	 * Disable ext4 delalloc feature, so block will be allocated
-	 * as soon as possible
-	 */
-	SAFE_MOUNT(cleanup, device, MNTPOINT, fs_type, 0, "nodelalloc");
-	mount_flag = 1;
-
-	SAFE_CHDIR(cleanup, MNTPOINT);
-	chdir_flag = 1;
-
-}
+static int parentfd = -1;
+static int childfd = -1;
 
 static void do_child(void)
 {
-	int fd, offset;
+	int offset;
+	int page_size;
 	char buf[FS_BLOCKSIZE];
 	char *addr = NULL;
 
-	/*
-	 * We have changed SIGBUS' handler in parent process by calling
-	 * tst_sig(FORK, DEF_HANDLER, NULL), so here just restore it.
-	 */
-	if (signal(SIGBUS, SIG_DFL) == SIG_ERR)
-		tst_brkm(TBROK | TERRNO, NULL, "signal(SIGBUS) failed");
+	page_size = getpagesize();
+
+	childfd = SAFE_OPEN(FILE_CHILD, O_RDWR | O_CREAT, 0666);
 
 	memset(buf, 'a', FS_BLOCKSIZE);
-	fd = SAFE_OPEN(NULL, "testfilec", O_RDWR);
-	SAFE_WRITE(NULL, 1, fd, buf, FS_BLOCKSIZE);
+	SAFE_WRITE(1, childfd, buf, FS_BLOCKSIZE);
 
 	/*
 	 * In case mremap() may fail because that memory area can not be
@@ -198,19 +50,19 @@  static void do_child(void)
 	 * we first do a mmap(page_size * 2) operation to reserve some
 	 * free address space.
 	 */
-	addr = SAFE_MMAP(NULL, NULL, page_size * 2, PROT_WRITE | PROT_READ,
-			 MAP_PRIVATE_EXCEPT_UCLINUX | MAP_ANONYMOUS, -1, 0);
-	SAFE_MUNMAP(NULL, addr, page_size * 2);
+	addr = SAFE_MMAP(NULL, page_size * 2, PROT_WRITE | PROT_READ,
+			 MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+	SAFE_MUNMAP(addr, page_size * 2);
 
-	addr = SAFE_MMAP(NULL, addr, FS_BLOCKSIZE, PROT_WRITE | PROT_READ,
-			 MAP_SHARED, fd, 0);
+	addr = SAFE_MMAP(addr, FS_BLOCKSIZE, PROT_WRITE | PROT_READ, MAP_SHARED, childfd, 0);
 
 	addr[0] = 'a';
 
-	SAFE_FTRUNCATE(NULL, fd, page_size * 2);
+	SAFE_FTRUNCATE(childfd, page_size * 2);
+
 	addr = mremap(addr, FS_BLOCKSIZE, 2 * page_size, 0);
 	if (addr == MAP_FAILED)
-		tst_brkm(TBROK | TERRNO, NULL, "mremap failed unexpectedly");
+		tst_brk(TBROK | TERRNO, "mremap failed unexpectedly");
 
 	/*
 	 * Let parent process consume FS free blocks as many as possible, then
@@ -220,26 +72,114 @@  static void do_child(void)
 	 * if not, the data 'A', 'B', 'C' will be silently discarded later when
 	 * kernel writepage is called, that means data corruption.
 	 */
-	TST_SAFE_CHECKPOINT_WAKE(NULL, 0);
-	TST_SAFE_CHECKPOINT_WAIT2(NULL, 0, 60*1000);
+	TST_CHECKPOINT_WAKE_AND_WAIT(0);
 
 	for (offset = FS_BLOCKSIZE; offset < page_size; offset += FS_BLOCKSIZE)
 		addr[offset] = 'a';
 
-	SAFE_MUNMAP(NULL, addr, 2 * page_size);
-	SAFE_CLOSE(NULL, fd);
-	exit(TFAIL);
+	SAFE_MUNMAP(addr, 2 * page_size);
+	SAFE_CLOSE(childfd);
+
+	exit(1);
+}
+
+static void run(void)
+{
+	int ret, status;
+	pid_t child;
+	char buf[FS_BLOCKSIZE];
+	int bug_reproduced = 0;
+
+	child = SAFE_FORK();
+	if (!child) {
+		do_child();
+		return;
+	}
+
+	parentfd = SAFE_OPEN(FILE_PARENT, O_RDWR | O_CREAT, 0666);
+
+	memset(buf, 'a', FS_BLOCKSIZE);
+
+	TST_CHECKPOINT_WAIT(0);
+
+	while (1) {
+		ret = write(parentfd, buf, FS_BLOCKSIZE);
+		if (ret < 0) {
+			if (errno == ENOSPC)
+				break;
+
+			tst_brk(TBROK | TERRNO, "write failed unexpectedly");
+		}
+	}
+
+	SAFE_CLOSE(parentfd);
+
+	TST_CHECKPOINT_WAKE(0);
+
+	SAFE_WAITPID(child, &status, 0);
+	if (WIFEXITED(status) && WEXITSTATUS(status) == 1) {
+		bug_reproduced = 1;
+	} else {
+		/*
+		 * If child process was killed by SIGBUS, bug is not reproduced.
+		 */
+		if (!WIFSIGNALED(status) || WTERMSIG(status) != SIGBUS) {
+			tst_brk(TBROK | TERRNO, "child process terminate unexpectedly with status %s",
+				tst_strstatus(status));
+		}
+	}
+
+	SAFE_UNLINK(FILE_PARENT);
+	SAFE_UNLINK(FILE_CHILD);
+
+	if (bug_reproduced)
+		tst_res(TFAIL, "bug is reproduced");
+	else
+		tst_res(TPASS, "bug is not reproduced");
 }
 
 static void cleanup(void)
 {
+	if (childfd >= 0)
+		SAFE_CLOSE(childfd);
+
 	if (parentfd >= 0)
-		close(parentfd);
-	if (chdir_flag && chdir(".."))
-		tst_resm(TWARN | TERRNO, "chdir('..') failed");
-	if (mount_flag && tst_umount(MNTPOINT) < 0)
-		tst_resm(TWARN | TERRNO, "umount device:%s failed", device);
-	if (device)
-		tst_release_device(device);
-	tst_rmdir();
+		SAFE_CLOSE(parentfd);
 }
+
+static struct tst_test test = {
+	.test_all = run,
+	.cleanup = cleanup,
+	.forks_child = 1,
+	.needs_root = 1,
+	.needs_device = 1,
+	.needs_checkpoints = 1,
+	.needs_tmpdir = 1,
+	.mount_device = 1,
+	.mntpoint = MNTPOINT,
+	.dev_fs_type = "ext4",
+	.dev_fs_opts =
+		(const char *const[]){
+			"-b",
+			"1024",
+			NULL,
+		},
+	.dev_extra_opts =
+		(const char *const[]){
+			"10240",
+			NULL,
+		},
+	.needs_cmds =
+		(const char *const[]){
+			"mkfs.ext4",
+			NULL,
+		},
+	.tags =
+		(const struct tst_tag[]){
+			{ "linux-git",
+			  "d6320cbfc92910a3e5f10c42d98c231c98db4f60" },
+			{ "linux-git",
+			  "0572639ff66dcffe62d37adfe4c4576f9fc398f4" },
+			{},
+		},
+};