Re: [PATCH v4 05/19] selftests/mm: make the swap cases' swapout reliable

[Mike Rapoport]

> collapse_swapin_single_pte() and collapse_max_ptes_swap() swap a range out
> and then require smaps to report exactly the count they asked for.  Two
> things keep that count from arriving.
> 
> MADV_PAGEOUT is best effort, so the count often turns up a moment late.
> 
> And wait_for_scan() leaves MADV_HUGEPAGE behind, so khugepaged is still

"... leaves range as eligible for collapsing, so khugepaged is still
working on it."

> working on the range.  Collapsing a range with up to max_ptes_swap pages
> swapped out means reading them back in, so the daemon empties the swap as
> fast as the case fills it.  On arm64 with 64K pages max_ptes_swap is 1024
> pages, which is 64M a step, and the case loses:

".. the case looses the race:"

> 
>     # Swapout 1024 of 8192 pages... Fail
>     not ok 10 collapse_max_ptes_swap
> 
> Ask again for up to two seconds, with the range held out of the daemon's
> reach while asking.  The collapse each case runs next puts MADV_HUGEPAGE
> back, so only the setup is affected.

"Retry several times for up to two seconds meanwhile holding the range
out of the khugepaged reach. The collapse each case runs next restores
MADV_NOHUGEPAGE, so only setup is affected"

> 
> If the pages still will not go, skip.  A machine with no swap, or swap too

"If the pages still won't swap out, skip".

> small, full, capped by a memcg or busy with writeback, is not the kernel
> under test refusing.  An error from madvise() itself still ends the run.
> 
> Assisted-by: Claude-Code:claude-opus-5
> Reviewed-by: Muhammad Usama Anjum <usama.anjum@xxxxxxx>
> Tested-by: Muhammad Usama Anjum <usama.anjum@xxxxxxx>
> Signed-off-by: Kiryl Shutsemau (Meta) <kas@xxxxxxxxxx>
>
> diff --git a/tools/testing/selftests/mm/khugepaged.c b/tools/testing/selftests/mm/khugepaged.c
> index b436b401e2ea..a3d14cc1a2bf 100644
> --- a/tools/testing/selftests/mm/khugepaged.c
> +++ b/tools/testing/selftests/mm/khugepaged.c
> @@ -241,6 +241,41 @@ static bool check_swap(void *addr, unsigned long size)
>  	return swap;
>  }
>  
> +/*
> + * Page the range out and wait for the swap count to say so.
> + *
> + * Two things get in the way.  MADV_PAGEOUT is best effort:
> + * shrink_folio_list() leaves a folio alone when it cannot reclaim it right
> + * away, and one still under writeback from an earlier pageout is the common
> + * case, so the count the caller asks for arrives a moment later.  And a range
> + * an earlier collapse left MADV_HUGEPAGE is one khugepaged is still working
> + * on: collapsing a range with up to max_ptes_swap pages swapped out means
> + * reading those pages back in, so the daemon undoes the pageout as fast as it
> + * is asked for.  Keep the range out of its reach; the collapse the caller runs
> + * next puts MADV_HUGEPAGE back.
> + *
> + * Failing to get the pages out is the machine's answer, not the kernel's --
> + * swap too small, swap full, a memcg cap, a folio still under writeback -- so
> + * callers skip rather than fail.  An error from madvise() is different, and
> + * ends the run here.

I'd replace this cumbersome comment with

> + */
> +static bool swapout_range(void *p, unsigned long size)
> +{
> +	int i;
> +

	/* prevent khugepaged from swapping into this range */

> +	if (madvise(p, size, MADV_NOHUGEPAGE))
> +		ksft_exit_fail_perror("madvise(MADV_NOHUGEPAGE)");
> +

	/*
	 * retry several times because MADV_PAGEOUT is best effort.
	 * sleep between the retries to give a chance for outstanding
	 * writebacks for pages being swapped out to finish.
	 */

-- 
Sincerely yours,
Mike.