On 25/07/2026 10:25 am, Dev Jain wrote: > > > On 24/07/26 9:27 pm, Muhammad Usama Anjum wrote: >> On 24/07/2026 4:23 pm, Dev Jain wrote: >>> >>> >>> On 24/07/26 5:23 pm, Muhammad Usama Anjum wrote: >>>> On 24/07/2026 12:46 pm, Dev Jain wrote: >>>>> >>>>> >>>>> 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) >>> >>> Also what if you migrated accidentally to node 0? In which case you >>> will return a false success below. >> What do you mean by "migrated accidentally to node 0"? Can you give an >> example? > > I meant it can happen that ret is 0, status is 0. Then you will pass > "status != n2" and the function will return 0. > > So probably just return -2 instead of status? I see. Interestingly, it seems sashiko caught the same thing. I'll fix return code in the next version.
-- Thanks, Usama
