Re: [PATCH v2] selftest/vm: add mremap expand merge offset test

On 02.01.23 15:44, Lorenzo Stoakes wrote:

Add a test to assert that we can mremap() and expand a mapping starting from an offset within an existing mapping. We unmap the last page in a 3 page mapping to ensure that the remap should always succeed, before remapping from the 2nd page.

This is additionally a regression test for the issue solved in "mm, mremap: fix mremap() expanding vma with addr inside vma" and confirmed to fail prior to the change and pass after it.

Finally, this patch updates the existing mremap expand merge test to check error conditions and reduce code duplication between the two tests.

Signed-off-by: Lorenzo Stoakes lstoakes@gmail.com

tools/testing/selftests/vm/mremap_test.c | 115 ++++++++++++++++++----- 1 file changed, 93 insertions(+), 22 deletions(-)

diff --git a/tools/testing/selftests/vm/mremap_test.c b/tools/testing/selftests/vm/mremap_test.c

...

• start = mmap(NULL, 3 * page_size, PROT_READ | PROT_WRITE,

•      MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
  

• if (start == MAP_FAILED) {

• ksft_print_msg("mmap failed: %s\n", strerror(errno));
  

I'd

ksft_test_result_fail(...) return;

• goto out;
  

• }
• munmap(start + page_size, page_size);
• remap = mremap(start, page_size, 2 * page_size, 0);
• if (remap == MAP_FAILED) {

• ksft_print_msg("mremap failed: %s\n", strerror(errno));
  

• munmap(start, page_size);
  

• munmap(start + 2 * page_size, page_size);
  

• goto out;
  

dito

ksft_test_result_fail(...) ... return;

• }
• success = is_range_mapped(maps_fp, start, start + 3 * page_size);
• munmap(start, 3 * page_size);

+out:

then you can drop the out label.

• if (success)

• ksft_test_result_pass("%s\n", test_name);
  

• else

• ksft_test_result_fail("%s\n", test_name);
  

+}

+/*

• • Similar to mremap_expand_merge() except instead of removing the middle page,
• • we remove the last then attempt to remap offset from the second page. This
• • should result in the mapping being restored to its former state.
• */

+static void mremap_expand_merge_offset(FILE *maps_fp, unsigned long page_size) +{

• char *test_name = "mremap expand merge offset";
• bool success = false;
• char *remap, *start;
• start = mmap(NULL, 3 * page_size, PROT_READ | PROT_WRITE,

•      MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
  

• if (start == MAP_FAILED) {

• ksft_print_msg("mmap failed: %s\n", strerror(errno));
  

• goto out;
  

• }
• /* Unmap final page to ensure we have space to expand. */
• munmap(start + 2 * page_size, page_size);
• remap = mremap(start + page_size, page_size, 2 * page_size, 0);
• if (remap == MAP_FAILED) {

• ksft_print_msg("mremap failed: %s\n", strerror(errno));
  

• munmap(start, 2 * page_size);
  

• goto out;
  

• }
• success = is_range_mapped(maps_fp, start, start + 3 * page_size);
• munmap(start, 3 * page_size);

+out:

dito.

if (success) ksft_test_result_pass("%s\n", test_name); else ksft_test_result_fail("%s\n", test_name);

• fclose(fp); }

/* @@ -385,6 +447,7 @@ int main(int argc, char **argv) struct test perf_test_cases[MAX_PERF_TEST]; int page_size; time_t t;

• FILE *maps_fp;

I'd simply use a global variable, same applies for page_size. But passing it around is also ok.

pattern_seed = (unsigned int) time(&t); @@ -458,7 +521,15 @@ int main(int argc, char **argv) run_mremap_test_case(test_cases[i], &failures, threshold_mb, pattern_seed);

• mremap_expand_merge(page_size);

• maps_fp = fopen("/proc/self/maps", "r");
• if (maps_fp == NULL) {

• ksft_print_msg("Failed to read /proc/self/maps: %s\n", strerror(errno));
  

Maybe simply fail the test completely and return -errno ?

• } else {

• mremap_expand_merge(maps_fp, page_size);
  

• mremap_expand_merge_offset(maps_fp, page_size);
  

• fclose(maps_fp);
  

No need to fclose, just keep it open ...

• }

if (run_perf_tests) { ksft_print_msg("\n%s\n",

Acked-by: David Hildenbrand david@redhat.com