[LTP] [PATCH v2 0/3] move_pages04: fixes and improvements

[LTP] [PATCH v2 0/3] move_pages04: fixes and improvements

[David Hildenbrand]

This is v2 of [1].

Fix and extend the move_pages04 test case to make it pass on upstream
kernels again. While at it, clean it up a bit and convert it to the new
test API.

[1] https://lkml.kernel.org/r/20240829141002.1962303-1-david@redhat.com

Cc: Jan Stancek <jstancek@redhat.com>
Cc: Cyril Hrubis <chrubis@suse.cz>

David Hildenbrand (3):
  move_pages04: check for "invalid area", "no page mapped" and "shared
    zero page mapped"
  move_pages04: remove special-casing for kernels < 4.3
  move_pages04: convert to new test API

 .../kernel/syscalls/move_pages/move_pages04.c | 267 +++++++++---------
 1 file changed, 127 insertions(+), 140 deletions(-)

-- 
2.46.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v2 1/3] move_pages04: check for "invalid area", "no page mapped" and "shared zero page mapped"

[David Hildenbrand]

While the kernel commit d899844e9c98 ("mm: fix status code which
move_pages() returns for zero page") fixed the return value when the
shared zero page was encountered to match what was stated in the man page,
it unfortunately also changed the behavior when no page is mapped yet --
when no page was faulted in/populated on demand.

Then, this test started failing, and we thought we would be testing for
the "zero page" case, but actually we were testing for the "no page mapped"
case, and didn't realize that the kernel commit had unintended side
effects.

As the kernel changed the behavior to return "-ENOENT" again for the
"no page mapped" case in commit 7dff875c9436 ("mm/migrate: convert
add_page_for_migration() from follow_page() to folio_walk") the test
starts failing again ... but for the wrong reason.

The man page clearly spells out that the expectation for the zero page is
"-EFAULT", and that "-EFAULT" can also be returned if "the memory area is
not mapped by the process" -- which means that there is no VMA/mmap()
covering that address.

The man page also documents that "-ENOENT" is returned when "The page is
not present", which includes "there is nothing mapped".

So let's fix the test and properly testing for all three separate things:
"invalid area/page", "no page mapped" and "shared zero page mapped".

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 .../kernel/syscalls/move_pages/move_pages04.c | 106 ++++++++++++++----
 1 file changed, 85 insertions(+), 21 deletions(-)

diff --git a/testcases/kernel/syscalls/move_pages/move_pages04.c b/testcases/kernel/syscalls/move_pages/move_pages04.c
index f53453ab4..9fea63afa 100644
--- a/testcases/kernel/syscalls/move_pages/move_pages04.c
+++ b/testcases/kernel/syscalls/move_pages/move_pages04.c
@@ -26,18 +26,29 @@
  *	move_pages04.c
  *
  * DESCRIPTION
- *      Failure when page does not exit.
+ *      Failure when the memory area is not valid, no page is mapped yet or
+ *      the shared zero page is mapped.
  *
  * ALGORITHM
  *
- *      1. Pass zero page (allocated, but not written to) as one of the
- *         page addresses to move_pages().
- *      2. Check if the corresponding status is set to:
+ *      1. Pass the address of a valid memory area where no where no page is
+ *         mapped yet (not read/written), the address of a valid memory area
+ *         where the shared zero page is mapped (read, but not written to)
+ *         and the address of an invalid memory area as page addresses to
+ *         move_pages().
+ *      2. Check if the corresponding status for "no page mapped" is set to
+ *         -ENOENT. Note that kernels >= 4.3 [1] and < 6.12 [2] wrongly returned
+ *         -EFAULT by accident.
+ *      3. Check if the corresponding status for "shared zero page" is set to:
  *         -ENOENT for kernels < 4.3
  *         -EFAULT for kernels >= 4.3 [1]
+ *      4. Check if the corresponding status for "invalid memory area" is set
+ *         to -EFAULT.
  *
  * [1]
  * d899844e9c98 "mm: fix status code which move_pages() returns for zero page"
+ * [2]
+ * 7dff875c9436 "mm/migrate: convert add_page_for_migration() from follow_page() to folio_walk"
  *
  * USAGE:  <for command-line>
  *      move_pages04 [-c n] [-i n] [-I x] [-P x] [-t]
@@ -64,10 +75,12 @@
 #include "test.h"
 #include "move_pages_support.h"
 
-#define TEST_PAGES 2
+#define TEST_PAGES 4
 #define TEST_NODES 2
 #define TOUCHED_PAGES 1
-#define UNTOUCHED_PAGE (TEST_PAGES - 1)
+#define NO_PAGE TOUCHED_PAGES
+#define ZERO_PAGE (NO_PAGE + 1)
+#define INVALID_PAGE (ZERO_PAGE + 1)
 
 void setup(void);
 void cleanup(void);
@@ -89,12 +102,12 @@ int main(int argc, char **argv)
 	int lc;
 	unsigned int from_node;
 	unsigned int to_node;
-	int ret, exp_status;
+	int ret, exp_zero_page_status;
 
 	if ((tst_kvercmp(4, 3, 0)) >= 0)
-		exp_status = -EFAULT;
+		exp_zero_page_status = -EFAULT;
 	else
-		exp_status = -ENOENT;
+		exp_zero_page_status = -ENOENT;
 
 	ret = get_allowed_nodes(NH_MEMS, 2, &from_node, &to_node);
 	if (ret < 0)
@@ -106,6 +119,7 @@ int main(int argc, char **argv)
 		int nodes[TEST_PAGES];
 		int status[TEST_PAGES];
 		unsigned long onepage = get_page_size();
+		char tmp;
 
 		/* reset tst_count in case we are looping */
 		tst_count = 0;
@@ -114,14 +128,44 @@ int main(int argc, char **argv)
 		if (ret == -1)
 			continue;
 
-		/* Allocate page and do not touch it. */
-		pages[UNTOUCHED_PAGE] = numa_alloc_onnode(onepage, from_node);
-		if (pages[UNTOUCHED_PAGE] == NULL) {
-			tst_resm(TBROK, "failed allocating page on node %d",
+		/*
+		 * Allocate memory and do not touch it. Consequently, no
+		 * page will be faulted in / mapped into the page tables.
+		 */
+		pages[NO_PAGE] = numa_alloc_onnode(onepage, from_node);
+		if (pages[NO_PAGE] == NULL) {
+			tst_resm(TBROK, "failed allocating memory on node %d",
 				 from_node);
 			goto err_free_pages;
 		}
 
+		/*
+		 * Allocate memory, read from it, but do not write to it. This
+		 * will populate the shared zeropage.
+		 */
+		pages[ZERO_PAGE] = numa_alloc_onnode(onepage, from_node);
+		if (pages[ZERO_PAGE] == NULL) {
+			tst_resm(TBROK, "failed allocating memory on node %d",
+				 from_node);
+			goto err_free_pages;
+		}
+		/* Make the compiler not optimize-out the read. */
+		tmp = *((char *)pages[ZERO_PAGE]);
+		asm volatile("" : "+r" (tmp));
+
+		/*
+		 * Temporarily allocate memory and free it immediately. Do this
+		 * as the last step so the area won't get reused before we're
+		 * done.
+		 */
+		pages[INVALID_PAGE] = numa_alloc_onnode(onepage, from_node);
+		if (pages[INVALID_PAGE] == NULL) {
+			tst_resm(TBROK, "failed allocating memory on node %d",
+				 from_node);
+			goto err_free_pages;
+		}
+		numa_free(pages[INVALID_PAGE], onepage);
+
 		for (i = 0; i < TEST_PAGES; i++)
 			nodes[i] = to_node;
 
@@ -135,20 +179,40 @@ int main(int argc, char **argv)
 			tst_resm(TINFO, "move_pages() returned %d", ret);
 		}
 
-		if (status[UNTOUCHED_PAGE] == exp_status) {
+		if (status[NO_PAGE] == -ENOENT) {
 			tst_resm(TPASS, "status[%d] has expected value",
-				 UNTOUCHED_PAGE);
+				 NO_PAGE);
 		} else {
 			tst_resm(TFAIL, "status[%d] is %s, expected %s",
-				UNTOUCHED_PAGE,
-				tst_strerrno(-status[UNTOUCHED_PAGE]),
-				tst_strerrno(-exp_status));
+				NO_PAGE,
+				tst_strerrno(-status[NO_PAGE]),
+				tst_strerrno(ENOENT));
+		}
+
+		if (status[ZERO_PAGE] == exp_zero_page_status) {
+			tst_resm(TPASS, "status[%d] has expected value",
+				 ZERO_PAGE);
+		} else {
+			tst_resm(TFAIL, "status[%d] is %s, expected %s",
+				ZERO_PAGE,
+				tst_strerrno(-status[ZERO_PAGE]),
+				tst_strerrno(-exp_zero_page_status));
+		}
+
+		if (status[INVALID_PAGE] == -EFAULT) {
+			tst_resm(TPASS, "status[%d] has expected value",
+				 INVALID_PAGE);
+		} else {
+			tst_resm(TFAIL, "status[%d] is %s, expected %s",
+				INVALID_PAGE,
+				tst_strerrno(-status[INVALID_PAGE]),
+				tst_strerrno(EFAULT));
 		}
 
 err_free_pages:
-		/* This is capable of freeing both the touched and
-		 * untouched pages.
-		 */
+		/* Memory for the invalid page was already freed. */
+		pages[INVALID_PAGE] = NULL;
+		/* This is capable of freeing all memory we allocated. */
 		free_pages(pages, TEST_PAGES);
 	}
 #else
-- 
2.46.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v2 2/3] move_pages04: remove special-casing for kernels < 4.3

[David Hildenbrand]

Let's stop pretending that kernels < 4.3 did the right thing: they
didn't. Let the test fail if we run on these kernels.

Suggested-by: Jan Stancek <jstancek@redhat.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 .../kernel/syscalls/move_pages/move_pages04.c      | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/testcases/kernel/syscalls/move_pages/move_pages04.c b/testcases/kernel/syscalls/move_pages/move_pages04.c
index 9fea63afa..7ad4ba78c 100644
--- a/testcases/kernel/syscalls/move_pages/move_pages04.c
+++ b/testcases/kernel/syscalls/move_pages/move_pages04.c
@@ -40,8 +40,7 @@
  *         -ENOENT. Note that kernels >= 4.3 [1] and < 6.12 [2] wrongly returned
  *         -EFAULT by accident.
  *      3. Check if the corresponding status for "shared zero page" is set to:
- *         -ENOENT for kernels < 4.3
- *         -EFAULT for kernels >= 4.3 [1]
+ *         -EFAULT. Note that kernels < 4.3 [1] wrongly returned -ENOENT.
  *      4. Check if the corresponding status for "invalid memory area" is set
  *         to -EFAULT.
  *
@@ -102,12 +101,7 @@ int main(int argc, char **argv)
 	int lc;
 	unsigned int from_node;
 	unsigned int to_node;
-	int ret, exp_zero_page_status;
-
-	if ((tst_kvercmp(4, 3, 0)) >= 0)
-		exp_zero_page_status = -EFAULT;
-	else
-		exp_zero_page_status = -ENOENT;
+	int ret;
 
 	ret = get_allowed_nodes(NH_MEMS, 2, &from_node, &to_node);
 	if (ret < 0)
@@ -189,14 +183,14 @@ int main(int argc, char **argv)
 				tst_strerrno(ENOENT));
 		}
 
-		if (status[ZERO_PAGE] == exp_zero_page_status) {
+		if (status[ZERO_PAGE] == -EFAULT) {
 			tst_resm(TPASS, "status[%d] has expected value",
 				 ZERO_PAGE);
 		} else {
 			tst_resm(TFAIL, "status[%d] is %s, expected %s",
 				ZERO_PAGE,
 				tst_strerrno(-status[ZERO_PAGE]),
-				tst_strerrno(-exp_zero_page_status));
+				tst_strerrno(EFAULT));
 		}
 
 		if (status[INVALID_PAGE] == -EFAULT) {
-- 
2.46.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v2 3/3] move_pages04: convert to new test API

[David Hildenbrand]

Let's convert the test to the new test API and cleanup the file header
a bit.

It's worth nothing that the "-c", "-P" and "-t" options are were not
working although documented.

Suggested-by: Cc: Cyril Hrubis <chrubis@suse.cz>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 .../kernel/syscalls/move_pages/move_pages04.c | 287 +++++++-----------
 1 file changed, 108 insertions(+), 179 deletions(-)

diff --git a/testcases/kernel/syscalls/move_pages/move_pages04.c b/testcases/kernel/syscalls/move_pages/move_pages04.c
index 7ad4ba78c..16db3f918 100644
--- a/testcases/kernel/syscalls/move_pages/move_pages04.c
+++ b/testcases/kernel/syscalls/move_pages/move_pages04.c
@@ -1,33 +1,14 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
 /*
  *   Copyright (c) 2008 Vijay Kumar B. <vijaykumar@bravegnu.org>
- *
- *   Based on testcases/kernel/syscalls/waitpid/waitpid01.c
- *   Original copyright message:
- *
  *   Copyright (c) International Business Machines  Corp., 2001
- *
- *   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, write to the Free Software
- *   Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
  */
 
-/*
- * NAME
- *	move_pages04.c
+/*\
+ * [Description]
  *
- * DESCRIPTION
- *      Failure when the memory area is not valid, no page is mapped yet or
- *      the shared zero page is mapped.
+ * Verify that move_pages() properly reports failures when the memory area is
+ * not valid, no page is mapped yet or the shared zero page is mapped.
  *
  * ALGORITHM
  *
@@ -48,21 +29,6 @@
  * d899844e9c98 "mm: fix status code which move_pages() returns for zero page"
  * [2]
  * 7dff875c9436 "mm/migrate: convert add_page_for_migration() from follow_page() to folio_walk"
- *
- * USAGE:  <for command-line>
- *      move_pages04 [-c n] [-i n] [-I x] [-P x] [-t]
- *      where,  -c n : Run n copies concurrently.
- *              -i n : Execute test n times.
- *              -I x : Execute test for x seconds.
- *              -P x : Pause for x seconds between iterations.
- *              -t   : Turn on syscall timing.
- *
- * History
- *	05/2008 Vijay Kumar
- *		Initial Version.
- *
- * Restrictions
- *	None
  */
 
 #include <sys/mman.h>
@@ -71,7 +37,7 @@
 #include <unistd.h>
 #include <signal.h>
 #include <errno.h>
-#include "test.h"
+#include "tst_test.h"
 #include "move_pages_support.h"
 
 #define TEST_PAGES 4
@@ -81,162 +47,125 @@
 #define ZERO_PAGE (NO_PAGE + 1)
 #define INVALID_PAGE (ZERO_PAGE + 1)
 
-void setup(void);
-void cleanup(void);
-
-char *TCID = "move_pages04";
-int TST_TOTAL = 1;
-
-typedef void (*sighandler_t) (int);
-
-int main(int argc, char **argv)
+static void run(void)
 {
-
-	tst_parse_opts(argc, argv, NULL, NULL);
-
-	setup();
-
 #ifdef HAVE_NUMA_V2
 	unsigned int i;
-	int lc;
 	unsigned int from_node;
 	unsigned int to_node;
 	int ret;
+	void *pages[TEST_PAGES] = { 0 };
+	int nodes[TEST_PAGES];
+	int status[TEST_PAGES];
+	unsigned long onepage = get_page_size();
+	char tmp;
 
 	ret = get_allowed_nodes(NH_MEMS, 2, &from_node, &to_node);
 	if (ret < 0)
-		tst_brkm(TBROK | TERRNO, cleanup, "get_allowed_nodes: %d", ret);
-
-	/* check for looping state if -i option is given */
-	for (lc = 0; TEST_LOOPING(lc); lc++) {
-		void *pages[TEST_PAGES] = { 0 };
-		int nodes[TEST_PAGES];
-		int status[TEST_PAGES];
-		unsigned long onepage = get_page_size();
-		char tmp;
-
-		/* reset tst_count in case we are looping */
-		tst_count = 0;
-
-		ret = alloc_pages_on_node(pages, TOUCHED_PAGES, from_node);
-		if (ret == -1)
-			continue;
-
-		/*
-		 * Allocate memory and do not touch it. Consequently, no
-		 * page will be faulted in / mapped into the page tables.
-		 */
-		pages[NO_PAGE] = numa_alloc_onnode(onepage, from_node);
-		if (pages[NO_PAGE] == NULL) {
-			tst_resm(TBROK, "failed allocating memory on node %d",
-				 from_node);
-			goto err_free_pages;
-		}
-
-		/*
-		 * Allocate memory, read from it, but do not write to it. This
-		 * will populate the shared zeropage.
-		 */
-		pages[ZERO_PAGE] = numa_alloc_onnode(onepage, from_node);
-		if (pages[ZERO_PAGE] == NULL) {
-			tst_resm(TBROK, "failed allocating memory on node %d",
-				 from_node);
-			goto err_free_pages;
-		}
-		/* Make the compiler not optimize-out the read. */
-		tmp = *((char *)pages[ZERO_PAGE]);
-		asm volatile("" : "+r" (tmp));
-
-		/*
-		 * Temporarily allocate memory and free it immediately. Do this
-		 * as the last step so the area won't get reused before we're
-		 * done.
-		 */
-		pages[INVALID_PAGE] = numa_alloc_onnode(onepage, from_node);
-		if (pages[INVALID_PAGE] == NULL) {
-			tst_resm(TBROK, "failed allocating memory on node %d",
-				 from_node);
-			goto err_free_pages;
-		}
-		numa_free(pages[INVALID_PAGE], onepage);
-
-		for (i = 0; i < TEST_PAGES; i++)
-			nodes[i] = to_node;
-
-		ret = numa_move_pages(0, TEST_PAGES, pages, nodes,
-				      status, MPOL_MF_MOVE);
-		if (ret == -1) {
-			tst_resm(TFAIL | TERRNO,
-				 "move_pages unexpectedly failed");
-			goto err_free_pages;
-		} else if (ret > 0) {
-			tst_resm(TINFO, "move_pages() returned %d", ret);
-		}
-
-		if (status[NO_PAGE] == -ENOENT) {
-			tst_resm(TPASS, "status[%d] has expected value",
-				 NO_PAGE);
-		} else {
-			tst_resm(TFAIL, "status[%d] is %s, expected %s",
-				NO_PAGE,
-				tst_strerrno(-status[NO_PAGE]),
-				tst_strerrno(ENOENT));
-		}
-
-		if (status[ZERO_PAGE] == -EFAULT) {
-			tst_resm(TPASS, "status[%d] has expected value",
-				 ZERO_PAGE);
-		} else {
-			tst_resm(TFAIL, "status[%d] is %s, expected %s",
-				ZERO_PAGE,
-				tst_strerrno(-status[ZERO_PAGE]),
-				tst_strerrno(EFAULT));
-		}
-
-		if (status[INVALID_PAGE] == -EFAULT) {
-			tst_resm(TPASS, "status[%d] has expected value",
-				 INVALID_PAGE);
-		} else {
-			tst_resm(TFAIL, "status[%d] is %s, expected %s",
-				INVALID_PAGE,
-				tst_strerrno(-status[INVALID_PAGE]),
-				tst_strerrno(EFAULT));
-		}
+		tst_brk(TBROK | TERRNO, "get_allowed_nodes: %d", ret);
 
-err_free_pages:
-		/* Memory for the invalid page was already freed. */
-		pages[INVALID_PAGE] = NULL;
-		/* This is capable of freeing all memory we allocated. */
-		free_pages(pages, TEST_PAGES);
+	ret = alloc_pages_on_node(pages, TOUCHED_PAGES, from_node);
+	if (ret == -1)
+		tst_brk(TBROK, "failed allocating memory on node %d",
+			from_node);
+
+	/*
+	 * Allocate memory and do not touch it. Consequently, no
+	 * page will be faulted in / mapped into the page tables.
+	 */
+	pages[NO_PAGE] = numa_alloc_onnode(onepage, from_node);
+	if (pages[NO_PAGE] == NULL)
+		tst_brk(TBROK, "failed allocating memory on node %d",
+			from_node);
+
+	/*
+	 * Allocate memory, read from it, but do not write to it. This
+	 * will populate the shared zeropage.
+	 */
+	pages[ZERO_PAGE] = numa_alloc_onnode(onepage, from_node);
+	if (pages[ZERO_PAGE] == NULL)
+		tst_brk(TBROK, "failed allocating memory on node %d",
+			from_node);
+	/* Make the compiler not optimize-out the read. */
+	tmp = *((char *)pages[ZERO_PAGE]);
+	asm volatile("" : "+r" (tmp));
+
+	/*
+	 * Temporarily allocate memory and free it immediately. Do this
+	 * as the last step so the area won't get reused before we're
+	 * done.
+	 */
+	pages[INVALID_PAGE] = numa_alloc_onnode(onepage, from_node);
+	if (pages[INVALID_PAGE] == NULL)
+		tst_brk(TBROK, "failed allocating memory on node %d",
+			from_node);
+	numa_free(pages[INVALID_PAGE], onepage);
+
+	for (i = 0; i < TEST_PAGES; i++)
+		nodes[i] = to_node;
+
+	ret = numa_move_pages(0, TEST_PAGES, pages, nodes,
+			      status, MPOL_MF_MOVE);
+	if (ret == -1) {
+		tst_res(TFAIL | TERRNO,
+			"move_pages unexpectedly failed");
+		goto err_free_pages;
+	} else if (ret > 0) {
+		tst_res(TINFO, "move_pages() returned %d", ret);
 	}
+
+	if (status[NO_PAGE] == -ENOENT) {
+		tst_res(TPASS, "status[%d] has expected value",
+			NO_PAGE);
+	} else {
+		tst_res(TFAIL, "status[%d] is %s, expected %s",
+			NO_PAGE,
+			tst_strerrno(-status[NO_PAGE]),
+			tst_strerrno(ENOENT));
+	}
+
+	if (status[ZERO_PAGE] == -EFAULT) {
+		tst_res(TPASS, "status[%d] has expected value",
+			ZERO_PAGE);
+	} else {
+		tst_res(TFAIL, "status[%d] is %s, expected %s",
+			ZERO_PAGE,
+			tst_strerrno(-status[ZERO_PAGE]),
+			tst_strerrno(EFAULT));
+	}
+
+	if (status[INVALID_PAGE] == -EFAULT) {
+		tst_res(TPASS, "status[%d] has expected value",
+			INVALID_PAGE);
+	} else {
+		tst_res(TFAIL, "status[%d] is %s, expected %s",
+			INVALID_PAGE,
+			tst_strerrno(-status[INVALID_PAGE]),
+			tst_strerrno(EFAULT));
+	}
+
+err_free_pages:
+	/* Memory for the invalid page was already freed. */
+	pages[INVALID_PAGE] = NULL;
+	/* This is capable of freeing all memory we allocated. */
+	free_pages(pages, TEST_PAGES);
 #else
-	tst_resm(TCONF, NUMA_ERROR_MSG);
+	tst_res(TCONF, NUMA_ERROR_MSG);
 #endif
-
-	cleanup();
-	tst_exit();
 }
 
-/*
- * setup() - performs all ONE TIME setup for this test
- */
 void setup(void)
 {
-
-	tst_sig(FORK, DEF_HANDLER, cleanup);
-
 	check_config(TEST_NODES);
-
-	/* Pause if that option was specified
-	 * TEST_PAUSE contains the code to fork the test with the -c option.
-	 */
-	TEST_PAUSE;
 }
 
-/*
- * cleanup() - performs all ONE TIME cleanup for this test at completion
- */
-void cleanup(void)
-{
-
-}
+static struct tst_test test = {
+	.test_all = run,
+	.setup = setup,
+	.max_runtime = 120,
+	.tags = (const struct tst_tag[]) {
+		{"linux-git", "d899844e9c98"},
+		{"linux-git", "7dff875c9436"},
+		{}
+	}
+};
-- 
2.46.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v2 0/3] move_pages04: fixes and improvements

[Jan Stancek]

On Tue, Oct 8, 2024 at 3:59 PM David Hildenbrand <david@redhat.com> wrote:
>
> This is v2 of [1].
>
> Fix and extend the move_pages04 test case to make it pass on upstream
> kernels again. While at it, clean it up a bit and convert it to the new
> test API.

Other than a small typo here (which can be fixed during push)

- *      1. Pass the address of a valid memory area where no where no page is
+ *      1. Pass the address of a valid memory area where no page is

it looks good to me, and behaves as expected with 6.12-rc1.

If people find it too noisy now for older kernels, we could add .min_kver = ""
to the test.

Reviewed-by: Jan Stancek <jstancek@redhat.com>

>
> [1] https://lkml.kernel.org/r/20240829141002.1962303-1-david@redhat.com
>
> Cc: Jan Stancek <jstancek@redhat.com>
> Cc: Cyril Hrubis <chrubis@suse.cz>
>
> David Hildenbrand (3):
>   move_pages04: check for "invalid area", "no page mapped" and "shared
>     zero page mapped"
>   move_pages04: remove special-casing for kernels < 4.3
>   move_pages04: convert to new test API
>
>  .../kernel/syscalls/move_pages/move_pages04.c | 267 +++++++++---------
>  1 file changed, 127 insertions(+), 140 deletions(-)
>
> --
> 2.46.1
>


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v2 3/3] move_pages04: convert to new test API

[Cyril Hrubis]

Hi!
> +static struct tst_test test = {
> +	.test_all = run,
> +	.setup = setup,
> +	.max_runtime = 120,

This seems to be a bit excessive, generally tests that do run in less
than a second are not supposed to set this field.

Is there any reason for this to be there? If not I can remove this line
along with fixing the typo found by Jan and fix a bit of the formatting
for the documentation comment that is actually parsed by asciidoc before
applying the changes.

> +	.tags = (const struct tst_tag[]) {
> +		{"linux-git", "d899844e9c98"},
> +		{"linux-git", "7dff875c9436"},
> +		{}
> +	}
> +};
> -- 
> 2.46.1
> 

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v2 3/3] move_pages04: convert to new test API

[David Hildenbrand]

On 17.10.24 17:27, Cyril Hrubis wrote:
> Hi!
>> +static struct tst_test test = {
>> +	.test_all = run,
>> +	.setup = setup,
>> +	.max_runtime = 120,
> 
> This seems to be a bit excessive, generally tests that do run in less
> than a second are not supposed to set this field.
> 
> Is there any reason for this to be there? 

No, copy and paste :)

> If not I can remove this line
> along with fixing the typo found by Jan and fix a bit of the formatting
> for the documentation comment that is actually parsed by asciidoc before
> applying the changes.


Perfect, thanks!


-- 
Cheers,

David / dhildenb


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v2 3/3] move_pages04: convert to new test API

[Cyril Hrubis]

Hi!
> > If not I can remove this line
> > along with fixing the typo found by Jan and fix a bit of the formatting
> > for the documentation comment that is actually parsed by asciidoc before
> > applying the changes.
> 
> 
> Perfect, thanks!

Pushed, thanks.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp