Re: [PATCH 2/2] selftests/mm: Do not fail test for a single migration failure

On 8/9/24 22:43, Shuah Khan wrote:

On 8/9/24 04:31, Dev Jain wrote:

quoted

Do not fail the test for just a single instance of migration failure,
since migration is a best-effort service.

The cover letter says:

"Given that migration is a best-effort service, it is wrong to fail the
test for just a single failure; hence, fail the test after 100 
consecutive
failures (where 100 is still a subjective choice)."

You do want to mention the above here.

Sure, shall update in v2.

The reason being, I would like to know what this does to the run-time of
this test if migration fails and retried 100 times.

Sure; just for the note, it won't affect the execution time of the test 
since
that is controlled by a timeout mechanism.

quoted

Signed-off-by: Dev Jain <dev.jain@arm.com>
Suggested-by: David Hildenbrand <redacted>
Reviewed-by: Ryan Roberts <ryan.roberts@arm.com>
Tested-by: Ryan Roberts <ryan.roberts@arm.com>
---
  tools/testing/selftests/mm/migration.c | 17 +++++++++++------
  1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/tools/testing/selftests/mm/migration.c 

b/tools/testing/selftests/mm/migration.c
index 6908569ef406..64bcbb7151cf 100644

--- a/tools/testing/selftests/mm/migration.c
+++ b/tools/testing/selftests/mm/migration.c

@@ -15,10 +15,10 @@

  #include <signal.h>
  #include <time.h>
  -#define TWOMEG (2<<20)
-#define RUNTIME (20)
-
-#define ALIGN(x, a) (((x) + (a - 1)) & (~((a) - 1)))
+#define TWOMEG        (2<<20)
+#define RUNTIME        (20)
+#define MAX_RETRIES    100
+#define ALIGN(x, a)    (((x) + (a - 1)) & (~((a) - 1)))
    FIXTURE(migration)
  {

@@ -65,6 +65,7 @@ int migrate(uint64_t *ptr, int n1, int n2)

      int ret, tmp;
      int status = 0;
      struct timespec ts1, ts2;
+    int failures = 0;
        if (clock_gettime(CLOCK_MONOTONIC, &ts1))
          return -1;

@@ -79,13 +80,17 @@ int migrate(uint64_t *ptr, int n1, int n2)

          ret = move_pages(0, 1, (void **) &ptr, &n2, &status,
                  MPOL_MF_MOVE_ALL);
          if (ret) {
-            if (ret > 0)
+            if (ret > 0) {
+                /* Migration is best effort; try again */
+                if (++failures < MAX_RETRIES)
+                    continue;
                  printf("Didn't migrate %d pages\n", ret);
+            }
              else
                  perror("Couldn't migrate pages");
              return -2;
          }
-
+        failures = 0;
          tmp = n2;
          n2 = n1;
          n1 = tmp;

thanks,
-- Shuah