[LTP] [PATCH] syscalls/move_pages12: Add new regression test

[LTP] [PATCH] syscalls/move_pages12: Add new regression test

[Guangwen Feng]

Fixed by:
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()

Signed-off-by: Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
---
 runtest/numa                                       |   1 +
 runtest/syscalls                                   |   1 +
 testcases/kernel/syscalls/.gitignore               |   1 +
 .../kernel/syscalls/move_pages/move_pages12.c      | 171 +++++++++++++++++++++
 4 files changed, 174 insertions(+)
 create mode 100644 testcases/kernel/syscalls/move_pages/move_pages12.c

diff --git a/runtest/numa b/runtest/numa
index 87f64da..5dc6f48 100644
--- a/runtest/numa
+++ b/runtest/numa
@@ -10,3 +10,4 @@ move_pages08 move_pages.sh 08
 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_pages.sh 12
diff --git a/runtest/syscalls b/runtest/syscalls
index 884ab80..9711cba 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -645,6 +645,7 @@ move_pages08 move_pages.sh 08
 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_pages.sh 12
 
 mprotect01 mprotect01
 mprotect02 mprotect02
diff --git a/testcases/kernel/syscalls/.gitignore b/testcases/kernel/syscalls/.gitignore
index 3201fa9..21ed466 100644
--- a/testcases/kernel/syscalls/.gitignore
+++ b/testcases/kernel/syscalls/.gitignore
@@ -589,6 +589,7 @@
 /move_pages/move_pages09
 /move_pages/move_pages10
 /move_pages/move_pages11
+/move_pages/move_pages12
 /mprotect/mprotect01
 /mprotect/mprotect02
 /mprotect/mprotect03
diff --git a/testcases/kernel/syscalls/move_pages/move_pages12.c b/testcases/kernel/syscalls/move_pages/move_pages12.c
new file mode 100644
index 0000000..593d4ea
--- /dev/null
+++ b/testcases/kernel/syscalls/move_pages/move_pages12.c
@@ -0,0 +1,171 @@
+/*
+ * 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/>.
+ */
+
+/*
+ * 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.
+ *
+ * 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()
+ */
+
+#include <errno.h>
+#include <stdio.h>
+#include <string.h>
+
+#include "tst_test.h"
+#include "tst_safe_stdio.h"
+#include "move_pages_support.h"
+
+#define LOOPS	10000
+#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
+#define TEST_ADDR	0x700000000000UL
+
+static int pgsz, hpsz;
+static long orig_hugepages;
+
+static int read_hugepagesize(void)
+{
+	FILE *fp;
+	char line[BUFSIZ], buf[BUFSIZ];
+	int val;
+
+	fp = SAFE_FOPEN(PATH_MEMINFO, "r");
+	while (fgets(line, BUFSIZ, fp) != NULL) {
+		if (sscanf(line, "%64s %d", buf, &val) == 2) {
+			if (strcmp(buf, "Hugepagesize:") == 0) {
+				SAFE_FCLOSE(fp);
+				return 1024 * val;
+			}
+		}
+	}
+
+	SAFE_FCLOSE(fp);
+	tst_brk(TBROK, "can't find \"%s\" in %s",
+		"Hugepagesize:", PATH_MEMINFO);
+}
+
+static void do_child(void)
+{
+	char *p;
+
+	while (1) {
+		p = SAFE_MMAP((void *)TEST_ADDR, TEST_PAGES * hpsz,
+			PROT_READ | PROT_WRITE,
+			MAP_PRIVATE | MAP_ANONYMOUS | MAP_HUGETLB, -1, 0);
+		if (p != (void *)TEST_ADDR)
+			tst_brk(TBROK, "Failed to mmap at desired addr");
+
+		memset(p, 0, TEST_PAGES * hpsz);
+
+		SAFE_MUNMAP(p, TEST_PAGES * hpsz);
+	}
+}
+
+static void verify_move_pages(void)
+{
+#if HAVE_NUMA_MOVE_PAGES
+	unsigned int node1, node2;
+	int i, j, ret;
+	int test_pages = TEST_PAGES * hpsz / pgsz;
+	int *nodes, *status;
+	void **pages;
+	pid_t cpid;
+
+	ret = get_allowed_nodes(NH_MEMS, TEST_NODES, &node1, &node2);
+	if (ret < 0)
+		tst_brk(TBROK | TERRNO, "get_allowed_nodes: %d", ret);
+
+	pages = SAFE_MALLOC(sizeof(char *) * test_pages + 1);
+	nodes = SAFE_MALLOC(sizeof(char *) * test_pages + 1);
+	status = SAFE_MALLOC(sizeof(char *) * test_pages + 1);
+
+	for (i = 0; i < test_pages; i++)
+		pages[i] = (void *)TEST_ADDR + i * pgsz;
+
+	cpid = SAFE_FORK();
+	if (cpid == 0)
+		do_child();
+
+	for (i = 0; i < LOOPS; 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(cpid, test_pages, pages, nodes, status,
+			MPOL_MF_MOVE_ALL));
+		if (TEST_RETURN) {
+			tst_res(TFAIL | TTERRNO, "move_pages failed");
+			break;
+		}
+	}
+
+	SAFE_KILL(cpid, SIGKILL);
+	SAFE_WAITPID(cpid, NULL, 0);
+
+	if (i == LOOPS)
+		tst_res(TPASS, "Bug not reproduced");
+#else
+	tst_res(TCONF, "move_pages support not found.");
+#endif
+}
+
+static void setup(void)
+{
+	check_config(TEST_NODES);
+
+	if (access(PATH_HUGEPAGES, F_OK))
+		tst_brk(TCONF, "Huge page not supported.");
+
+	pgsz = (int)get_page_size();
+	hpsz = read_hugepagesize();
+
+	SAFE_FILE_SCANF(PATH_NR_HUGEPAGES, "%ld", &orig_hugepages);
+	SAFE_FILE_PRINTF(PATH_NR_HUGEPAGES, "%d", 40);
+}
+
+static void cleanup(void)
+{
+	SAFE_FILE_PRINTF(PATH_NR_HUGEPAGES, "%ld", orig_hugepages);
+}
+
+static struct tst_test test = {
+	.tid = "move_pages12",
+	.min_kver = "2.6.32",
+	.needs_root = 1,
+	.forks_child = 1,
+	.setup = setup,
+	.cleanup = cleanup,
+	.test_all = verify_move_pages,
+};
-- 
1.8.4.2

[LTP] [PATCH] syscalls/move_pages12: Add new regression test

[Cyril Hrubis]

Hi!
> diff --git a/runtest/numa b/runtest/numa
> index 87f64da..5dc6f48 100644
> --- a/runtest/numa
> +++ b/runtest/numa
> @@ -10,3 +10,4 @@ move_pages08 move_pages.sh 08
>  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_pages.sh 12
> diff --git a/runtest/syscalls b/runtest/syscalls
> index 884ab80..9711cba 100644
> --- a/runtest/syscalls
> +++ b/runtest/syscalls
> @@ -645,6 +645,7 @@ move_pages08 move_pages.sh 08
>  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_pages.sh 12

I do not like this wrapper. Can we rather get rid of it?

The move_pages* testcases seems to have ifdefs and are compiled as stubs
that produce TCONF when HAVE_NUMA_MOVE_PAGES was not defined at compile
time. Even the one you wrote. So there is no need for this kind of
wrapper at all.

At least don't use the script with new tests.

>  mprotect01 mprotect01
>  mprotect02 mprotect02
> diff --git a/testcases/kernel/syscalls/.gitignore b/testcases/kernel/syscalls/.gitignore
> index 3201fa9..21ed466 100644
> --- a/testcases/kernel/syscalls/.gitignore
> +++ b/testcases/kernel/syscalls/.gitignore
> @@ -589,6 +589,7 @@
>  /move_pages/move_pages09
>  /move_pages/move_pages10
>  /move_pages/move_pages11
> +/move_pages/move_pages12
>  /mprotect/mprotect01
>  /mprotect/mprotect02
>  /mprotect/mprotect03
> diff --git a/testcases/kernel/syscalls/move_pages/move_pages12.c b/testcases/kernel/syscalls/move_pages/move_pages12.c
> new file mode 100644
> index 0000000..593d4ea
> --- /dev/null
> +++ b/testcases/kernel/syscalls/move_pages/move_pages12.c
> @@ -0,0 +1,171 @@
> +/*
> + * 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/>.
> + */
> +
> +/*
> + * 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.
> + *
> + * 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()
> + */
> +
> +#include <errno.h>
> +#include <stdio.h>
> +#include <string.h>
> +
> +#include "tst_test.h"
> +#include "tst_safe_stdio.h"
> +#include "move_pages_support.h"
> +
> +#define LOOPS	10000
> +#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
> +#define TEST_ADDR	0x700000000000UL
                           ^
			Why do we need this specific address?

> +static int pgsz, hpsz;
> +static long orig_hugepages;
> +
> +static int read_hugepagesize(void)
> +{
> +	FILE *fp;
> +	char line[BUFSIZ], buf[BUFSIZ];
> +	int val;
> +
> +	fp = SAFE_FOPEN(PATH_MEMINFO, "r");
> +	while (fgets(line, BUFSIZ, fp) != NULL) {
> +		if (sscanf(line, "%64s %d", buf, &val) == 2) {
> +			if (strcmp(buf, "Hugepagesize:") == 0) {
> +				SAFE_FCLOSE(fp);
> +				return 1024 * val;
> +			}
> +		}
> +	}
> +
> +	SAFE_FCLOSE(fp);
> +	tst_brk(TBROK, "can't find \"%s\" in %s",
> +		"Hugepagesize:", PATH_MEMINFO);
> +}

We have SAFE_FILE_LINES_SCANF() exactly for this purpose.

This function could be replaced with:

SAFE_FILE_LINES_SCANF(PATH_MEMINFO, "Hugepagesize: %d", &val);

> +static void do_child(void)
> +{
> +	char *p;
> +
> +	while (1) {
> +		p = SAFE_MMAP((void *)TEST_ADDR, TEST_PAGES * hpsz,
> +			PROT_READ | PROT_WRITE,
> +			MAP_PRIVATE | MAP_ANONYMOUS | MAP_HUGETLB, -1, 0);
> +		if (p != (void *)TEST_ADDR)
> +			tst_brk(TBROK, "Failed to mmap at desired addr");
> +
> +		memset(p, 0, TEST_PAGES * hpsz);
> +
> +		SAFE_MUNMAP(p, TEST_PAGES * hpsz);
> +	}
> +}
> +
> +static void verify_move_pages(void)
> +{
> +#if HAVE_NUMA_MOVE_PAGES

Can we rather ifdef around the whole test code and use TST_TEST_TCONF()?

See for example the kernel/syscalls/flistxattr/flistxattr01.c testcase.

> +	unsigned int node1, node2;
> +	int i, j, ret;
> +	int test_pages = TEST_PAGES * hpsz / pgsz;
> +	int *nodes, *status;
> +	void **pages;
> +	pid_t cpid;
> +
> +	ret = get_allowed_nodes(NH_MEMS, TEST_NODES, &node1, &node2);
> +	if (ret < 0)
> +		tst_brk(TBROK | TERRNO, "get_allowed_nodes: %d", ret);
> +
> +	pages = SAFE_MALLOC(sizeof(char *) * test_pages + 1);
> +	nodes = SAFE_MALLOC(sizeof(char *) * test_pages + 1);
> +	status = SAFE_MALLOC(sizeof(char *) * test_pages + 1);
> +
> +	for (i = 0; i < test_pages; i++)
> +		pages[i] = (void *)TEST_ADDR + i * pgsz;
> +
> +	cpid = SAFE_FORK();
> +	if (cpid == 0)
> +		do_child();
> +
> +	for (i = 0; i < LOOPS; 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(cpid, test_pages, pages, nodes, status,
> +			MPOL_MF_MOVE_ALL));
> +		if (TEST_RETURN) {
> +			tst_res(TFAIL | TTERRNO, "move_pages failed");
> +			break;
> +		}
> +	}
> +
> +	SAFE_KILL(cpid, SIGKILL);
> +	SAFE_WAITPID(cpid, NULL, 0);
> +
> +	if (i == LOOPS)
> +		tst_res(TPASS, "Bug not reproduced");
> +#else
> +	tst_res(TCONF, "move_pages support not found.");
> +#endif
> +}
> +
> +static void setup(void)
> +{
> +	check_config(TEST_NODES);
> +
> +	if (access(PATH_HUGEPAGES, F_OK))
> +		tst_brk(TCONF, "Huge page not supported.");
> +
> +	pgsz = (int)get_page_size();
> +	hpsz = read_hugepagesize();
> +
> +	SAFE_FILE_SCANF(PATH_NR_HUGEPAGES, "%ld", &orig_hugepages);
> +	SAFE_FILE_PRINTF(PATH_NR_HUGEPAGES, "%d", 40);

Huh, this is higly suspicious. What do you want to have here? Does the
test need 40 hugepages?

Have you considered that hugepages are enabled on the system already and
that there are some hugepages allocated as well? Shouldn't we rather try
to increase the limit to the number of already used hugepages + 40?

> +}
> +
> +static void cleanup(void)
> +{
> +	SAFE_FILE_PRINTF(PATH_NR_HUGEPAGES, "%ld", orig_hugepages);
> +}
> +
> +static struct tst_test test = {
> +	.tid = "move_pages12",
> +	.min_kver = "2.6.32",
> +	.needs_root = 1,
> +	.forks_child = 1,
> +	.setup = setup,
> +	.cleanup = cleanup,
> +	.test_all = verify_move_pages,
> +};
> -- 
> 1.8.4.2
> 
> 
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH] syscalls/move_pages12: Add new regression test

[Guangwen Feng]

Hi Cyril,

Thanks for your review.

On 01/24/2017 12:59 AM, Cyril Hrubis wrote:
> Hi!
>> diff --git a/runtest/numa b/runtest/numa
>> index 87f64da..5dc6f48 100644
>> --- a/runtest/numa
>> +++ b/runtest/numa
>> @@ -10,3 +10,4 @@ move_pages08 move_pages.sh 08
>>  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_pages.sh 12
>> diff --git a/runtest/syscalls b/runtest/syscalls
>> index 884ab80..9711cba 100644
>> --- a/runtest/syscalls
>> +++ b/runtest/syscalls
>> @@ -645,6 +645,7 @@ move_pages08 move_pages.sh 08
>>  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_pages.sh 12
> 
> I do not like this wrapper. Can we rather get rid of it?
> 
> The move_pages* testcases seems to have ifdefs and are compiled as stubs
> that produce TCONF when HAVE_NUMA_MOVE_PAGES was not defined at compile
> time. Even the one you wrote. So there is no need for this kind of
> wrapper at all.
> 
> At least don't use the script with new tests.

OK, I got it.

> 
>>  mprotect01 mprotect01
>>  mprotect02 mprotect02
>> diff --git a/testcases/kernel/syscalls/.gitignore b/testcases/kernel/syscalls/.gitignore
>> index 3201fa9..21ed466 100644
>> --- a/testcases/kernel/syscalls/.gitignore
>> +++ b/testcases/kernel/syscalls/.gitignore
>> @@ -589,6 +589,7 @@
>>  /move_pages/move_pages09
>>  /move_pages/move_pages10
>>  /move_pages/move_pages11
>> +/move_pages/move_pages12
>>  /mprotect/mprotect01
>>  /mprotect/mprotect02
>>  /mprotect/mprotect03
>> diff --git a/testcases/kernel/syscalls/move_pages/move_pages12.c b/testcases/kernel/syscalls/move_pages/move_pages12.c
>> new file mode 100644
>> index 0000000..593d4ea
>> --- /dev/null
>> +++ b/testcases/kernel/syscalls/move_pages/move_pages12.c
>> @@ -0,0 +1,171 @@
>> +/*
>> + * 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/>.
>> + */
>> +
>> +/*
>> + * 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.
>> + *
>> + * 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()
>> + */
>> +
>> +#include <errno.h>
>> +#include <stdio.h>
>> +#include <string.h>
>> +
>> +#include "tst_test.h"
>> +#include "tst_safe_stdio.h"
>> +#include "move_pages_support.h"
>> +
>> +#define LOOPS	10000
>> +#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
>> +#define TEST_ADDR	0x700000000000UL
>                            ^
> 			Why do we need this specific address?

It is not necessary indeed, we can get page addr via mmap(NULL...)
and pass it to move_pages(), I will rewrite this, thanks.

> 
>> +static int pgsz, hpsz;
>> +static long orig_hugepages;
>> +
>> +static int read_hugepagesize(void)
>> +{
>> +	FILE *fp;
>> +	char line[BUFSIZ], buf[BUFSIZ];
>> +	int val;
>> +
>> +	fp = SAFE_FOPEN(PATH_MEMINFO, "r");
>> +	while (fgets(line, BUFSIZ, fp) != NULL) {
>> +		if (sscanf(line, "%64s %d", buf, &val) == 2) {
>> +			if (strcmp(buf, "Hugepagesize:") == 0) {
>> +				SAFE_FCLOSE(fp);
>> +				return 1024 * val;
>> +			}
>> +		}
>> +	}
>> +
>> +	SAFE_FCLOSE(fp);
>> +	tst_brk(TBROK, "can't find \"%s\" in %s",
>> +		"Hugepagesize:", PATH_MEMINFO);
>> +}
> 
> We have SAFE_FILE_LINES_SCANF() exactly for this purpose.
> 
> This function could be replaced with:
> 
> SAFE_FILE_LINES_SCANF(PATH_MEMINFO, "Hugepagesize: %d", &val);

OK, I see, thanks.

> 
>> +static void do_child(void)
>> +{
>> +	char *p;
>> +
>> +	while (1) {
>> +		p = SAFE_MMAP((void *)TEST_ADDR, TEST_PAGES * hpsz,
>> +			PROT_READ | PROT_WRITE,
>> +			MAP_PRIVATE | MAP_ANONYMOUS | MAP_HUGETLB, -1, 0);
>> +		if (p != (void *)TEST_ADDR)
>> +			tst_brk(TBROK, "Failed to mmap at desired addr");
>> +
>> +		memset(p, 0, TEST_PAGES * hpsz);
>> +
>> +		SAFE_MUNMAP(p, TEST_PAGES * hpsz);
>> +	}
>> +}
>> +
>> +static void verify_move_pages(void)
>> +{
>> +#if HAVE_NUMA_MOVE_PAGES
> 
> Can we rather ifdef around the whole test code and use TST_TEST_TCONF()?
> 
> See for example the kernel/syscalls/flistxattr/flistxattr01.c testcase.

OK, thanks.

> 
>> +	unsigned int node1, node2;
>> +	int i, j, ret;
>> +	int test_pages = TEST_PAGES * hpsz / pgsz;
>> +	int *nodes, *status;
>> +	void **pages;
>> +	pid_t cpid;
>> +
>> +	ret = get_allowed_nodes(NH_MEMS, TEST_NODES, &node1, &node2);
>> +	if (ret < 0)
>> +		tst_brk(TBROK | TERRNO, "get_allowed_nodes: %d", ret);
>> +
>> +	pages = SAFE_MALLOC(sizeof(char *) * test_pages + 1);
>> +	nodes = SAFE_MALLOC(sizeof(char *) * test_pages + 1);
>> +	status = SAFE_MALLOC(sizeof(char *) * test_pages + 1);
>> +
>> +	for (i = 0; i < test_pages; i++)
>> +		pages[i] = (void *)TEST_ADDR + i * pgsz;
>> +
>> +	cpid = SAFE_FORK();
>> +	if (cpid == 0)
>> +		do_child();
>> +
>> +	for (i = 0; i < LOOPS; 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(cpid, test_pages, pages, nodes, status,
>> +			MPOL_MF_MOVE_ALL));
>> +		if (TEST_RETURN) {
>> +			tst_res(TFAIL | TTERRNO, "move_pages failed");
>> +			break;
>> +		}
>> +	}
>> +
>> +	SAFE_KILL(cpid, SIGKILL);
>> +	SAFE_WAITPID(cpid, NULL, 0);
>> +
>> +	if (i == LOOPS)
>> +		tst_res(TPASS, "Bug not reproduced");
>> +#else
>> +	tst_res(TCONF, "move_pages support not found.");
>> +#endif
>> +}
>> +
>> +static void setup(void)
>> +{
>> +	check_config(TEST_NODES);
>> +
>> +	if (access(PATH_HUGEPAGES, F_OK))
>> +		tst_brk(TCONF, "Huge page not supported.");
>> +
>> +	pgsz = (int)get_page_size();
>> +	hpsz = read_hugepagesize();
>> +
>> +	SAFE_FILE_SCANF(PATH_NR_HUGEPAGES, "%ld", &orig_hugepages);
>> +	SAFE_FILE_PRINTF(PATH_NR_HUGEPAGES, "%d", 40);
> 
> Huh, this is higly suspicious. What do you want to have here? Does the
> test need 40 hugepages?
> 
> Have you considered that hugepages are enabled on the system already and
> that there are some hugepages allocated as well? Shouldn't we rather try
> to increase the limit to the number of already used hugepages + 40?

Yea, actually we don't need that much, I will rewrite this, thanks.

Best Regards,
Guangwen Feng

> 
>> +}
>> +
>> +static void cleanup(void)
>> +{
>> +	SAFE_FILE_PRINTF(PATH_NR_HUGEPAGES, "%ld", orig_hugepages);
>> +}
>> +
>> +static struct tst_test test = {
>> +	.tid = "move_pages12",
>> +	.min_kver = "2.6.32",
>> +	.needs_root = 1,
>> +	.forks_child = 1,
>> +	.setup = setup,
>> +	.cleanup = cleanup,
>> +	.test_all = verify_move_pages,
>> +};
>> -- 
>> 1.8.4.2
>>
>>
>>
>>
>> -- 
>> Mailing list info: https://lists.linux.it/listinfo/ltp
> 

[LTP] [PATCH v2] syscalls/move_pages12: Add new regression test

[Guangwen Feng]

Fixed by:
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()

Signed-off-by: Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
---
 runtest/numa                                       |   1 +
 runtest/syscalls                                   |   1 +
 testcases/kernel/syscalls/.gitignore               |   1 +
 .../kernel/syscalls/move_pages/move_pages12.c      | 165 +++++++++++++++++++++
 4 files changed, 168 insertions(+)
 create mode 100644 testcases/kernel/syscalls/move_pages/move_pages12.c

diff --git a/runtest/numa b/runtest/numa
index 87f64da..dcf4948 100644
--- a/runtest/numa
+++ b/runtest/numa
@@ -10,3 +10,4 @@ move_pages08 move_pages.sh 08
 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
diff --git a/runtest/syscalls b/runtest/syscalls
index dc03c4c..48f6492 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -645,6 +645,7 @@ move_pages08 move_pages.sh 08
 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
 
 mprotect01 mprotect01
 mprotect02 mprotect02
diff --git a/testcases/kernel/syscalls/.gitignore b/testcases/kernel/syscalls/.gitignore
index 91dccef..bebaa89 100644
--- a/testcases/kernel/syscalls/.gitignore
+++ b/testcases/kernel/syscalls/.gitignore
@@ -589,6 +589,7 @@
 /move_pages/move_pages09
 /move_pages/move_pages10
 /move_pages/move_pages11
+/move_pages/move_pages12
 /mprotect/mprotect01
 /mprotect/mprotect02
 /mprotect/mprotect03
diff --git a/testcases/kernel/syscalls/move_pages/move_pages12.c b/testcases/kernel/syscalls/move_pages/move_pages12.c
new file mode 100644
index 0000000..f0c5a9f
--- /dev/null
+++ b/testcases/kernel/syscalls/move_pages/move_pages12.c
@@ -0,0 +1,165 @@
+/*
+ * 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/>.
+ */
+
+/*
+ * 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.
+ *
+ * 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()
+ */
+
+#include <errno.h>
+#include <unistd.h>
+#include <string.h>
+#include <sys/types.h>
+
+#include "tst_test.h"
+#include "move_pages_support.h"
+
+#if HAVE_NUMA_MOVE_PAGES
+
+#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
+
+static int pgsz, hpsz;
+static long orig_hugepages;
+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 + 1);
+	nodes = SAFE_MALLOC(sizeof(char *) * test_pages + 1);
+	status = SAFE_MALLOC(sizeof(char *) * test_pages + 1);
+
+	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;
+	pid_t cpid = -1;
+	int status;
+
+	for (i = 0; i < LOOPS; i++) {
+		addr = SAFE_MMAP(NULL, TEST_PAGES * hpsz,
+			PROT_READ | PROT_WRITE,
+			MAP_PRIVATE | MAP_ANONYMOUS | MAP_HUGETLB, -1, 0);
+
+		memset(addr, 0, TEST_PAGES * hpsz);
+
+		SAFE_MUNMAP(addr, TEST_PAGES * hpsz);
+
+		if (i == 0) {
+			cpid = SAFE_FORK();
+			if (cpid == 0)
+				do_child();
+		}
+	}
+
+	if (i == LOOPS) {
+		SAFE_KILL(cpid, SIGKILL);
+		SAFE_WAITPID(cpid, &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");
+
+	pgsz = (int)get_page_size();
+	SAFE_FILE_LINES_SCANF(PATH_MEMINFO, "Hugepagesize: %d", &hpsz);
+	hpsz *= 1024;
+
+	SAFE_FILE_LINES_SCANF(PATH_MEMINFO, "MemFree: %d", &memfree);
+	memfree *= 1024;
+	if (4 * hpsz > memfree)
+		tst_brk(TBROK, "RAM not enough");
+
+	SAFE_FILE_SCANF(PATH_NR_HUGEPAGES, "%ld", &orig_hugepages);
+	SAFE_FILE_PRINTF(PATH_NR_HUGEPAGES, "%ld", orig_hugepages + 4);
+
+	ret = get_allowed_nodes(NH_MEMS, TEST_NODES, &node1, &node2);
+	if (ret < 0)
+		tst_brk(TBROK | TERRNO, "get_allowed_nodes: %d", ret);
+}
+
+static void cleanup(void)
+{
+	SAFE_FILE_PRINTF(PATH_NR_HUGEPAGES, "%ld", orig_hugepages);
+}
+
+static struct tst_test test = {
+	.tid = "move_pages12",
+	.min_kver = "2.6.32",
+	.needs_root = 1,
+	.forks_child = 1,
+	.setup = setup,
+	.cleanup = cleanup,
+	.test_all = do_test,
+};
+
+#else
+	tst_res(TCONF, "move_pages support not found");
+#endif
-- 
1.8.4.2

[LTP] [PATCH v2] syscalls/move_pages12: Add new regression test

[Guangwen Feng]

Hi!

Sorry, I find there is something wrong with the V2,
please ignore it, I will send a V3.

Best Regards,
Guangwen Feng

On 02/08/2017 05:19 PM, Guangwen Feng wrote:
> Fixed by:
> 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()
> 
> Signed-off-by: Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
> ---
>  runtest/numa                                       |   1 +
>  runtest/syscalls                                   |   1 +
>  testcases/kernel/syscalls/.gitignore               |   1 +
>  .../kernel/syscalls/move_pages/move_pages12.c      | 165 +++++++++++++++++++++
>  4 files changed, 168 insertions(+)
>  create mode 100644 testcases/kernel/syscalls/move_pages/move_pages12.c
> 
> diff --git a/runtest/numa b/runtest/numa
> index 87f64da..dcf4948 100644
> --- a/runtest/numa
> +++ b/runtest/numa
> @@ -10,3 +10,4 @@ move_pages08 move_pages.sh 08
>  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
> diff --git a/runtest/syscalls b/runtest/syscalls
> index dc03c4c..48f6492 100644
> --- a/runtest/syscalls
> +++ b/runtest/syscalls
> @@ -645,6 +645,7 @@ move_pages08 move_pages.sh 08
>  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
>  
>  mprotect01 mprotect01
>  mprotect02 mprotect02
> diff --git a/testcases/kernel/syscalls/.gitignore b/testcases/kernel/syscalls/.gitignore
> index 91dccef..bebaa89 100644
> --- a/testcases/kernel/syscalls/.gitignore
> +++ b/testcases/kernel/syscalls/.gitignore
> @@ -589,6 +589,7 @@
>  /move_pages/move_pages09
>  /move_pages/move_pages10
>  /move_pages/move_pages11
> +/move_pages/move_pages12
>  /mprotect/mprotect01
>  /mprotect/mprotect02
>  /mprotect/mprotect03
> diff --git a/testcases/kernel/syscalls/move_pages/move_pages12.c b/testcases/kernel/syscalls/move_pages/move_pages12.c
> new file mode 100644
> index 0000000..f0c5a9f
> --- /dev/null
> +++ b/testcases/kernel/syscalls/move_pages/move_pages12.c
> @@ -0,0 +1,165 @@
> +/*
> + * 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/>.
> + */
> +
> +/*
> + * 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.
> + *
> + * 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()
> + */
> +
> +#include <errno.h>
> +#include <unistd.h>
> +#include <string.h>
> +#include <sys/types.h>
> +
> +#include "tst_test.h"
> +#include "move_pages_support.h"
> +
> +#if HAVE_NUMA_MOVE_PAGES
> +
> +#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
> +
> +static int pgsz, hpsz;
> +static long orig_hugepages;
> +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 + 1);
> +	nodes = SAFE_MALLOC(sizeof(char *) * test_pages + 1);
> +	status = SAFE_MALLOC(sizeof(char *) * test_pages + 1);
> +
> +	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;
> +	pid_t cpid = -1;
> +	int status;
> +
> +	for (i = 0; i < LOOPS; i++) {
> +		addr = SAFE_MMAP(NULL, TEST_PAGES * hpsz,
> +			PROT_READ | PROT_WRITE,
> +			MAP_PRIVATE | MAP_ANONYMOUS | MAP_HUGETLB, -1, 0);
> +
> +		memset(addr, 0, TEST_PAGES * hpsz);
> +
> +		SAFE_MUNMAP(addr, TEST_PAGES * hpsz);
> +
> +		if (i == 0) {
> +			cpid = SAFE_FORK();
> +			if (cpid == 0)
> +				do_child();
> +		}
> +	}
> +
> +	if (i == LOOPS) {
> +		SAFE_KILL(cpid, SIGKILL);
> +		SAFE_WAITPID(cpid, &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");
> +
> +	pgsz = (int)get_page_size();
> +	SAFE_FILE_LINES_SCANF(PATH_MEMINFO, "Hugepagesize: %d", &hpsz);
> +	hpsz *= 1024;
> +
> +	SAFE_FILE_LINES_SCANF(PATH_MEMINFO, "MemFree: %d", &memfree);
> +	memfree *= 1024;
> +	if (4 * hpsz > memfree)
> +		tst_brk(TBROK, "RAM not enough");
> +
> +	SAFE_FILE_SCANF(PATH_NR_HUGEPAGES, "%ld", &orig_hugepages);
> +	SAFE_FILE_PRINTF(PATH_NR_HUGEPAGES, "%ld", orig_hugepages + 4);
> +
> +	ret = get_allowed_nodes(NH_MEMS, TEST_NODES, &node1, &node2);
> +	if (ret < 0)
> +		tst_brk(TBROK | TERRNO, "get_allowed_nodes: %d", ret);
> +}
> +
> +static void cleanup(void)
> +{
> +	SAFE_FILE_PRINTF(PATH_NR_HUGEPAGES, "%ld", orig_hugepages);
> +}
> +
> +static struct tst_test test = {
> +	.tid = "move_pages12",
> +	.min_kver = "2.6.32",
> +	.needs_root = 1,
> +	.forks_child = 1,
> +	.setup = setup,
> +	.cleanup = cleanup,
> +	.test_all = do_test,
> +};
> +
> +#else
> +	tst_res(TCONF, "move_pages support not found");
> +#endif
> 

[LTP] [PATCH v3] syscalls/move_pages12: Add new regression test

[Guangwen Feng]

Fixed by:
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()

Signed-off-by: Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
---
 runtest/numa                                       |   1 +
 runtest/syscalls                                   |   1 +
 testcases/kernel/syscalls/.gitignore               |   1 +
 .../kernel/syscalls/move_pages/move_pages12.c      | 165 +++++++++++++++++++++
 4 files changed, 168 insertions(+)
 create mode 100644 testcases/kernel/syscalls/move_pages/move_pages12.c

diff --git a/runtest/numa b/runtest/numa
index 87f64da..dcf4948 100644
--- a/runtest/numa
+++ b/runtest/numa
@@ -10,3 +10,4 @@ move_pages08 move_pages.sh 08
 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
diff --git a/runtest/syscalls b/runtest/syscalls
index dc03c4c..48f6492 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -645,6 +645,7 @@ move_pages08 move_pages.sh 08
 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
 
 mprotect01 mprotect01
 mprotect02 mprotect02
diff --git a/testcases/kernel/syscalls/.gitignore b/testcases/kernel/syscalls/.gitignore
index 91dccef..bebaa89 100644
--- a/testcases/kernel/syscalls/.gitignore
+++ b/testcases/kernel/syscalls/.gitignore
@@ -589,6 +589,7 @@
 /move_pages/move_pages09
 /move_pages/move_pages10
 /move_pages/move_pages11
+/move_pages/move_pages12
 /mprotect/mprotect01
 /mprotect/mprotect02
 /mprotect/mprotect03
diff --git a/testcases/kernel/syscalls/move_pages/move_pages12.c b/testcases/kernel/syscalls/move_pages/move_pages12.c
new file mode 100644
index 0000000..f0c5a9f
--- /dev/null
+++ b/testcases/kernel/syscalls/move_pages/move_pages12.c
@@ -0,0 +1,165 @@
+/*
+ * 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/>.
+ */
+
+/*
+ * 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.
+ *
+ * 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()
+ */
+
+#include <errno.h>
+#include <unistd.h>
+#include <string.h>
+#include <sys/types.h>
+
+#include "tst_test.h"
+#include "move_pages_support.h"
+
+#if HAVE_NUMA_MOVE_PAGES
+
+#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
+
+static int pgsz, hpsz;
+static long orig_hugepages;
+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 + 1);
+	nodes = SAFE_MALLOC(sizeof(char *) * test_pages + 1);
+	status = SAFE_MALLOC(sizeof(char *) * test_pages + 1);
+
+	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;
+	pid_t cpid = -1;
+	int status;
+
+	for (i = 0; i < LOOPS; i++) {
+		addr = SAFE_MMAP(NULL, TEST_PAGES * hpsz,
+			PROT_READ | PROT_WRITE,
+			MAP_PRIVATE | MAP_ANONYMOUS | MAP_HUGETLB, -1, 0);
+
+		memset(addr, 0, TEST_PAGES * hpsz);
+
+		SAFE_MUNMAP(addr, TEST_PAGES * hpsz);
+
+		if (i == 0) {
+			cpid = SAFE_FORK();
+			if (cpid == 0)
+				do_child();
+		}
+	}
+
+	if (i == LOOPS) {
+		SAFE_KILL(cpid, SIGKILL);
+		SAFE_WAITPID(cpid, &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");
+
+	pgsz = (int)get_page_size();
+	SAFE_FILE_LINES_SCANF(PATH_MEMINFO, "Hugepagesize: %d", &hpsz);
+	hpsz *= 1024;
+
+	SAFE_FILE_LINES_SCANF(PATH_MEMINFO, "MemFree: %d", &memfree);
+	memfree *= 1024;
+	if (4 * hpsz > memfree)
+		tst_brk(TBROK, "RAM not enough");
+
+	SAFE_FILE_SCANF(PATH_NR_HUGEPAGES, "%ld", &orig_hugepages);
+	SAFE_FILE_PRINTF(PATH_NR_HUGEPAGES, "%ld", orig_hugepages + 4);
+
+	ret = get_allowed_nodes(NH_MEMS, TEST_NODES, &node1, &node2);
+	if (ret < 0)
+		tst_brk(TBROK | TERRNO, "get_allowed_nodes: %d", ret);
+}
+
+static void cleanup(void)
+{
+	SAFE_FILE_PRINTF(PATH_NR_HUGEPAGES, "%ld", orig_hugepages);
+}
+
+static struct tst_test test = {
+	.tid = "move_pages12",
+	.min_kver = "2.6.32",
+	.needs_root = 1,
+	.forks_child = 1,
+	.setup = setup,
+	.cleanup = cleanup,
+	.test_all = do_test,
+};
+
+#else
+	tst_res(TCONF, "move_pages support not found");
+#endif
-- 
1.8.4.2

[LTP] [PATCH v3] syscalls/move_pages12: Add new regression test

[Cyril Hrubis]

Hi!
> +#include "tst_test.h"
> +#include "move_pages_support.h"
> +
> +#if HAVE_NUMA_MOVE_PAGES
> +
> +#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
> +
> +static int pgsz, hpsz;
> +static long orig_hugepages;
> +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 + 1);
> +	nodes = SAFE_MALLOC(sizeof(char *) * test_pages + 1);
> +	status = SAFE_MALLOC(sizeof(char *) * test_pages + 1);

I know that this is taken from to original reproduced, but these
allocations appears to be wrong. Both nodes and status are, as far as I
can tell, arrays of integers, so this should in fact be:

pages = SAFE_MALLOC(sizeof(char *) * test_pages);
nodes = SAFE_MALLOC(sizeof(int) * test_pages);
status = SAFE_MALLOC(sizeof(int) * test_pages);

I'm not even sure why there is + 1 in the original code, what is that
extra byte usefull for.

Does the reproducer still work once we allocate these arrays correctly?

> +	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;
> +	pid_t cpid = -1;
> +	int status;
> +
> +	for (i = 0; i < LOOPS; i++) {
> +		addr = SAFE_MMAP(NULL, TEST_PAGES * hpsz,
> +			PROT_READ | PROT_WRITE,
> +			MAP_PRIVATE | MAP_ANONYMOUS | MAP_HUGETLB, -1, 0);
> +
> +		memset(addr, 0, TEST_PAGES * hpsz);
> +
> +		SAFE_MUNMAP(addr, TEST_PAGES * hpsz);
> +
> +		if (i == 0) {
> +			cpid = SAFE_FORK();
> +			if (cpid == 0)
> +				do_child();
> +		}
> +	}
> +
> +	if (i == LOOPS) {
> +		SAFE_KILL(cpid, SIGKILL);
> +		SAFE_WAITPID(cpid, &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");
> +
> +	pgsz = (int)get_page_size();
> +	SAFE_FILE_LINES_SCANF(PATH_MEMINFO, "Hugepagesize: %d", &hpsz);
> +	hpsz *= 1024;
> +
> +	SAFE_FILE_LINES_SCANF(PATH_MEMINFO, "MemFree: %d", &memfree);
> +	memfree *= 1024;
> +	if (4 * hpsz > memfree)
> +		tst_brk(TBROK, "RAM not enough");
                                   ^
				This should rather be "Not enough free RAM"

Or something similar, but that is minor.

> +	SAFE_FILE_SCANF(PATH_NR_HUGEPAGES, "%ld", &orig_hugepages);
> +	SAFE_FILE_PRINTF(PATH_NR_HUGEPAGES, "%ld", orig_hugepages + 4);
> +
> +	ret = get_allowed_nodes(NH_MEMS, TEST_NODES, &node1, &node2);
> +	if (ret < 0)
> +		tst_brk(TBROK | TERRNO, "get_allowed_nodes: %d", ret);
> +}
> +
> +static void cleanup(void)
> +{
> +	SAFE_FILE_PRINTF(PATH_NR_HUGEPAGES, "%ld", orig_hugepages);
> +}
> +
> +static struct tst_test test = {
> +	.tid = "move_pages12",
> +	.min_kver = "2.6.32",
> +	.needs_root = 1,
> +	.forks_child = 1,
> +	.setup = setup,
> +	.cleanup = cleanup,
> +	.test_all = do_test,
> +};
> +
> +#else
> +	tst_res(TCONF, "move_pages support not found");
> +#endif

The rest looks good.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v3] syscalls/move_pages12: Add new regression test

[Guangwen Feng]

Hi!

On 02/09/2017 06:56 PM, Cyril Hrubis wrote:
> Hi!
>> +#include "tst_test.h"
>> +#include "move_pages_support.h"
>> +
>> +#if HAVE_NUMA_MOVE_PAGES
>> +
>> +#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
>> +
>> +static int pgsz, hpsz;
>> +static long orig_hugepages;
>> +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 + 1);
>> +	nodes = SAFE_MALLOC(sizeof(char *) * test_pages + 1);
>> +	status = SAFE_MALLOC(sizeof(char *) * test_pages + 1);
> 
> I know that this is taken from to original reproduced, but these
> allocations appears to be wrong. Both nodes and status are, as far as I
> can tell, arrays of integers, so this should in fact be:
> 
> pages = SAFE_MALLOC(sizeof(char *) * test_pages);
> nodes = SAFE_MALLOC(sizeof(int) * test_pages);
> status = SAFE_MALLOC(sizeof(int) * test_pages);
> 
> I'm not even sure why there is + 1 in the original code, what is that
> extra byte usefull for.
> 
> Does the reproducer still work once we allocate these arrays correctly?

Yes, it works well with the correct allocation,
and it looks like there is no need to add the "+ 1", thanks.

> 
>> +	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;
>> +	pid_t cpid = -1;
>> +	int status;
>> +
>> +	for (i = 0; i < LOOPS; i++) {
>> +		addr = SAFE_MMAP(NULL, TEST_PAGES * hpsz,
>> +			PROT_READ | PROT_WRITE,
>> +			MAP_PRIVATE | MAP_ANONYMOUS | MAP_HUGETLB, -1, 0);
>> +
>> +		memset(addr, 0, TEST_PAGES * hpsz);
>> +
>> +		SAFE_MUNMAP(addr, TEST_PAGES * hpsz);
>> +
>> +		if (i == 0) {
>> +			cpid = SAFE_FORK();
>> +			if (cpid == 0)
>> +				do_child();
>> +		}
>> +	}
>> +
>> +	if (i == LOOPS) {
>> +		SAFE_KILL(cpid, SIGKILL);
>> +		SAFE_WAITPID(cpid, &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");
>> +
>> +	pgsz = (int)get_page_size();
>> +	SAFE_FILE_LINES_SCANF(PATH_MEMINFO, "Hugepagesize: %d", &hpsz);
>> +	hpsz *= 1024;
>> +
>> +	SAFE_FILE_LINES_SCANF(PATH_MEMINFO, "MemFree: %d", &memfree);
>> +	memfree *= 1024;
>> +	if (4 * hpsz > memfree)
>> +		tst_brk(TBROK, "RAM not enough");
>                                    ^
> 				This should rather be "Not enough free RAM"
> 
> Or something similar, but that is minor.
> 

I will modify here as your description, thanks.

Best Regards,
Guangwen Feng 

>> +	SAFE_FILE_SCANF(PATH_NR_HUGEPAGES, "%ld", &orig_hugepages);
>> +	SAFE_FILE_PRINTF(PATH_NR_HUGEPAGES, "%ld", orig_hugepages + 4);
>> +
>> +	ret = get_allowed_nodes(NH_MEMS, TEST_NODES, &node1, &node2);
>> +	if (ret < 0)
>> +		tst_brk(TBROK | TERRNO, "get_allowed_nodes: %d", ret);
>> +}
>> +
>> +static void cleanup(void)
>> +{
>> +	SAFE_FILE_PRINTF(PATH_NR_HUGEPAGES, "%ld", orig_hugepages);
>> +}
>> +
>> +static struct tst_test test = {
>> +	.tid = "move_pages12",
>> +	.min_kver = "2.6.32",
>> +	.needs_root = 1,
>> +	.forks_child = 1,
>> +	.setup = setup,
>> +	.cleanup = cleanup,
>> +	.test_all = do_test,
>> +};
>> +
>> +#else
>> +	tst_res(TCONF, "move_pages support not found");
>> +#endif
> 
> The rest looks good.
> 

[LTP] [PATCH v4] syscalls/move_pages12: Add new regression test

[Guangwen Feng]

Fixed by:
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()

Signed-off-by: Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
---
 runtest/numa                                       |   1 +
 runtest/syscalls                                   |   1 +
 testcases/kernel/syscalls/.gitignore               |   1 +
 .../kernel/syscalls/move_pages/move_pages12.c      | 165 +++++++++++++++++++++
 4 files changed, 168 insertions(+)
 create mode 100644 testcases/kernel/syscalls/move_pages/move_pages12.c

diff --git a/runtest/numa b/runtest/numa
index 87f64da..dcf4948 100644
--- a/runtest/numa
+++ b/runtest/numa
@@ -10,3 +10,4 @@ move_pages08 move_pages.sh 08
 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
diff --git a/runtest/syscalls b/runtest/syscalls
index dc03c4c..48f6492 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -645,6 +645,7 @@ move_pages08 move_pages.sh 08
 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
 
 mprotect01 mprotect01
 mprotect02 mprotect02
diff --git a/testcases/kernel/syscalls/.gitignore b/testcases/kernel/syscalls/.gitignore
index 91dccef..bebaa89 100644
--- a/testcases/kernel/syscalls/.gitignore
+++ b/testcases/kernel/syscalls/.gitignore
@@ -589,6 +589,7 @@
 /move_pages/move_pages09
 /move_pages/move_pages10
 /move_pages/move_pages11
+/move_pages/move_pages12
 /mprotect/mprotect01
 /mprotect/mprotect02
 /mprotect/mprotect03
diff --git a/testcases/kernel/syscalls/move_pages/move_pages12.c b/testcases/kernel/syscalls/move_pages/move_pages12.c
new file mode 100644
index 0000000..ac5f7ad
--- /dev/null
+++ b/testcases/kernel/syscalls/move_pages/move_pages12.c
@@ -0,0 +1,165 @@
+/*
+ * 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/>.
+ */
+
+/*
+ * 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.
+ *
+ * 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()
+ */
+
+#include <errno.h>
+#include <unistd.h>
+#include <string.h>
+#include <sys/types.h>
+
+#include "tst_test.h"
+#include "move_pages_support.h"
+
+#if HAVE_NUMA_MOVE_PAGES
+
+#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
+
+static int pgsz, hpsz;
+static long orig_hugepages;
+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;
+	pid_t cpid = -1;
+	int status;
+
+	for (i = 0; i < LOOPS; i++) {
+		addr = SAFE_MMAP(NULL, TEST_PAGES * hpsz,
+			PROT_READ | PROT_WRITE,
+			MAP_PRIVATE | MAP_ANONYMOUS | MAP_HUGETLB, -1, 0);
+
+		memset(addr, 0, TEST_PAGES * hpsz);
+
+		SAFE_MUNMAP(addr, TEST_PAGES * hpsz);
+
+		if (i == 0) {
+			cpid = SAFE_FORK();
+			if (cpid == 0)
+				do_child();
+		}
+	}
+
+	if (i == LOOPS) {
+		SAFE_KILL(cpid, SIGKILL);
+		SAFE_WAITPID(cpid, &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");
+
+	pgsz = (int)get_page_size();
+	SAFE_FILE_LINES_SCANF(PATH_MEMINFO, "Hugepagesize: %d", &hpsz);
+	hpsz *= 1024;
+
+	SAFE_FILE_LINES_SCANF(PATH_MEMINFO, "MemFree: %d", &memfree);
+	memfree *= 1024;
+	if (4 * hpsz > memfree)
+		tst_brk(TBROK, "Not enough free RAM");
+
+	SAFE_FILE_SCANF(PATH_NR_HUGEPAGES, "%ld", &orig_hugepages);
+	SAFE_FILE_PRINTF(PATH_NR_HUGEPAGES, "%ld", orig_hugepages + 4);
+
+	ret = get_allowed_nodes(NH_MEMS, TEST_NODES, &node1, &node2);
+	if (ret < 0)
+		tst_brk(TBROK | TERRNO, "get_allowed_nodes: %d", ret);
+}
+
+static void cleanup(void)
+{
+	SAFE_FILE_PRINTF(PATH_NR_HUGEPAGES, "%ld", orig_hugepages);
+}
+
+static struct tst_test test = {
+	.tid = "move_pages12",
+	.min_kver = "2.6.32",
+	.needs_root = 1,
+	.forks_child = 1,
+	.setup = setup,
+	.cleanup = cleanup,
+	.test_all = do_test,
+};
+
+#else
+	tst_res(TCONF, "move_pages support not found");
+#endif
-- 
1.8.4.2

[LTP] [PATCH v4] syscalls/move_pages12: Add new regression test

[Cyril Hrubis]

Hi!
> +static void do_test(void)
> +{
> +	int i;
> +	pid_t cpid = -1;
> +	int status;
> +
> +	for (i = 0; i < LOOPS; i++) {
> +		addr = SAFE_MMAP(NULL, TEST_PAGES * hpsz,
> +			PROT_READ | PROT_WRITE,
> +			MAP_PRIVATE | MAP_ANONYMOUS | MAP_HUGETLB, -1, 0);
> +
> +		memset(addr, 0, TEST_PAGES * hpsz);
> +
> +		SAFE_MUNMAP(addr, TEST_PAGES * hpsz);
> +
> +		if (i == 0) {
> +			cpid = SAFE_FORK();
> +			if (cpid == 0)
> +				do_child();
> +		}
> +	}

Looking at this piece of code there was a reason the test was using
fixed address. Since once the child forks the addr in parent returned by
mmap() may change and the child would not be working with the same
memory at all. And while this would be probably working fine most of the
time as the kernel will likely return the same address over and over
we may as well fix this by doing one mmap() munmap() cycle to get an
address to pass to the mmap() in this loop.

i.e.
	...

	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);

	cpid = SAFE_FORK();
	if (cpid == 0)
		do_child();

	for (i = 0; i < LOOPS; i++) {
		void *ptr;

		ptr = SAFE_MMAP(addr, TEST_PAGES * hpsz,
		                PROT_READ | PROT_WRITE,
			        MAP_PRIVATE | MAP_ANONYMOUS | MAP_HUGETLB, -1, 0);

		if (ptr != addr)
			tst_brk(...);

		memset(addr);

		SAFE_UNMAP(addr, TEST_PAGES * hpsz)
	}

This would ensure that both parent and child are working on the same
address.


> +	if (i == LOOPS) {
> +		SAFE_KILL(cpid, SIGKILL);
> +		SAFE_WAITPID(cpid, &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");
> +
> +	pgsz = (int)get_page_size();
> +	SAFE_FILE_LINES_SCANF(PATH_MEMINFO, "Hugepagesize: %d", &hpsz);
> +	hpsz *= 1024;
> +
> +	SAFE_FILE_LINES_SCANF(PATH_MEMINFO, "MemFree: %d", &memfree);
> +	memfree *= 1024;
> +	if (4 * hpsz > memfree)
> +		tst_brk(TBROK, "Not enough free RAM");
> +
> +	SAFE_FILE_SCANF(PATH_NR_HUGEPAGES, "%ld", &orig_hugepages);
> +	SAFE_FILE_PRINTF(PATH_NR_HUGEPAGES, "%ld", orig_hugepages + 4);
> +
> +	ret = get_allowed_nodes(NH_MEMS, TEST_NODES, &node1, &node2);
> +	if (ret < 0)
> +		tst_brk(TBROK | TERRNO, "get_allowed_nodes: %d", ret);
> +}
> +
> +static void cleanup(void)
> +{
> +	SAFE_FILE_PRINTF(PATH_NR_HUGEPAGES, "%ld", orig_hugepages);
> +}
> +
> +static struct tst_test test = {
> +	.tid = "move_pages12",
> +	.min_kver = "2.6.32",
> +	.needs_root = 1,
> +	.forks_child = 1,
> +	.setup = setup,
> +	.cleanup = cleanup,
> +	.test_all = do_test,
> +};
> +
> +#else
> +	tst_res(TCONF, "move_pages support not found");
> +#endif
> -- 
> 1.8.4.2
> 
> 
> 

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v5] syscalls/move_pages12: Add new regression test

[Guangwen Feng]

Fixed by:
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()

Signed-off-by: Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
---
 runtest/numa                                       |   1 +
 runtest/syscalls                                   |   1 +
 testcases/kernel/syscalls/.gitignore               |   1 +
 .../kernel/syscalls/move_pages/move_pages12.c      | 172 +++++++++++++++++++++
 4 files changed, 175 insertions(+)
 create mode 100644 testcases/kernel/syscalls/move_pages/move_pages12.c

diff --git a/runtest/numa b/runtest/numa
index 87f64da..dcf4948 100644
--- a/runtest/numa
+++ b/runtest/numa
@@ -10,3 +10,4 @@ move_pages08 move_pages.sh 08
 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
diff --git a/runtest/syscalls b/runtest/syscalls
index dc03c4c..48f6492 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -645,6 +645,7 @@ move_pages08 move_pages.sh 08
 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
 
 mprotect01 mprotect01
 mprotect02 mprotect02
diff --git a/testcases/kernel/syscalls/.gitignore b/testcases/kernel/syscalls/.gitignore
index 91dccef..bebaa89 100644
--- a/testcases/kernel/syscalls/.gitignore
+++ b/testcases/kernel/syscalls/.gitignore
@@ -589,6 +589,7 @@
 /move_pages/move_pages09
 /move_pages/move_pages10
 /move_pages/move_pages11
+/move_pages/move_pages12
 /mprotect/mprotect01
 /mprotect/mprotect02
 /mprotect/mprotect03
diff --git a/testcases/kernel/syscalls/move_pages/move_pages12.c b/testcases/kernel/syscalls/move_pages/move_pages12.c
new file mode 100644
index 0000000..1a40d6c
--- /dev/null
+++ b/testcases/kernel/syscalls/move_pages/move_pages12.c
@@ -0,0 +1,172 @@
+/*
+ * 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/>.
+ */
+
+/*
+ * 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.
+ *
+ * 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()
+ */
+
+#include <errno.h>
+#include <unistd.h>
+#include <string.h>
+#include <sys/types.h>
+
+#include "tst_test.h"
+#include "move_pages_support.h"
+
+#if HAVE_NUMA_MOVE_PAGES
+
+#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
+
+static int pgsz, hpsz;
+static long orig_hugepages;
+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;
+	pid_t cpid = -1;
+	int status;
+
+	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);
+
+	cpid = SAFE_FORK();
+	if (cpid == 0)
+		do_child();
+
+	for (i = 0; i < LOOPS; i++) {
+		void *ptr;
+
+		ptr = SAFE_MMAP(NULL, TEST_PAGES * 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);
+
+		SAFE_MUNMAP(addr, TEST_PAGES * hpsz);
+	}
+
+	if (i == LOOPS) {
+		SAFE_KILL(cpid, SIGKILL);
+		SAFE_WAITPID(cpid, &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");
+
+	pgsz = (int)get_page_size();
+	SAFE_FILE_LINES_SCANF(PATH_MEMINFO, "Hugepagesize: %d", &hpsz);
+	hpsz *= 1024;
+
+	SAFE_FILE_LINES_SCANF(PATH_MEMINFO, "MemFree: %d", &memfree);
+	memfree *= 1024;
+	if (4 * hpsz > memfree)
+		tst_brk(TBROK, "Not enough free RAM");
+
+	SAFE_FILE_SCANF(PATH_NR_HUGEPAGES, "%ld", &orig_hugepages);
+	SAFE_FILE_PRINTF(PATH_NR_HUGEPAGES, "%ld", orig_hugepages + 4);
+
+	ret = get_allowed_nodes(NH_MEMS, TEST_NODES, &node1, &node2);
+	if (ret < 0)
+		tst_brk(TBROK | TERRNO, "get_allowed_nodes: %d", ret);
+}
+
+static void cleanup(void)
+{
+	SAFE_FILE_PRINTF(PATH_NR_HUGEPAGES, "%ld", orig_hugepages);
+}
+
+static struct tst_test test = {
+	.tid = "move_pages12",
+	.min_kver = "2.6.32",
+	.needs_root = 1,
+	.forks_child = 1,
+	.setup = setup,
+	.cleanup = cleanup,
+	.test_all = do_test,
+};
+
+#else
+	tst_res(TCONF, "move_pages support not found");
+#endif
-- 
1.8.4.2

[LTP] [PATCH v5] syscalls/move_pages12: Add new regression test

[Cyril Hrubis]

Hi!
Pushed, thanks.

-- 
Cyril Hrubis
chrubis@suse.cz