[PATCH RESEND 0/9] selftests/mm: improve mremap_test

[PATCH RESEND 0/9] selftests/mm: improve mremap_test

[Sarthak Sharma]

This series fixes several correctness issues in mremap_test and
simplifies and strengthens its data validation.

Patch 1 converts the mremap_test to use kselftest helpers, removes
manual tracking of failed tests and corrects some spelling errors.

Patches 2 to 5 fix userfaultfd skipping, unexpected mremap success
handling, multi-VMA data validation and failure reporting when data
corruption is detected.

Patch 6 removes the randomization and uses a simple pattern based approach.

Patch 7 removes perf tests and timing infrastructure.

Patch 8 removes validation threshold and always validates complete mappings.

Patch 9 strengthens the multi VMA validation by also checking the
mapping state of holes after remapping.

---
Sarthak Sharma (9):
  selftests/mm: mremap_test: use kselftest helpers
  selftests/mm: mremap_test: skip test when userfaultfd is unavailable
  selftests/mm: mremap_test: fail unexpected mremap successes
  selftests/mm: mremap_test: correct multiple VMA range size
  selftests/mm: mremap_test: fail on data corruption
  selftests/mm: mremap_test: replace random data with deterministic
    pattern
  selftests/mm: mremap_test: remove perf tests and timing
  selftests/mm: mremap_test: remove validation threshold
  selftests/mm: mremap_test: strengthen multi VMA validation

 tools/testing/selftests/mm/mremap_test.c | 600 +++++++++--------------
 1 file changed, 220 insertions(+), 380 deletions(-)

-- 
2.53.0

[PATCH RESEND 1/9] selftests/mm: mremap_test: use kselftest helpers

[Sarthak Sharma]

mremap_test currently uses a lot of fprintf() and perror()
calls. It also uses a variable "failures" to track the number
of failed table driven tests.

Use ksft_print_msg() and ksft_perror() for diagnostics.
Remove the variable "failures" and let kselftest counters
handle the final exit status. Use ksft_finished() at
the end instead of manually checking if failures > 0. Replace

if (success)
	ksft_test_result_pass(...);
else
	ksft_test_result_fail(...);

calls with ksft_test_result(success, ...);

Also correct the duplicated "mremap" in "mremap move within
range" and the spelling of "dontunmap".

Signed-off-by: Sarthak Sharma <sarthak.sharma@arm.com>
---
 tools/testing/selftests/mm/mremap_test.c | 222 +++++++++--------------
 1 file changed, 89 insertions(+), 133 deletions(-)

diff --git a/tools/testing/selftests/mm/mremap_test.c b/tools/testing/selftests/mm/mremap_test.c
index 97abf4713cc5..5532d3151ed0 100644
--- a/tools/testing/selftests/mm/mremap_test.c
+++ b/tools/testing/selftests/mm/mremap_test.c
@@ -257,10 +257,7 @@ static void mremap_expand_merge(FILE *maps_fp, unsigned long page_size)
 	munmap(start, 3 * page_size);
 
 out:
-	if (success)
-		ksft_test_result_pass("%s\n", test_name);
-	else
-		ksft_test_result_fail("%s\n", test_name);
+	ksft_test_result(success, "%s\n", test_name);
 }
 
 /*
@@ -297,10 +294,7 @@ static void mremap_expand_merge_offset(FILE *maps_fp, unsigned long page_size)
 	munmap(start, 3 * page_size);
 
 out:
-	if (success)
-		ksft_test_result_pass("%s\n", test_name);
-	else
-		ksft_test_result_fail("%s\n", test_name);
+	ksft_test_result(success, "%s\n", test_name);
 }
 
 /*
@@ -319,7 +313,7 @@ static void mremap_expand_merge_offset(FILE *maps_fp, unsigned long page_size)
  */
 static void mremap_move_within_range(unsigned int pattern_seed, char *rand_addr)
 {
-	char *test_name = "mremap mremap move within range";
+	char *test_name = "mremap move within range";
 	void *src, *dest;
 	unsigned int i, success = 1;
 
@@ -327,7 +321,7 @@ static void mremap_move_within_range(unsigned int pattern_seed, char *rand_addr)
 	void *ptr = mmap(NULL, size, PROT_READ | PROT_WRITE,
 			 MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
 	if (ptr == MAP_FAILED) {
-		perror("mmap");
+		ksft_perror("mmap");
 		success = 0;
 		goto out;
 	}
@@ -344,7 +338,7 @@ static void mremap_move_within_range(unsigned int pattern_seed, char *rand_addr)
 	void *new_ptr = mremap(src + SIZE_MB(1), SIZE_MB(1), SIZE_MB(1),
 						   MREMAP_MAYMOVE | MREMAP_FIXED, dest + SIZE_MB(1));
 	if (new_ptr == MAP_FAILED) {
-		perror("mremap");
+		ksft_perror("mremap");
 		success = 0;
 		goto out;
 	}
@@ -365,12 +359,9 @@ static void mremap_move_within_range(unsigned int pattern_seed, char *rand_addr)
 
 out:
 	if (munmap(ptr, size) == -1)
-		perror("munmap");
+		ksft_perror("munmap");
 
-	if (success)
-		ksft_test_result_pass("%s\n", test_name);
-	else
-		ksft_test_result_fail("%s\n", test_name);
+	ksft_test_result(success, "%s\n", test_name);
 }
 
 static bool is_multiple_vma_range_ok(unsigned int pattern_seed,
@@ -415,7 +406,7 @@ static void mremap_move_multiple_vmas(unsigned int pattern_seed,
 	ptr = mmap(NULL, size, PROT_READ | PROT_WRITE,
 		   MAP_PRIVATE | MAP_ANON, -1, 0);
 	if (ptr == MAP_FAILED) {
-		perror("mmap");
+		ksft_perror("mmap");
 		success = false;
 		goto out;
 	}
@@ -423,12 +414,12 @@ static void mremap_move_multiple_vmas(unsigned int pattern_seed,
 	tgt_ptr = mmap(NULL, 2 * size, PROT_READ | PROT_WRITE,
 		       MAP_PRIVATE | MAP_ANON, -1, 0);
 	if (tgt_ptr == MAP_FAILED) {
-		perror("mmap");
+		ksft_perror("mmap");
 		success = false;
 		goto out;
 	}
 	if (munmap(tgt_ptr, 2 * size)) {
-		perror("munmap");
+		ksft_perror("munmap");
 		success = false;
 		goto out_unmap;
 	}
@@ -446,7 +437,7 @@ static void mremap_move_multiple_vmas(unsigned int pattern_seed,
 			continue;
 
 		if (munmap(&ptr[i * page_size], page_size)) {
-			perror("munmap");
+			ksft_perror("munmap");
 			success = false;
 			goto out_unmap;
 		}
@@ -466,7 +457,7 @@ static void mremap_move_multiple_vmas(unsigned int pattern_seed,
 
 	/* First, just move the whole thing. */
 	if (mremap(ptr, size, size, mremap_flags, tgt_ptr) == MAP_FAILED) {
-		perror("mremap");
+		ksft_perror("mremap");
 		success = false;
 		goto out_unmap;
 	}
@@ -479,7 +470,7 @@ static void mremap_move_multiple_vmas(unsigned int pattern_seed,
 	/* Move next to itself. */
 	if (mremap(tgt_ptr, size, size, mremap_flags,
 		   &tgt_ptr[size]) == MAP_FAILED) {
-		perror("mremap");
+		ksft_perror("mremap");
 		success = false;
 		goto out_unmap;
 	}
@@ -492,14 +483,14 @@ static void mremap_move_multiple_vmas(unsigned int pattern_seed,
 	/* Map a range to overwrite. */
 	if (mmap(tgt_ptr, size, PROT_NONE,
 		 MAP_PRIVATE | MAP_ANON | MAP_FIXED, -1, 0) == MAP_FAILED) {
-		perror("mmap tgt");
+		ksft_perror("mmap tgt");
 		success = false;
 		goto out_unmap;
 	}
 	/* Move and overwrite. */
 	if (mremap(&tgt_ptr[size], size, size,
 		   mremap_flags, tgt_ptr) == MAP_FAILED) {
-		perror("mremap");
+		ksft_perror("mremap");
 		success = false;
 		goto out_unmap;
 	}
@@ -511,17 +502,12 @@ static void mremap_move_multiple_vmas(unsigned int pattern_seed,
 
 out_unmap:
 	if (munmap(tgt_ptr, 2 * size))
-		perror("munmap tgt");
+		ksft_perror("munmap tgt");
 	if (munmap(ptr, size))
-		perror("munmap src");
+		ksft_perror("munmap src");
 
 out:
-	if (success)
-		ksft_test_result_pass("%s%s\n", test_name,
-				      dont_unmap ? " [dontunnmap]" : "");
-	else
-		ksft_test_result_fail("%s%s\n", test_name,
-				      dont_unmap ? " [dontunnmap]" : "");
+	ksft_test_result(success, "%s%s\n", test_name, dont_unmap ? " [dontunmap]" : "");
 }
 
 static void mremap_shrink_multiple_vmas(unsigned long page_size,
@@ -537,7 +523,7 @@ static void mremap_shrink_multiple_vmas(unsigned long page_size,
 	ptr = mmap(NULL, size, PROT_READ | PROT_WRITE,
 		   MAP_PRIVATE | MAP_ANON, -1, 0);
 	if (ptr == MAP_FAILED) {
-		perror("mmap");
+		ksft_perror("mmap");
 		success = false;
 		goto out;
 	}
@@ -545,12 +531,12 @@ static void mremap_shrink_multiple_vmas(unsigned long page_size,
 	tgt_ptr = mmap(NULL, size, PROT_READ | PROT_WRITE,
 		       MAP_PRIVATE | MAP_ANON, -1, 0);
 	if (tgt_ptr == MAP_FAILED) {
-		perror("mmap");
+		ksft_perror("mmap");
 		success = false;
 		goto out;
 	}
 	if (munmap(tgt_ptr, size)) {
-		perror("munmap");
+		ksft_perror("munmap");
 		success = false;
 		goto out_unmap;
 	}
@@ -564,7 +550,7 @@ static void mremap_shrink_multiple_vmas(unsigned long page_size,
 	 */
 	for (i = 1; i < 10; i += 2) {
 		if (munmap(&ptr[i * page_size], page_size)) {
-			perror("munmap");
+			ksft_perror("munmap");
 			success = false;
 			goto out_unmap;
 		}
@@ -584,23 +570,18 @@ static void mremap_shrink_multiple_vmas(unsigned long page_size,
 			     tgt_ptr);
 
 	if (res == MAP_FAILED) {
-		perror("mremap");
+		ksft_perror("mremap");
 		success = false;
 		goto out_unmap;
 	}
 
 out_unmap:
 	if (munmap(tgt_ptr, size))
-		perror("munmap tgt");
+		ksft_perror("munmap tgt");
 	if (munmap(ptr, size))
-		perror("munmap src");
+		ksft_perror("munmap src");
 out:
-	if (success)
-		ksft_test_result_pass("%s%s\n", test_name,
-				      inplace ? " [inplace]" : "");
-	else
-		ksft_test_result_fail("%s%s\n", test_name,
-				      inplace ? " [inplace]" : "");
+	ksft_test_result(success, "%s%s\n", test_name, inplace ? " [inplace]" : "");
 }
 
 static void mremap_move_multiple_vmas_split(unsigned int pattern_seed,
@@ -620,7 +601,7 @@ static void mremap_move_multiple_vmas_split(unsigned int pattern_seed,
 	ptr = mmap(NULL, size, PROT_READ | PROT_WRITE,
 		   MAP_PRIVATE | MAP_ANON, -1, 0);
 	if (ptr == MAP_FAILED) {
-		perror("mmap");
+		ksft_perror("mmap");
 		success = false;
 		goto out;
 	}
@@ -628,12 +609,12 @@ static void mremap_move_multiple_vmas_split(unsigned int pattern_seed,
 	tgt_ptr = mmap(NULL, size, PROT_READ | PROT_WRITE,
 		       MAP_PRIVATE | MAP_ANON, -1, 0);
 	if (tgt_ptr == MAP_FAILED) {
-		perror("mmap");
+		ksft_perror("mmap");
 		success = false;
 		goto out;
 	}
 	if (munmap(tgt_ptr, size)) {
-		perror("munmap");
+		ksft_perror("munmap");
 		success = false;
 		goto out_unmap;
 	}
@@ -647,7 +628,7 @@ static void mremap_move_multiple_vmas_split(unsigned int pattern_seed,
 	 *  0 1 2 3 4   5 6 7 8 9  pattern offset
 	 */
 	if (munmap(&ptr[5 * page_size], page_size)) {
-		perror("munmap");
+		ksft_perror("munmap");
 		success = false;
 		goto out_unmap;
 	}
@@ -683,7 +664,7 @@ static void mremap_move_multiple_vmas_split(unsigned int pattern_seed,
 	 */
 	if (mremap(&ptr[2 * page_size], size - 3 * page_size, size - 3 * page_size,
 		   mremap_flags, tgt_ptr) == MAP_FAILED) {
-		perror("mremap");
+		ksft_perror("mremap");
 		success = false;
 		goto out_unmap;
 	}
@@ -714,16 +695,11 @@ static void mremap_move_multiple_vmas_split(unsigned int pattern_seed,
 
 out_unmap:
 	if (munmap(tgt_ptr, size))
-		perror("munmap tgt");
+		ksft_perror("munmap tgt");
 	if (munmap(ptr, size))
-		perror("munmap src");
+		ksft_perror("munmap src");
 out:
-	if (success)
-		ksft_test_result_pass("%s%s\n", test_name,
-				      dont_unmap ? " [dontunnmap]" : "");
-	else
-		ksft_test_result_fail("%s%s\n", test_name,
-				      dont_unmap ? " [dontunnmap]" : "");
+	ksft_test_result(success, "%s%s\n", test_name, dont_unmap ? " [dontunmap]" : "");
 }
 
 #ifdef __NR_userfaultfd
@@ -744,16 +720,16 @@ static void mremap_move_multi_invalid_vmas(FILE *maps_fp,
 	uffd = syscall(__NR_userfaultfd, O_NONBLOCK);
 	if (uffd == -1) {
 		err = errno;
-		perror("userfaultfd");
+		ksft_perror("userfaultfd");
 		if (err == EPERM) {
-			ksft_test_result_skip("%s - missing uffd", test_name);
+			ksft_test_result_skip("%s - missing uffd\n", test_name);
 			return;
 		}
 		success = false;
 		goto out;
 	}
 	if (ioctl(uffd, UFFDIO_API, &api)) {
-		perror("ioctl UFFDIO_API");
+		ksft_perror("ioctl UFFDIO_API");
 		success = false;
 		goto out_close_uffd;
 	}
@@ -761,19 +737,19 @@ static void mremap_move_multi_invalid_vmas(FILE *maps_fp,
 	ptr = mmap(NULL, size, PROT_READ | PROT_WRITE,
 		   MAP_PRIVATE | MAP_ANON, -1, 0);
 	if (ptr == MAP_FAILED) {
-		perror("mmap");
+		ksft_perror("mmap");
 		success = false;
 		goto out_close_uffd;
 	}
 
 	tgt_ptr = mmap(NULL, size, PROT_NONE, MAP_PRIVATE | MAP_ANON, -1, 0);
 	if (tgt_ptr == MAP_FAILED) {
-		perror("mmap");
+		ksft_perror("mmap");
 		success = false;
 		goto out_close_uffd;
 	}
 	if (munmap(tgt_ptr, size)) {
-		perror("munmap");
+		ksft_perror("munmap");
 		success = false;
 		goto out_unmap;
 	}
@@ -799,12 +775,12 @@ static void mremap_move_multi_invalid_vmas(FILE *maps_fp,
 		};
 
 		if (ioctl(uffd, UFFDIO_REGISTER, &reg) == -1) {
-			perror("ioctl UFFDIO_REGISTER");
+			ksft_perror("ioctl UFFDIO_REGISTER");
 			success = false;
 			goto out_unmap;
 		}
 		if (munmap(unmap_ptr, page_size)) {
-			perror("munmap");
+			ksft_perror("munmap");
 			success = false;
 			goto out_unmap;
 		}
@@ -819,19 +795,18 @@ static void mremap_move_multi_invalid_vmas(FILE *maps_fp,
 	res = mremap(ptr, size, size, MREMAP_MAYMOVE | MREMAP_FIXED, tgt_ptr);
 	err = errno;
 	if (res != MAP_FAILED) {
-		fprintf(stderr, "mremap() succeeded for multi VMA uffd armed\n");
+		ksft_print_msg("mremap() succeeded for multi VMA uffd armed\n");
 		success = false;
 		goto out_unmap;
 	}
 	if (err != EFAULT) {
 		errno = err;
-		perror("mremap() unexpected error");
+		ksft_perror("mremap() unexpected error");
 		success = false;
 		goto out_unmap;
 	}
 	if (is_ptr_mapped(maps_fp, tgt_ptr, page_size)) {
-		fprintf(stderr,
-			"Invalid uffd-armed VMA at start of multi range moved\n");
+		ksft_print_msg("Invalid uffd-armed VMA at start of multi range moved\n");
 		success = false;
 		goto out_unmap;
 	}
@@ -843,7 +818,7 @@ static void mremap_move_multi_invalid_vmas(FILE *maps_fp,
 	res = mremap(ptr, page_size, page_size,
 		     MREMAP_MAYMOVE | MREMAP_FIXED, tgt_ptr);
 	if (res == MAP_FAILED) {
-		perror("mremap single invalid-multi VMA");
+		ksft_perror("mremap single invalid-multi VMA");
 		success = false;
 		goto out_unmap;
 	}
@@ -853,14 +828,14 @@ static void mremap_move_multi_invalid_vmas(FILE *maps_fp,
 	 * move valid) VMA at the start of ptr range.
 	 */
 	if (munmap(tgt_ptr, page_size)) {
-		perror("munmap");
+		ksft_perror("munmap");
 		success = false;
 		goto out_unmap;
 	}
 	res = mmap(ptr, page_size, PROT_READ | PROT_WRITE,
 		   MAP_PRIVATE | MAP_ANON | MAP_FIXED, -1, 0);
 	if (res == MAP_FAILED) {
-		perror("mmap");
+		ksft_perror("mmap");
 		success = false;
 		goto out_unmap;
 	}
@@ -872,18 +847,18 @@ static void mremap_move_multi_invalid_vmas(FILE *maps_fp,
 	res = mremap(ptr, size, size, MREMAP_MAYMOVE | MREMAP_FIXED, tgt_ptr);
 	err = errno;
 	if (res != MAP_FAILED) {
-		fprintf(stderr, "mremap() succeeded for multi VMA uffd armed\n");
+		ksft_print_msg("mremap() succeeded for multi VMA uffd armed\n");
 		success = false;
 		goto out_unmap;
 	}
 	if (err != EFAULT) {
 		errno = err;
-		perror("mremap() unexpected error");
+		ksft_perror("mremap() unexpected error");
 		success = false;
 		goto out_unmap;
 	}
 	if (!is_ptr_mapped(maps_fp, tgt_ptr, page_size)) {
-		fprintf(stderr, "Valid VMA not moved\n");
+		ksft_print_msg("Valid VMA not moved\n");
 		success = false;
 		goto out_unmap;
 	}
@@ -894,12 +869,12 @@ static void mremap_move_multi_invalid_vmas(FILE *maps_fp,
 	 * multi-move VMAs.
 	 */
 	if (munmap(tgt_ptr, page_size)) {
-		perror("munmap");
+		ksft_perror("munmap");
 		success = false;
 		goto out_unmap;
 	}
 	if (munmap(ptr, size - 2 * page_size)) {
-		perror("munmap");
+		ksft_perror("munmap");
 		success = false;
 		goto out_unmap;
 	}
@@ -908,7 +883,7 @@ static void mremap_move_multi_invalid_vmas(FILE *maps_fp,
 			   PROT_READ | PROT_WRITE,
 			   MAP_PRIVATE | MAP_ANON | MAP_FIXED, -1, 0);
 		if (res == MAP_FAILED) {
-			perror("mmap");
+			ksft_perror("mmap");
 			success = false;
 			goto out_unmap;
 		}
@@ -921,13 +896,13 @@ static void mremap_move_multi_invalid_vmas(FILE *maps_fp,
 	res = mremap(ptr, size, size, MREMAP_MAYMOVE | MREMAP_FIXED, tgt_ptr);
 	err = errno;
 	if (res != MAP_FAILED) {
-		fprintf(stderr, "mremap() succeeded for multi VMA uffd armed\n");
+		ksft_print_msg("mremap() succeeded for multi VMA uffd armed\n");
 		success = false;
 		goto out_unmap;
 	}
 	if (err != EFAULT) {
 		errno = err;
-		perror("mremap() unexpected error");
+		ksft_perror("mremap() unexpected error");
 		success = false;
 		goto out_unmap;
 	}
@@ -937,11 +912,11 @@ static void mremap_move_multi_invalid_vmas(FILE *maps_fp,
 				&tgt_ptr[i * page_size], page_size);
 
 		if (i < 8 && !is_mapped) {
-			fprintf(stderr, "Valid VMA not moved at %d\n", i);
+			ksft_print_msg("Valid VMA not moved at %d\n", i);
 			success = false;
 			goto out_unmap;
 		} else if (i == 8 && is_mapped) {
-			fprintf(stderr, "Invalid VMA moved at %d\n", i);
+			ksft_print_msg("Invalid VMA moved at %d\n", i);
 			success = false;
 			goto out_unmap;
 		}
@@ -949,23 +924,20 @@ static void mremap_move_multi_invalid_vmas(FILE *maps_fp,
 
 out_unmap:
 	if (munmap(tgt_ptr, size))
-		perror("munmap tgt");
+		ksft_perror("munmap tgt");
 	if (munmap(ptr, size))
-		perror("munmap src");
+		ksft_perror("munmap src");
 out_close_uffd:
 	close(uffd);
 out:
-	if (success)
-		ksft_test_result_pass("%s\n", test_name);
-	else
-		ksft_test_result_fail("%s\n", test_name);
+	ksft_test_result(success, "%s\n", test_name);
 }
 #else
 static void mremap_move_multi_invalid_vmas(FILE *maps_fp, unsigned long page_size)
 {
 	char *test_name = "mremap move multiple invalid vmas";
 
-	ksft_test_result_skip("%s - missing uffd", test_name);
+	ksft_test_result_skip("%s - missing uffd\n", test_name);
 }
 #endif /* __NR_userfaultfd */
 
@@ -1124,7 +1096,7 @@ static void mremap_move_1mb_from_start(unsigned int pattern_seed,
 	void *new_ptr = mremap(src + SIZE_MB(1), SIZE_MB(1), SIZE_MB(1),
 						   MREMAP_MAYMOVE | MREMAP_FIXED, dest + SIZE_MB(1));
 	if (new_ptr == MAP_FAILED) {
-		perror("mremap");
+		ksft_perror("mremap");
 		success = 0;
 		goto out;
 	}
@@ -1145,59 +1117,49 @@ static void mremap_move_1mb_from_start(unsigned int pattern_seed,
 
 out:
 	if (src && munmap(src, c.region_size) == -1)
-		perror("munmap src");
+		ksft_perror("munmap src");
 
 	if (dest && munmap(dest, c.region_size) == -1)
-		perror("munmap dest");
+		ksft_perror("munmap dest");
 
-	if (success)
-		ksft_test_result_pass("%s\n", test_name);
-	else
-		ksft_test_result_fail("%s\n", test_name);
+	ksft_test_result(success, "%s\n", test_name);
 }
 
-static void run_mremap_test_case(struct test test_case, int *failures,
-				 unsigned int threshold_mb,
+static void run_mremap_test_case(struct test test_case, unsigned int threshold_mb,
 				 char *rand_addr)
 {
 	long long remap_time = remap_region(test_case.config, threshold_mb,
 					    rand_addr);
 
 	if (remap_time < 0) {
-		if (test_case.expect_failure) {
-			ksft_print_msg("%s: expected mremap failure\n",
-				       test_case.name);
-			ksft_test_result_xfail("%s\n", test_case.name);
-		} else {
+		if (test_case.expect_failure)
+			ksft_test_result_xfail("%s: expected mremap failure\n",
+					       test_case.name);
+		else
 			ksft_test_result_fail("%s\n", test_case.name);
-			*failures += 1;
-		}
 	} else {
 		/*
 		 * Comparing mremap time is only applicable if entire region
 		 * was faulted in.
 		 */
 		if (threshold_mb == VALIDATION_NO_THRESHOLD ||
-		    test_case.config.region_size <= threshold_mb * _1MB) {
-			ksft_print_msg("%s: mremap time: %12lldns\n",
-				       test_case.name, remap_time);
+		    test_case.config.region_size <= threshold_mb * _1MB)
+			ksft_test_result_pass("%s: mremap time: %12lldns\n",
+					      test_case.name, remap_time);
+		else
 			ksft_test_result_pass("%s\n", test_case.name);
-		} else {
-			ksft_test_result_pass("%s\n", test_case.name);
-		}
 	}
 }
 
 static void usage(const char *cmd)
 {
-	fprintf(stderr,
-		"Usage: %s [[-t <threshold_mb>] [-p <pattern_seed>]]\n"
-		"-t\t only validate threshold_mb of the remapped region\n"
-		"  \t if 0 is supplied no threshold is used; all tests\n"
-		"  \t are run and remapped regions validated fully.\n"
-		"  \t The default threshold used is 4MB.\n"
-		"-p\t provide a seed to generate the random pattern for\n"
-		"  \t validating the remapped region.\n", cmd);
+	ksft_print_msg("Usage: %s [[-t <threshold_mb>] [-p <pattern_seed>]]\n", cmd);
+	ksft_print_msg("-t\t only validate threshold_mb of the remapped region\n");
+	ksft_print_msg("  \t if 0 is supplied no threshold is used; all tests\n");
+	ksft_print_msg("  \t are run and remapped regions validated fully.\n");
+	ksft_print_msg("  \t The default threshold used is 4MB.\n");
+	ksft_print_msg("-p\t provide a seed to generate the random pattern for\n");
+	ksft_print_msg("  \t validating the remapped region.\n");
 }
 
 static int parse_args(int argc, char **argv, unsigned int *threshold_mb,
@@ -1232,7 +1194,6 @@ static int parse_args(int argc, char **argv, unsigned int *threshold_mb,
 #define MAX_PERF_TEST 3
 int main(int argc, char **argv)
 {
-	int failures = 0;
 	unsigned int i;
 	int run_perf_tests;
 	unsigned int threshold_mb = VALIDATION_DEFAULT_THRESHOLD;
@@ -1260,7 +1221,7 @@ int main(int argc, char **argv)
 	pattern_seed = (unsigned int) time(&t);
 
 	if (parse_args(argc, argv, &threshold_mb, &pattern_seed) < 0)
-		exit(EXIT_FAILURE);
+		ksft_exit_fail_msg("Invalid arguments\n");
 
 	ksft_print_msg("Test configs:\n");
 	ksft_print_msg("threshold_mb=%u\n", threshold_mb);
@@ -1282,7 +1243,7 @@ int main(int argc, char **argv)
 	rand_addr = (char *)mmap(NULL, rand_size, PROT_READ | PROT_WRITE,
 				 MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
 	if (rand_addr == MAP_FAILED) {
-		perror("mmap");
+		ksft_perror("mmap");
 		ksft_exit_fail_msg("cannot mmap rand_addr\n");
 	}
 
@@ -1361,8 +1322,7 @@ int main(int argc, char **argv)
 		      ARRAY_SIZE(perf_test_cases) : 0) + num_expand_tests + num_misc_tests);
 
 	for (i = 0; i < ARRAY_SIZE(test_cases); i++)
-		run_mremap_test_case(test_cases[i], &failures, threshold_mb,
-				     rand_addr);
+		run_mremap_test_case(test_cases[i], threshold_mb, rand_addr);
 
 	maps_fp = fopen("/proc/self/maps", "r");
 
@@ -1387,18 +1347,14 @@ int main(int argc, char **argv)
 	fclose(maps_fp);
 
 	if (run_perf_tests) {
-		ksft_print_msg("\n%s\n",
-		 "mremap HAVE_MOVE_PMD/PUD optimization time comparison for 1GB region:");
+		ksft_print_msg("%s\n",
+			       "mremap HAVE_MOVE_PMD/PUD optimization time comparison for 1GB region:");
 		for (i = 0; i < ARRAY_SIZE(perf_test_cases); i++)
-			run_mremap_test_case(perf_test_cases[i], &failures,
-					     threshold_mb,
+			run_mremap_test_case(perf_test_cases[i], threshold_mb,
 					     rand_addr);
 	}
 
 	munmap(rand_addr, rand_size);
 
-	if (failures > 0)
-		ksft_exit_fail();
-	else
-		ksft_exit_pass();
+	ksft_finished();
 }
-- 
2.53.0

Re: [PATCH RESEND 1/9] selftests/mm: mremap_test: use kselftest helpers

[David Hildenbrand (Arm)]

On 9/24/26 07:00, Sarthak Sharma wrote:
> mremap_test currently uses a lot of fprintf() and perror()
> calls. It also uses a variable "failures" to track the number
> of failed table driven tests.
> 
> Use ksft_print_msg() and ksft_perror() for diagnostics.
> Remove the variable "failures" and let kselftest counters
> handle the final exit status. Use ksft_finished() at
> the end instead of manually checking if failures > 0. Replace
> 
> if (success)
> 	ksft_test_result_pass(...);
> else
> 	ksft_test_result_fail(...);
> 
> calls with ksft_test_result(success, ...);
> 
> Also correct the duplicated "mremap" in "mremap move within
> range" and the spelling of "dontunmap".
> 
> Signed-off-by: Sarthak Sharma <sarthak.sharma@arm.com>
> ---

[...]

>  #endif /* __NR_userfaultfd */
>  
> @@ -1124,7 +1096,7 @@ static void mremap_move_1mb_from_start(unsigned int pattern_seed,
>  	void *new_ptr = mremap(src + SIZE_MB(1), SIZE_MB(1), SIZE_MB(1),
>  						   MREMAP_MAYMOVE | MREMAP_FIXED, dest + SIZE_MB(1));
>  	if (new_ptr == MAP_FAILED) {
> -		perror("mremap");
> +		ksft_perror("mremap");
>  		success = 0;
>  		goto out;
>  	}
> @@ -1145,59 +1117,49 @@ static void mremap_move_1mb_from_start(unsigned int pattern_seed,
>  
>  out:
>  	if (src && munmap(src, c.region_size) == -1)

While at it ... why the comparison with -1. And why do we worry about munmap()
failing at all? We generally ignore these errors on the exit path, as it's
unlikely we would ever hit them, and there isn't a lot we can do.

So maybe just remove printing errors entirely?

if (src)
	munmap(src, c.region_size)

...

> -		perror("munmap src");
> +		ksft_perror("munmap src");
>  
>  	if (dest && munmap(dest, c.region_size) == -1)
> -		perror("munmap dest");
> +		ksft_perror("munmap dest");
>  
> -	if (success)
> -		ksft_test_result_pass("%s\n", test_name);
> -	else
> -		ksft_test_result_fail("%s\n", test_name);
> +	ksft_test_result(success, "%s\n", test_name);
>  }

[...]

>  static void usage(const char *cmd)
>  {
> -	fprintf(stderr,
> -		"Usage: %s [[-t <threshold_mb>] [-p <pattern_seed>]]\n"
> -		"-t\t only validate threshold_mb of the remapped region\n"
> -		"  \t if 0 is supplied no threshold is used; all tests\n"
> -		"  \t are run and remapped regions validated fully.\n"
> -		"  \t The default threshold used is 4MB.\n"
> -		"-p\t provide a seed to generate the random pattern for\n"
> -		"  \t validating the remapped region.\n", cmd);
> +	ksft_print_msg("Usage: %s [[-t <threshold_mb>] [-p <pattern_seed>]]\n", cmd);
> +	ksft_print_msg("-t\t only validate threshold_mb of the remapped region\n");
> +	ksft_print_msg("  \t if 0 is supplied no threshold is used; all tests\n");
> +	ksft_print_msg("  \t are run and remapped regions validated fully.\n");
> +	ksft_print_msg("  \t The default threshold used is 4MB.\n");
> +	ksft_print_msg("-p\t provide a seed to generate the random pattern for\n");
> +	ksft_print_msg("  \t validating the remapped region.\n");
>  }

That looks odd, as we will now print this as "# ". I would have assumed that
removing all parameters as the first patch would make things cleaner?

So as a first patch I think we should just remove the parameters entirely. They
are unused by our infrastrcture:

	run_vmtests.sh:CATEGORY="mremap" run_test ./mremap_test

Does anything speak against that?

>  
>  static int parse_args(int argc, char **argv, unsigned int *threshold_mb,
> @@ -1232,7 +1194,6 @@ static int parse_args(int argc, char **argv, unsigned int *threshold_mb,
>  #define MAX_PERF_TEST 3
>  int main(int argc, char **argv)
>  {
> -	int failures = 0;
>  	unsigned int i;
>  	int run_perf_tests;
>  	unsigned int threshold_mb = VALIDATION_DEFAULT_THRESHOLD;
> @@ -1260,7 +1221,7 @@ int main(int argc, char **argv)
>  	pattern_seed = (unsigned int) time(&t);
>  
>  	if (parse_args(argc, argv, &threshold_mb, &pattern_seed) < 0)
> -		exit(EXIT_FAILURE);
> +		ksft_exit_fail_msg("Invalid arguments\n");
>  
>  	ksft_print_msg("Test configs:\n");
>  	ksft_print_msg("threshold_mb=%u\n", threshold_mb);
> @@ -1282,7 +1243,7 @@ int main(int argc, char **argv)
>  	rand_addr = (char *)mmap(NULL, rand_size, PROT_READ | PROT_WRITE,
>  				 MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
>  	if (rand_addr == MAP_FAILED) {
> -		perror("mmap");
> +		ksft_perror("mmap");
>  		ksft_exit_fail_msg("cannot mmap rand_addr\n");

Would be better combined like:

ksft_exit_fail_msg("cannot mmap rand_addr: %s\n", strerror(errno));

?

-- 
Cheers,

David

Re: [PATCH RESEND 1/9] selftests/mm: mremap_test: use kselftest helpers

[Sarthak Sharma]

Hi David!

On 9/29/26 1:28 PM, David Hildenbrand (Arm) wrote:
> On 9/24/26 07:00, Sarthak Sharma wrote:
>> mremap_test currently uses a lot of fprintf() and perror()
>> calls. It also uses a variable "failures" to track the number
>> of failed table driven tests.
>>
>> Use ksft_print_msg() and ksft_perror() for diagnostics.
>> Remove the variable "failures" and let kselftest counters
>> handle the final exit status. Use ksft_finished() at
>> the end instead of manually checking if failures > 0. Replace
>>
>> if (success)
>> 	ksft_test_result_pass(...);
>> else
>> 	ksft_test_result_fail(...);
>>
>> calls with ksft_test_result(success, ...);
>>
>> Also correct the duplicated "mremap" in "mremap move within
>> range" and the spelling of "dontunmap".
>>
>> Signed-off-by: Sarthak Sharma <sarthak.sharma@arm.com>
>> ---
> 
> [...]
> 
>>  #endif /* __NR_userfaultfd */
>>  
>> @@ -1124,7 +1096,7 @@ static void mremap_move_1mb_from_start(unsigned int pattern_seed,
>>  	void *new_ptr = mremap(src + SIZE_MB(1), SIZE_MB(1), SIZE_MB(1),
>>  						   MREMAP_MAYMOVE | MREMAP_FIXED, dest + SIZE_MB(1));
>>  	if (new_ptr == MAP_FAILED) {
>> -		perror("mremap");
>> +		ksft_perror("mremap");
>>  		success = 0;
>>  		goto out;
>>  	}
>> @@ -1145,59 +1117,49 @@ static void mremap_move_1mb_from_start(unsigned int pattern_seed,
>>  
>>  out:
>>  	if (src && munmap(src, c.region_size) == -1)
> 
> While at it ... why the comparison with -1. And why do we worry about munmap()
> failing at all? We generally ignore these errors on the exit path, as it's
> unlikely we would ever hit them, and there isn't a lot we can do.
> 
> So maybe just remove printing errors entirely?
> 
> if (src)
> 	munmap(src, c.region_size)

Yup, this check is not supposed to be on the exit path. I'll fix this.

> 
> ...
> 
>> -		perror("munmap src");
>> +		ksft_perror("munmap src");
>>  
>>  	if (dest && munmap(dest, c.region_size) == -1)
>> -		perror("munmap dest");
>> +		ksft_perror("munmap dest");
>>  
>> -	if (success)
>> -		ksft_test_result_pass("%s\n", test_name);
>> -	else
>> -		ksft_test_result_fail("%s\n", test_name);
>> +	ksft_test_result(success, "%s\n", test_name);
>>  }
> 
> [...]
> 
>>  static void usage(const char *cmd)
>>  {
>> -	fprintf(stderr,
>> -		"Usage: %s [[-t <threshold_mb>] [-p <pattern_seed>]]\n"
>> -		"-t\t only validate threshold_mb of the remapped region\n"
>> -		"  \t if 0 is supplied no threshold is used; all tests\n"
>> -		"  \t are run and remapped regions validated fully.\n"
>> -		"  \t The default threshold used is 4MB.\n"
>> -		"-p\t provide a seed to generate the random pattern for\n"
>> -		"  \t validating the remapped region.\n", cmd);
>> +	ksft_print_msg("Usage: %s [[-t <threshold_mb>] [-p <pattern_seed>]]\n", cmd);
>> +	ksft_print_msg("-t\t only validate threshold_mb of the remapped region\n");
>> +	ksft_print_msg("  \t if 0 is supplied no threshold is used; all tests\n");
>> +	ksft_print_msg("  \t are run and remapped regions validated fully.\n");
>> +	ksft_print_msg("  \t The default threshold used is 4MB.\n");
>> +	ksft_print_msg("-p\t provide a seed to generate the random pattern for\n");
>> +	ksft_print_msg("  \t validating the remapped region.\n");
>>  }
> 
> That looks odd, as we will now print this as "# ". I would have assumed that
> removing all parameters as the first patch would make things cleaner?
> 
> So as a first patch I think we should just remove the parameters entirely. They
> are unused by our infrastrcture:
> 
> 	run_vmtests.sh:CATEGORY="mremap" run_test ./mremap_test
> 
> Does anything speak against that?

Okay, I'd kept the 7th and 8th patch of the series for that. I'll make
them the first and second ones then, this will remove all this churn.

Or maybe remove all parameters altogether in the first patch itself.

> 
>>  
>>  static int parse_args(int argc, char **argv, unsigned int *threshold_mb,
>> @@ -1232,7 +1194,6 @@ static int parse_args(int argc, char **argv, unsigned int *threshold_mb,
>>  #define MAX_PERF_TEST 3
>>  int main(int argc, char **argv)
>>  {
>> -	int failures = 0;
>>  	unsigned int i;
>>  	int run_perf_tests;
>>  	unsigned int threshold_mb = VALIDATION_DEFAULT_THRESHOLD;
>> @@ -1260,7 +1221,7 @@ int main(int argc, char **argv)
>>  	pattern_seed = (unsigned int) time(&t);
>>  
>>  	if (parse_args(argc, argv, &threshold_mb, &pattern_seed) < 0)
>> -		exit(EXIT_FAILURE);
>> +		ksft_exit_fail_msg("Invalid arguments\n");
>>  
>>  	ksft_print_msg("Test configs:\n");
>>  	ksft_print_msg("threshold_mb=%u\n", threshold_mb);
>> @@ -1282,7 +1243,7 @@ int main(int argc, char **argv)
>>  	rand_addr = (char *)mmap(NULL, rand_size, PROT_READ | PROT_WRITE,
>>  				 MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
>>  	if (rand_addr == MAP_FAILED) {
>> -		perror("mmap");
>> +		ksft_perror("mmap");
>>  		ksft_exit_fail_msg("cannot mmap rand_addr\n");
> 
> Would be better combined like:
> 
> ksft_exit_fail_msg("cannot mmap rand_addr: %s\n", strerror(errno));
> 
> ?

Yup, or maybe with a ksft_exit_fail_perror() directly. Will change.

[PATCH RESEND 2/9] selftests/mm: mremap_test: skip test when userfaultfd is unavailable

[Sarthak Sharma]

The userfaultfd syscall can return ENOSYS even when __NR_userfaultfd is
defined by the architecture headers, for example when the kernel is
built without CONFIG_USERFAULTFD. The test currently skips for EPERM
when the caller lacks permission to use userfaultfd but treats
ENOSYS as a failure.

Handle ENOSYS like EPERM and skip the mremap_move_multi_invalid_vmas()
test when userfaultfd syscall is not implemented.

Fixes: 742d3663a577 ("selftests/mm: add test for invalid multi VMA operations")
Signed-off-by: Sarthak Sharma <sarthak.sharma@arm.com>
---
 tools/testing/selftests/mm/mremap_test.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/mm/mremap_test.c b/tools/testing/selftests/mm/mremap_test.c
index 5532d3151ed0..a5813a5e8f5b 100644
--- a/tools/testing/selftests/mm/mremap_test.c
+++ b/tools/testing/selftests/mm/mremap_test.c
@@ -721,7 +721,7 @@ static void mremap_move_multi_invalid_vmas(FILE *maps_fp,
 	if (uffd == -1) {
 		err = errno;
 		ksft_perror("userfaultfd");
-		if (err == EPERM) {
+		if (err == EPERM || err == ENOSYS) {
 			ksft_test_result_skip("%s - missing uffd\n", test_name);
 			return;
 		}
-- 
2.53.0

Re: [PATCH RESEND 2/9] selftests/mm: mremap_test: skip test when userfaultfd is unavailable

[David Hildenbrand (Arm)]

On 9/24/26 07:00, Sarthak Sharma wrote:
> The userfaultfd syscall can return ENOSYS even when __NR_userfaultfd is
> defined by the architecture headers, for example when the kernel is
> built without CONFIG_USERFAULTFD. The test currently skips for EPERM
> when the caller lacks permission to use userfaultfd but treats
> ENOSYS as a failure.
> 
> Handle ENOSYS like EPERM and skip the mremap_move_multi_invalid_vmas()
> test when userfaultfd syscall is not implemented.
> 
> Fixes: 742d3663a577 ("selftests/mm: add test for invalid multi VMA operations")
> Signed-off-by: Sarthak Sharma <sarthak.sharma@arm.com>
> ---
>  tools/testing/selftests/mm/mremap_test.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/mm/mremap_test.c b/tools/testing/selftests/mm/mremap_test.c
> index 5532d3151ed0..a5813a5e8f5b 100644
> --- a/tools/testing/selftests/mm/mremap_test.c
> +++ b/tools/testing/selftests/mm/mremap_test.c
> @@ -721,7 +721,7 @@ static void mremap_move_multi_invalid_vmas(FILE *maps_fp,
>  	if (uffd == -1) {
>  		err = errno;
>  		ksft_perror("userfaultfd");

Should we rather move that below the if? Doesn't quite make sense to print the
error if we are skipping and printing a proper message.

Maybe that should go into the previous patch?

> -		if (err == EPERM) {
> +		if (err == EPERM || err == ENOSYS) {
>  			ksft_test_result_skip("%s - missing uffd\n", test_name);
>  			return;
>  		}

Apart from that LGTM.

Acked-by: David Hildenbrand (Arm) <david@kernel.org>

-- 
Cheers,

David

Re: [PATCH RESEND 2/9] selftests/mm: mremap_test: skip test when userfaultfd is unavailable

[Sarthak Sharma]

Hi David!

On 9/29/26 1:30 PM, David Hildenbrand (Arm) wrote:
> On 9/24/26 07:00, Sarthak Sharma wrote:
>> The userfaultfd syscall can return ENOSYS even when __NR_userfaultfd is
>> defined by the architecture headers, for example when the kernel is
>> built without CONFIG_USERFAULTFD. The test currently skips for EPERM
>> when the caller lacks permission to use userfaultfd but treats
>> ENOSYS as a failure.
>>
>> Handle ENOSYS like EPERM and skip the mremap_move_multi_invalid_vmas()
>> test when userfaultfd syscall is not implemented.
>>
>> Fixes: 742d3663a577 ("selftests/mm: add test for invalid multi VMA operations")
>> Signed-off-by: Sarthak Sharma <sarthak.sharma@arm.com>
>> ---
>>  tools/testing/selftests/mm/mremap_test.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/tools/testing/selftests/mm/mremap_test.c b/tools/testing/selftests/mm/mremap_test.c
>> index 5532d3151ed0..a5813a5e8f5b 100644
>> --- a/tools/testing/selftests/mm/mremap_test.c
>> +++ b/tools/testing/selftests/mm/mremap_test.c
>> @@ -721,7 +721,7 @@ static void mremap_move_multi_invalid_vmas(FILE *maps_fp,
>>  	if (uffd == -1) {
>>  		err = errno;
>>  		ksft_perror("userfaultfd");
> 
> Should we rather move that below the if? Doesn't quite make sense to print the
> error if we are skipping and printing a proper message.

Yes, I'll put it below the if block.

> 
> Maybe that should go into the previous patch?

Yes, will include in the ksft conversion patch.

> 
>> -		if (err == EPERM) {
>> +		if (err == EPERM || err == ENOSYS) {
>>  			ksft_test_result_skip("%s - missing uffd\n", test_name);
>>  			return;
>>  		}
> 
> Apart from that LGTM.
> 
> Acked-by: David Hildenbrand (Arm) <david@kernel.org>

Thanks!

[PATCH RESEND 3/9] selftests/mm: mremap_test: fail unexpected mremap successes

[Sarthak Sharma]

run_mremap_test_case() reports a test as pass when remap_region()
succeeds. But if the test was expecting failure and remap succeeds,
it should be treated as a failure and not a success.

Check test_case.expect_failure on the successful remap path. If
it expected failure, report a failure instead of success.

Fixes: 7df666253f26 ("kselftests: vm: add mremap tests")
Signed-off-by: Sarthak Sharma <sarthak.sharma@arm.com>
---
 tools/testing/selftests/mm/mremap_test.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/mm/mremap_test.c b/tools/testing/selftests/mm/mremap_test.c
index a5813a5e8f5b..ba9b7f7566d0 100644
--- a/tools/testing/selftests/mm/mremap_test.c
+++ b/tools/testing/selftests/mm/mremap_test.c
@@ -1138,12 +1138,15 @@ static void run_mremap_test_case(struct test test_case, unsigned int threshold_m
 		else
 			ksft_test_result_fail("%s\n", test_case.name);
 	} else {
+		if (test_case.expect_failure)
+			ksft_test_result_fail("%s: unexpected mremap success\n",
+					      test_case.name);
 		/*
 		 * Comparing mremap time is only applicable if entire region
 		 * was faulted in.
 		 */
-		if (threshold_mb == VALIDATION_NO_THRESHOLD ||
-		    test_case.config.region_size <= threshold_mb * _1MB)
+		else if (threshold_mb == VALIDATION_NO_THRESHOLD ||
+			 test_case.config.region_size <= threshold_mb * _1MB)
 			ksft_test_result_pass("%s: mremap time: %12lldns\n",
 					      test_case.name, remap_time);
 		else
-- 
2.53.0

Re: [PATCH RESEND 3/9] selftests/mm: mremap_test: fail unexpected mremap successes

[David Hildenbrand (Arm)]

On 9/24/26 07:00, Sarthak Sharma wrote:
> run_mremap_test_case() reports a test as pass when remap_region()
> succeeds. But if the test was expecting failure and remap succeeds,
> it should be treated as a failure and not a success.
> 
> Check test_case.expect_failure on the successful remap path. If
> it expected failure, report a failure instead of success.
> 
> Fixes: 7df666253f26 ("kselftests: vm: add mremap tests")
> Signed-off-by: Sarthak Sharma <sarthak.sharma@arm.com>
> ---
>  tools/testing/selftests/mm/mremap_test.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/testing/selftests/mm/mremap_test.c b/tools/testing/selftests/mm/mremap_test.c
> index a5813a5e8f5b..ba9b7f7566d0 100644
> --- a/tools/testing/selftests/mm/mremap_test.c
> +++ b/tools/testing/selftests/mm/mremap_test.c
> @@ -1138,12 +1138,15 @@ static void run_mremap_test_case(struct test test_case, unsigned int threshold_m
>  		else
>  			ksft_test_result_fail("%s\n", test_case.name);
>  	} else {
> +		if (test_case.expect_failure)
> +			ksft_test_result_fail("%s: unexpected mremap success\n",
> +					      test_case.name);
>  		/*
>  		 * Comparing mremap time is only applicable if entire region
>  		 * was faulted in.
>  		 */
> -		if (threshold_mb == VALIDATION_NO_THRESHOLD ||
> -		    test_case.config.region_size <= threshold_mb * _1MB)
> +		else if (threshold_mb == VALIDATION_NO_THRESHOLD ||
> +			 test_case.config.region_size <= threshold_mb * _1MB)
>  			ksft_test_result_pass("%s: mremap time: %12lldns\n",
>  					      test_case.name, remap_time);
>  		else

That sounds about right to me.

Acked-by: David Hildenbrand (Arm) <david@kernel.org>

-- 
Cheers,

David

[PATCH RESEND 4/9] selftests/mm: mremap_test: correct multiple VMA range size

[Sarthak Sharma]

In the test mremap_move_multiple_vmas(), pages 4
to 6 form a single VMA. But the helper function
is_multiple_vma_range_ok() checks pages 4 and 5 as
one range and treats page 6 separately.

Check all three pages as a single range and skip
the page 6 iteration to match the actual VMA layout.

Fixes: d53f248258e1 ("tools/testing/selftests: extend mremap_test to test multi-VMA mremap")
Signed-off-by: Sarthak Sharma <sarthak.sharma@arm.com>
---
 tools/testing/selftests/mm/mremap_test.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/mm/mremap_test.c b/tools/testing/selftests/mm/mremap_test.c
index ba9b7f7566d0..869df22d46db 100644
--- a/tools/testing/selftests/mm/mremap_test.c
+++ b/tools/testing/selftests/mm/mremap_test.c
@@ -373,7 +373,10 @@ static bool is_multiple_vma_range_ok(unsigned int pattern_seed,
 	for (i = 0; i <= 10; i += 2) {
 		int j;
 		char *buf = &ptr[i * page_size];
-		size_t size = i == 4 ? 2 * page_size : page_size;
+		size_t size = i == 4 ? 3 * page_size : page_size;
+
+		if (i == 6)
+			continue;
 
 		for (j = 0; j < size; j++) {
 			char chr = rand();
-- 
2.53.0

[PATCH RESEND 5/9] selftests/mm: mremap_test: fail on data corruption

[Sarthak Sharma]

In mremap_move_multiple_vmas_split(), if data is corrupted
after mremap, the control jumps to cleanup without marking
the test as failed.

Set success to false before moving to cleanup.

Fixes: 7d6597dfef11 ("tools/testing/selftests: explicitly test split multi VMA mremap move")
Signed-off-by: Sarthak Sharma <sarthak.sharma@arm.com>
---
 tools/testing/selftests/mm/mremap_test.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/testing/selftests/mm/mremap_test.c b/tools/testing/selftests/mm/mremap_test.c
index 869df22d46db..3b42c4f2473c 100644
--- a/tools/testing/selftests/mm/mremap_test.c
+++ b/tools/testing/selftests/mm/mremap_test.c
@@ -691,6 +691,7 @@ static void mremap_move_multiple_vmas_split(unsigned int pattern_seed,
 			if (chr != buf[j]) {
 				ksft_print_msg("page %d offset %d corrupted, expected %d got %d\n",
 					       i, j, chr, buf[j]);
+				success = false;
 				goto out_unmap;
 			}
 		}
-- 
2.53.0

[PATCH RESEND 6/9] selftests/mm: mremap_test: replace random data with deterministic pattern

[Sarthak Sharma]

mremap_test uses a random data stream to detect corruption after remap
operations. This requires seed handling and byte by byte validation,
which is inefficient.

Replace it with a deterministic pattern where every word in a page
contains its one-based page index. Use memcpy() and memcmp() to
initialize and validate the mappings. Remove the pattern seed and
its command line option. Also update the comment diagrams to
reflect the new deterministic pattern.

Suggested-by: David Hildenbrand (Arm) <david@kernel.org>
Signed-off-by: Sarthak Sharma <sarthak.sharma@arm.com>
---
 tools/testing/selftests/mm/mremap_test.c | 251 +++++++++--------------
 1 file changed, 95 insertions(+), 156 deletions(-)

diff --git a/tools/testing/selftests/mm/mremap_test.c b/tools/testing/selftests/mm/mremap_test.c
index 3b42c4f2473c..7b4c9ab3bf14 100644
--- a/tools/testing/selftests/mm/mremap_test.c
+++ b/tools/testing/selftests/mm/mremap_test.c
@@ -15,6 +15,7 @@
 #include <syscall.h>
 #include <time.h>
 #include <stdbool.h>
+#include <stdint.h>
 
 #include "kselftest.h"
 
@@ -61,6 +62,9 @@ enum {
 	PUD = _1GB,
 };
 
+static uint32_t *pattern;
+static size_t pattern_size;
+
 #define PTE page_size
 
 #define MAKE_TEST(source_align, destination_align, size,	\
@@ -311,11 +315,11 @@ static void mremap_expand_merge_offset(FILE *maps_fp, unsigned long page_size)
  *
  * |DDDDddddSSSSssss|
  */
-static void mremap_move_within_range(unsigned int pattern_seed, char *rand_addr)
+static void mremap_move_within_range(void)
 {
 	char *test_name = "mremap move within range";
 	void *src, *dest;
-	unsigned int i, success = 1;
+	unsigned int success = 1;
 
 	size_t size = SIZE_MB(20);
 	void *ptr = mmap(NULL, size, PROT_READ | PROT_WRITE,
@@ -331,12 +335,12 @@ static void mremap_move_within_range(unsigned int pattern_seed, char *rand_addr)
 	src = (void *)((unsigned long)src & ~(SIZE_MB(2) - 1));
 
 	/* Set byte pattern for source block. */
-	memcpy(src, rand_addr, SIZE_MB(2));
+	memcpy(src, (char *)pattern + SIZE_MB(2), SIZE_MB(2));
 
 	dest = src - SIZE_MB(2);
 
 	void *new_ptr = mremap(src + SIZE_MB(1), SIZE_MB(1), SIZE_MB(1),
-						   MREMAP_MAYMOVE | MREMAP_FIXED, dest + SIZE_MB(1));
+			       MREMAP_MAYMOVE | MREMAP_FIXED, dest + SIZE_MB(1));
 	if (new_ptr == MAP_FAILED) {
 		ksft_perror("mremap");
 		success = 0;
@@ -344,17 +348,9 @@ static void mremap_move_within_range(unsigned int pattern_seed, char *rand_addr)
 	}
 
 	/* Verify byte pattern after remapping */
-	srand(pattern_seed);
-	for (i = 0; i < SIZE_MB(1); i++) {
-		char c = (char) rand();
-
-		if (((char *)src)[i] != c) {
-			ksft_print_msg("Data at src at %d got corrupted due to unrelated mremap\n",
-				       i);
-			ksft_print_msg("Expected: %#x\t Got: %#x\n", c & 0xff,
-					((char *) src)[i] & 0xff);
-			success = 0;
-		}
+	if (memcmp(src, (char *)pattern + SIZE_MB(2), SIZE_MB(1))) {
+		ksft_print_msg("Source data was corrupted\n");
+		success = 0;
 	}
 
 out:
@@ -364,37 +360,27 @@ static void mremap_move_within_range(unsigned int pattern_seed, char *rand_addr)
 	ksft_test_result(success, "%s\n", test_name);
 }
 
-static bool is_multiple_vma_range_ok(unsigned int pattern_seed,
-				     char *ptr, unsigned long page_size)
+static bool is_multiple_vma_range_ok(char *ptr, unsigned long page_size)
 {
 	int i;
 
-	srand(pattern_seed);
 	for (i = 0; i <= 10; i += 2) {
-		int j;
-		char *buf = &ptr[i * page_size];
 		size_t size = i == 4 ? 3 * page_size : page_size;
 
 		if (i == 6)
 			continue;
 
-		for (j = 0; j < size; j++) {
-			char chr = rand();
-
-			if (chr != buf[j]) {
-				ksft_print_msg("page %d offset %d corrupted, expected %d got %d\n",
-					       i, j, chr, buf[j]);
-				return false;
-			}
+		if (memcmp(ptr + i * page_size, (char *)pattern + i * page_size,
+			   size)) {
+			ksft_print_msg("Data in VMA starting at page %d got corrupted\n",
+				       i);
+			return false;
 		}
 	}
-
 	return true;
 }
 
-static void mremap_move_multiple_vmas(unsigned int pattern_seed,
-				      unsigned long page_size,
-				      bool dont_unmap)
+static void mremap_move_multiple_vmas(unsigned long page_size, bool dont_unmap)
 {
 	int mremap_flags = MREMAP_FIXED | MREMAP_MAYMOVE;
 	char *test_name = "mremap move multiple vmas";
@@ -427,13 +413,15 @@ static void mremap_move_multiple_vmas(unsigned int pattern_seed,
 		goto out_unmap;
 	}
 
+	memcpy(ptr, pattern, size);
+
 	/*
 	 * Unmap so we end up with:
 	 *
 	 *  0   2   4 5 6   8   10 offset in buffer
 	 * |*| |*| |*****| |*| |*|
 	 * |*| |*| |*****| |*| |*|
-	 *  0   1   2 3 4   5   6  pattern offset
+	 *  0   2   4 5 6   8   10  pattern offset
 	 */
 	for (i = 1; i < 10; i += 2) {
 		if (i == 5)
@@ -446,18 +434,6 @@ static void mremap_move_multiple_vmas(unsigned int pattern_seed,
 		}
 	}
 
-	srand(pattern_seed);
-
-	/* Set up random patterns. */
-	for (i = 0; i <= 10; i += 2) {
-		int j;
-		size_t size = i == 4 ? 2 * page_size : page_size;
-		char *buf = &ptr[i * page_size];
-
-		for (j = 0; j < size; j++)
-			buf[j] = rand();
-	}
-
 	/* First, just move the whole thing. */
 	if (mremap(ptr, size, size, mremap_flags, tgt_ptr) == MAP_FAILED) {
 		ksft_perror("mremap");
@@ -465,7 +441,7 @@ static void mremap_move_multiple_vmas(unsigned int pattern_seed,
 		goto out_unmap;
 	}
 	/* Check move was ok. */
-	if (!is_multiple_vma_range_ok(pattern_seed, tgt_ptr, page_size)) {
+	if (!is_multiple_vma_range_ok(tgt_ptr, page_size)) {
 		success = false;
 		goto out_unmap;
 	}
@@ -478,7 +454,7 @@ static void mremap_move_multiple_vmas(unsigned int pattern_seed,
 		goto out_unmap;
 	}
 	/* Check that the move is ok. */
-	if (!is_multiple_vma_range_ok(pattern_seed, &tgt_ptr[size], page_size)) {
+	if (!is_multiple_vma_range_ok(&tgt_ptr[size], page_size)) {
 		success = false;
 		goto out_unmap;
 	}
@@ -498,7 +474,7 @@ static void mremap_move_multiple_vmas(unsigned int pattern_seed,
 		goto out_unmap;
 	}
 	/* Check that the move is ok. */
-	if (!is_multiple_vma_range_ok(pattern_seed, tgt_ptr, page_size)) {
+	if (!is_multiple_vma_range_ok(tgt_ptr, page_size)) {
 		success = false;
 		goto out_unmap;
 	}
@@ -587,8 +563,7 @@ static void mremap_shrink_multiple_vmas(unsigned long page_size,
 	ksft_test_result(success, "%s%s\n", test_name, inplace ? " [inplace]" : "");
 }
 
-static void mremap_move_multiple_vmas_split(unsigned int pattern_seed,
-					    unsigned long page_size,
+static void mremap_move_multiple_vmas_split(unsigned long page_size,
 					    bool dont_unmap)
 {
 	char *test_name = "mremap move multiple vmas split";
@@ -622,13 +597,15 @@ static void mremap_move_multiple_vmas_split(unsigned int pattern_seed,
 		goto out_unmap;
 	}
 
+	memcpy(ptr, pattern, size);
+
 	/*
 	 * Unmap so we end up with:
 	 *
 	 *  0 1 2 3 4 5 6 7 8 9 10 offset in buffer
 	 * |**********| |*******|
 	 * |**********| |*******|
-	 *  0 1 2 3 4   5 6 7 8 9  pattern offset
+	 *  0 1 2 3 4 5 6 7 8 9 10  pattern offset
 	 */
 	if (munmap(&ptr[5 * page_size], page_size)) {
 		ksft_perror("munmap");
@@ -636,19 +613,6 @@ static void mremap_move_multiple_vmas_split(unsigned int pattern_seed,
 		goto out_unmap;
 	}
 
-	/* Set up random patterns. */
-	srand(pattern_seed);
-	for (i = 0; i < 10; i++) {
-		int j;
-		char *buf = &ptr[i * page_size];
-
-		if (i == 5)
-			continue;
-
-		for (j = 0; j < page_size; j++)
-			buf[j] = rand();
-	}
-
 	/*
 	 * Move the below:
 	 *
@@ -656,14 +620,14 @@ static void mremap_move_multiple_vmas_split(unsigned int pattern_seed,
 	 *  0 1 2 3 4 5 6 7 8 9 10 offset in buffer
 	 * |**********| |*******|
 	 * |**********| |*******|
-	 *  0 1 2 3 4   5 6 7 8 9  pattern offset
+	 *  0 1 2 3 4 5 6 7 8 9 10  pattern offset
 	 *
 	 * Into:
 	 *
 	 * 0 1 2 3 4 5 6 7 offset in buffer
 	 * |*****| |*****|
 	 * |*****| |*****|
-	 * 2 3 4   5 6 7   pattern offset
+	 * 2 3 4 5 6 7 8 9 pattern offset
 	 */
 	if (mremap(&ptr[2 * page_size], size - 3 * page_size, size - 3 * page_size,
 		   mremap_flags, tgt_ptr) == MAP_FAILED) {
@@ -672,28 +636,16 @@ static void mremap_move_multiple_vmas_split(unsigned int pattern_seed,
 		goto out_unmap;
 	}
 
-	/* Offset into random pattern. */
-	srand(pattern_seed);
-	for (i = 0; i < 2 * page_size; i++)
-		rand();
-
 	/* Check pattern. */
 	for (i = 0; i < 7; i++) {
-		int j;
-		char *buf = &tgt_ptr[i * page_size];
-
 		if (i == 3)
 			continue;
 
-		for (j = 0; j < page_size; j++) {
-			char chr = rand();
-
-			if (chr != buf[j]) {
-				ksft_print_msg("page %d offset %d corrupted, expected %d got %d\n",
-					       i, j, chr, buf[j]);
-				success = false;
-				goto out_unmap;
-			}
+		if (memcmp(tgt_ptr + i * page_size,
+			   (char *)pattern + (i + 2) * page_size, page_size)) {
+			ksft_print_msg("Data in page %d got corrupted\n", i + 2);
+			success = false;
+			goto out_unmap;
 		}
 	}
 
@@ -946,13 +898,13 @@ static void mremap_move_multi_invalid_vmas(FILE *maps_fp, unsigned long page_siz
 #endif /* __NR_userfaultfd */
 
 /* Returns the time taken for the remap on success else returns -1. */
-static long long remap_region(struct config c, unsigned int threshold_mb,
-			      char *rand_addr)
+static long long remap_region(struct config c, unsigned int threshold_mb)
 {
 	void *addr, *tmp_addr, *src_addr, *dest_addr, *dest_preamble_addr = NULL;
 	struct timespec t_start = {0, 0}, t_end = {0, 0};
 	long long  start_ns, end_ns, align_mask, ret, offset;
 	unsigned long long threshold;
+	char *preamble_pattern;
 
 	if (threshold_mb == VALIDATION_NO_THRESHOLD)
 		threshold = c.region_size;
@@ -966,7 +918,7 @@ static long long remap_region(struct config c, unsigned int threshold_mb,
 	}
 
 	/* Set byte pattern for source block. */
-	memcpy(src_addr, rand_addr, threshold);
+	memcpy(src_addr, pattern, threshold);
 
 	/* Mask to zero out lower bits of address for alignment */
 	align_mask = ~(c.dest_alignment - 1);
@@ -1006,8 +958,9 @@ static long long remap_region(struct config c, unsigned int threshold_mb,
 			goto clean_up_src;
 		}
 
+		preamble_pattern = (char *)pattern + pattern_size - c.dest_preamble_size;
 		/* Set byte pattern for the dest preamble block. */
-		memcpy(dest_preamble_addr, rand_addr, c.dest_preamble_size);
+		memcpy(dest_preamble_addr, preamble_pattern, c.dest_preamble_size);
 	}
 
 	clock_gettime(CLOCK_MONOTONIC, &t_start);
@@ -1022,7 +975,7 @@ static long long remap_region(struct config c, unsigned int threshold_mb,
 	}
 
 	/* Verify byte pattern after remapping */
-	if (memcmp(dest_addr, rand_addr, threshold)) {
+	if (memcmp(dest_addr, pattern, threshold)) {
 		ksft_print_msg("Data after remap doesn't match\n");
 		ret = -1;
 		goto clean_up_dest;
@@ -1030,7 +983,7 @@ static long long remap_region(struct config c, unsigned int threshold_mb,
 
 	/* Verify the dest preamble byte pattern after remapping */
 	if (c.dest_preamble_size &&
-	    memcmp(dest_preamble_addr, rand_addr, c.dest_preamble_size)) {
+	    memcmp(dest_preamble_addr, preamble_pattern, c.dest_preamble_size)) {
 		ksft_print_msg("Preamble data after remap doesn't match\n");
 		ret = -1;
 		goto clean_up_dest;
@@ -1062,12 +1015,11 @@ static long long remap_region(struct config c, unsigned int threshold_mb,
  * the beginning of the mapping just because the aligned
  * down address landed on a mapping that maybe does not exist.
  */
-static void mremap_move_1mb_from_start(unsigned int pattern_seed,
-				       char *rand_addr)
+static void mremap_move_1mb_from_start(void)
 {
 	char *test_name = "mremap move 1mb from start at 1MB+256KB aligned src";
 	void *src = NULL, *dest = NULL;
-	unsigned int i, success = 1;
+	unsigned int success = 1;
 
 	/* Config to reuse get_source_mapping() to do an aligned mmap. */
 	struct config c = {
@@ -1089,7 +1041,7 @@ static void mremap_move_1mb_from_start(unsigned int pattern_seed,
 	}
 
 	/* Set byte pattern for source block. */
-	memcpy(src, rand_addr, SIZE_MB(2));
+	memcpy(src, pattern, SIZE_MB(2));
 
 	/*
 	 * Unmap the beginning of dest so that the aligned address
@@ -1098,7 +1050,7 @@ static void mremap_move_1mb_from_start(unsigned int pattern_seed,
 	munmap(dest, SIZE_MB(1));
 
 	void *new_ptr = mremap(src + SIZE_MB(1), SIZE_MB(1), SIZE_MB(1),
-						   MREMAP_MAYMOVE | MREMAP_FIXED, dest + SIZE_MB(1));
+			       MREMAP_MAYMOVE | MREMAP_FIXED, dest + SIZE_MB(1));
 	if (new_ptr == MAP_FAILED) {
 		ksft_perror("mremap");
 		success = 0;
@@ -1106,17 +1058,9 @@ static void mremap_move_1mb_from_start(unsigned int pattern_seed,
 	}
 
 	/* Verify byte pattern after remapping */
-	srand(pattern_seed);
-	for (i = 0; i < SIZE_MB(1); i++) {
-		char c = (char) rand();
-
-		if (((char *)src)[i] != c) {
-			ksft_print_msg("Data at src at %d got corrupted due to unrelated mremap\n",
-				       i);
-			ksft_print_msg("Expected: %#x\t Got: %#x\n", c & 0xff,
-					((char *) src)[i] & 0xff);
-			success = 0;
-		}
+	if (memcmp(src, pattern, SIZE_MB(1))) {
+		ksft_print_msg("Data before the remapped range was corrupted\n");
+		success = 0;
 	}
 
 out:
@@ -1129,11 +1073,10 @@ static void mremap_move_1mb_from_start(unsigned int pattern_seed,
 	ksft_test_result(success, "%s\n", test_name);
 }
 
-static void run_mremap_test_case(struct test test_case, unsigned int threshold_mb,
-				 char *rand_addr)
+static void run_mremap_test_case(struct test test_case,
+				 unsigned int threshold_mb)
 {
-	long long remap_time = remap_region(test_case.config, threshold_mb,
-					    rand_addr);
+	long long remap_time = remap_region(test_case.config, threshold_mb);
 
 	if (remap_time < 0) {
 		if (test_case.expect_failure)
@@ -1160,29 +1103,22 @@ static void run_mremap_test_case(struct test test_case, unsigned int threshold_m
 
 static void usage(const char *cmd)
 {
-	ksft_print_msg("Usage: %s [[-t <threshold_mb>] [-p <pattern_seed>]]\n", cmd);
+	ksft_print_msg("Usage: %s [-t <threshold_mb>]\n", cmd);
 	ksft_print_msg("-t\t only validate threshold_mb of the remapped region\n");
 	ksft_print_msg("  \t if 0 is supplied no threshold is used; all tests\n");
 	ksft_print_msg("  \t are run and remapped regions validated fully.\n");
 	ksft_print_msg("  \t The default threshold used is 4MB.\n");
-	ksft_print_msg("-p\t provide a seed to generate the random pattern for\n");
-	ksft_print_msg("  \t validating the remapped region.\n");
 }
 
-static int parse_args(int argc, char **argv, unsigned int *threshold_mb,
-		      unsigned int *pattern_seed)
+static int parse_args(int argc, char **argv, unsigned int *threshold_mb)
 {
-	const char *optstr = "t:p:";
 	int opt;
 
-	while ((opt = getopt(argc, argv, optstr)) != -1) {
+	while ((opt = getopt(argc, argv, "t:")) != -1) {
 		switch (opt) {
 		case 't':
 			*threshold_mb = atoi(optarg);
 			break;
-		case 'p':
-			*pattern_seed = atoi(optarg);
-			break;
 		default:
 			usage(argv[0]);
 			return -1;
@@ -1197,6 +1133,21 @@ static int parse_args(int argc, char **argv, unsigned int *threshold_mb,
 	return 0;
 }
 
+static void fill_pattern(uint32_t *pattern, size_t pattern_size, size_t page_size)
+{
+	size_t nr_pages = pattern_size / page_size;
+	size_t words_per_page = page_size / sizeof(uint32_t);
+	size_t page, word;
+
+	for (page = 0; page < nr_pages; page++) {
+		uint32_t *page_addr = pattern + page * words_per_page;
+		uint32_t val = page + 1;
+
+		for (word = 0; word < words_per_page; word++)
+			page_addr[word] = val;
+	}
+}
+
 #define MAX_TEST 15
 #define MAX_PERF_TEST 3
 int main(int argc, char **argv)
@@ -1210,57 +1161,46 @@ int main(int argc, char **argv)
 	size_t max_test_constant_region_size = _2MB;
 	size_t dest_preamble_size = 10 * _4MB;
 
-	unsigned int pattern_seed;
-	char *rand_addr;
-	size_t rand_size;
 	int num_expand_tests = 2;
 	int num_misc_tests = 9;
 	struct test test_cases[MAX_TEST] = {};
 	struct test perf_test_cases[MAX_PERF_TEST];
 	int page_size;
-	time_t t;
 	FILE *maps_fp;
 
 	ksft_print_header();
 
 	get_mmap_min_addr();
 
-	pattern_seed = (unsigned int) time(&t);
-
-	if (parse_args(argc, argv, &threshold_mb, &pattern_seed) < 0)
+	if (parse_args(argc, argv, &threshold_mb) < 0)
 		ksft_exit_fail_msg("Invalid arguments\n");
 
 	ksft_print_msg("Test configs:\n");
 	ksft_print_msg("threshold_mb=%u\n", threshold_mb);
-	ksft_print_msg("pattern_seed=%u\n", pattern_seed);
 
 	/*
-	 * set preallocated random array according to test configs; see the
-	 * functions for the logic of setting the size
+	 * Set a preallocated array where page[i] contains i+1
 	 */
 	if (!threshold_mb)
-		rand_size = MAX(max_test_variable_region_size,
-				max_test_constant_region_size);
+		pattern_size = MAX(max_test_variable_region_size,
+				   max_test_constant_region_size);
 	else
-		rand_size = MAX(MIN(threshold_mb * _1MB,
-				    max_test_variable_region_size),
-				max_test_constant_region_size);
-	rand_size = MAX(dest_preamble_size, rand_size);
-
-	rand_addr = (char *)mmap(NULL, rand_size, PROT_READ | PROT_WRITE,
-				 MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
-	if (rand_addr == MAP_FAILED) {
+		pattern_size = MAX(MIN(threshold_mb * _1MB,
+				       max_test_variable_region_size),
+				   max_test_constant_region_size);
+	pattern_size = MAX(dest_preamble_size, pattern_size);
+
+	pattern = mmap(NULL, pattern_size, PROT_READ | PROT_WRITE,
+		       MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+	if (pattern == MAP_FAILED) {
 		ksft_perror("mmap");
-		ksft_exit_fail_msg("cannot mmap rand_addr\n");
+		ksft_exit_fail_msg("cannot mmap pattern\n");
 	}
 
-	/* fill stream of random bytes */
-	srand(pattern_seed);
-	for (unsigned long i = 0; i < rand_size; ++i)
-		rand_addr[i] = (char) rand();
-
 	page_size = sysconf(_SC_PAGESIZE);
 
+	fill_pattern(pattern, pattern_size, page_size);
+
 	/* Expected mremap failures */
 	test_cases[0] =	MAKE_TEST(page_size, page_size, page_size,
 				  OVERLAPPING, EXPECT_FAILURE,
@@ -1329,26 +1269,26 @@ int main(int argc, char **argv)
 		      ARRAY_SIZE(perf_test_cases) : 0) + num_expand_tests + num_misc_tests);
 
 	for (i = 0; i < ARRAY_SIZE(test_cases); i++)
-		run_mremap_test_case(test_cases[i], threshold_mb, rand_addr);
+		run_mremap_test_case(test_cases[i], threshold_mb);
 
 	maps_fp = fopen("/proc/self/maps", "r");
 
 	if (maps_fp == NULL) {
-		munmap(rand_addr, rand_size);
+		munmap(pattern, pattern_size);
 		ksft_exit_fail_msg("Failed to read /proc/self/maps: %s\n", strerror(errno));
 	}
 
 	mremap_expand_merge(maps_fp, page_size);
 	mremap_expand_merge_offset(maps_fp, page_size);
 
-	mremap_move_within_range(pattern_seed, rand_addr);
-	mremap_move_1mb_from_start(pattern_seed, rand_addr);
+	mremap_move_within_range();
+	mremap_move_1mb_from_start();
 	mremap_shrink_multiple_vmas(page_size, /* inplace= */true);
 	mremap_shrink_multiple_vmas(page_size, /* inplace= */false);
-	mremap_move_multiple_vmas(pattern_seed, page_size, /* dontunmap= */ false);
-	mremap_move_multiple_vmas(pattern_seed, page_size, /* dontunmap= */ true);
-	mremap_move_multiple_vmas_split(pattern_seed, page_size, /* dontunmap= */ false);
-	mremap_move_multiple_vmas_split(pattern_seed, page_size, /* dontunmap= */ true);
+	mremap_move_multiple_vmas(page_size, /* dontunmap= */ false);
+	mremap_move_multiple_vmas(page_size, /* dontunmap= */ true);
+	mremap_move_multiple_vmas_split(page_size, /* dontunmap= */ false);
+	mremap_move_multiple_vmas_split(page_size, /* dontunmap= */ true);
 	mremap_move_multi_invalid_vmas(maps_fp, page_size);
 
 	fclose(maps_fp);
@@ -1357,11 +1297,10 @@ int main(int argc, char **argv)
 		ksft_print_msg("%s\n",
 			       "mremap HAVE_MOVE_PMD/PUD optimization time comparison for 1GB region:");
 		for (i = 0; i < ARRAY_SIZE(perf_test_cases); i++)
-			run_mremap_test_case(perf_test_cases[i], threshold_mb,
-					     rand_addr);
+			run_mremap_test_case(perf_test_cases[i], threshold_mb);
 	}
 
-	munmap(rand_addr, rand_size);
+	munmap(pattern, pattern_size);
 
 	ksft_finished();
 }
-- 
2.53.0

[PATCH RESEND 7/9] selftests/mm: mremap_test: remove perf tests and timing

[Sarthak Sharma]

mremap_test contains three perf tests that report the time
taken to move 1GB mappings with different page table level
alignments. Although they validate the remapped data,
their main purpose it to report timing information. The
recorded time also does not affect the test result. Also,
they are skipped with the default validation threshold.

Remove these tests and the timing infrastructure from
mremap_test.c. The regular tests continue to cover PUD
and PMD aligned remaps.

Suggested-by: David Hildenbrand (Arm) <david@kernel.org>
Signed-off-by: Sarthak Sharma <sarthak.sharma@arm.com>
---
 tools/testing/selftests/mm/mremap_test.c | 61 +++---------------------
 1 file changed, 7 insertions(+), 54 deletions(-)

diff --git a/tools/testing/selftests/mm/mremap_test.c b/tools/testing/selftests/mm/mremap_test.c
index 7b4c9ab3bf14..04ea072f8cd1 100644
--- a/tools/testing/selftests/mm/mremap_test.c
+++ b/tools/testing/selftests/mm/mremap_test.c
@@ -13,7 +13,6 @@
 #include <sys/ioctl.h>
 #include <sys/mman.h>
 #include <syscall.h>
-#include <time.h>
 #include <stdbool.h>
 #include <stdint.h>
 
@@ -23,7 +22,6 @@
 #define EXPECT_FAILURE 1
 #define NON_OVERLAPPING 0
 #define OVERLAPPING 1
-#define NS_PER_SEC 1000000000ULL
 #define VALIDATION_DEFAULT_THRESHOLD 4	/* 4MB */
 #define VALIDATION_NO_THRESHOLD 0	/* Verify the entire region */
 
@@ -897,12 +895,11 @@ static void mremap_move_multi_invalid_vmas(FILE *maps_fp, unsigned long page_siz
 }
 #endif /* __NR_userfaultfd */
 
-/* Returns the time taken for the remap on success else returns -1. */
-static long long remap_region(struct config c, unsigned int threshold_mb)
+static int remap_region(struct config c, unsigned int threshold_mb)
 {
 	void *addr, *tmp_addr, *src_addr, *dest_addr, *dest_preamble_addr = NULL;
-	struct timespec t_start = {0, 0}, t_end = {0, 0};
-	long long  start_ns, end_ns, align_mask, ret, offset;
+	long long align_mask, offset;
+	int ret = 0;
 	unsigned long long threshold;
 	char *preamble_pattern;
 
@@ -963,10 +960,8 @@ static long long remap_region(struct config c, unsigned int threshold_mb)
 		memcpy(dest_preamble_addr, preamble_pattern, c.dest_preamble_size);
 	}
 
-	clock_gettime(CLOCK_MONOTONIC, &t_start);
 	dest_addr = mremap(src_addr, c.region_size, c.region_size,
 					  MREMAP_MAYMOVE|MREMAP_FIXED, (char *) addr);
-	clock_gettime(CLOCK_MONOTONIC, &t_end);
 
 	if (dest_addr == MAP_FAILED) {
 		ksft_print_msg("mremap failed: %s\n", strerror(errno));
@@ -989,16 +984,7 @@ static long long remap_region(struct config c, unsigned int threshold_mb)
 		goto clean_up_dest;
 	}
 
-	start_ns = t_start.tv_sec * NS_PER_SEC + t_start.tv_nsec;
-	end_ns = t_end.tv_sec * NS_PER_SEC + t_end.tv_nsec;
-	ret = end_ns - start_ns;
-
-/*
- * Since the destination address is specified using MREMAP_FIXED, subsequent
- * mremap will unmap any previous mapping at the address range specified by
- * dest_addr and region_size. This significantly affects the remap time of
- * subsequent tests. So we clean up mappings after each test.
- */
+/* Clean up mappings after each test. */
 clean_up_dest:
 	munmap(dest_addr, c.region_size);
 clean_up_dest_preamble:
@@ -1076,9 +1062,9 @@ static void mremap_move_1mb_from_start(void)
 static void run_mremap_test_case(struct test test_case,
 				 unsigned int threshold_mb)
 {
-	long long remap_time = remap_region(test_case.config, threshold_mb);
+	int ret = remap_region(test_case.config, threshold_mb);
 
-	if (remap_time < 0) {
+	if (ret < 0) {
 		if (test_case.expect_failure)
 			ksft_test_result_xfail("%s: expected mremap failure\n",
 					       test_case.name);
@@ -1088,14 +1074,6 @@ static void run_mremap_test_case(struct test test_case,
 		if (test_case.expect_failure)
 			ksft_test_result_fail("%s: unexpected mremap success\n",
 					      test_case.name);
-		/*
-		 * Comparing mremap time is only applicable if entire region
-		 * was faulted in.
-		 */
-		else if (threshold_mb == VALIDATION_NO_THRESHOLD ||
-			 test_case.config.region_size <= threshold_mb * _1MB)
-			ksft_test_result_pass("%s: mremap time: %12lldns\n",
-					      test_case.name, remap_time);
 		else
 			ksft_test_result_pass("%s\n", test_case.name);
 	}
@@ -1149,11 +1127,9 @@ static void fill_pattern(uint32_t *pattern, size_t pattern_size, size_t page_siz
 }
 
 #define MAX_TEST 15
-#define MAX_PERF_TEST 3
 int main(int argc, char **argv)
 {
 	unsigned int i;
-	int run_perf_tests;
 	unsigned int threshold_mb = VALIDATION_DEFAULT_THRESHOLD;
 
 	/* hard-coded test configs */
@@ -1164,7 +1140,6 @@ int main(int argc, char **argv)
 	int num_expand_tests = 2;
 	int num_misc_tests = 9;
 	struct test test_cases[MAX_TEST] = {};
-	struct test perf_test_cases[MAX_PERF_TEST];
 	int page_size;
 	FILE *maps_fp;
 
@@ -1251,22 +1226,7 @@ int main(int argc, char **argv)
 				  "5MB mremap - Source 1MB-aligned, Dest 1MB-aligned with 40MB Preamble");
 	test_cases[14].config.dest_preamble_size = 10 * _4MB;
 
-	perf_test_cases[0] =  MAKE_TEST(page_size, page_size, _1GB, NON_OVERLAPPING, EXPECT_SUCCESS,
-					"1GB mremap - Source PTE-aligned, Destination PTE-aligned");
-	/*
-	 * mremap 1GB region - Page table level aligned time
-	 * comparison.
-	 */
-	perf_test_cases[1] = MAKE_TEST(PMD, PMD, _1GB, NON_OVERLAPPING, EXPECT_SUCCESS,
-				       "1GB mremap - Source PMD-aligned, Destination PMD-aligned");
-	perf_test_cases[2] = MAKE_TEST(PUD, PUD, _1GB, NON_OVERLAPPING, EXPECT_SUCCESS,
-				       "1GB mremap - Source PUD-aligned, Destination PUD-aligned");
-
-	run_perf_tests =  (threshold_mb == VALIDATION_NO_THRESHOLD) ||
-				(threshold_mb * _1MB >= _1GB);
-
-	ksft_set_plan(ARRAY_SIZE(test_cases) + (run_perf_tests ?
-		      ARRAY_SIZE(perf_test_cases) : 0) + num_expand_tests + num_misc_tests);
+	ksft_set_plan(ARRAY_SIZE(test_cases) + num_expand_tests + num_misc_tests);
 
 	for (i = 0; i < ARRAY_SIZE(test_cases); i++)
 		run_mremap_test_case(test_cases[i], threshold_mb);
@@ -1293,13 +1253,6 @@ int main(int argc, char **argv)
 
 	fclose(maps_fp);
 
-	if (run_perf_tests) {
-		ksft_print_msg("%s\n",
-			       "mremap HAVE_MOVE_PMD/PUD optimization time comparison for 1GB region:");
-		for (i = 0; i < ARRAY_SIZE(perf_test_cases); i++)
-			run_mremap_test_case(perf_test_cases[i], threshold_mb);
-	}
-
 	munmap(pattern, pattern_size);
 
 	ksft_finished();
-- 
2.53.0

[PATCH RESEND 8/9] selftests/mm: mremap_test: remove validation threshold

[Sarthak Sharma]

mremap_test validates only the first 4MB of large mappings by default.
The amount of data validated can be changed using the -t command line
option, with zero selecting the complete region.

Remove the validation threshold and its command line option. Always
initialize and validate the complete remapped region.

Suggested-by: David Hildenbrand (Arm) <david@kernel.org>
Signed-off-by: Sarthak Sharma <sarthak.sharma@arm.com>
---
 tools/testing/selftests/mm/mremap_test.c | 84 +++---------------------
 1 file changed, 8 insertions(+), 76 deletions(-)

diff --git a/tools/testing/selftests/mm/mremap_test.c b/tools/testing/selftests/mm/mremap_test.c
index 04ea072f8cd1..77e952556be1 100644
--- a/tools/testing/selftests/mm/mremap_test.c
+++ b/tools/testing/selftests/mm/mremap_test.c
@@ -22,13 +22,6 @@
 #define EXPECT_FAILURE 1
 #define NON_OVERLAPPING 0
 #define OVERLAPPING 1
-#define VALIDATION_DEFAULT_THRESHOLD 4	/* 4MB */
-#define VALIDATION_NO_THRESHOLD 0	/* Verify the entire region */
-
-#ifndef MIN
-#define MIN(X, Y) ((X) < (Y) ? (X) : (Y))
-#define MAX(X, Y) ((X) > (Y) ? (X) : (Y))
-#endif
 #define SIZE_MB(m) ((size_t)m * (1024 * 1024))
 #define SIZE_KB(k) ((size_t)k * 1024)
 
@@ -61,7 +54,7 @@ enum {
 };
 
 static uint32_t *pattern;
-static size_t pattern_size;
+static const size_t pattern_size = _2GB;
 
 #define PTE page_size
 
@@ -895,19 +888,13 @@ static void mremap_move_multi_invalid_vmas(FILE *maps_fp, unsigned long page_siz
 }
 #endif /* __NR_userfaultfd */
 
-static int remap_region(struct config c, unsigned int threshold_mb)
+static int remap_region(struct config c)
 {
 	void *addr, *tmp_addr, *src_addr, *dest_addr, *dest_preamble_addr = NULL;
 	long long align_mask, offset;
 	int ret = 0;
-	unsigned long long threshold;
 	char *preamble_pattern;
 
-	if (threshold_mb == VALIDATION_NO_THRESHOLD)
-		threshold = c.region_size;
-	else
-		threshold = MIN(threshold_mb * _1MB, c.region_size);
-
 	src_addr = get_source_mapping(c);
 	if (!src_addr) {
 		ret = -1;
@@ -915,7 +902,7 @@ static int remap_region(struct config c, unsigned int threshold_mb)
 	}
 
 	/* Set byte pattern for source block. */
-	memcpy(src_addr, pattern, threshold);
+	memcpy(src_addr, pattern, c.region_size);
 
 	/* Mask to zero out lower bits of address for alignment */
 	align_mask = ~(c.dest_alignment - 1);
@@ -970,7 +957,7 @@ static int remap_region(struct config c, unsigned int threshold_mb)
 	}
 
 	/* Verify byte pattern after remapping */
-	if (memcmp(dest_addr, pattern, threshold)) {
+	if (memcmp(dest_addr, pattern, c.region_size)) {
 		ksft_print_msg("Data after remap doesn't match\n");
 		ret = -1;
 		goto clean_up_dest;
@@ -1059,10 +1046,9 @@ static void mremap_move_1mb_from_start(void)
 	ksft_test_result(success, "%s\n", test_name);
 }
 
-static void run_mremap_test_case(struct test test_case,
-				 unsigned int threshold_mb)
+static void run_mremap_test_case(struct test test_case)
 {
-	int ret = remap_region(test_case.config, threshold_mb);
+	int ret = remap_region(test_case.config);
 
 	if (ret < 0) {
 		if (test_case.expect_failure)
@@ -1079,38 +1065,6 @@ static void run_mremap_test_case(struct test test_case,
 	}
 }
 
-static void usage(const char *cmd)
-{
-	ksft_print_msg("Usage: %s [-t <threshold_mb>]\n", cmd);
-	ksft_print_msg("-t\t only validate threshold_mb of the remapped region\n");
-	ksft_print_msg("  \t if 0 is supplied no threshold is used; all tests\n");
-	ksft_print_msg("  \t are run and remapped regions validated fully.\n");
-	ksft_print_msg("  \t The default threshold used is 4MB.\n");
-}
-
-static int parse_args(int argc, char **argv, unsigned int *threshold_mb)
-{
-	int opt;
-
-	while ((opt = getopt(argc, argv, "t:")) != -1) {
-		switch (opt) {
-		case 't':
-			*threshold_mb = atoi(optarg);
-			break;
-		default:
-			usage(argv[0]);
-			return -1;
-		}
-	}
-
-	if (optind < argc) {
-		usage(argv[0]);
-		return -1;
-	}
-
-	return 0;
-}
-
 static void fill_pattern(uint32_t *pattern, size_t pattern_size, size_t page_size)
 {
 	size_t nr_pages = pattern_size / page_size;
@@ -1127,16 +1081,9 @@ static void fill_pattern(uint32_t *pattern, size_t pattern_size, size_t page_siz
 }
 
 #define MAX_TEST 15
-int main(int argc, char **argv)
+int main(void)
 {
 	unsigned int i;
-	unsigned int threshold_mb = VALIDATION_DEFAULT_THRESHOLD;
-
-	/* hard-coded test configs */
-	size_t max_test_variable_region_size = _2GB;
-	size_t max_test_constant_region_size = _2MB;
-	size_t dest_preamble_size = 10 * _4MB;
-
 	int num_expand_tests = 2;
 	int num_misc_tests = 9;
 	struct test test_cases[MAX_TEST] = {};
@@ -1147,24 +1094,9 @@ int main(int argc, char **argv)
 
 	get_mmap_min_addr();
 
-	if (parse_args(argc, argv, &threshold_mb) < 0)
-		ksft_exit_fail_msg("Invalid arguments\n");
-
-	ksft_print_msg("Test configs:\n");
-	ksft_print_msg("threshold_mb=%u\n", threshold_mb);
-
 	/*
 	 * Set a preallocated array where page[i] contains i+1
 	 */
-	if (!threshold_mb)
-		pattern_size = MAX(max_test_variable_region_size,
-				   max_test_constant_region_size);
-	else
-		pattern_size = MAX(MIN(threshold_mb * _1MB,
-				       max_test_variable_region_size),
-				   max_test_constant_region_size);
-	pattern_size = MAX(dest_preamble_size, pattern_size);
-
 	pattern = mmap(NULL, pattern_size, PROT_READ | PROT_WRITE,
 		       MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
 	if (pattern == MAP_FAILED) {
@@ -1229,7 +1161,7 @@ int main(int argc, char **argv)
 	ksft_set_plan(ARRAY_SIZE(test_cases) + num_expand_tests + num_misc_tests);
 
 	for (i = 0; i < ARRAY_SIZE(test_cases); i++)
-		run_mremap_test_case(test_cases[i], threshold_mb);
+		run_mremap_test_case(test_cases[i]);
 
 	maps_fp = fopen("/proc/self/maps", "r");
 
-- 
2.53.0

[PATCH RESEND 9/9] selftests/mm: mremap_test: strengthen multi VMA validation

[Sarthak Sharma]

Tests in mremap_test.c where a multi VMA range is involved only
check if the data remains consistent across mapped pages after
remap. They do not verify the mapping state of gaps in the
remapped range.

Add helpers to check whether a range is mapped with the
expected permissions using /proc/self/maps. Use them to
verify that gaps remain unmapped after regular remaps and
retain PROT_NONE permissions when remapping over an
existing mapping with PROT_NONE permissions.

Signed-off-by: Sarthak Sharma <sarthak.sharma@arm.com>
---
 tools/testing/selftests/mm/mremap_test.c | 91 +++++++++++++++++++-----
 1 file changed, 72 insertions(+), 19 deletions(-)

diff --git a/tools/testing/selftests/mm/mremap_test.c b/tools/testing/selftests/mm/mremap_test.c
index 77e952556be1..1d9934b9a60b 100644
--- a/tools/testing/selftests/mm/mremap_test.c
+++ b/tools/testing/selftests/mm/mremap_test.c
@@ -124,13 +124,15 @@ static unsigned long long get_mmap_min_addr(void)
 }
 
 /*
- * Using /proc/self/maps, assert that the specified address range is contained
- * within a single mapping.
+ * Using /proc/self/maps, check whether the specified address range is contained
+ * within a single mapping with the expected permissions, if supplied.
  */
-static bool is_range_mapped(FILE *maps_fp, unsigned long start,
-			    unsigned long end)
+static bool is_range_mapped_with_perms(FILE *maps_fp, unsigned long start,
+				       unsigned long end,
+				       const char *expected_perms)
 {
 	char *line = NULL;
+	char perms[5];
 	size_t len = 0;
 	bool success = false;
 	unsigned long first_val, second_val;
@@ -138,12 +140,13 @@ static bool is_range_mapped(FILE *maps_fp, unsigned long start,
 	rewind(maps_fp);
 
 	while (getline(&line, &len, maps_fp) != -1) {
-		if (sscanf(line, "%lx-%lx", &first_val, &second_val) != 2) {
+		if (sscanf(line, "%lx-%lx %4s", &first_val, &second_val, perms) != 3) {
 			ksft_exit_fail_msg("cannot parse /proc/self/maps\n");
 			break;
 		}
 
-		if (first_val <= start && second_val >= end) {
+		if (first_val <= start && second_val >= end &&
+		    (!expected_perms || !strcmp(perms, expected_perms))) {
 			success = true;
 			fflush(maps_fp);
 			break;
@@ -154,6 +157,12 @@ static bool is_range_mapped(FILE *maps_fp, unsigned long start,
 	return success;
 }
 
+static bool is_range_mapped(FILE *maps_fp, unsigned long start,
+			    unsigned long end)
+{
+	return is_range_mapped_with_perms(maps_fp, start, end, NULL);
+}
+
 /* Check if [ptr, ptr + size) mapped in /proc/self/maps. */
 static bool is_ptr_mapped(FILE *maps_fp, void *ptr, unsigned long size)
 {
@@ -163,6 +172,20 @@ static bool is_ptr_mapped(FILE *maps_fp, void *ptr, unsigned long size)
 	return is_range_mapped(maps_fp, start, end);
 }
 
+/*
+ * Check if [ptr, ptr + size) is mapped with the required permissions in
+ * /proc/self/maps.
+ */
+static bool is_ptr_mapped_with_perms(FILE *maps_fp, void *ptr,
+				     unsigned long size,
+				     const char *expected_perms)
+{
+	unsigned long start = (unsigned long)ptr;
+	unsigned long end = start + size;
+
+	return is_range_mapped_with_perms(maps_fp, start, end, expected_perms);
+}
+
 /*
  * Returns the start address of the mapping on success, else returns
  * NULL on failure.
@@ -351,16 +374,37 @@ static void mremap_move_within_range(void)
 	ksft_test_result(success, "%s\n", test_name);
 }
 
-static bool is_multiple_vma_range_ok(char *ptr, unsigned long page_size)
+static bool is_multiple_vma_range_ok(FILE *maps_fp, char *ptr,
+				     unsigned long page_size, bool expect_gaps_mapped)
 {
 	int i;
 
-	for (i = 0; i <= 10; i += 2) {
+	for (i = 0; i <= 10; i++) {
 		size_t size = i == 4 ? 3 * page_size : page_size;
 
-		if (i == 6)
+		/* Pages 4, 5 and 6 are part of the same VMA. */
+		if (i == 5 || i == 6)
 			continue;
 
+		/* Odd pages correspond to gaps between the source VMAs. */
+		if (i & 1) {
+			if (expect_gaps_mapped) {
+				if (!is_ptr_mapped_with_perms(maps_fp,
+							      ptr + i * page_size,
+							      page_size, "---p")) {
+					ksft_print_msg("Page %d PROT_NONE mapping overwritten\n",
+						       i);
+					return false;
+				}
+			} else if (is_ptr_mapped(maps_fp, ptr + i * page_size,
+						 page_size)) {
+				ksft_print_msg("Page %d is unexpectedly mapped\n", i);
+				return false;
+			}
+
+			continue;
+		}
+
 		if (memcmp(ptr + i * page_size, (char *)pattern + i * page_size,
 			   size)) {
 			ksft_print_msg("Data in VMA starting at page %d got corrupted\n",
@@ -371,7 +415,8 @@ static bool is_multiple_vma_range_ok(char *ptr, unsigned long page_size)
 	return true;
 }
 
-static void mremap_move_multiple_vmas(unsigned long page_size, bool dont_unmap)
+static void mremap_move_multiple_vmas(FILE *maps_fp, unsigned long page_size,
+				      bool dont_unmap)
 {
 	int mremap_flags = MREMAP_FIXED | MREMAP_MAYMOVE;
 	char *test_name = "mremap move multiple vmas";
@@ -432,7 +477,7 @@ static void mremap_move_multiple_vmas(unsigned long page_size, bool dont_unmap)
 		goto out_unmap;
 	}
 	/* Check move was ok. */
-	if (!is_multiple_vma_range_ok(tgt_ptr, page_size)) {
+	if (!is_multiple_vma_range_ok(maps_fp, tgt_ptr, page_size, false)) {
 		success = false;
 		goto out_unmap;
 	}
@@ -445,7 +490,7 @@ static void mremap_move_multiple_vmas(unsigned long page_size, bool dont_unmap)
 		goto out_unmap;
 	}
 	/* Check that the move is ok. */
-	if (!is_multiple_vma_range_ok(&tgt_ptr[size], page_size)) {
+	if (!is_multiple_vma_range_ok(maps_fp, &tgt_ptr[size], page_size, false)) {
 		success = false;
 		goto out_unmap;
 	}
@@ -465,7 +510,7 @@ static void mremap_move_multiple_vmas(unsigned long page_size, bool dont_unmap)
 		goto out_unmap;
 	}
 	/* Check that the move is ok. */
-	if (!is_multiple_vma_range_ok(tgt_ptr, page_size)) {
+	if (!is_multiple_vma_range_ok(maps_fp, tgt_ptr, page_size, true)) {
 		success = false;
 		goto out_unmap;
 	}
@@ -554,7 +599,8 @@ static void mremap_shrink_multiple_vmas(unsigned long page_size,
 	ksft_test_result(success, "%s%s\n", test_name, inplace ? " [inplace]" : "");
 }
 
-static void mremap_move_multiple_vmas_split(unsigned long page_size,
+static void mremap_move_multiple_vmas_split(FILE *maps_fp,
+					    unsigned long page_size,
 					    bool dont_unmap)
 {
 	char *test_name = "mremap move multiple vmas split";
@@ -629,8 +675,15 @@ static void mremap_move_multiple_vmas_split(unsigned long page_size,
 
 	/* Check pattern. */
 	for (i = 0; i < 7; i++) {
-		if (i == 3)
+		if (i == 3) {
+			if (is_ptr_mapped(maps_fp, tgt_ptr + i * page_size,
+					  page_size)) {
+				ksft_print_msg("Page %d is unexpectedly mapped\n", i);
+				success = false;
+				goto out_unmap;
+			}
 			continue;
+		}
 
 		if (memcmp(tgt_ptr + i * page_size,
 			   (char *)pattern + (i + 2) * page_size, page_size)) {
@@ -1177,10 +1230,10 @@ int main(void)
 	mremap_move_1mb_from_start();
 	mremap_shrink_multiple_vmas(page_size, /* inplace= */true);
 	mremap_shrink_multiple_vmas(page_size, /* inplace= */false);
-	mremap_move_multiple_vmas(page_size, /* dontunmap= */ false);
-	mremap_move_multiple_vmas(page_size, /* dontunmap= */ true);
-	mremap_move_multiple_vmas_split(page_size, /* dontunmap= */ false);
-	mremap_move_multiple_vmas_split(page_size, /* dontunmap= */ true);
+	mremap_move_multiple_vmas(maps_fp, page_size, /* dontunmap= */ false);
+	mremap_move_multiple_vmas(maps_fp, page_size, /* dontunmap= */ true);
+	mremap_move_multiple_vmas_split(maps_fp, page_size, /* dontunmap= */ false);
+	mremap_move_multiple_vmas_split(maps_fp, page_size, /* dontunmap= */ true);
 	mremap_move_multi_invalid_vmas(maps_fp, page_size);
 
 	fclose(maps_fp);
-- 
2.53.0

Re: [PATCH RESEND 0/9] selftests/mm: improve mremap_test

[Sarthak Sharma]

On 9/24/26 10:30 AM, Sarthak Sharma wrote:
> This series fixes several correctness issues in mremap_test and
> simplifies and strengthens its data validation.
> 
> Patch 1 converts the mremap_test to use kselftest helpers, removes
> manual tracking of failed tests and corrects some spelling errors.
> 
> Patches 2 to 5 fix userfaultfd skipping, unexpected mremap success
> handling, multi-VMA data validation and failure reporting when data
> corruption is detected.
> 
> Patch 6 removes the randomization and uses a simple pattern based approach.
> 
> Patch 7 removes perf tests and timing infrastructure.
> 
> Patch 8 removes validation threshold and always validates complete mappings.
> 
> Patch 9 strengthens the multi VMA validation by also checking the
> mapping state of holes after remapping.

Hello everyone! Just wanted to check if someone has had a chance to look
at the series.

Also, Sashiko has a concern [1], and I had the same while posting the
series. I hope I can get some opinion from the community.

Currently, for PUD remap tests, we allocate a source mapping of 2GB. We
only fault in the first threshold_mb amount of memory, remap the whole
region, and validate the already faulted threshold_mb amount of memory,
which is, by default, equal to 4MB and can be changed by the user by
supplying a command line option.

Since we plan to remove all command line options from teh selftests, I
removed threshold_mb altogether, following some discussion on the list
[2]. This would now cause a 2GB source mapping, and its contents copied
from another 2GB buffer. This causes the process to have a 4GB RSS.
Sashiko says that this can cause OOM killing in small CI machines.

Would it be okay to keep a 4GB RSS in this case, or should we find some
other way of validating a part of the whole range instead?

[1]
https://sashiko.dev/#/patchset/20260924050009.19974-1-sarthak.sharma%40arm.com

[2]
https://lore.kernel.org/all/2e34b619-085f-4a9c-bb41-bc024fd40dd7@kernel.org/

Re: [PATCH RESEND 0/9] selftests/mm: improve mremap_test

[Kalesh Singh]

On Mon, Sep 28, 2026 at 10:54 PM Sarthak Sharma <sarthak.sharma@arm.com> wrote:
>
>
>
> On 9/24/26 10:30 AM, Sarthak Sharma wrote:
> > This series fixes several correctness issues in mremap_test and
> > simplifies and strengthens its data validation.
> >
> > Patch 1 converts the mremap_test to use kselftest helpers, removes
> > manual tracking of failed tests and corrects some spelling errors.
> >
> > Patches 2 to 5 fix userfaultfd skipping, unexpected mremap success
> > handling, multi-VMA data validation and failure reporting when data
> > corruption is detected.
> >
> > Patch 6 removes the randomization and uses a simple pattern based approach.
> >
> > Patch 7 removes perf tests and timing infrastructure.
> >
> > Patch 8 removes validation threshold and always validates complete mappings.
> >
> > Patch 9 strengthens the multi VMA validation by also checking the
> > mapping state of holes after remapping.
>
> Hello everyone! Just wanted to check if someone has had a chance to look
> at the series.
>
> Also, Sashiko has a concern [1], and I had the same while posting the
> series. I hope I can get some opinion from the community.
>
> Currently, for PUD remap tests, we allocate a source mapping of 2GB. We
> only fault in the first threshold_mb amount of memory, remap the whole
> region, and validate the already faulted threshold_mb amount of memory,
> which is, by default, equal to 4MB and can be changed by the user by
> supplying a command line option.
>
> Since we plan to remove all command line options from teh selftests, I
> removed threshold_mb altogether, following some discussion on the list
> [2]. This would now cause a 2GB source mapping, and its contents copied
> from another 2GB buffer. This causes the process to have a 4GB RSS.
> Sashiko says that this can cause OOM killing in small CI machines.
>
> Would it be okay to keep a 4GB RSS in this case, or should we find some
> other way of validating a part of the whole range instead?

Hi Sarthak,

IIRC when I initially introduced the test, John was concerned that
validating the whole range would significantly increase the duration
of the mm selftests; this is why the threshold was introduced. Please
check how much it increases if we validate the full range (with
David's suggestions) and if it's no longer a concern from other folks.
I am fine with removing the threshold.

Thanks,
Kalesh

>
> [1]
> https://sashiko.dev/#/patchset/20260924050009.19974-1-sarthak.sharma%40arm.com
>
> [2]
> https://lore.kernel.org/all/2e34b619-085f-4a9c-bb41-bc024fd40dd7@kernel.org/

Re: [PATCH RESEND 0/9] selftests/mm: improve mremap_test

[David Hildenbrand (Arm)]

On 9/29/26 09:32, Kalesh Singh wrote:
> On Mon, Sep 28, 2026 at 10:54 PM Sarthak Sharma <sarthak.sharma@arm.com> wrote:
>>
>>
>>
>> On 9/24/26 10:30 AM, Sarthak Sharma wrote:
>>> This series fixes several correctness issues in mremap_test and
>>> simplifies and strengthens its data validation.
>>>
>>> Patch 1 converts the mremap_test to use kselftest helpers, removes
>>> manual tracking of failed tests and corrects some spelling errors.
>>>
>>> Patches 2 to 5 fix userfaultfd skipping, unexpected mremap success
>>> handling, multi-VMA data validation and failure reporting when data
>>> corruption is detected.
>>>
>>> Patch 6 removes the randomization and uses a simple pattern based approach.
>>>
>>> Patch 7 removes perf tests and timing infrastructure.
>>>
>>> Patch 8 removes validation threshold and always validates complete mappings.
>>>
>>> Patch 9 strengthens the multi VMA validation by also checking the
>>> mapping state of holes after remapping.
>>
>> Hello everyone! Just wanted to check if someone has had a chance to look
>> at the series.
>>
>> Also, Sashiko has a concern [1], and I had the same while posting the
>> series. I hope I can get some opinion from the community.
>>
>> Currently, for PUD remap tests, we allocate a source mapping of 2GB. We
>> only fault in the first threshold_mb amount of memory, remap the whole
>> region, and validate the already faulted threshold_mb amount of memory,
>> which is, by default, equal to 4MB and can be changed by the user by
>> supplying a command line option.
>>
>> Since we plan to remove all command line options from teh selftests, I
>> removed threshold_mb altogether, following some discussion on the list
>> [2]. This would now cause a 2GB source mapping, and its contents copied
>> from another 2GB buffer. This causes the process to have a 4GB RSS.

Yeah, that's a lot for small CI systems indeed.

>> Sashiko says that this can cause OOM killing in small CI machines.
>>
>> Would it be okay to keep a 4GB RSS in this case, or should we find some
>> other way of validating a part of the whole range instead?
> 
> Hi Sarthak,
> 
> IIRC when I initially introduced the test, John was concerned that
> validating the whole range would significantly increase the duration
> of the mm selftests; this is why the threshold was introduced. Please
> check how much it increases if we validate the full range (with
> David's suggestions) and if it's no longer a concern from other folks.
> I am fine with removing the threshold.

As discussed off-list, I guess it makes more sense to validate a couple of pages
at the beginning, the middle and the end?

-- 
Cheers,

David

Re: [PATCH RESEND 0/9] selftests/mm: improve mremap_test

[Sarthak Sharma]

Hi Kalesh and David!

On 9/29/26 1:17 PM, David Hildenbrand (Arm) wrote:
> On 9/29/26 09:32, Kalesh Singh wrote:
>> On Mon, Sep 28, 2026 at 10:54 PM Sarthak Sharma <sarthak.sharma@arm.com> wrote:
>>>
>>>
>>>
>>> On 9/24/26 10:30 AM, Sarthak Sharma wrote:
>>>> This series fixes several correctness issues in mremap_test and
>>>> simplifies and strengthens its data validation.
>>>>
>>>> Patch 1 converts the mremap_test to use kselftest helpers, removes
>>>> manual tracking of failed tests and corrects some spelling errors.
>>>>
>>>> Patches 2 to 5 fix userfaultfd skipping, unexpected mremap success
>>>> handling, multi-VMA data validation and failure reporting when data
>>>> corruption is detected.
>>>>
>>>> Patch 6 removes the randomization and uses a simple pattern based approach.
>>>>
>>>> Patch 7 removes perf tests and timing infrastructure.
>>>>
>>>> Patch 8 removes validation threshold and always validates complete mappings.
>>>>
>>>> Patch 9 strengthens the multi VMA validation by also checking the
>>>> mapping state of holes after remapping.
>>>
>>> Hello everyone! Just wanted to check if someone has had a chance to look
>>> at the series.
>>>
>>> Also, Sashiko has a concern [1], and I had the same while posting the
>>> series. I hope I can get some opinion from the community.
>>>
>>> Currently, for PUD remap tests, we allocate a source mapping of 2GB. We
>>> only fault in the first threshold_mb amount of memory, remap the whole
>>> region, and validate the already faulted threshold_mb amount of memory,
>>> which is, by default, equal to 4MB and can be changed by the user by
>>> supplying a command line option.
>>>
>>> Since we plan to remove all command line options from teh selftests, I
>>> removed threshold_mb altogether, following some discussion on the list
>>> [2]. This would now cause a 2GB source mapping, and its contents copied
>>> from another 2GB buffer. This causes the process to have a 4GB RSS.
> 
> Yeah, that's a lot for small CI systems indeed.
> 
>>> Sashiko says that this can cause OOM killing in small CI machines.
>>>
>>> Would it be okay to keep a 4GB RSS in this case, or should we find some
>>> other way of validating a part of the whole range instead?
>>
>> Hi Sarthak,
>>
>> IIRC when I initially introduced the test, John was concerned that
>> validating the whole range would significantly increase the duration
>> of the mm selftests; this is why the threshold was introduced. Please
>> check how much it increases if we validate the full range (with
>> David's suggestions) and if it's no longer a concern from other folks.
>> I am fine with removing the threshold.

I tested on an Orion O6. Here's the difference before and after removing
the threshold right now.

Before:
0.02user 0.07system 0:00.10elapsed 98%CPU (0avgtext+0avgdata
87332maxresident)k
0inputs+0outputs (0major+21133minor)pagefaults 0swaps

After:
1.67user 4.91system 0:06.63elapsed 99%CPU (0avgtext+0avgdata
4195564maxresident)k
0inputs+0outputs (0major+2225058minor)pagefaults 0swaps

I think instead of the time taken, the more concerning thing here is RSS
(85MB vs 4GB).

> 
> As discussed off-list, I guess it makes more sense to validate a couple of pages
> at the beginning, the middle and the end?

Yup, makes sense. I will implement this then.

Re: [PATCH RESEND 0/9] selftests/mm: improve mremap_test

[Lorenzo Stoakes (ARM)]

On Thu, Sep 24, 2026 at 10:30:00AM +0530, Sarthak Sharma wrote:
> This series fixes several correctness issues in mremap_test and
> simplifies and strengthens its data validation.
>
> Patch 1 converts the mremap_test to use kselftest helpers, removes
> manual tracking of failed tests and corrects some spelling errors.
>
> Patches 2 to 5 fix userfaultfd skipping, unexpected mremap success
> handling, multi-VMA data validation and failure reporting when data
> corruption is detected.
>
> Patch 6 removes the randomization and uses a simple pattern based approach.
>
> Patch 7 removes perf tests and timing infrastructure.
>
> Patch 8 removes validation threshold and always validates complete mappings.
>
> Patch 9 strengthens the multi VMA validation by also checking the
> mapping state of holes after remapping.

Even if I don't get a chance to look through this, just want to say thanks for
taking a look at this! The mremap tests have always been hairy, when I added
extra tests there I didn't love what I saw so it's great to see it get some
attention! :)

>
> ---
> Sarthak Sharma (9):
>   selftests/mm: mremap_test: use kselftest helpers
>   selftests/mm: mremap_test: skip test when userfaultfd is unavailable
>   selftests/mm: mremap_test: fail unexpected mremap successes
>   selftests/mm: mremap_test: correct multiple VMA range size
>   selftests/mm: mremap_test: fail on data corruption
>   selftests/mm: mremap_test: replace random data with deterministic
>     pattern
>   selftests/mm: mremap_test: remove perf tests and timing
>   selftests/mm: mremap_test: remove validation threshold
>   selftests/mm: mremap_test: strengthen multi VMA validation
>
>  tools/testing/selftests/mm/mremap_test.c | 600 +++++++++--------------
>  1 file changed, 220 insertions(+), 380 deletions(-)
>
> --
> 2.53.0
>

--
Cheers, Lorenzo