selftests/mm fixes for arm64

The selftests runner pipes the test program's stdout to tap_prefix. The presence of the pipe means that the test program sets its stdout to be fully buffered (as aposed to line buffered when directly connected to the terminal). The block buffering means that there is often content in

arm64 does not support the soft-dirty PTE bit. However, the soft-dirty test suite is currently run unconditionally and therefore generates spurious test failures on arm64. There are also some tests in madv_populate which assume it is supported.

For soft-dirty lets disable the whole suite for arm64; it is no longer built and run_vmtests.sh will skip it if its not present.

For madv_populate, we need a runtime mechanism so that the remaining tests continue to be run. Unfortunately, the only way to determine if the soft-dirty dirty bit is supported is to write to a page, then see if the bit is set in /proc/self/pagemap. But the tests that we want to conditionally execute are testing precicesly this. So if we introduced this feature check, we could accedentally turn a real failure (on a system that claims to support soft-dirty) into a skip. So instead, do the check based on architecture; for arm64, we report that soft-dirty is not supported.

Signed-off-by Ryan Roberts <[email protected]>

---
  tools/testing/selftests/mm/Makefile        |  5 ++++-
  tools/testing/selftests/mm/madv_populate.c | 26 ++++++++++++++++++++--
  tools/testing/selftests/mm/run_vmtests.sh  |  5 ++++-
  3 files changed, 32 insertions(+), 4 deletions(-)

tools/testing/selftests/mm/Makefile +4 -1

index 66d7c07dc177..3514697fc2db 100644
@@ -63,12 +63,15 @@ TEST_GEN_PROGS += thuge-gen
  TEST_GEN_PROGS += transhuge-stress
  TEST_GEN_PROGS += uffd-stress
  TEST_GEN_PROGS += uffd-unit-tests
-TEST_GEN_PROGS += soft-dirty
  TEST_GEN_PROGS += split_huge_page_test
  TEST_GEN_PROGS += ksm_tests
  TEST_GEN_PROGS += ksm_functional_tests
  TEST_GEN_PROGS += mdwe_test
 
+ifneq ($(ARCH),arm64)
+TEST_GEN_PROGS += soft-dirty
+endif
+
  ifeq ($(ARCH),x86_64)
  CAN_BUILD_I386 := $(shell ./../x86/check_cc.sh "$(CC)" ../x86/trivial_32bit_program.c -m32)
  CAN_BUILD_X86_64 := $(shell ./../x86/check_cc.sh "$(CC)" ../x86/trivial_64bit_program.c)

tools/testing/selftests/mm/madv_populate.c +6 -0

index 60547245e479..17bcb07f19f3 100644
@@ -264,14 +264,35 @@ static void test_softdirty(void)
  	munmap(addr, SIZE);
  }
 
+static int system_has_softdirty(void)
+{
+	/*
+	 * There is no way to check if the kernel supports soft-dirty, other
+	 * than by writing to a page and seeing if the bit was set. But the
+	 * tests are intended to check that the bit gets set when it should, so

thuge-gen was previously only munmapping part of the mmapped buffer, which caused us to run out of 1G huge pages for a later part of the test. Fix this by munmapping the whole buffer. Based on the code, it looks like a typo rather than an intention to keep some of the buffer mapped.

thuge-gen was also calling mmap with SHM_HUGETLB flag (bit 11 set), which is actually MAP_DENYWRITE in mmap context. The man page says this flag is ignored in modern kernels. I'm pretty sure from the context that the author intended to pass the MAP_HUGETLB flag so I've fixed that up too.

Signed-off-by Ryan Roberts <[email protected]>

---
  tools/testing/selftests/mm/thuge-gen.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

tools/testing/selftests/mm/thuge-gen.c +1 -1

index 380ab5f0a534..16ed4dfa7359 100644
@@ -139,7 +139,7 @@ void test_mmap(unsigned long size, unsigned flags)
  		before, after, before - after, size);
  	assert(size == getpagesize() || (before - after) == NUM_PAGES);
  	show(size);
-	err = munmap(map, size);
+	err = munmap(map, size * NUM_PAGES);
  	assert(!err);
  }
 

The migration test currently has a number of robustness problems that cause it to hang and leak resources.

Timeout: There are 3 tests, which each previously ran for 60 seconds. However, the timeout in mm/settings for a single test binary was set to 45 seconds. So when run using run_kselftest.sh, the top level timeout would trigger before the test binary was finished. Solve this by meeting in the middle; each of the 3 tests now runs for 20 seconds (for a total of 60), and the top level timeout is set to 90 seconds.

Leaking child processes: the shared_anon test fork()s some children but then an ASSERT() fires before the test kills those children. The assert causes immediate exit of the parent and leaking of the children. Furthermore, if run using the run_kselftest.sh wrapper, the wrapper would get stuck waiting for those children to exit, which never happens. Solve this by deferring any asserts until after the children are killed. The same pattern is used for the threaded tests for uniformity.

With these changes, the test binary now runs to completion on arm64, with 2 tests passing and the shared_anon test failing.

Signed-off-by Ryan Roberts <[email protected]>

---
  tools/testing/selftests/mm/migration.c | 14 ++++++++++----
  tools/testing/selftests/mm/settings    |  2 +-
  2 files changed, 11 insertions(+), 5 deletions(-)

tools/testing/selftests/mm/migration.c +2 -1

index 379581567f27..189d7d9070e8 100644
@@ -15,7 +15,7 @@
  #include <time.h>
 
  #define TWOMEG (2<<20)
-#define RUNTIME (60)
+#define RUNTIME (20)
 
  #define ALIGN(x, a) (((x) + (a - 1)) & (~((a) - 1)))
 

@@ -118,6 +118,7 @@ TEST_F_TIMEOUT(migration, private_anon, 2*RUNTIME)
  {
  	uint64_t *ptr;
  	int i;
+	int ret;
 
  	if (self->nthreads < 2 || self->n1 < 0 || self->n2 < 0)
  		SKIP(return, "Not enough threads or NUMA nodes available");

tools/testing/selftests/mm/migration.c +1 -0

  }
 
  /*

@@ -144,6 +146,7 @@ TEST_F_TIMEOUT(migration, shared_anon, 2*RUNTIME)
  	pid_t pid;
  	uint64_t *ptr;
  	int i;
+	int ret;
 
  	if (self->nthreads < 2 || self->n1 < 0 || self->n2 < 0)
  		SKIP(return, "Not enough threads or NUMA nodes available");

Quoted 47 lines · Ryan Roberts

The migration test currently has a number of robustness problems that cause it to hang and leak resources.

Timeout: There are 3 tests, which each previously ran for 60 seconds. However, the timeout in mm/settings for a single test binary was set to 45 seconds. So when run using run_kselftest.sh, the top level timeout would trigger before the test binary was finished. Solve this by meeting in the middle; each of the 3 tests now runs for 20 seconds (for a total of 60), and the top level timeout is set to 90 seconds.

Leaking child processes: the shared_anon test fork()s some children but then an ASSERT() fires before the test kills those children. The assert causes immediate exit of the parent and leaking of the children. Furthermore, if run using the run_kselftest.sh wrapper, the wrapper would get stuck waiting for those children to exit, which never happens. Solve this by deferring any asserts until after the children are killed. The same pattern is used for the threaded tests for uniformity.

With these changes, the test binary now runs to completion on arm64, with 2 tests passing and the shared_anon test failing.

Signed-off-by Ryan Roberts <[email protected]>

---
  tools/testing/selftests/mm/migration.c | 14 ++++++++++---- tools/testing/selftests/mm/settings    |  2 +-

  2 files changed, 11 insertions(+), 5 deletions(-)

tools/testing/selftests/mm/migration.c +1 -1

index 379581567f27..189d7d9070e8 100644
@@ -15,7 +15,7 @@
   #include <time.h>
     #define TWOMEG (2<<20)
-#define RUNTIME (60)
+#define RUNTIME (20)
     #define ALIGN(x, a) (((x) + (a - 1)) & (~((a) - 1)))
   @@ -118,6 +118,7 @@ TEST_F_TIMEOUT(migration, private_anon, 2*RUNTIME)
   {
       uint64_t *ptr;

      int i;
+    int ret;
        if (self->nthreads < 2 || self->n1 < 0 || self->n2 < 0) SKIP(return, "Not enough threads or NUMA nodes available");

tools/testing/selftests/mm/migration.c +0 -0

@@ -131,9 +132,10 @@ TEST_F_TIMEOUT(migration, private_anon, 2*RUNTIME)
           if (pthread_create(&self->threads[i], NULL, access_mem, ptr))

tools/testing/selftests/mm/migration.c TEST_F_TIMEOUT(migration, private_anon, 2*RUNTIME) · line 138

Quoted 6 lines

tools/testing/selftests/mm/migration.c +2 -0

               perror("Couldn't create thread");
   -    ASSERT_EQ(migrate(ptr, self->n1, self->n2), 0);
+    ret = migrate(ptr, self->n1, self->n2);
       for (i = 0; i < self->nthreads - 1; i++)
           ASSERT_EQ(pthread_cancel(self->threads[i]), 0);
+    ASSERT_EQ(ret, 0);

Why is that required? This does not involve fork.

Quoted 13 lines

tools/testing/selftests/mm/migration.c +1 -0

   }
     /*

@@ -144,6 +146,7 @@ TEST_F_TIMEOUT(migration, shared_anon, 2*RUNTIME)
       pid_t pid;
       uint64_t *ptr;
       int i;
+    int ret;
         if (self->nthreads < 2 || self->n1 < 0 || self->n2 < 0)
           SKIP(return, "Not enough threads or NUMA nodes available");

@@ -161,9 +164,10 @@ TEST_F_TIMEOUT(migration, shared_anon, 2*RUNTIME)
               self->pids[i] = pid;
       }

tools/testing/selftests/mm/migration.c TEST_F_TIMEOUT(migration, shared_anon, 2*RUNTIME) · line 170

Quoted 5 lines

tools/testing/selftests/mm/migration.c +2 -0

   -    ASSERT_EQ(migrate(ptr, self->n1, self->n2), 0);
+    ret = migrate(ptr, self->n1, self->n2);
       for (i = 0; i < self->nthreads - 1; i++)
           ASSERT_EQ(kill(self->pids[i], SIGTERM), 0);
+    ASSERT_EQ(ret, 0);

Might be cleaner to also:

tools/testing/selftests/mm/migration.c +5 -2

index 379581567f27..b3f12b9847ec 100644
@@ -11,6 +11,7 @@
  #include <numaif.h>
  #include <sys/mman.h>
  #include <sys/types.h>
+#include <sys/prctl.h>
  #include <signal.h>
  #include <time.h>
 

@@ -155,10 +156,12 @@ TEST_F_TIMEOUT(migration, shared_anon, 2*RUNTIME)
         memset(ptr, 0xde, TWOMEG);
         for (i = 0; i < self->nthreads - 1; i++) {
                 pid = fork();
-               if (!pid)
+               if (!pid) {
+                       prctl(PR_SET_PDEATHSIG, SIGHUP);
                         access_mem(ptr);
-               else
+               } else {
                         self->pids[i] = pid;
+               }
         }
 
         ASSERT_EQ(migrate(ptr, self->n1, self->n2), 0);

Then, whenever the parent dies, all child processes get zapped.

Quoted 48 lines · David Hildenbrand

Quoted 47 lines · Ryan Roberts

The migration test currently has a number of robustness problems that cause it to hang and leak resources.

Timeout: There are 3 tests, which each previously ran for 60 seconds. However, the timeout in mm/settings for a single test binary was set to 45 seconds. So when run using run_kselftest.sh, the top level timeout would trigger before the test binary was finished. Solve this by meeting in the middle; each of the 3 tests now runs for 20 seconds (for a total of 60), and the top level timeout is set to 90 seconds.

Leaking child processes: the shared_anon test fork()s some children but then an ASSERT() fires before the test kills those children. The assert causes immediate exit of the parent and leaking of the children. Furthermore, if run using the run_kselftest.sh wrapper, the wrapper would get stuck waiting for those children to exit, which never happens. Solve this by deferring any asserts until after the children are killed. The same pattern is used for the threaded tests for uniformity.

With these changes, the test binary now runs to completion on arm64, with 2 tests passing and the shared_anon test failing.

Signed-off-by Ryan Roberts <[email protected]>

---
   tools/testing/selftests/mm/migration.c | 14 ++++++++++----
   tools/testing/selftests/mm/settings    |  2 +-
   2 files changed, 11 insertions(+), 5 deletions(-)

tools/testing/selftests/mm/migration.c +1 -1

index 379581567f27..189d7d9070e8 100644
@@ -15,7 +15,7 @@
   #include <time.h>
     #define TWOMEG (2<<20)
-#define RUNTIME (60)
+#define RUNTIME (20)
     #define ALIGN(x, a) (((x) + (a - 1)) & (~((a) - 1)))
   @@ -118,6 +118,7 @@ TEST_F_TIMEOUT(migration, private_anon, 2*RUNTIME)
   {
       uint64_t *ptr;

      int i;
+    int ret;
        if (self->nthreads < 2 || self->n1 < 0 || self->n2 < 0) SKIP(return, "Not enough threads or NUMA nodes available");

tools/testing/selftests/mm/migration.c +0 -0

@@ -131,9 +132,10 @@ TEST_F_TIMEOUT(migration, private_anon, 2*RUNTIME)
           if (pthread_create(&self->threads[i], NULL, access_mem, ptr))

Quoted 8 lines

tools/testing/selftests/mm/migration.c TEST_F_TIMEOUT(migration, private_anon, 2*RUNTIME) · line 138

Quoted 6 lines

tools/testing/selftests/mm/migration.c +2 -0

               perror("Couldn't create thread");
   -    ASSERT_EQ(migrate(ptr, self->n1, self->n2), 0);
+    ret = migrate(ptr, self->n1, self->n2);
       for (i = 0; i < self->nthreads - 1; i++)
           ASSERT_EQ(pthread_cancel(self->threads[i]), 0);
+    ASSERT_EQ(ret, 0);

Why is that required? This does not involve fork.

It's not required. I was just trying to keep everything aligned to the same pattern.

Quoted 13 lines

Quoted 13 lines

tools/testing/selftests/mm/migration.c +1 -0

   }
     /*

@@ -144,6 +146,7 @@ TEST_F_TIMEOUT(migration, shared_anon, 2*RUNTIME)
       pid_t pid;
       uint64_t *ptr;
       int i;
+    int ret;
         if (self->nthreads < 2 || self->n1 < 0 || self->n2 < 0)
           SKIP(return, "Not enough threads or NUMA nodes available");

@@ -161,9 +164,10 @@ TEST_F_TIMEOUT(migration, shared_anon, 2*RUNTIME)
               self->pids[i] = pid;
       }

Quoted 8 lines

tools/testing/selftests/mm/migration.c TEST_F_TIMEOUT(migration, shared_anon, 2*RUNTIME) · line 170

Quoted 5 lines

tools/testing/selftests/mm/migration.c +2 -0

   -    ASSERT_EQ(migrate(ptr, self->n1, self->n2), 0);
+    ret = migrate(ptr, self->n1, self->n2);
       for (i = 0; i < self->nthreads - 1; i++)
           ASSERT_EQ(kill(self->pids[i], SIGTERM), 0);
+    ASSERT_EQ(ret, 0);

Might be cleaner to also:

Or instead of? I agree this is neater, so will undo the moving of the ASSERT() and rely on this prctl.

Quoted 82 lines · Ryan Roberts

Quoted 48 lines · David Hildenbrand

Quoted 47 lines · Ryan Roberts

The migration test currently has a number of robustness problems that cause it to hang and leak resources.

Timeout: There are 3 tests, which each previously ran for 60 seconds. However, the timeout in mm/settings for a single test binary was set to 45 seconds. So when run using run_kselftest.sh, the top level timeout would trigger before the test binary was finished. Solve this by meeting in the middle; each of the 3 tests now runs for 20 seconds (for a total of 60), and the top level timeout is set to 90 seconds.

Leaking child processes: the shared_anon test fork()s some children but then an ASSERT() fires before the test kills those children. The assert causes immediate exit of the parent and leaking of the children. Furthermore, if run using the run_kselftest.sh wrapper, the wrapper would get stuck waiting for those children to exit, which never happens. Solve this by deferring any asserts until after the children are killed. The same pattern is used for the threaded tests for uniformity.

With these changes, the test binary now runs to completion on arm64, with 2 tests passing and the shared_anon test failing.

Signed-off-by Ryan Roberts <[email protected]>

---
    tools/testing/selftests/mm/migration.c | 14 ++++++++++----
    tools/testing/selftests/mm/settings    |  2 +-
    2 files changed, 11 insertions(+), 5 deletions(-)

tools/testing/selftests/mm/migration.c +1 -1

index 379581567f27..189d7d9070e8 100644
@@ -15,7 +15,7 @@
    #include <time.h>
      #define TWOMEG (2<<20)
-#define RUNTIME (60)
+#define RUNTIME (20)
      #define ALIGN(x, a) (((x) + (a - 1)) & (~((a) - 1)))
    @@ -118,6 +118,7 @@ TEST_F_TIMEOUT(migration, private_anon, 2*RUNTIME)
    {
        uint64_t *ptr;

      int i;
+    int ret;
        if (self->nthreads < 2 || self->n1 < 0 || self->n2 < 0) SKIP(return, "Not enough threads or NUMA nodes available");

tools/testing/selftests/mm/migration.c +0 -0

@@ -131,9 +132,10 @@ TEST_F_TIMEOUT(migration, private_anon, 2*RUNTIME)
            if (pthread_create(&self->threads[i], NULL, access_mem, ptr))

Quoted 8 lines

tools/testing/selftests/mm/migration.c TEST_F_TIMEOUT(migration, private_anon, 2*RUNTIME) · line 138

Quoted 6 lines

tools/testing/selftests/mm/migration.c +2 -0

                perror("Couldn't create thread");
    -    ASSERT_EQ(migrate(ptr, self->n1, self->n2), 0);
+    ret = migrate(ptr, self->n1, self->n2);
        for (i = 0; i < self->nthreads - 1; i++)
            ASSERT_EQ(pthread_cancel(self->threads[i]), 0);
+    ASSERT_EQ(ret, 0);

Why is that required? This does not involve fork.

It's not required. I was just trying to keep everything aligned to the same pattern.

Quoted 13 lines

Quoted 13 lines

tools/testing/selftests/mm/migration.c +1 -0

    }
      /*

@@ -144,6 +146,7 @@ TEST_F_TIMEOUT(migration, shared_anon, 2*RUNTIME)
        pid_t pid;
        uint64_t *ptr;
        int i;
+    int ret;
          if (self->nthreads < 2 || self->n1 < 0 || self->n2 < 0)
            SKIP(return, "Not enough threads or NUMA nodes available");

@@ -161,9 +164,10 @@ TEST_F_TIMEOUT(migration, shared_anon, 2*RUNTIME)
                self->pids[i] = pid;
        }

Quoted 8 lines

tools/testing/selftests/mm/migration.c TEST_F_TIMEOUT(migration, shared_anon, 2*RUNTIME) · line 170

Quoted 5 lines

tools/testing/selftests/mm/migration.c +2 -0

    -    ASSERT_EQ(migrate(ptr, self->n1, self->n2), 0);
+    ret = migrate(ptr, self->n1, self->n2);
        for (i = 0; i < self->nthreads - 1; i++)
            ASSERT_EQ(kill(self->pids[i], SIGTERM), 0);
+    ASSERT_EQ(ret, 0);

Might be cleaner to also:

Quoted 2 lines

Or instead of? I agree this is neater, so will undo the moving of the ASSERT() and rely on this prctl.

I was thinking about possible races when our parent process already quits before our child managed to set the prctl. prctl() won't do anything in that case, hmmmm.

But similarly, existing code might already trigger the migrate() + kill before the child processes even started to access_mem().

Racy :)

Quoted 91 lines · David Hildenbrand

Quoted 83 lines · Ryan Roberts

Quoted 48 lines · David Hildenbrand

Quoted 47 lines · Ryan Roberts

The migration test currently has a number of robustness problems that cause it to hang and leak resources.

Timeout: There are 3 tests, which each previously ran for 60 seconds. However, the timeout in mm/settings for a single test binary was set to 45 seconds. So when run using run_kselftest.sh, the top level timeout would trigger before the test binary was finished. Solve this by meeting in the middle; each of the 3 tests now runs for 20 seconds (for a total of 60), and the top level timeout is set to 90 seconds.

Leaking child processes: the shared_anon test fork()s some children but then an ASSERT() fires before the test kills those children. The assert causes immediate exit of the parent and leaking of the children. Furthermore, if run using the run_kselftest.sh wrapper, the wrapper would get stuck waiting for those children to exit, which never happens. Solve this by deferring any asserts until after the children are killed. The same pattern is used for the threaded tests for uniformity.

With these changes, the test binary now runs to completion on arm64, with 2 tests passing and the shared_anon test failing.

Signed-off-by Ryan Roberts <[email protected]>

---
    tools/testing/selftests/mm/migration.c | 14 ++++++++++---- tools/testing/selftests/mm/settings    |  2 +-

    2 files changed, 11 insertions(+), 5 deletions(-)

tools/testing/selftests/mm/migration.c +1 -1

index 379581567f27..189d7d9070e8 100644
@@ -15,7 +15,7 @@
     #include <time.h>
       #define TWOMEG (2<<20)
-#define RUNTIME (60)
+#define RUNTIME (20)
       #define ALIGN(x, a) (((x) + (a - 1)) & (~((a) - 1)))
     @@ -118,6 +118,7 @@ TEST_F_TIMEOUT(migration, private_anon, 2*RUNTIME)
     {
         uint64_t *ptr;

        int i;
+    int ret;
          if (self->nthreads < 2 || self->n1 < 0 || self->n2 < 0) SKIP(return, "Not enough threads or NUMA nodes available");

tools/testing/selftests/mm/migration.c +0 -0

@@ -131,9 +132,10 @@ TEST_F_TIMEOUT(migration, private_anon, 2*RUNTIME)
             if (pthread_create(&self->threads[i], NULL, access_mem, ptr))

Quoted 8 lines

tools/testing/selftests/mm/migration.c TEST_F_TIMEOUT(migration, private_anon, 2*RUNTIME) · line 138

Quoted 6 lines

tools/testing/selftests/mm/migration.c +2 -0

                 perror("Couldn't create thread");
     -    ASSERT_EQ(migrate(ptr, self->n1, self->n2), 0);
+    ret = migrate(ptr, self->n1, self->n2);
         for (i = 0; i < self->nthreads - 1; i++)
             ASSERT_EQ(pthread_cancel(self->threads[i]), 0);
+    ASSERT_EQ(ret, 0);

Why is that required? This does not involve fork.

It's not required. I was just trying to keep everything aligned to the same pattern.

Quoted 13 lines

Quoted 13 lines

tools/testing/selftests/mm/migration.c +1 -0

     }
       /*

@@ -144,6 +146,7 @@ TEST_F_TIMEOUT(migration, shared_anon, 2*RUNTIME)
         pid_t pid;
         uint64_t *ptr;
         int i;
+    int ret;
           if (self->nthreads < 2 || self->n1 < 0 || self->n2 < 0)
             SKIP(return, "Not enough threads or NUMA nodes available");

@@ -161,9 +164,10 @@ TEST_F_TIMEOUT(migration, shared_anon, 2*RUNTIME)
                 self->pids[i] = pid;
         }

Quoted 8 lines

tools/testing/selftests/mm/migration.c TEST_F_TIMEOUT(migration, shared_anon, 2*RUNTIME) · line 170

Quoted 5 lines

tools/testing/selftests/mm/migration.c +2 -0

     -    ASSERT_EQ(migrate(ptr, self->n1, self->n2), 0);
+    ret = migrate(ptr, self->n1, self->n2);
         for (i = 0; i < self->nthreads - 1; i++)
             ASSERT_EQ(kill(self->pids[i], SIGTERM), 0);
+    ASSERT_EQ(ret, 0);

Might be cleaner to also:

Quoted 2 lines

Or instead of? I agree this is neater, so will undo the moving of the ASSERT() and rely on this prctl.

I was thinking about possible races when our parent process already quits before our child managed to set the prctl. prctl() won't do anything in that case, hmmmm.

Quoted 5 lines

But similarly, existing code might already trigger the migrate() + kill before the child processes even started to access_mem().

Racy :)

Maybe what would work, is checking after the prctl() in the child if the parent is already gone.

Quoted 92 lines · David Hildenbrand

Quoted 91 lines · David Hildenbrand

Quoted 83 lines · Ryan Roberts

Quoted 48 lines · David Hildenbrand

Quoted 47 lines · Ryan Roberts

The migration test currently has a number of robustness problems that cause it to hang and leak resources.

Timeout: There are 3 tests, which each previously ran for 60 seconds. However, the timeout in mm/settings for a single test binary was set to 45 seconds. So when run using run_kselftest.sh, the top level timeout would trigger before the test binary was finished. Solve this by meeting in the middle; each of the 3 tests now runs for 20 seconds (for a total of 60), and the top level timeout is set to 90 seconds.

Leaking child processes: the shared_anon test fork()s some children but then an ASSERT() fires before the test kills those children. The assert causes immediate exit of the parent and leaking of the children. Furthermore, if run using the run_kselftest.sh wrapper, the wrapper would get stuck waiting for those children to exit, which never happens. Solve this by deferring any asserts until after the children are killed. The same pattern is used for the threaded tests for uniformity.

With these changes, the test binary now runs to completion on arm64, with 2 tests passing and the shared_anon test failing.

Signed-off-by Ryan Roberts <[email protected]>

---
     tools/testing/selftests/mm/migration.c | 14 ++++++++++----
     tools/testing/selftests/mm/settings    |  2 +-
     2 files changed, 11 insertions(+), 5 deletions(-)

tools/testing/selftests/mm/migration.c +1 -1

index 379581567f27..189d7d9070e8 100644
@@ -15,7 +15,7 @@
     #include <time.h>
       #define TWOMEG (2<<20)
-#define RUNTIME (60)
+#define RUNTIME (20)
       #define ALIGN(x, a) (((x) + (a - 1)) & (~((a) - 1)))
     @@ -118,6 +118,7 @@ TEST_F_TIMEOUT(migration, private_anon, 2*RUNTIME)
     {
         uint64_t *ptr;

        int i;
+    int ret;
          if (self->nthreads < 2 || self->n1 < 0 || self->n2 < 0) SKIP(return, "Not enough threads or NUMA nodes available");

tools/testing/selftests/mm/migration.c +0 -0

@@ -131,9 +132,10 @@ TEST_F_TIMEOUT(migration, private_anon, 2*RUNTIME)
             if (pthread_create(&self->threads[i], NULL, access_mem, ptr))

Quoted 8 lines

tools/testing/selftests/mm/migration.c TEST_F_TIMEOUT(migration, private_anon, 2*RUNTIME) · line 138

Quoted 6 lines

tools/testing/selftests/mm/migration.c +2 -0

                 perror("Couldn't create thread");
     -    ASSERT_EQ(migrate(ptr, self->n1, self->n2), 0);
+    ret = migrate(ptr, self->n1, self->n2);
         for (i = 0; i < self->nthreads - 1; i++)
             ASSERT_EQ(pthread_cancel(self->threads[i]), 0);
+    ASSERT_EQ(ret, 0);

Why is that required? This does not involve fork.

It's not required. I was just trying to keep everything aligned to the same pattern.

Quoted 13 lines

Quoted 13 lines

tools/testing/selftests/mm/migration.c +1 -0

     }
       /*

@@ -144,6 +146,7 @@ TEST_F_TIMEOUT(migration, shared_anon, 2*RUNTIME)
         pid_t pid;
         uint64_t *ptr;
         int i;
+    int ret;
           if (self->nthreads < 2 || self->n1 < 0 || self->n2 < 0)
             SKIP(return, "Not enough threads or NUMA nodes available");

@@ -161,9 +164,10 @@ TEST_F_TIMEOUT(migration, shared_anon, 2*RUNTIME)
                 self->pids[i] = pid;
         }

Quoted 8 lines

tools/testing/selftests/mm/migration.c TEST_F_TIMEOUT(migration, shared_anon, 2*RUNTIME) · line 170

Quoted 5 lines

tools/testing/selftests/mm/migration.c +2 -0

     -    ASSERT_EQ(migrate(ptr, self->n1, self->n2), 0);
+    ret = migrate(ptr, self->n1, self->n2);
         for (i = 0; i < self->nthreads - 1; i++)
             ASSERT_EQ(kill(self->pids[i], SIGTERM), 0);
+    ASSERT_EQ(ret, 0);

Might be cleaner to also:

Quoted 2 lines

Or instead of? I agree this is neater, so will undo the moving of the ASSERT() and rely on this prctl.

I was thinking about possible races when our parent process already quits before our child managed to set the prctl. prctl() won't do anything in that case, hmmmm.

Quoted 8 lines

Quoted 5 lines

But similarly, existing code might already trigger the migrate() + kill before the child processes even started to access_mem().

Racy :)

Maybe what would work, is checking after the prctl() in the child if the parent is already gone.

Like this?

	if (!pid) {
		prctl(PR_SET_PDEATHSIG, SIGHUP);
		/* Parent may have died before prctl so check now. */
		if (getppid() == 1)
			kill(getpid(), SIGHUP);
		access_mem(ptr);
	}

Until now, transhuge-stress runs until its explicitly killed, so when invoked by run_kselftest.sh, it would run until the test timeout, then it would be killed and the test would be marked as failed.

Add a new, optional command line parameter that allows the user to specify the duration in seconds that the program should run. The program exits after this duration with a success (0) exit code. If the argument is omitted the old behacvior remains.

On it's own, this doesn't quite solve our problem because run_kselftest.sh does not allow passing parameters to the program under test. But we will shortly move this to run_vmtests.sh, which does allow parameter passing.

Signed-off-by Ryan Roberts <[email protected]>

---
  tools/testing/selftests/mm/transhuge-stress.c | 12 ++++++++++--
  1 file changed, 10 insertions(+), 2 deletions(-)

tools/testing/selftests/mm/transhuge-stress.c +7 -2

index ba9d37ad3a89..c61fb9350b8c 100644
@@ -25,13 +25,14 @@ int main(int argc, char **argv)
  {
  	size_t ram, len;
  	void *ptr, *p;
-	struct timespec a, b;
+	struct timespec start, a, b;
  	int i = 0;
  	char *name = NULL;
  	double s;
  	uint8_t *map;
  	size_t map_len;
  	int pagemap_fd;
+	int duration = 0;
 
  	ram = sysconf(_SC_PHYS_PAGES);
  	if (ram > SIZE_MAX / psize() / 4)

@@ -42,9 +43,11 @@ int main(int argc, char **argv)
 
  	while (++i < argc) {
  		if (!strcmp(argv[i], "-h"))
-			errx(1, "usage: %s [size in MiB]", argv[0]);
+			errx(1, "usage: %s [-f <filename>] [-d <duration>] [size in MiB]", argv[0]);
  		else if (!strcmp(argv[i], "-f"))
  			name = argv[++i];
+		else if (!strcmp(argv[i], "-d"))
+			duration = atoi(argv[++i]);
  		else
  			len = atoll(argv[i]) << 20;
  	}

@@ -78,6 +81,8 @@ int main(int argc, char **argv)
  	if (!map)
  		errx(2, "map malloc");
 
+	clock_gettime(CLOCK_MONOTONIC, &start);
+
  	while (1) {
  		int nr_succeed = 0, nr_failed = 0, nr_pages = 0;
 

It is very unclear to me how one is supposed to run all the mm selftests consistently and get clear results.

Most of the test programs are launched by both run_vmtests.sh and run_kselftest.sh:

hugepage-mmap
hugepage-shm
map_hugetlb
hugepage-mremap
hugepage-vmemmap
hugetlb-madvise
map_fixed_noreplace
gup_test
gup_longterm
uffd-unit-tests
uffd-stress
compaction_test
on-fault-limit
map_populate
mlock-random-test
mlock2-tests
mrelease_test
mremap_test
thuge-gen
virtual_address_range
va_high_addr_switch
mremap_dontunmap
hmm-tests
madv_populate
memfd_secret
ksm_tests
ksm_functional_tests
soft-dirty
cow

However, of this set, when launched by run_vmtests.sh, some of the programs are invoked multiple times with different arguments. When invoked by run_kselftest.sh, they are invoked without arguments (and as a consequence, some fail immediately).

Some test programs are only launched by run_vmtests.sh:

test_vmalloc.sh

And some test programs and only launched by run_kselftest.sh:

khugepaged
migration
mkdirty
transhuge-stress
split_huge_page_test
mdwe_test
write_to_hugetlbfs

Furthermore, run_vmtests.sh is invoked by run_kselftest.sh, so in this case all the test programs invoked by both scripts are run twice!

Needless to say, this is a bit of a mess. In the absence of fully understanding the history here, it looks to me like the best solution is to launch ALL test programs from run_vmtests.sh, and ONLY invoke run_vmtests.sh from run_kselftest.sh. This way, we get full control over the parameters, each program is only invoked the intended number of times, and regardless of which script is used, the same tests get run in the same way.

The only drawback is that if using run_kselftest.sh, it's top-level tap result reporting reports only a single test and it fails if any of the contained tests fail. I don't see this as a big deal though since we still see all the nested reporting from multiple layers. The other issue with this is that all of run_vmtests.sh must execute within a single kselftest timeout period, so let's increase that to something more suitable.

In the Makefile, TEST_GEN_PROGS will compile and install the tests and will add them to the list of tests that run_kselftest.sh will run. TEST_GEN_FILES will compile and install the tests but will not add them to the test list. So let's move all the programs from TEST_GEN_PROGS to TEST_GEN_FILES so that they are built but not executed by run_kselftest.sh. Note that run_vmtests.sh is added to TEST_PROGS, which means it ends up in the test list. (the lack of "_GEN" means it won't be compiled, but simply copied).

Signed-off-by Ryan Roberts <[email protected]>

---

Quoted 83 lines · Ryan Roberts

It is very unclear to me how one is supposed to run all the mm selftests consistently and get clear results.

Most of the test programs are launched by both run_vmtests.sh and run_kselftest.sh:

hugepage-mmap
hugepage-shm
map_hugetlb
hugepage-mremap
hugepage-vmemmap
hugetlb-madvise
map_fixed_noreplace
gup_test
gup_longterm
uffd-unit-tests
uffd-stress
compaction_test
on-fault-limit
map_populate
mlock-random-test
mlock2-tests
mrelease_test
mremap_test
thuge-gen
virtual_address_range
va_high_addr_switch
mremap_dontunmap
hmm-tests
madv_populate
memfd_secret
ksm_tests
ksm_functional_tests
soft-dirty
cow

However, of this set, when launched by run_vmtests.sh, some of the programs are invoked multiple times with different arguments. When invoked by run_kselftest.sh, they are invoked without arguments (and as a consequence, some fail immediately).

Some test programs are only launched by run_vmtests.sh:

test_vmalloc.sh

And some test programs and only launched by run_kselftest.sh:

khugepaged
migration
mkdirty
transhuge-stress
split_huge_page_test
mdwe_test
write_to_hugetlbfs

Furthermore, run_vmtests.sh is invoked by run_kselftest.sh, so in this case all the test programs invoked by both scripts are run twice!

Needless to say, this is a bit of a mess. In the absence of fully understanding the history here, it looks to me like the best solution is to launch ALL test programs from run_vmtests.sh, and ONLY invoke run_vmtests.sh from run_kselftest.sh. This way, we get full control over the parameters, each program is only invoked the intended number of times, and regardless of which script is used, the same tests get run in the same way.

The only drawback is that if using run_kselftest.sh, it's top-level tap result reporting reports only a single test and it fails if any of the contained tests fail. I don't see this as a big deal though since we still see all the nested reporting from multiple layers. The other issue with this is that all of run_vmtests.sh must execute within a single kselftest timeout period, so let's increase that to something more suitable.

In the Makefile, TEST_GEN_PROGS will compile and install the tests and will add them to the list of tests that run_kselftest.sh will run. TEST_GEN_FILES will compile and install the tests but will not add them to the test list. So let's move all the programs from TEST_GEN_PROGS to TEST_GEN_FILES so that they are built but not executed by run_kselftest.sh. Note that run_vmtests.sh is added to TEST_PROGS, which means it ends up in the test list. (the lack of "_GEN" means it won't be compiled, but simply copied).

Quoted 2 lines

Signed-off-by Ryan Roberts <[email protected]>

---

Acked-by David Hildenbrand <[email protected]>

Quoted 84 lines · David Hildenbrand

Quoted 83 lines · Ryan Roberts

It is very unclear to me how one is supposed to run all the mm selftests consistently and get clear results.

Most of the test programs are launched by both run_vmtests.sh and run_kselftest.sh:

hugepage-mmap
hugepage-shm
map_hugetlb
hugepage-mremap
hugepage-vmemmap
hugetlb-madvise
map_fixed_noreplace
gup_test
gup_longterm
uffd-unit-tests
uffd-stress
compaction_test
on-fault-limit
map_populate
mlock-random-test
mlock2-tests
mrelease_test
mremap_test
thuge-gen
virtual_address_range
va_high_addr_switch
mremap_dontunmap
hmm-tests
madv_populate
memfd_secret
ksm_tests
ksm_functional_tests
soft-dirty
cow

However, of this set, when launched by run_vmtests.sh, some of the programs are invoked multiple times with different arguments. When invoked by run_kselftest.sh, they are invoked without arguments (and as a consequence, some fail immediately).

Some test programs are only launched by run_vmtests.sh:

test_vmalloc.sh

And some test programs and only launched by run_kselftest.sh:

khugepaged
migration
mkdirty
transhuge-stress
split_huge_page_test
mdwe_test
write_to_hugetlbfs

Furthermore, run_vmtests.sh is invoked by run_kselftest.sh, so in this case all the test programs invoked by both scripts are run twice!

Needless to say, this is a bit of a mess. In the absence of fully understanding the history here, it looks to me like the best solution is to launch ALL test programs from run_vmtests.sh, and ONLY invoke run_vmtests.sh from run_kselftest.sh. This way, we get full control over the parameters, each program is only invoked the intended number of times, and regardless of which script is used, the same tests get run in the same way.

The only drawback is that if using run_kselftest.sh, it's top-level tap result reporting reports only a single test and it fails if any of the contained tests fail. I don't see this as a big deal though since we still see all the nested reporting from multiple layers. The other issue with this is that all of run_vmtests.sh must execute within a single kselftest timeout period, so let's increase that to something more suitable.

In the Makefile, TEST_GEN_PROGS will compile and install the tests and will add them to the list of tests that run_kselftest.sh will run. TEST_GEN_FILES will compile and install the tests but will not add them to the test list. So let's move all the programs from TEST_GEN_PROGS to TEST_GEN_FILES so that they are built but not executed by run_kselftest.sh. Note that run_vmtests.sh is added to TEST_PROGS, which means it ends up in the test list. (the lack of "_GEN" means it won't be compiled, but simply copied).

Quoted 4 lines

Quoted 2 lines

Signed-off-by Ryan Roberts <[email protected]>

---

Acked-by David Hildenbrand <[email protected]>

Thanks for letting me know, David. Sorry for the late response, still catching up things.

I used to justify that from mm/ itself that everything should be PROG, but I see that from higher level where TEST_GEN_FILE|PROG is really used this makes sense. As long as vm_utils.o will be properly linked I'll be happy enough..

Acked-by Peter Xu <[email protected]>

Thanks,