Re: [PATCH v2 5/5] selftests/mm: retry migration failures for the full runtime

[Muhammad Usama Anjum]

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 <usama.anjum@xxxxxxx>
>>>>>> ---
>>>>>
>>>>> 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