On 24/07/26 3:54 pm, Muhammad Usama Anjum wrote:
> move_pages() is best effort and can temporarily fail when concurrent
> faults race with page unmapping. A busy shared-anon workload can exhaust
> the current 100 retries long before the intended 20-second runtime and
> produce a false failure.
>
> Use the full runtime as the retry window. Since the initial page location
> is unknown, require it to reach both alternating NUMA targets to confirm
> that cross-node migration made progress despite transient contention.
>
> Signed-off-by: Muhammad Usama Anjum <[email protected]>
> ---
Makes sense, but see below.
> Changes since v1:
> - Retry per-page failures for the full runtime
> - Verify that both alternating NUMA targets are reached
> ---
> tools/testing/selftests/mm/migration.c | 39 ++++++++++++++------------
> 1 file changed, 21 insertions(+), 18 deletions(-)
>
> diff --git a/tools/testing/selftests/mm/migration.c
> b/tools/testing/selftests/mm/migration.c
> index 29f7492453d43..4d55a424058a9 100644
> --- a/tools/testing/selftests/mm/migration.c
> +++ b/tools/testing/selftests/mm/migration.c
> @@ -7,7 +7,7 @@
> #include "kselftest_harness.h"
> #include "hugepage_settings.h"
>
> -#include <strings.h>
> +#include <string.h>
> #include <pthread.h>
> #include <numa.h>
> #include <numaif.h>
> @@ -20,7 +20,6 @@
>
> #define TWOMEG (2<<20)
> #define RUNTIME (20)
> -#define MAX_RETRIES 100
> #define ALIGN(x, a) (((x) + (a - 1)) & (~((a) - 1)))
>
> HUGETLB_SETUP_DEFAULT_PAGES(1)
> @@ -110,7 +109,7 @@ int migrate(uint64_t *ptr, int n1, int n2)
> int ret, tmp;
> int status = 0;
> struct timespec ts1, ts2;
> - int failures = 0;
> + int success = 0;
>
> if (clock_gettime(CLOCK_MONOTONIC, &ts1))
> return -1;
> @@ -119,29 +118,33 @@ int migrate(uint64_t *ptr, int n1, int n2)
> if (clock_gettime(CLOCK_MONOTONIC, &ts2))
> return -1;
>
> - if (ts2.tv_sec - ts1.tv_sec >= RUNTIME)
> - return 0;
> + if (ts2.tv_sec - ts1.tv_sec >= RUNTIME) {
> + /* Reaching both targets verifies a cross-node move. */
> + if (success >= 2)
> + return 0;
> + else
> + return -2;
> + }
>
> ret = move_pages(0, 1, (void **) &ptr, &n2, &status,
> MPOL_MF_MOVE_ALL);
> - if (ret) {
> - 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;
> + if (ret < 0) {
> + perror("Couldn't migrate pages");
> + return ret;
> }
> - failures = 0;
> + /* Migration is best effort. Try again */
> + if (ret > 0 || status < 0)
old code wasn't using status, so why now?
> + continue;
> + if (status != n2) {
> + printf("Page is on node %d instead of target node %d\n",
> + status, n2);
> + return status;
> + }
> + success++;
> tmp = n2;
> n2 = n1;
> n1 = tmp;
> }
> -
> - return 0;
> }
>
> void *access_mem(void *ptr)
