All of lore.kernel.org
 help / color / mirror / Atom feed
* [LTP] [PATCH 1/2] syscalls/move_pages: move the common code to move_hugepages_support
@ 2017-12-01  3:48 Xiao Yang
  2017-12-01  3:48 ` [LTP] [PATCH 2/2] syscalls/move_pages13.c: Add new regression test Xiao Yang
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Xiao Yang @ 2017-12-01  3:48 UTC (permalink / raw)
  To: ltp

Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com>
---
 testcases/kernel/syscalls/move_pages/Makefile      |   2 +-
 .../syscalls/move_pages/move_hugepages_support.c   | 131 ++++++++++++++++++++
 .../syscalls/move_pages/move_hugepages_support.h   |  49 ++++++++
 .../kernel/syscalls/move_pages/move_pages12.c      | 134 ++-------------------
 4 files changed, 190 insertions(+), 126 deletions(-)
 create mode 100644 testcases/kernel/syscalls/move_pages/move_hugepages_support.c
 create mode 100644 testcases/kernel/syscalls/move_pages/move_hugepages_support.h

diff --git a/testcases/kernel/syscalls/move_pages/Makefile b/testcases/kernel/syscalls/move_pages/Makefile
index 9892770..17e0f1e 100644
--- a/testcases/kernel/syscalls/move_pages/Makefile
+++ b/testcases/kernel/syscalls/move_pages/Makefile
@@ -26,7 +26,7 @@ INSTALL_TARGETS		:= move_pages.sh
 
 MAKE_TARGETS		:= $(patsubst $(abs_srcdir)/%.c,%,$(wildcard $(abs_srcdir)/*[0-9].c))
 
-$(MAKE_TARGETS): %: %.o move_pages_support.o
+$(MAKE_TARGETS): %: %.o move_pages_support.o move_hugepages_support.o
 
 LDLIBS			+= -lpthread -lrt
 
diff --git a/testcases/kernel/syscalls/move_pages/move_hugepages_support.c b/testcases/kernel/syscalls/move_pages/move_hugepages_support.c
new file mode 100644
index 0000000..096d91a
--- /dev/null
+++ b/testcases/kernel/syscalls/move_pages/move_hugepages_support.c
@@ -0,0 +1,131 @@
+/*
+ * Copyright (c) 2017 FUJITSU LIMITED. All rights reserved.
+ * Author: Xiao Yang <yangx.jy@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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#define TST_NO_DEFAULT_MAIN
+#include "tst_test.h"
+#include "move_hugepages_support.h"
+
+#ifdef HAVE_NUMA_V2
+void alloc_free_huge_on_node(unsigned int node, size_t size, int hgpsz)
+{
+	char *mem;
+	long ret;
+	struct bitmask *bm;
+
+	tst_res(TINFO, "Allocating and freeing %zu hugepages on node %u",
+		size / hgpsz, node);
+
+	mem = mmap(NULL, size, PROT_READ | PROT_WRITE,
+		   MAP_PRIVATE | MAP_ANONYMOUS | MAP_HUGETLB, -1, 0);
+	if (mem == MAP_FAILED) {
+		if (errno == ENOMEM)
+			tst_brk(TCONF, "Cannot allocate huge pages");
+
+		tst_brk(TBROK | TERRNO, "mmap(..., MAP_HUGETLB, ...) failed");
+	}
+
+	bm = numa_bitmask_alloc(numa_max_possible_node() + 1);
+	if (!bm)
+		tst_brk(TBROK | TERRNO, "numa_bitmask_alloc() failed");
+
+	numa_bitmask_setbit(bm, node);
+
+	ret = mbind(mem, size, MPOL_BIND, bm->maskp, bm->size + 1, 0);
+	if (ret) {
+		if (errno == ENOMEM)
+			tst_brk(TCONF, "Cannot mbind huge pages");
+
+		tst_brk(TBROK | TERRNO, "mbind() failed");
+	}
+
+	TEST(mlock(mem, size));
+	if (TEST_RETURN) {
+		SAFE_MUNMAP(mem, size);
+		if (TEST_ERRNO == ENOMEM || TEST_ERRNO == EAGAIN)
+			tst_brk(TCONF, "Cannot lock huge pages");
+		tst_brk(TBROK | TTERRNO, "mlock failed");
+	}
+
+	numa_bitmask_free(bm);
+
+	SAFE_MUNMAP(mem, size);
+}
+
+void do_hugepages_move(int count, int hpgsz, int pgsz, void *addr,
+		       unsigned int node1, unsigned int node2)
+{
+	int test_pages = count * hpgsz / pgsz;
+	int i, j;
+	int *nodes, *status;
+	void **pages;
+	pid_t ppid = getppid();
+
+	pages = SAFE_MALLOC(sizeof(char *) * test_pages);
+	nodes = SAFE_MALLOC(sizeof(int) * test_pages);
+	status = SAFE_MALLOC(sizeof(int) * test_pages);
+
+	for (i = 0; i < test_pages; i++)
+		pages[i] = addr + i * pgsz;
+
+	for (i = 0; ; i++) {
+		for (j = 0; j < test_pages; j++) {
+			if (i % 2 == 0)
+				nodes[j] = node1;
+			else
+				nodes[j] = node2;
+			status[j] = 0;
+		}
+
+		TEST(numa_move_pages(ppid, test_pages,
+			pages, nodes, status, MPOL_MF_MOVE_ALL));
+		if (TEST_RETURN) {
+			tst_res(TFAIL | TTERRNO, "move_pages failed");
+			break;
+		}
+	}
+
+	exit(0);
+}
+
+long modify_node_nr_hgpgs(char *path_hgpgs_node, size_t size,
+			  unsigned int node, int hgpsz, long add_num)
+{
+	long orig_hgpgs_node = -1;
+
+	snprintf(path_hgpgs_node, size,
+		 "/sys/devices/system/node/node%u/hugepages/hugepages-%dkB/nr_hugepages",
+		 node, hgpsz);
+
+	if (!access(path_hgpgs_node, F_OK)) {
+		SAFE_FILE_SCANF(path_hgpgs_node, "%ld", &orig_hgpgs_node);
+		tst_res(TINFO,
+			"Increasing %dkB hugepages pool on node %u to %ld",
+			hgpsz, node, orig_hgpgs_node + add_num);
+		SAFE_FILE_PRINTF(path_hgpgs_node, "%ld",
+				 orig_hgpgs_node + add_num);
+	}
+
+	return orig_hgpgs_node;
+}
+
+void restore_orig_hgpgs_value(char *path_hgpgs, long orig_value)
+{
+	if (orig_value != -1)
+		SAFE_FILE_PRINTF(path_hgpgs, "%ld", orig_value);
+}
+#endif
diff --git a/testcases/kernel/syscalls/move_pages/move_hugepages_support.h b/testcases/kernel/syscalls/move_pages/move_hugepages_support.h
new file mode 100644
index 0000000..98f357b
--- /dev/null
+++ b/testcases/kernel/syscalls/move_pages/move_hugepages_support.h
@@ -0,0 +1,49 @@
+/*
+ * Copyright (c) 2017 FUJITSU LIMITED. All rights reserved.
+ * Author: Xiao Yang <yangx.jy@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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program;  if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef MOVE_HUGEPAGES_SUPPORT_H
+#define MOVE_HUGEPAGES_SUPPORT_H
+
+#include "config.h"
+#include <errno.h>
+#include <sys/types.h>
+#include <unistd.h>
+#include <stdlib.h>
+#include <stdio.h>
+# if HAVE_NUMA_H
+#include <numa.h>
+# endif
+# if HAVE_NUMAIF_H
+#include <numaif.h>
+# endif
+#include <sys/mman.h>
+#include "lapi/mmap.h"
+
+#define PATH_MEMINFO	"/proc/meminfo"
+#define PATH_NR_HUGEPAGES	"/proc/sys/vm/nr_hugepages"
+#define PATH_HUGEPAGES	"/sys/kernel/mm/hugepages/"
+
+# ifdef HAVE_NUMA_V2
+void alloc_free_huge_on_node(unsigned int node, size_t size, int hgpsz);
+void do_hugepages_move(int count, int hpgsz, int pgsz, void *addr,
+		       unsigned int node1, unsigned int node2);
+long modify_node_nr_hgpgs(char *path_hgpgs_node, size_t size,
+			  unsigned int node, int hgpsz, long add_num);
+void restore_orig_hgpgs_value(char *path_hgpgs, long orig_value);
+# endif
+#endif
diff --git a/testcases/kernel/syscalls/move_pages/move_pages12.c b/testcases/kernel/syscalls/move_pages/move_pages12.c
index 1ac7bc1..9c66304 100644
--- a/testcases/kernel/syscalls/move_pages/move_pages12.c
+++ b/testcases/kernel/syscalls/move_pages/move_pages12.c
@@ -41,14 +41,12 @@
 
 #include "tst_test.h"
 #include "move_pages_support.h"
+#include "move_hugepages_support.h"
 #include "lapi/mmap.h"
 
 #ifdef HAVE_NUMA_V2
 
 #define LOOPS	1000
-#define PATH_MEMINFO	"/proc/meminfo"
-#define PATH_NR_HUGEPAGES	"/proc/sys/vm/nr_hugepages"
-#define PATH_HUGEPAGES	"/sys/kernel/mm/hugepages/"
 #define TEST_PAGES	2
 #define TEST_NODES	2
 
@@ -61,41 +59,6 @@ static long orig_hugepages_node2 = -1;
 static unsigned int node1, node2;
 static void *addr;
 
-static void do_child(void)
-{
-	int test_pages = TEST_PAGES * hpsz / pgsz;
-	int i, j;
-	int *nodes, *status;
-	void **pages;
-	pid_t ppid = getppid();
-
-	pages = SAFE_MALLOC(sizeof(char *) * test_pages);
-	nodes = SAFE_MALLOC(sizeof(int) * test_pages);
-	status = SAFE_MALLOC(sizeof(int) * test_pages);
-
-	for (i = 0; i < test_pages; i++)
-		pages[i] = addr + i * pgsz;
-
-	for (i = 0; ; i++) {
-		for (j = 0; j < test_pages; j++) {
-			if (i % 2 == 0)
-				nodes[j] = node1;
-			else
-				nodes[j] = node2;
-			status[j] = 0;
-		}
-
-		TEST(numa_move_pages(ppid, test_pages,
-			pages, nodes, status, MPOL_MF_MOVE_ALL));
-		if (TEST_RETURN) {
-			tst_res(TFAIL | TTERRNO, "move_pages failed");
-			break;
-		}
-	}
-
-	exit(0);
-}
-
 static void do_test(void)
 {
 	int i;
@@ -109,7 +72,7 @@ static void do_test(void)
 
 	cpid = SAFE_FORK();
 	if (cpid == 0)
-		do_child();
+		do_hugepages_move(TEST_PAGES, hpsz, pgsz, addr, node1, node2);
 
 	for (i = 0; i < LOOPS; i++) {
 		void *ptr;
@@ -133,51 +96,6 @@ static void do_test(void)
 	}
 }
 
-static void alloc_free_huge_on_node(unsigned int node, size_t size)
-{
-	char *mem;
-	long ret;
-	struct bitmask *bm;
-
-	tst_res(TINFO, "Allocating and freeing %zu hugepages on node %u",
-		size / hpsz, node);
-
-	mem = mmap(NULL, size, PROT_READ | PROT_WRITE,
-		   MAP_PRIVATE | MAP_ANONYMOUS | MAP_HUGETLB, -1, 0);
-	if (mem == MAP_FAILED) {
-		if (errno == ENOMEM)
-			tst_brk(TCONF, "Cannot allocate huge pages");
-
-		tst_brk(TBROK | TERRNO, "mmap(..., MAP_HUGETLB, ...) failed");
-	}
-
-	bm = numa_bitmask_alloc(numa_max_possible_node() + 1);
-	if (!bm)
-		tst_brk(TBROK | TERRNO, "numa_bitmask_alloc() failed");
-
-	numa_bitmask_setbit(bm, node);
-
-	ret = mbind(mem, size, MPOL_BIND, bm->maskp, bm->size + 1, 0);
-	if (ret) {
-		if (errno == ENOMEM)
-			tst_brk(TCONF, "Cannot mbind huge pages");
-
-		tst_brk(TBROK | TERRNO, "mbind() failed");
-	}
-
-	TEST(mlock(mem, size));
-	if (TEST_RETURN) {
-		SAFE_MUNMAP(mem, size);
-		if (TEST_ERRNO == ENOMEM || TEST_ERRNO == EAGAIN)
-			tst_brk(TCONF, "Cannot lock huge pages");
-		tst_brk(TBROK | TTERRNO, "mlock failed");
-	}
-
-	numa_bitmask_free(bm);
-
-	SAFE_MUNMAP(mem, size);
-}
-
 static void setup(void)
 {
 	int memfree, ret;
@@ -200,33 +118,8 @@ static void setup(void)
 	if (4 * hpsz > memfree)
 		tst_brk(TBROK, "Not enough free RAM");
 
-	snprintf(path_hugepages_node1, sizeof(path_hugepages_node1),
-		 "/sys/devices/system/node/node%u/hugepages/hugepages-%dkB/nr_hugepages",
-		 node1, hpsz);
-
-	snprintf(path_hugepages_node2, sizeof(path_hugepages_node2),
-		 "/sys/devices/system/node/node%u/hugepages/hugepages-%dkB/nr_hugepages",
-		 node2, hpsz);
-
-	if (!access(path_hugepages_node1, F_OK)) {
-		SAFE_FILE_SCANF(path_hugepages_node1,
-				"%ld", &orig_hugepages_node1);
-		tst_res(TINFO,
-			"Increasing %dkB hugepages pool on node %u to %ld",
-			hpsz, node1, orig_hugepages_node1 + 4);
-		SAFE_FILE_PRINTF(path_hugepages_node1,
-				 "%ld", orig_hugepages_node1 + 4);
-	}
-
-	if (!access(path_hugepages_node2, F_OK)) {
-		SAFE_FILE_SCANF(path_hugepages_node2,
-				"%ld", &orig_hugepages_node2);
-		tst_res(TINFO,
-			"Increasing %dkB hugepages pool on node %u to %ld",
-			hpsz, node2, orig_hugepages_node2 + 4);
-		SAFE_FILE_PRINTF(path_hugepages_node2,
-				 "%ld", orig_hugepages_node2 + 4);
-	}
+	orig_hugepages_node1 = modify_node_nr_hgpgs(path_hugepages_node1, sizeof(path_hugepages_node1), node1, hpsz, 4);
+	orig_hugepages_node2 = modify_node_nr_hgpgs(path_hugepages_node2, sizeof(path_hugepages_node2), node2, hpsz, 4);
 
 	hpsz *= 1024;
 
@@ -237,24 +130,15 @@ static void setup(void)
 		SAFE_FILE_PRINTF(PATH_NR_HUGEPAGES, "%ld", orig_hugepages + 8);
 	}
 
-	alloc_free_huge_on_node(node1, 4L * hpsz);
-	alloc_free_huge_on_node(node2, 4L * hpsz);
+	alloc_free_huge_on_node(node1, 4L * hpsz, hpsz);
+	alloc_free_huge_on_node(node2, 4L * hpsz, hpsz);
 }
 
 static void cleanup(void)
 {
-	if (orig_hugepages != -1)
-		SAFE_FILE_PRINTF(PATH_NR_HUGEPAGES, "%ld", orig_hugepages);
-
-	if (orig_hugepages_node1 != -1) {
-		SAFE_FILE_PRINTF(path_hugepages_node1,
-				 "%ld", orig_hugepages_node1);
-	}
-
-	if (orig_hugepages_node2 != -1) {
-		SAFE_FILE_PRINTF(path_hugepages_node2,
-				 "%ld", orig_hugepages_node2);
-	}
+	restore_orig_hgpgs_value(PATH_NR_HUGEPAGES, orig_hugepages);
+	restore_orig_hgpgs_value(path_hugepages_node1, orig_hugepages_node1);
+	restore_orig_hgpgs_value(path_hugepages_node2, orig_hugepages_node2);
 }
 
 static struct tst_test test = {
-- 
1.8.3.1




^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [LTP] [PATCH 2/2] syscalls/move_pages13.c: Add new regression test
  2017-12-01  3:48 [LTP] [PATCH 1/2] syscalls/move_pages: move the common code to move_hugepages_support Xiao Yang
@ 2017-12-01  3:48 ` Xiao Yang
  2019-04-16  8:48   ` [LTP] [PATCH v2] syscalls/move_page12.c: " Yang Xu
  2018-02-08  1:21 ` [LTP] [PATCH 1/2] syscalls/move_pages: move the common code to move_hugepages_support Xiao Yang
  2019-05-20 13:55 ` Cyril Hrubis
  2 siblings, 1 reply; 11+ messages in thread
From: Xiao Yang @ 2017-12-01  3:48 UTC (permalink / raw)
  To: ltp

The bug has been fixed in kernel:
'c9d398fa2378("mm, hugetlb: use pte_present() instead
 of pmd_present() in follow_huge_pmd()")'

It is easily for RHEL7.4GA to trigger the bug, but hardly
for upstream kernel.

Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com>
Signed-off-by: Jie Fei <feij.fnst@cn.fujitsu.com>
---
 runtest/numa                                       |   1 +
 runtest/syscalls                                   |   1 +
 testcases/kernel/syscalls/.gitignore               |   1 +
 .../kernel/syscalls/move_pages/move_pages13.c      | 158 +++++++++++++++++++++
 4 files changed, 161 insertions(+)
 create mode 100644 testcases/kernel/syscalls/move_pages/move_pages13.c

diff --git a/runtest/numa b/runtest/numa
index dcf4948..7757874 100644
--- a/runtest/numa
+++ b/runtest/numa
@@ -11,3 +11,4 @@ move_pages09 move_pages.sh 09
 move_pages10 move_pages.sh 10
 move_pages11 cd $LTPROOT/testcases/bin && chown root move_pages11 && chmod 04755 move_pages11 && move_pages.sh 11
 move_pages12 move_pages12
+move_pages13 move_pages13
diff --git a/runtest/syscalls b/runtest/syscalls
index 14089ac..904333a 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -665,6 +665,7 @@ move_pages09 move_pages.sh 09
 move_pages10 move_pages.sh 10
 move_pages11 cd $LTPROOT/testcases/bin && chown root move_pages11 && chmod 04755 move_pages11 && move_pages.sh 11
 move_pages12 move_pages12
+move_pages13 move_pages13
 
 mprotect01 mprotect01
 mprotect02 mprotect02
diff --git a/testcases/kernel/syscalls/.gitignore b/testcases/kernel/syscalls/.gitignore
index 12a136e..bf99657 100644
--- a/testcases/kernel/syscalls/.gitignore
+++ b/testcases/kernel/syscalls/.gitignore
@@ -606,6 +606,7 @@
 /move_pages/move_pages10
 /move_pages/move_pages11
 /move_pages/move_pages12
+/move_pages/move_pages13
 /mprotect/mprotect01
 /mprotect/mprotect02
 /mprotect/mprotect03
diff --git a/testcases/kernel/syscalls/move_pages/move_pages13.c b/testcases/kernel/syscalls/move_pages/move_pages13.c
new file mode 100644
index 0000000..2f1554b
--- /dev/null
+++ b/testcases/kernel/syscalls/move_pages/move_pages13.c
@@ -0,0 +1,158 @@
+/*
+ * Copyright (c) 2017 FUJITSU LIMITED. All rights reserved.
+ * Author(s): Xiao Yang <yangx.jy@cn.fujitsu.com>
+ *            Jie Fei <feij.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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program, if not, see <http://www.gnu.org/licenses/>.
+ */
+
+/*
+ * Description:
+ * This is a regression test for the race condition, where move_pages()
+ * and soft offline are called on a single hugetlb page concurrently.
+ *
+ * This bug can crash the buggy kernel, and was fixed by:
+ *
+ * commit c9d398fa237882ea07167e23bcfc5e6847066518
+ * Author: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
+ * Date:   Fri Mar 31 15:11:55 2017 -0700
+ *
+ *     mm, hugetlb: use pte_present() instead of pmd_present() in follow_huge_pmd()
+ */
+
+#include <errno.h>
+#include <unistd.h>
+#include <string.h>
+#include <stdio.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+
+#include "tst_test.h"
+#include "move_pages_support.h"
+#include "move_hugepages_support.h"
+#include "lapi/mmap.h"
+
+#ifdef HAVE_NUMA_V2
+
+#define LOOPS   2000
+#define TEST_PAGES	2
+#define TEST_NODES	2
+
+static int pgsz, hpsz;
+static long orig_hugepages = -1;
+static char path_hugepages_node1[PATH_MAX];
+static char path_hugepages_node2[PATH_MAX];
+static long orig_hugepages_node1 = -1;
+static long orig_hugepages_node2 = -1;
+static unsigned int node1, node2;
+static void *addr;
+
+static void do_test(void)
+{
+	int i, status;
+	void *test_addr;
+	pid_t pid = -1;
+
+	addr = SAFE_MMAP(NULL, TEST_PAGES * hpsz, PROT_READ | PROT_WRITE,
+			 MAP_PRIVATE | MAP_ANONYMOUS | MAP_HUGETLB, -1, 0);
+
+	SAFE_MUNMAP(addr, TEST_PAGES * hpsz);
+
+	pid = SAFE_FORK();
+	if (pid == 0)
+		do_hugepages_move(TEST_PAGES, hpsz, pgsz, addr, node1, node2);
+
+	for (i = 0; i < LOOPS; i++) {
+		test_addr = SAFE_MMAP(NULL, TEST_PAGES * hpsz, PROT_READ | PROT_WRITE,
+				      MAP_PRIVATE | MAP_ANONYMOUS | MAP_HUGETLB, -1, 0);
+		if (test_addr != addr)
+			tst_brk(TBROK, "Failed to mmap@desired addr");
+
+		memset(test_addr, 0, TEST_PAGES * hpsz);
+
+		if (madvise(test_addr, TEST_PAGES * hpsz, MADV_SOFT_OFFLINE)) {
+			if (errno == EINVAL) {
+				tst_brk(TCONF, "madvise() didn't support "
+					"MADV_SOFT_OFFLINE");
+			}
+		}
+
+		SAFE_MUNMAP(test_addr, TEST_PAGES * hpsz);
+	}
+
+	if (i == LOOPS) {
+		SAFE_KILL(pid, SIGKILL);
+		SAFE_WAITPID(pid, &status, 0);
+		if (!WIFEXITED(status))
+			tst_res(TPASS, "Bug not reproduced");
+	}
+}
+
+static void setup(void)
+{
+	int memfree, ret;
+
+	check_config(TEST_NODES);
+
+	if (access(PATH_HUGEPAGES, F_OK))
+		tst_brk(TCONF, "Huge page not supported");
+
+	ret = get_allowed_nodes(NH_MEMS, TEST_NODES, &node1, &node2);
+	if (ret < 0)
+		tst_brk(TBROK | TERRNO, "get_allowed_nodes: %d", ret);
+
+	pgsz = (int)get_page_size();
+	SAFE_FILE_LINES_SCANF(PATH_MEMINFO, "Hugepagesize: %d", &hpsz);
+
+	SAFE_FILE_LINES_SCANF(PATH_MEMINFO, "MemFree: %d", &memfree);
+	tst_res(TINFO, "Free RAM %d kB", memfree);
+
+	if (4 * hpsz > memfree)
+		tst_brk(TBROK, "Not enough free RAM");
+
+	orig_hugepages_node1 = modify_node_nr_hgpgs(path_hugepages_node1, sizeof(path_hugepages_node1), node1, hpsz, 4);
+	orig_hugepages_node2 = modify_node_nr_hgpgs(path_hugepages_node2, sizeof(path_hugepages_node2), node2, hpsz, 4);
+
+	hpsz *= 1024;
+
+	if (orig_hugepages_node1 == -1 || orig_hugepages_node2 == -1) {
+		SAFE_FILE_SCANF(PATH_NR_HUGEPAGES, "%ld", &orig_hugepages);
+		tst_res(TINFO, "Increasing global hugepages pool to %ld",
+			orig_hugepages + 8);
+		SAFE_FILE_PRINTF(PATH_NR_HUGEPAGES, "%ld", orig_hugepages + 8);
+	}
+
+	alloc_free_huge_on_node(node1, 4L * hpsz, hpsz);
+	alloc_free_huge_on_node(node2, 4L * hpsz, hpsz);
+}
+
+static void cleanup(void)
+{
+	restore_orig_hgpgs_value(PATH_NR_HUGEPAGES, orig_hugepages);
+	restore_orig_hgpgs_value(path_hugepages_node1, orig_hugepages_node1);
+	restore_orig_hgpgs_value(path_hugepages_node2, orig_hugepages_node2);
+}
+
+static struct tst_test test = {
+	.min_kver = "2.6.33",
+	.needs_root = 1,
+	.forks_child = 1,
+	.setup = setup,
+	.cleanup = cleanup,
+	.test_all = do_test,
+};
+
+#else
+	TST_TEST_TCONF("test requires libnuma >= 2 and it's development packages");
+#endif
-- 
1.8.3.1




^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [LTP] [PATCH 1/2] syscalls/move_pages: move the common code to move_hugepages_support
  2017-12-01  3:48 [LTP] [PATCH 1/2] syscalls/move_pages: move the common code to move_hugepages_support Xiao Yang
  2017-12-01  3:48 ` [LTP] [PATCH 2/2] syscalls/move_pages13.c: Add new regression test Xiao Yang
@ 2018-02-08  1:21 ` Xiao Yang
  2019-05-20 13:55 ` Cyril Hrubis
  2 siblings, 0 replies; 11+ messages in thread
From: Xiao Yang @ 2018-02-08  1:21 UTC (permalink / raw)
  To: ltp

Hi,

Ping. :-)

Thanks,
Xiao Yang
On 2017/12/01 11:48, Xiao Yang wrote:
> Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com>
> ---
>  testcases/kernel/syscalls/move_pages/Makefile      |   2 +-
>  .../syscalls/move_pages/move_hugepages_support.c   | 131 ++++++++++++++++++++
>  .../syscalls/move_pages/move_hugepages_support.h   |  49 ++++++++
>  .../kernel/syscalls/move_pages/move_pages12.c      | 134 ++-------------------
>  4 files changed, 190 insertions(+), 126 deletions(-)
>  create mode 100644 testcases/kernel/syscalls/move_pages/move_hugepages_support.c
>  create mode 100644 testcases/kernel/syscalls/move_pages/move_hugepages_support.h
>
> diff --git a/testcases/kernel/syscalls/move_pages/Makefile b/testcases/kernel/syscalls/move_pages/Makefile
> index 9892770..17e0f1e 100644
> --- a/testcases/kernel/syscalls/move_pages/Makefile
> +++ b/testcases/kernel/syscalls/move_pages/Makefile
> @@ -26,7 +26,7 @@ INSTALL_TARGETS		:= move_pages.sh
>  
>  MAKE_TARGETS		:= $(patsubst $(abs_srcdir)/%.c,%,$(wildcard $(abs_srcdir)/*[0-9].c))
>  
> -$(MAKE_TARGETS): %: %.o move_pages_support.o
> +$(MAKE_TARGETS): %: %.o move_pages_support.o move_hugepages_support.o
>  
>  LDLIBS			+= -lpthread -lrt
>  
> diff --git a/testcases/kernel/syscalls/move_pages/move_hugepages_support.c b/testcases/kernel/syscalls/move_pages/move_hugepages_support.c
> new file mode 100644
> index 0000000..096d91a
> --- /dev/null
> +++ b/testcases/kernel/syscalls/move_pages/move_hugepages_support.c
> @@ -0,0 +1,131 @@
> +/*
> + * Copyright (c) 2017 FUJITSU LIMITED. All rights reserved.
> + * Author: Xiao Yang <yangx.jy@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.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#define TST_NO_DEFAULT_MAIN
> +#include "tst_test.h"
> +#include "move_hugepages_support.h"
> +
> +#ifdef HAVE_NUMA_V2
> +void alloc_free_huge_on_node(unsigned int node, size_t size, int hgpsz)
> +{
> +	char *mem;
> +	long ret;
> +	struct bitmask *bm;
> +
> +	tst_res(TINFO, "Allocating and freeing %zu hugepages on node %u",
> +		size / hgpsz, node);
> +
> +	mem = mmap(NULL, size, PROT_READ | PROT_WRITE,
> +		   MAP_PRIVATE | MAP_ANONYMOUS | MAP_HUGETLB, -1, 0);
> +	if (mem == MAP_FAILED) {
> +		if (errno == ENOMEM)
> +			tst_brk(TCONF, "Cannot allocate huge pages");
> +
> +		tst_brk(TBROK | TERRNO, "mmap(..., MAP_HUGETLB, ...) failed");
> +	}
> +
> +	bm = numa_bitmask_alloc(numa_max_possible_node() + 1);
> +	if (!bm)
> +		tst_brk(TBROK | TERRNO, "numa_bitmask_alloc() failed");
> +
> +	numa_bitmask_setbit(bm, node);
> +
> +	ret = mbind(mem, size, MPOL_BIND, bm->maskp, bm->size + 1, 0);
> +	if (ret) {
> +		if (errno == ENOMEM)
> +			tst_brk(TCONF, "Cannot mbind huge pages");
> +
> +		tst_brk(TBROK | TERRNO, "mbind() failed");
> +	}
> +
> +	TEST(mlock(mem, size));
> +	if (TEST_RETURN) {
> +		SAFE_MUNMAP(mem, size);
> +		if (TEST_ERRNO == ENOMEM || TEST_ERRNO == EAGAIN)
> +			tst_brk(TCONF, "Cannot lock huge pages");
> +		tst_brk(TBROK | TTERRNO, "mlock failed");
> +	}
> +
> +	numa_bitmask_free(bm);
> +
> +	SAFE_MUNMAP(mem, size);
> +}
> +
> +void do_hugepages_move(int count, int hpgsz, int pgsz, void *addr,
> +		       unsigned int node1, unsigned int node2)
> +{
> +	int test_pages = count * hpgsz / pgsz;
> +	int i, j;
> +	int *nodes, *status;
> +	void **pages;
> +	pid_t ppid = getppid();
> +
> +	pages = SAFE_MALLOC(sizeof(char *) * test_pages);
> +	nodes = SAFE_MALLOC(sizeof(int) * test_pages);
> +	status = SAFE_MALLOC(sizeof(int) * test_pages);
> +
> +	for (i = 0; i < test_pages; i++)
> +		pages[i] = addr + i * pgsz;
> +
> +	for (i = 0; ; i++) {
> +		for (j = 0; j < test_pages; j++) {
> +			if (i % 2 == 0)
> +				nodes[j] = node1;
> +			else
> +				nodes[j] = node2;
> +			status[j] = 0;
> +		}
> +
> +		TEST(numa_move_pages(ppid, test_pages,
> +			pages, nodes, status, MPOL_MF_MOVE_ALL));
> +		if (TEST_RETURN) {
> +			tst_res(TFAIL | TTERRNO, "move_pages failed");
> +			break;
> +		}
> +	}
> +
> +	exit(0);
> +}
> +
> +long modify_node_nr_hgpgs(char *path_hgpgs_node, size_t size,
> +			  unsigned int node, int hgpsz, long add_num)
> +{
> +	long orig_hgpgs_node = -1;
> +
> +	snprintf(path_hgpgs_node, size,
> +		 "/sys/devices/system/node/node%u/hugepages/hugepages-%dkB/nr_hugepages",
> +		 node, hgpsz);
> +
> +	if (!access(path_hgpgs_node, F_OK)) {
> +		SAFE_FILE_SCANF(path_hgpgs_node, "%ld", &orig_hgpgs_node);
> +		tst_res(TINFO,
> +			"Increasing %dkB hugepages pool on node %u to %ld",
> +			hgpsz, node, orig_hgpgs_node + add_num);
> +		SAFE_FILE_PRINTF(path_hgpgs_node, "%ld",
> +				 orig_hgpgs_node + add_num);
> +	}
> +
> +	return orig_hgpgs_node;
> +}
> +
> +void restore_orig_hgpgs_value(char *path_hgpgs, long orig_value)
> +{
> +	if (orig_value != -1)
> +		SAFE_FILE_PRINTF(path_hgpgs, "%ld", orig_value);
> +}
> +#endif
> diff --git a/testcases/kernel/syscalls/move_pages/move_hugepages_support.h b/testcases/kernel/syscalls/move_pages/move_hugepages_support.h
> new file mode 100644
> index 0000000..98f357b
> --- /dev/null
> +++ b/testcases/kernel/syscalls/move_pages/move_hugepages_support.h
> @@ -0,0 +1,49 @@
> +/*
> + * Copyright (c) 2017 FUJITSU LIMITED. All rights reserved.
> + * Author: Xiao Yang <yangx.jy@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.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program;  if not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#ifndef MOVE_HUGEPAGES_SUPPORT_H
> +#define MOVE_HUGEPAGES_SUPPORT_H
> +
> +#include "config.h"
> +#include <errno.h>
> +#include <sys/types.h>
> +#include <unistd.h>
> +#include <stdlib.h>
> +#include <stdio.h>
> +# if HAVE_NUMA_H
> +#include <numa.h>
> +# endif
> +# if HAVE_NUMAIF_H
> +#include <numaif.h>
> +# endif
> +#include <sys/mman.h>
> +#include "lapi/mmap.h"
> +
> +#define PATH_MEMINFO	"/proc/meminfo"
> +#define PATH_NR_HUGEPAGES	"/proc/sys/vm/nr_hugepages"
> +#define PATH_HUGEPAGES	"/sys/kernel/mm/hugepages/"
> +
> +# ifdef HAVE_NUMA_V2
> +void alloc_free_huge_on_node(unsigned int node, size_t size, int hgpsz);
> +void do_hugepages_move(int count, int hpgsz, int pgsz, void *addr,
> +		       unsigned int node1, unsigned int node2);
> +long modify_node_nr_hgpgs(char *path_hgpgs_node, size_t size,
> +			  unsigned int node, int hgpsz, long add_num);
> +void restore_orig_hgpgs_value(char *path_hgpgs, long orig_value);
> +# endif
> +#endif
> diff --git a/testcases/kernel/syscalls/move_pages/move_pages12.c b/testcases/kernel/syscalls/move_pages/move_pages12.c
> index 1ac7bc1..9c66304 100644
> --- a/testcases/kernel/syscalls/move_pages/move_pages12.c
> +++ b/testcases/kernel/syscalls/move_pages/move_pages12.c
> @@ -41,14 +41,12 @@
>  
>  #include "tst_test.h"
>  #include "move_pages_support.h"
> +#include "move_hugepages_support.h"
>  #include "lapi/mmap.h"
>  
>  #ifdef HAVE_NUMA_V2
>  
>  #define LOOPS	1000
> -#define PATH_MEMINFO	"/proc/meminfo"
> -#define PATH_NR_HUGEPAGES	"/proc/sys/vm/nr_hugepages"
> -#define PATH_HUGEPAGES	"/sys/kernel/mm/hugepages/"
>  #define TEST_PAGES	2
>  #define TEST_NODES	2
>  
> @@ -61,41 +59,6 @@ static long orig_hugepages_node2 = -1;
>  static unsigned int node1, node2;
>  static void *addr;
>  
> -static void do_child(void)
> -{
> -	int test_pages = TEST_PAGES * hpsz / pgsz;
> -	int i, j;
> -	int *nodes, *status;
> -	void **pages;
> -	pid_t ppid = getppid();
> -
> -	pages = SAFE_MALLOC(sizeof(char *) * test_pages);
> -	nodes = SAFE_MALLOC(sizeof(int) * test_pages);
> -	status = SAFE_MALLOC(sizeof(int) * test_pages);
> -
> -	for (i = 0; i < test_pages; i++)
> -		pages[i] = addr + i * pgsz;
> -
> -	for (i = 0; ; i++) {
> -		for (j = 0; j < test_pages; j++) {
> -			if (i % 2 == 0)
> -				nodes[j] = node1;
> -			else
> -				nodes[j] = node2;
> -			status[j] = 0;
> -		}
> -
> -		TEST(numa_move_pages(ppid, test_pages,
> -			pages, nodes, status, MPOL_MF_MOVE_ALL));
> -		if (TEST_RETURN) {
> -			tst_res(TFAIL | TTERRNO, "move_pages failed");
> -			break;
> -		}
> -	}
> -
> -	exit(0);
> -}
> -
>  static void do_test(void)
>  {
>  	int i;
> @@ -109,7 +72,7 @@ static void do_test(void)
>  
>  	cpid = SAFE_FORK();
>  	if (cpid == 0)
> -		do_child();
> +		do_hugepages_move(TEST_PAGES, hpsz, pgsz, addr, node1, node2);
>  
>  	for (i = 0; i < LOOPS; i++) {
>  		void *ptr;
> @@ -133,51 +96,6 @@ static void do_test(void)
>  	}
>  }
>  
> -static void alloc_free_huge_on_node(unsigned int node, size_t size)
> -{
> -	char *mem;
> -	long ret;
> -	struct bitmask *bm;
> -
> -	tst_res(TINFO, "Allocating and freeing %zu hugepages on node %u",
> -		size / hpsz, node);
> -
> -	mem = mmap(NULL, size, PROT_READ | PROT_WRITE,
> -		   MAP_PRIVATE | MAP_ANONYMOUS | MAP_HUGETLB, -1, 0);
> -	if (mem == MAP_FAILED) {
> -		if (errno == ENOMEM)
> -			tst_brk(TCONF, "Cannot allocate huge pages");
> -
> -		tst_brk(TBROK | TERRNO, "mmap(..., MAP_HUGETLB, ...) failed");
> -	}
> -
> -	bm = numa_bitmask_alloc(numa_max_possible_node() + 1);
> -	if (!bm)
> -		tst_brk(TBROK | TERRNO, "numa_bitmask_alloc() failed");
> -
> -	numa_bitmask_setbit(bm, node);
> -
> -	ret = mbind(mem, size, MPOL_BIND, bm->maskp, bm->size + 1, 0);
> -	if (ret) {
> -		if (errno == ENOMEM)
> -			tst_brk(TCONF, "Cannot mbind huge pages");
> -
> -		tst_brk(TBROK | TERRNO, "mbind() failed");
> -	}
> -
> -	TEST(mlock(mem, size));
> -	if (TEST_RETURN) {
> -		SAFE_MUNMAP(mem, size);
> -		if (TEST_ERRNO == ENOMEM || TEST_ERRNO == EAGAIN)
> -			tst_brk(TCONF, "Cannot lock huge pages");
> -		tst_brk(TBROK | TTERRNO, "mlock failed");
> -	}
> -
> -	numa_bitmask_free(bm);
> -
> -	SAFE_MUNMAP(mem, size);
> -}
> -
>  static void setup(void)
>  {
>  	int memfree, ret;
> @@ -200,33 +118,8 @@ static void setup(void)
>  	if (4 * hpsz > memfree)
>  		tst_brk(TBROK, "Not enough free RAM");
>  
> -	snprintf(path_hugepages_node1, sizeof(path_hugepages_node1),
> -		 "/sys/devices/system/node/node%u/hugepages/hugepages-%dkB/nr_hugepages",
> -		 node1, hpsz);
> -
> -	snprintf(path_hugepages_node2, sizeof(path_hugepages_node2),
> -		 "/sys/devices/system/node/node%u/hugepages/hugepages-%dkB/nr_hugepages",
> -		 node2, hpsz);
> -
> -	if (!access(path_hugepages_node1, F_OK)) {
> -		SAFE_FILE_SCANF(path_hugepages_node1,
> -				"%ld", &orig_hugepages_node1);
> -		tst_res(TINFO,
> -			"Increasing %dkB hugepages pool on node %u to %ld",
> -			hpsz, node1, orig_hugepages_node1 + 4);
> -		SAFE_FILE_PRINTF(path_hugepages_node1,
> -				 "%ld", orig_hugepages_node1 + 4);
> -	}
> -
> -	if (!access(path_hugepages_node2, F_OK)) {
> -		SAFE_FILE_SCANF(path_hugepages_node2,
> -				"%ld", &orig_hugepages_node2);
> -		tst_res(TINFO,
> -			"Increasing %dkB hugepages pool on node %u to %ld",
> -			hpsz, node2, orig_hugepages_node2 + 4);
> -		SAFE_FILE_PRINTF(path_hugepages_node2,
> -				 "%ld", orig_hugepages_node2 + 4);
> -	}
> +	orig_hugepages_node1 = modify_node_nr_hgpgs(path_hugepages_node1, sizeof(path_hugepages_node1), node1, hpsz, 4);
> +	orig_hugepages_node2 = modify_node_nr_hgpgs(path_hugepages_node2, sizeof(path_hugepages_node2), node2, hpsz, 4);
>  
>  	hpsz *= 1024;
>  
> @@ -237,24 +130,15 @@ static void setup(void)
>  		SAFE_FILE_PRINTF(PATH_NR_HUGEPAGES, "%ld", orig_hugepages + 8);
>  	}
>  
> -	alloc_free_huge_on_node(node1, 4L * hpsz);
> -	alloc_free_huge_on_node(node2, 4L * hpsz);
> +	alloc_free_huge_on_node(node1, 4L * hpsz, hpsz);
> +	alloc_free_huge_on_node(node2, 4L * hpsz, hpsz);
>  }
>  
>  static void cleanup(void)
>  {
> -	if (orig_hugepages != -1)
> -		SAFE_FILE_PRINTF(PATH_NR_HUGEPAGES, "%ld", orig_hugepages);
> -
> -	if (orig_hugepages_node1 != -1) {
> -		SAFE_FILE_PRINTF(path_hugepages_node1,
> -				 "%ld", orig_hugepages_node1);
> -	}
> -
> -	if (orig_hugepages_node2 != -1) {
> -		SAFE_FILE_PRINTF(path_hugepages_node2,
> -				 "%ld", orig_hugepages_node2);
> -	}
> +	restore_orig_hgpgs_value(PATH_NR_HUGEPAGES, orig_hugepages);
> +	restore_orig_hgpgs_value(path_hugepages_node1, orig_hugepages_node1);
> +	restore_orig_hgpgs_value(path_hugepages_node2, orig_hugepages_node2);
>  }
>  
>  static struct tst_test test = {




^ permalink raw reply	[flat|nested] 11+ messages in thread

* [LTP] [PATCH v2] syscalls/move_page12.c: Add new regression test
  2017-12-01  3:48 ` [LTP] [PATCH 2/2] syscalls/move_pages13.c: Add new regression test Xiao Yang
@ 2019-04-16  8:48   ` Yang Xu
  2019-05-20 14:41     ` Cyril Hrubis
  0 siblings, 1 reply; 11+ messages in thread
From: Yang Xu @ 2019-04-16  8:48 UTC (permalink / raw)
  To: ltp

The bug has been fixed in kernel: 'c9d398fa2378 ("mm, hugetlb:
use pte_present() instead of pmd_present() in follow_huge_pmd()").

Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com>
Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
---
 .../kernel/syscalls/move_pages/move_pages12.c | 100 ++++++++++++------
 1 file changed, 67 insertions(+), 33 deletions(-)

diff --git a/testcases/kernel/syscalls/move_pages/move_pages12.c b/testcases/kernel/syscalls/move_pages/move_pages12.c
index 04fda8bef..e9035a18e 100644
--- a/testcases/kernel/syscalls/move_pages/move_pages12.c
+++ b/testcases/kernel/syscalls/move_pages/move_pages12.c
@@ -1,35 +1,40 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
 /*
  * Copyright (c) 2016 Fujitsu Ltd.
  *  Author: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
  *  Ported: Guangwen Feng <fenggw-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.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program, if not, see <http://www.gnu.org/licenses/>.
+ *  Ported: Xiao Yang <yangx.jy@cn.fujitsu.com>
+ *  Ported: Yang Xu <xuyang2018.jy@cn.jujitsu.com>
  */
 
 /*
- * This is a regression test for the race condition between move_pages()
- * and freeing hugepages, where move_pages() calls follow_page(FOLL_GET)
- * for hugepages internally and tries to get its refcount without
- * preventing concurrent freeing.
+ * Description:
  *
- * This test can crash the buggy kernel, and the bug was fixed in:
+ * Test #1:
+ *  This is a regression test for the race condition between move_pages()
+ *  and freeing hugepages, where move_pages() calls follow_page(FOLL_GET)
+ *  for hugepages internally and tries to get its refcount without
+ *  preventing concurrent freeing.
  *
- *  commit e66f17ff71772b209eed39de35aaa99ba819c93d
- *  Author: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
- *  Date:   Wed Feb 11 15:25:22 2015 -0800
+ *  This test can crash the buggy kernel, and the bug was fixed in:
+ *
+ *   commit e66f17ff71772b209eed39de35aaa99ba819c93d
+ *   Author: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
+ *   Date:   Wed Feb 11 15:25:22 2015 -0800
  *
- *  mm/hugetlb: take page table lock in follow_huge_pmd()
+ *   mm/hugetlb: take page table lock in follow_huge_pmd()
+ *
+ *  Test #2:
+ *   This is a regression test for the race condition, where move_pages()
+ *   and soft offline are called on a single hugetlb page concurrently.
+ *
+ *   This bug can crash the buggy kernel, and was fixed by:
+ *
+ *   commit c9d398fa237882ea07167e23bcfc5e6847066518
+ *   Author: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
+ *   Date:   Fri Mar 31 15:11:55 2017 -0700
+ *
+ *   mm, hugetlb: use pte_present() instead of pmd_present() in follow_huge_pmd()
  */
 
 #include <errno.h>
@@ -49,9 +54,17 @@
 #define PATH_MEMINFO	"/proc/meminfo"
 #define PATH_NR_HUGEPAGES	"/proc/sys/vm/nr_hugepages"
 #define PATH_HUGEPAGES	"/sys/kernel/mm/hugepages/"
-#define TEST_PAGES	2
+#define PATH_PAGEMAP	"/proc/self/pagemap"
 #define TEST_NODES	2
 
+static struct tcase {
+	int tpages;
+	int offline;
+} tcases[] = {
+	{2, 0},
+	{2, 1},
+};
+
 static int pgsz, hpsz;
 static long orig_hugepages = -1;
 static char path_hugepages_node1[PATH_MAX];
@@ -61,9 +74,21 @@ static long orig_hugepages_node2 = -1;
 static unsigned int node1, node2;
 static void *addr;
 
-static void do_child(void)
+static void do_soft_offline(int tpgs)
 {
-	int test_pages = TEST_PAGES * hpsz / pgsz;
+	if (madvise(addr, tpgs * hpsz, MADV_SOFT_OFFLINE) == -1) {
+		if (errno == EINVAL) {
+			tst_brk(TCONF,
+				"madvise() didn't support MADV_SOFT_OFFLINE");
+		} else {
+			tst_brk(TBROK | TERRNO, "madvise() failed");
+		}
+	}
+}
+
+static void do_child(int tpgs)
+{
+	int test_pages = tpgs * hpsz / pgsz;
 	int i, j;
 	int *nodes, *status;
 	void **pages;
@@ -96,34 +121,42 @@ static void do_child(void)
 	exit(0);
 }
 
-static void do_test(void)
+static void do_test(unsigned int n)
 {
 	int i;
 	pid_t cpid = -1;
 	int status;
 	unsigned int twenty_percent = (tst_timeout_remaining() / 5);
 
-	addr = SAFE_MMAP(NULL, TEST_PAGES * hpsz, PROT_READ | PROT_WRITE,
+	if (tcases[n].offline) {
+		if (access(PATH_PAGEMAP, F_OK))
+			tst_brk(TCONF, "pagemap was not supported");
+	}
+
+	addr = SAFE_MMAP(NULL, tcases[n].tpages * hpsz, PROT_READ | PROT_WRITE,
 		MAP_PRIVATE | MAP_ANONYMOUS | MAP_HUGETLB, -1, 0);
 
-	SAFE_MUNMAP(addr, TEST_PAGES * hpsz);
+	SAFE_MUNMAP(addr, tcases[n].tpages * hpsz);
 
 	cpid = SAFE_FORK();
 	if (cpid == 0)
-		do_child();
+		do_child(tcases[n].tpages);
 
 	for (i = 0; i < LOOPS; i++) {
 		void *ptr;
 
-		ptr = SAFE_MMAP(NULL, TEST_PAGES * hpsz,
+		ptr = SAFE_MMAP(NULL, tcases[n].tpages * hpsz,
 			PROT_READ | PROT_WRITE,
 			MAP_PRIVATE | MAP_ANONYMOUS | MAP_HUGETLB, -1, 0);
 		if (ptr != addr)
 			tst_brk(TBROK, "Failed to mmap@desired addr");
 
-		memset(addr, 0, TEST_PAGES * hpsz);
+		memset(addr, 0, tcases[n].tpages * hpsz);
+
+		if (tcases[n].offline)
+			do_soft_offline(tcases[n].tpages);
 
-		SAFE_MUNMAP(addr, TEST_PAGES * hpsz);
+		SAFE_MUNMAP(addr, tcases[n].tpages * hpsz);
 
 		if (tst_timeout_remaining() < twenty_percent)
 			break;
@@ -266,7 +299,8 @@ static struct tst_test test = {
 	.forks_child = 1,
 	.setup = setup,
 	.cleanup = cleanup,
-	.test_all = do_test,
+	.test = do_test,
+	.tcnt = ARRAY_SIZE(tcases),
 };
 
 #else
-- 
2.18.1




^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [LTP] [PATCH 1/2] syscalls/move_pages: move the common code to move_hugepages_support
  2017-12-01  3:48 [LTP] [PATCH 1/2] syscalls/move_pages: move the common code to move_hugepages_support Xiao Yang
  2017-12-01  3:48 ` [LTP] [PATCH 2/2] syscalls/move_pages13.c: Add new regression test Xiao Yang
  2018-02-08  1:21 ` [LTP] [PATCH 1/2] syscalls/move_pages: move the common code to move_hugepages_support Xiao Yang
@ 2019-05-20 13:55 ` Cyril Hrubis
  2 siblings, 0 replies; 11+ messages in thread
From: Cyril Hrubis @ 2019-05-20 13:55 UTC (permalink / raw)
  To: ltp

Hi!
> diff --git a/testcases/kernel/syscalls/move_pages/move_hugepages_support.c b/testcases/kernel/syscalls/move_pages/move_hugepages_support.c
> new file mode 100644
> index 0000000..096d91a
> --- /dev/null
> +++ b/testcases/kernel/syscalls/move_pages/move_hugepages_support.c
> @@ -0,0 +1,131 @@
> +/*
> + * Copyright (c) 2017 FUJITSU LIMITED. All rights reserved.
> + * Author: Xiao Yang <yangx.jy@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.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#define TST_NO_DEFAULT_MAIN
> +#include "tst_test.h"
> +#include "move_hugepages_support.h"
> +
> +#ifdef HAVE_NUMA_V2
> +void alloc_free_huge_on_node(unsigned int node, size_t size, int hgpsz)
> +{
> +	char *mem;
> +	long ret;
> +	struct bitmask *bm;
> +
> +	tst_res(TINFO, "Allocating and freeing %zu hugepages on node %u",
> +		size / hgpsz, node);
> +
> +	mem = mmap(NULL, size, PROT_READ | PROT_WRITE,
> +		   MAP_PRIVATE | MAP_ANONYMOUS | MAP_HUGETLB, -1, 0);
> +	if (mem == MAP_FAILED) {
> +		if (errno == ENOMEM)
> +			tst_brk(TCONF, "Cannot allocate huge pages");
> +
> +		tst_brk(TBROK | TERRNO, "mmap(..., MAP_HUGETLB, ...) failed");
> +	}
> +
> +	bm = numa_bitmask_alloc(numa_max_possible_node() + 1);
> +	if (!bm)
> +		tst_brk(TBROK | TERRNO, "numa_bitmask_alloc() failed");
> +
> +	numa_bitmask_setbit(bm, node);
> +
> +	ret = mbind(mem, size, MPOL_BIND, bm->maskp, bm->size + 1, 0);
> +	if (ret) {
> +		if (errno == ENOMEM)
> +			tst_brk(TCONF, "Cannot mbind huge pages");
> +
> +		tst_brk(TBROK | TERRNO, "mbind() failed");
> +	}
> +
> +	TEST(mlock(mem, size));
> +	if (TEST_RETURN) {
> +		SAFE_MUNMAP(mem, size);
> +		if (TEST_ERRNO == ENOMEM || TEST_ERRNO == EAGAIN)
> +			tst_brk(TCONF, "Cannot lock huge pages");
> +		tst_brk(TBROK | TTERRNO, "mlock failed");
> +	}
> +
> +	numa_bitmask_free(bm);
> +
> +	SAFE_MUNMAP(mem, size);
> +}
> +
> +void do_hugepages_move(int count, int hpgsz, int pgsz, void *addr,
> +		       unsigned int node1, unsigned int node2)
> +{
> +	int test_pages = count * hpgsz / pgsz;
> +	int i, j;
> +	int *nodes, *status;
> +	void **pages;
> +	pid_t ppid = getppid();
> +
> +	pages = SAFE_MALLOC(sizeof(char *) * test_pages);
> +	nodes = SAFE_MALLOC(sizeof(int) * test_pages);
> +	status = SAFE_MALLOC(sizeof(int) * test_pages);
> +
> +	for (i = 0; i < test_pages; i++)
> +		pages[i] = addr + i * pgsz;
> +
> +	for (i = 0; ; i++) {
> +		for (j = 0; j < test_pages; j++) {
> +			if (i % 2 == 0)
> +				nodes[j] = node1;
> +			else
> +				nodes[j] = node2;
> +			status[j] = 0;
> +		}
> +
> +		TEST(numa_move_pages(ppid, test_pages,
> +			pages, nodes, status, MPOL_MF_MOVE_ALL));
> +		if (TEST_RETURN) {
> +			tst_res(TFAIL | TTERRNO, "move_pages failed");
> +			break;
> +		}
> +	}
> +
> +	exit(0);
> +}
> +
> +long modify_node_nr_hgpgs(char *path_hgpgs_node, size_t size,
> +			  unsigned int node, int hgpsz, long add_num)
> +{
> +	long orig_hgpgs_node = -1;
> +
> +	snprintf(path_hgpgs_node, size,
> +		 "/sys/devices/system/node/node%u/hugepages/hugepages-%dkB/nr_hugepages",
> +		 node, hgpsz);
> +
> +	if (!access(path_hgpgs_node, F_OK)) {
> +		SAFE_FILE_SCANF(path_hgpgs_node, "%ld", &orig_hgpgs_node);
> +		tst_res(TINFO,
> +			"Increasing %dkB hugepages pool on node %u to %ld",
> +			hgpsz, node, orig_hgpgs_node + add_num);
> +		SAFE_FILE_PRINTF(path_hgpgs_node, "%ld",
> +				 orig_hgpgs_node + add_num);
> +	}
> +
> +	return orig_hgpgs_node;
> +}
> +
> +void restore_orig_hgpgs_value(char *path_hgpgs, long orig_value)
> +{
> +	if (orig_value != -1)
> +		SAFE_FILE_PRINTF(path_hgpgs, "%ld", orig_value);
> +}

I guess that it would be a bit easier just to put these three functions
into the header as well....

Other than that it's fine.

-- 
Cyril Hrubis
chrubis@suse.cz

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [LTP] [PATCH v2] syscalls/move_page12.c: Add new regression test
  2019-04-16  8:48   ` [LTP] [PATCH v2] syscalls/move_page12.c: " Yang Xu
@ 2019-05-20 14:41     ` Cyril Hrubis
  2019-05-21  9:48       ` xuyang
  2019-05-21  9:51       ` [LTP] [PATCH v3] syscalls/move_page12: " Yang Xu
  0 siblings, 2 replies; 11+ messages in thread
From: Cyril Hrubis @ 2019-05-20 14:41 UTC (permalink / raw)
  To: ltp

Hi!
>  #include <errno.h>
> @@ -49,9 +54,17 @@
>  #define PATH_MEMINFO	"/proc/meminfo"
>  #define PATH_NR_HUGEPAGES	"/proc/sys/vm/nr_hugepages"
>  #define PATH_HUGEPAGES	"/sys/kernel/mm/hugepages/"
> -#define TEST_PAGES	2
> +#define PATH_PAGEMAP	"/proc/self/pagemap"
>  #define TEST_NODES	2
>  
> +static struct tcase {
> +	int tpages;
> +	int offline;
> +} tcases[] = {
> +	{2, 0},
> +	{2, 1},
> +};
> +
>  static int pgsz, hpsz;
>  static long orig_hugepages = -1;
>  static char path_hugepages_node1[PATH_MAX];
> @@ -61,9 +74,21 @@ static long orig_hugepages_node2 = -1;
>  static unsigned int node1, node2;
>  static void *addr;
>  
> -static void do_child(void)
> +static void do_soft_offline(int tpgs)
>  {
> -	int test_pages = TEST_PAGES * hpsz / pgsz;
> +	if (madvise(addr, tpgs * hpsz, MADV_SOFT_OFFLINE) == -1) {
> +		if (errno == EINVAL) {
> +			tst_brk(TCONF,
> +				"madvise() didn't support MADV_SOFT_OFFLINE");

Can we change this to tst_res() and return a value from this function so
that the second test could be skipped without exitting the whole test?

The thing is that as far as we are implementing more than one testcase
in a test unsupported features that are only relevant for a one test
should not exit the whole testcase in case of looping (the -i
parameter).

> +		} else {
> +			tst_brk(TBROK | TERRNO, "madvise() failed");
> +		}
> +	}
> +}
> +
> +static void do_child(int tpgs)
> +{
> +	int test_pages = tpgs * hpsz / pgsz;
>  	int i, j;
>  	int *nodes, *status;
>  	void **pages;
> @@ -96,34 +121,42 @@ static void do_child(void)
>  	exit(0);
>  }
>  
> -static void do_test(void)
> +static void do_test(unsigned int n)
>  {
>  	int i;
>  	pid_t cpid = -1;
>  	int status;
>  	unsigned int twenty_percent = (tst_timeout_remaining() / 5);
>  
> -	addr = SAFE_MMAP(NULL, TEST_PAGES * hpsz, PROT_READ | PROT_WRITE,
> +	if (tcases[n].offline) {
> +		if (access(PATH_PAGEMAP, F_OK))
> +			tst_brk(TCONF, "pagemap was not supported");

Here as well, can we do tst_res() and return instead?

> +	}
> +
> +	addr = SAFE_MMAP(NULL, tcases[n].tpages * hpsz, PROT_READ | PROT_WRITE,
>  		MAP_PRIVATE | MAP_ANONYMOUS | MAP_HUGETLB, -1, 0);
>  
> -	SAFE_MUNMAP(addr, TEST_PAGES * hpsz);
> +	SAFE_MUNMAP(addr, tcases[n].tpages * hpsz);
>  
>  	cpid = SAFE_FORK();
>  	if (cpid == 0)
> -		do_child();
> +		do_child(tcases[n].tpages);
>  
>  	for (i = 0; i < LOOPS; i++) {
>  		void *ptr;
>  
> -		ptr = SAFE_MMAP(NULL, TEST_PAGES * hpsz,
> +		ptr = SAFE_MMAP(NULL, tcases[n].tpages * hpsz,
>  			PROT_READ | PROT_WRITE,
>  			MAP_PRIVATE | MAP_ANONYMOUS | MAP_HUGETLB, -1, 0);
>  		if (ptr != addr)
>  			tst_brk(TBROK, "Failed to mmap at desired addr");
>  
> -		memset(addr, 0, TEST_PAGES * hpsz);
> +		memset(addr, 0, tcases[n].tpages * hpsz);
> +
> +		if (tcases[n].offline)
> +			do_soft_offline(tcases[n].tpages);


And here we should check the return value and return if the madvise in
the function has returned EINVAL.


> -		SAFE_MUNMAP(addr, TEST_PAGES * hpsz);
> +		SAFE_MUNMAP(addr, tcases[n].tpages * hpsz);
>  
>  		if (tst_timeout_remaining() < twenty_percent)
>  			break;
> @@ -266,7 +299,8 @@ static struct tst_test test = {
>  	.forks_child = 1,
>  	.setup = setup,
>  	.cleanup = cleanup,
> -	.test_all = do_test,
> +	.test = do_test,
> +	.tcnt = ARRAY_SIZE(tcases),
>  };
>  
>  #else

Otherwise it looks good.

-- 
Cyril Hrubis
chrubis@suse.cz

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [LTP] [PATCH v2] syscalls/move_page12.c: Add new regression test
  2019-05-20 14:41     ` Cyril Hrubis
@ 2019-05-21  9:48       ` xuyang
  2019-05-21  9:51       ` [LTP] [PATCH v3] syscalls/move_page12: " Yang Xu
  1 sibling, 0 replies; 11+ messages in thread
From: xuyang @ 2019-05-21  9:48 UTC (permalink / raw)
  To: ltp

   Hi cryil

> Hi!
>>   #include<errno.h>
>> @@ -49,9 +54,17 @@
>>   #define PATH_MEMINFO	"/proc/meminfo"
>>   #define PATH_NR_HUGEPAGES	"/proc/sys/vm/nr_hugepages"
>>   #define PATH_HUGEPAGES	"/sys/kernel/mm/hugepages/"
>> -#define TEST_PAGES	2
>> +#define PATH_PAGEMAP	"/proc/self/pagemap"
>>   #define TEST_NODES	2
>>
>> +static struct tcase {
>> +	int tpages;
>> +	int offline;
>> +} tcases[] = {
>> +	{2, 0},
>> +	{2, 1},
>> +};
>> +
>>   static int pgsz, hpsz;
>>   static long orig_hugepages = -1;
>>   static char path_hugepages_node1[PATH_MAX];
>> @@ -61,9 +74,21 @@ static long orig_hugepages_node2 = -1;
>>   static unsigned int node1, node2;
>>   static void *addr;
>>
>> -static void do_child(void)
>> +static void do_soft_offline(int tpgs)
>>   {
>> -	int test_pages = TEST_PAGES * hpsz / pgsz;
>> +	if (madvise(addr, tpgs * hpsz, MADV_SOFT_OFFLINE) == -1) {
>> +		if (errno == EINVAL) {
>> +			tst_brk(TCONF,
>> +				"madvise() didn't support MADV_SOFT_OFFLINE");
> Can we change this to tst_res() and return a value from this function so
> that the second test could be skipped without exitting the whole test?
>
> The thing is that as far as we are implementing more than one testcase
> in a test unsupported features that are only relevant for a one test
> should not exit the whole testcase in case of looping (the -i
> parameter).
   OK.  I will change it to tst_res in v3 patch soon.

>> +		} else {
>> +			tst_brk(TBROK | TERRNO, "madvise() failed");
>> +		}
>> +	}
>> +}
>> +
>> +static void do_child(int tpgs)
>> +{
>> +	int test_pages = tpgs * hpsz / pgsz;
>>   	int i, j;
>>   	int *nodes, *status;
>>   	void **pages;
>> @@ -96,34 +121,42 @@ static void do_child(void)
>>   	exit(0);
>>   }
>>
>> -static void do_test(void)
>> +static void do_test(unsigned int n)
>>   {
>>   	int i;
>>   	pid_t cpid = -1;
>>   	int status;
>>   	unsigned int twenty_percent = (tst_timeout_remaining() / 5);
>>
>> -	addr = SAFE_MMAP(NULL, TEST_PAGES * hpsz, PROT_READ | PROT_WRITE,
>> +	if (tcases[n].offline) {
>> +		if (access(PATH_PAGEMAP, F_OK))
>> +			tst_brk(TCONF, "pagemap was not supported");
> Here as well, can we do tst_res() and return instead?
>
   PATH_PAGEMAP is useless in this case. I will remove it .

>> +	}
>> +
>> +	addr = SAFE_MMAP(NULL, tcases[n].tpages * hpsz, PROT_READ | PROT_WRITE,
>>   		MAP_PRIVATE | MAP_ANONYMOUS | MAP_HUGETLB, -1, 0);
>>
>> -	SAFE_MUNMAP(addr, TEST_PAGES * hpsz);
>> +	SAFE_MUNMAP(addr, tcases[n].tpages * hpsz);
>>
>>   	cpid = SAFE_FORK();
>>   	if (cpid == 0)
>> -		do_child();
>> +		do_child(tcases[n].tpages);
>>
>>   	for (i = 0; i<  LOOPS; i++) {
>>   		void *ptr;
>>
>> -		ptr = SAFE_MMAP(NULL, TEST_PAGES * hpsz,
>> +		ptr = SAFE_MMAP(NULL, tcases[n].tpages * hpsz,
>>   			PROT_READ | PROT_WRITE,
>>   			MAP_PRIVATE | MAP_ANONYMOUS | MAP_HUGETLB, -1, 0);
>>   		if (ptr != addr)
>>   			tst_brk(TBROK, "Failed to mmap at desired addr");
>>
>> -		memset(addr, 0, TEST_PAGES * hpsz);
>> +		memset(addr, 0, tcases[n].tpages * hpsz);
>> +
>> +		if (tcases[n].offline)
>> +			do_soft_offline(tcases[n].tpages);
>
> And here we should check the return value and return if the madvise in
> the function has returned EINVAL.
>
>
I got it.

>> -		SAFE_MUNMAP(addr, TEST_PAGES * hpsz);
>> +		SAFE_MUNMAP(addr, tcases[n].tpages * hpsz);
>>
>>   		if (tst_timeout_remaining()<  twenty_percent)
>>   			break;
>> @@ -266,7 +299,8 @@ static struct tst_test test = {
>>   	.forks_child = 1,
>>   	.setup = setup,
>>   	.cleanup = cleanup,
>> -	.test_all = do_test,
>> +	.test = do_test,
>> +	.tcnt = ARRAY_SIZE(tcases),
>>   };
>>
>>   #else
> Otherwise it looks good.
>




^ permalink raw reply	[flat|nested] 11+ messages in thread

* [LTP] [PATCH v3] syscalls/move_page12: Add new regression test
  2019-05-20 14:41     ` Cyril Hrubis
  2019-05-21  9:48       ` xuyang
@ 2019-05-21  9:51       ` Yang Xu
  2019-05-21 13:47         ` Cyril Hrubis
  1 sibling, 1 reply; 11+ messages in thread
From: Yang Xu @ 2019-05-21  9:51 UTC (permalink / raw)
  To: ltp

The bug has been fixed in kernel: 'c9d398fa2378 ("mm, hugetlb:
use pte_present() instead of pmd_present() in follow_huge_pmd()").

Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com>
---
 .../kernel/syscalls/move_pages/move_pages12.c | 104 ++++++++++++------
 1 file changed, 70 insertions(+), 34 deletions(-)

diff --git a/testcases/kernel/syscalls/move_pages/move_pages12.c b/testcases/kernel/syscalls/move_pages/move_pages12.c
index 04fda8bef..104467565 100644
--- a/testcases/kernel/syscalls/move_pages/move_pages12.c
+++ b/testcases/kernel/syscalls/move_pages/move_pages12.c
@@ -1,35 +1,41 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
 /*
- * Copyright (c) 2016 Fujitsu Ltd.
+ *  Copyright (c) 2019 FUJITSU LIMITED. All rights reserved.
  *  Author: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
  *  Ported: Guangwen Feng <fenggw-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.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program, if not, see <http://www.gnu.org/licenses/>.
+ *  Ported: Xiao Yang <yangx.jy@cn.fujitsu.com>
+ *  Ported: Yang Xu <xuyang2018.jy@cn.jujitsu.com>
  */
 
 /*
- * This is a regression test for the race condition between move_pages()
- * and freeing hugepages, where move_pages() calls follow_page(FOLL_GET)
- * for hugepages internally and tries to get its refcount without
- * preventing concurrent freeing.
+ * Description:
  *
- * This test can crash the buggy kernel, and the bug was fixed in:
+ * Test #1:
+ *  This is a regression test for the race condition between move_pages()
+ *  and freeing hugepages, where move_pages() calls follow_page(FOLL_GET)
+ *  for hugepages internally and tries to get its refcount without
+ *  preventing concurrent freeing.
  *
- *  commit e66f17ff71772b209eed39de35aaa99ba819c93d
- *  Author: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
- *  Date:   Wed Feb 11 15:25:22 2015 -0800
+ *  This test can crash the buggy kernel, and the bug was fixed in:
+ *
+ *   commit e66f17ff71772b209eed39de35aaa99ba819c93d
+ *   Author: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
+ *   Date:   Wed Feb 11 15:25:22 2015 -0800
+ *
+ *   mm/hugetlb: take page table lock in follow_huge_pmd()
+ *
+ *  Test #2:
+ *   This is a regression test for the race condition, where move_pages()
+ *   and soft offline are called on a single hugetlb page concurrently.
+ *
+ *   This bug can crash the buggy kernel, and was fixed by:
  *
- *  mm/hugetlb: take page table lock in follow_huge_pmd()
+ *   commit c9d398fa237882ea07167e23bcfc5e6847066518
+ *   Author: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
+ *   Date:   Fri Mar 31 15:11:55 2017 -0700
+ *
+ *   mm, hugetlb: use pte_present() instead of pmd_present() in
+ *   follow_huge_pmd()
  */
 
 #include <errno.h>
@@ -49,9 +55,16 @@
 #define PATH_MEMINFO	"/proc/meminfo"
 #define PATH_NR_HUGEPAGES	"/proc/sys/vm/nr_hugepages"
 #define PATH_HUGEPAGES	"/sys/kernel/mm/hugepages/"
-#define TEST_PAGES	2
 #define TEST_NODES	2
 
+static struct tcase {
+	int tpages;
+	int offline;
+} tcases[] = {
+	{2, 0},
+	{2, 1},
+};
+
 static int pgsz, hpsz;
 static long orig_hugepages = -1;
 static char path_hugepages_node1[PATH_MAX];
@@ -61,9 +74,19 @@ static long orig_hugepages_node2 = -1;
 static unsigned int node1, node2;
 static void *addr;
 
-static void do_child(void)
+static int do_soft_offline(int tpgs)
+{
+	if (madvise(addr, tpgs * hpsz, MADV_SOFT_OFFLINE) == -1) {
+		if (errno != EINVAL)
+			tst_res(TFAIL | TTERRNO, "madvise failed");
+		return errno;
+	}
+	return 0;
+}
+
+static void do_child(int tpgs)
 {
-	int test_pages = TEST_PAGES * hpsz / pgsz;
+	int test_pages = tpgs * hpsz / pgsz;
 	int i, j;
 	int *nodes, *status;
 	void **pages;
@@ -96,34 +119,46 @@ static void do_child(void)
 	exit(0);
 }
 
-static void do_test(void)
+static void do_test(unsigned int n)
 {
 	int i;
 	pid_t cpid = -1;
 	int status;
 	unsigned int twenty_percent = (tst_timeout_remaining() / 5);
 
-	addr = SAFE_MMAP(NULL, TEST_PAGES * hpsz, PROT_READ | PROT_WRITE,
+	addr = SAFE_MMAP(NULL, tcases[n].tpages * hpsz, PROT_READ | PROT_WRITE,
 		MAP_PRIVATE | MAP_ANONYMOUS | MAP_HUGETLB, -1, 0);
 
-	SAFE_MUNMAP(addr, TEST_PAGES * hpsz);
+	SAFE_MUNMAP(addr, tcases[n].tpages * hpsz);
 
 	cpid = SAFE_FORK();
 	if (cpid == 0)
-		do_child();
+		do_child(tcases[n].tpages);
 
 	for (i = 0; i < LOOPS; i++) {
 		void *ptr;
 
-		ptr = SAFE_MMAP(NULL, TEST_PAGES * hpsz,
+		ptr = SAFE_MMAP(NULL, tcases[n].tpages * hpsz,
 			PROT_READ | PROT_WRITE,
 			MAP_PRIVATE | MAP_ANONYMOUS | MAP_HUGETLB, -1, 0);
 		if (ptr != addr)
 			tst_brk(TBROK, "Failed to mmap@desired addr");
 
-		memset(addr, 0, TEST_PAGES * hpsz);
+		memset(addr, 0, tcases[n].tpages * hpsz);
+
+		if (tcases[n].offline) {
+			if (do_soft_offline(tcases[n].tpages) == EINVAL) {
+				SAFE_KILL(cpid, SIGKILL);
+				SAFE_WAITPID(cpid, &status, 0);
+				SAFE_MUNMAP(addr, tcases[n].tpages * hpsz);
+				tst_res(TFAIL,
+					"madvise() didn't support "
+					"MADV_SOFT_OFFLINE");
+				return;
+			}
+		}
 
-		SAFE_MUNMAP(addr, TEST_PAGES * hpsz);
+		SAFE_MUNMAP(addr, tcases[n].tpages * hpsz);
 
 		if (tst_timeout_remaining() < twenty_percent)
 			break;
@@ -266,7 +301,8 @@ static struct tst_test test = {
 	.forks_child = 1,
 	.setup = setup,
 	.cleanup = cleanup,
-	.test_all = do_test,
+	.test = do_test,
+	.tcnt = ARRAY_SIZE(tcases),
 };
 
 #else
-- 
2.18.1




^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [LTP] [PATCH v3] syscalls/move_page12: Add new regression test
  2019-05-21  9:51       ` [LTP] [PATCH v3] syscalls/move_page12: " Yang Xu
@ 2019-05-21 13:47         ` Cyril Hrubis
  2019-05-22  1:43           ` [LTP] [PATCH v4] syscalls/move_pages12: " Yang Xu
  0 siblings, 1 reply; 11+ messages in thread
From: Cyril Hrubis @ 2019-05-21 13:47 UTC (permalink / raw)
  To: ltp

Hi!
> +		if (tcases[n].offline) {
> +			if (do_soft_offline(tcases[n].tpages) == EINVAL) {
> +				SAFE_KILL(cpid, SIGKILL);
> +				SAFE_WAITPID(cpid, &status, 0);
> +				SAFE_MUNMAP(addr, tcases[n].tpages * hpsz);
> +				tst_res(TFAIL,
					^
					Shouldn't this be TCONF instead?

> +					"madvise() didn't support "
> +					"MADV_SOFT_OFFLINE");
                                             ^
					 LKML coding style prefers not
					 to break up user visible
					 strings, so this string should
					 be on one line even if the line
					 is longer than 80 chars.

> +				return;
> +			}
> +		}

Other than that it looks good.

-- 
Cyril Hrubis
chrubis@suse.cz

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [LTP] [PATCH v4] syscalls/move_pages12: Add new regression test
  2019-05-21 13:47         ` Cyril Hrubis
@ 2019-05-22  1:43           ` Yang Xu
  2019-05-22 11:45             ` Cyril Hrubis
  0 siblings, 1 reply; 11+ messages in thread
From: Yang Xu @ 2019-05-22  1:43 UTC (permalink / raw)
  To: ltp

The bug has been fixed in kernel: 'c9d398fa2378 ("mm, hugetlb:
use pte_present() instead of pmd_present() in follow_huge_pmd()").

Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com>
---
 .../kernel/syscalls/move_pages/move_pages12.c | 103 ++++++++++++------
 1 file changed, 69 insertions(+), 34 deletions(-)

diff --git a/testcases/kernel/syscalls/move_pages/move_pages12.c b/testcases/kernel/syscalls/move_pages/move_pages12.c
index 04fda8bef..569bbfbc2 100644
--- a/testcases/kernel/syscalls/move_pages/move_pages12.c
+++ b/testcases/kernel/syscalls/move_pages/move_pages12.c
@@ -1,35 +1,41 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
 /*
- * Copyright (c) 2016 Fujitsu Ltd.
+ *  Copyright (c) 2019 FUJITSU LIMITED. All rights reserved.
  *  Author: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
  *  Ported: Guangwen Feng <fenggw-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.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program, if not, see <http://www.gnu.org/licenses/>.
+ *  Ported: Xiao Yang <yangx.jy@cn.fujitsu.com>
+ *  Ported: Yang Xu <xuyang2018.jy@cn.jujitsu.com>
  */
 
 /*
- * This is a regression test for the race condition between move_pages()
- * and freeing hugepages, where move_pages() calls follow_page(FOLL_GET)
- * for hugepages internally and tries to get its refcount without
- * preventing concurrent freeing.
+ * Description:
  *
- * This test can crash the buggy kernel, and the bug was fixed in:
+ * Test #1:
+ *  This is a regression test for the race condition between move_pages()
+ *  and freeing hugepages, where move_pages() calls follow_page(FOLL_GET)
+ *  for hugepages internally and tries to get its refcount without
+ *  preventing concurrent freeing.
  *
- *  commit e66f17ff71772b209eed39de35aaa99ba819c93d
- *  Author: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
- *  Date:   Wed Feb 11 15:25:22 2015 -0800
+ *  This test can crash the buggy kernel, and the bug was fixed in:
+ *
+ *   commit e66f17ff71772b209eed39de35aaa99ba819c93d
+ *   Author: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
+ *   Date:   Wed Feb 11 15:25:22 2015 -0800
+ *
+ *   mm/hugetlb: take page table lock in follow_huge_pmd()
+ *
+ *  Test #2:
+ *   This is a regression test for the race condition, where move_pages()
+ *   and soft offline are called on a single hugetlb page concurrently.
+ *
+ *   This bug can crash the buggy kernel, and was fixed by:
  *
- *  mm/hugetlb: take page table lock in follow_huge_pmd()
+ *   commit c9d398fa237882ea07167e23bcfc5e6847066518
+ *   Author: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
+ *   Date:   Fri Mar 31 15:11:55 2017 -0700
+ *
+ *   mm, hugetlb: use pte_present() instead of pmd_present() in
+ *   follow_huge_pmd()
  */
 
 #include <errno.h>
@@ -49,9 +55,16 @@
 #define PATH_MEMINFO	"/proc/meminfo"
 #define PATH_NR_HUGEPAGES	"/proc/sys/vm/nr_hugepages"
 #define PATH_HUGEPAGES	"/sys/kernel/mm/hugepages/"
-#define TEST_PAGES	2
 #define TEST_NODES	2
 
+static struct tcase {
+	int tpages;
+	int offline;
+} tcases[] = {
+	{2, 0},
+	{2, 1},
+};
+
 static int pgsz, hpsz;
 static long orig_hugepages = -1;
 static char path_hugepages_node1[PATH_MAX];
@@ -61,9 +74,19 @@ static long orig_hugepages_node2 = -1;
 static unsigned int node1, node2;
 static void *addr;
 
-static void do_child(void)
+static int do_soft_offline(int tpgs)
+{
+	if (madvise(addr, tpgs * hpsz, MADV_SOFT_OFFLINE) == -1) {
+		if (errno != EINVAL)
+			tst_res(TFAIL | TTERRNO, "madvise failed");
+		return errno;
+	}
+	return 0;
+}
+
+static void do_child(int tpgs)
 {
-	int test_pages = TEST_PAGES * hpsz / pgsz;
+	int test_pages = tpgs * hpsz / pgsz;
 	int i, j;
 	int *nodes, *status;
 	void **pages;
@@ -96,34 +119,45 @@ static void do_child(void)
 	exit(0);
 }
 
-static void do_test(void)
+static void do_test(unsigned int n)
 {
 	int i;
 	pid_t cpid = -1;
 	int status;
 	unsigned int twenty_percent = (tst_timeout_remaining() / 5);
 
-	addr = SAFE_MMAP(NULL, TEST_PAGES * hpsz, PROT_READ | PROT_WRITE,
+	addr = SAFE_MMAP(NULL, tcases[n].tpages * hpsz, PROT_READ | PROT_WRITE,
 		MAP_PRIVATE | MAP_ANONYMOUS | MAP_HUGETLB, -1, 0);
 
-	SAFE_MUNMAP(addr, TEST_PAGES * hpsz);
+	SAFE_MUNMAP(addr, tcases[n].tpages * hpsz);
 
 	cpid = SAFE_FORK();
 	if (cpid == 0)
-		do_child();
+		do_child(tcases[n].tpages);
 
 	for (i = 0; i < LOOPS; i++) {
 		void *ptr;
 
-		ptr = SAFE_MMAP(NULL, TEST_PAGES * hpsz,
+		ptr = SAFE_MMAP(NULL, tcases[n].tpages * hpsz,
 			PROT_READ | PROT_WRITE,
 			MAP_PRIVATE | MAP_ANONYMOUS | MAP_HUGETLB, -1, 0);
 		if (ptr != addr)
 			tst_brk(TBROK, "Failed to mmap@desired addr");
 
-		memset(addr, 0, TEST_PAGES * hpsz);
+		memset(addr, 0, tcases[n].tpages * hpsz);
+
+		if (tcases[n].offline) {
+			if (do_soft_offline(tcases[n].tpages) == EINVAL) {
+				SAFE_KILL(cpid, SIGKILL);
+				SAFE_WAITPID(cpid, &status, 0);
+				SAFE_MUNMAP(addr, tcases[n].tpages * hpsz);
+				tst_res(TCONF,
+					"madvise() didn't support MADV_SOFT_OFFLINE");
+				return;
+			}
+		}
 
-		SAFE_MUNMAP(addr, TEST_PAGES * hpsz);
+		SAFE_MUNMAP(addr, tcases[n].tpages * hpsz);
 
 		if (tst_timeout_remaining() < twenty_percent)
 			break;
@@ -266,7 +300,8 @@ static struct tst_test test = {
 	.forks_child = 1,
 	.setup = setup,
 	.cleanup = cleanup,
-	.test_all = do_test,
+	.test = do_test,
+	.tcnt = ARRAY_SIZE(tcases),
 };
 
 #else
-- 
2.18.1




^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [LTP] [PATCH v4] syscalls/move_pages12: Add new regression test
  2019-05-22  1:43           ` [LTP] [PATCH v4] syscalls/move_pages12: " Yang Xu
@ 2019-05-22 11:45             ` Cyril Hrubis
  0 siblings, 0 replies; 11+ messages in thread
From: Cyril Hrubis @ 2019-05-22 11:45 UTC (permalink / raw)
  To: ltp

Hi!
Pushed with a minor change, thanks.

> - * Copyright (c) 2016 Fujitsu Ltd.
> + *  Copyright (c) 2019 FUJITSU LIMITED. All rights reserved.

Since the original copyright was 2016 I changed new line to range i.e.
Copyright (c) 2016-2019 FUJITSU ...

-- 
Cyril Hrubis
chrubis@suse.cz

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2019-05-22 11:45 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-12-01  3:48 [LTP] [PATCH 1/2] syscalls/move_pages: move the common code to move_hugepages_support Xiao Yang
2017-12-01  3:48 ` [LTP] [PATCH 2/2] syscalls/move_pages13.c: Add new regression test Xiao Yang
2019-04-16  8:48   ` [LTP] [PATCH v2] syscalls/move_page12.c: " Yang Xu
2019-05-20 14:41     ` Cyril Hrubis
2019-05-21  9:48       ` xuyang
2019-05-21  9:51       ` [LTP] [PATCH v3] syscalls/move_page12: " Yang Xu
2019-05-21 13:47         ` Cyril Hrubis
2019-05-22  1:43           ` [LTP] [PATCH v4] syscalls/move_pages12: " Yang Xu
2019-05-22 11:45             ` Cyril Hrubis
2018-02-08  1:21 ` [LTP] [PATCH 1/2] syscalls/move_pages: move the common code to move_hugepages_support Xiao Yang
2019-05-20 13:55 ` Cyril Hrubis

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.