Re: [PATCH 4/4] selftests: mm: add mTHP collapse test cases

On 7/28/26 4:13 PM, Baolin Wang wrote:
> Added a new command 'mthp_khugepaged' for mTHP collapse, along with the '-c'
> parameter to specify the collapse order. Additionally, added mTHP collapse
> test cases for 'collapse_full', 'collapse_empty', and 'collapse_single_mthp'
> for anonymous folios. All khugepaged test cases passed.
> 
> Signed-off-by: Baolin Wang <[email protected]>
> ---
>   tools/testing/selftests/mm/khugepaged.c   | 130 ++++++++++++++++++----
>   tools/testing/selftests/mm/run_vmtests.sh |   2 +
>   2 files changed, 113 insertions(+), 19 deletions(-)
> 
> diff --git a/tools/testing/selftests/mm/khugepaged.c b/tools/testing/selftests/mm/khugepaged.c
> index f69be6be0ecd..e21a6ec38363 100644
> --- a/tools/testing/selftests/mm/khugepaged.c
> +++ b/tools/testing/selftests/mm/khugepaged.c
> @@ -26,9 +26,11 @@
>   
>   #define BASE_ADDR ((void *)(1UL << 30))
>   static unsigned long hpage_pmd_size;
> +static int hpage_pmd_order;
>   static unsigned long page_size;
>   static int hpage_pmd_nr;
>   static int anon_order;
> +static int collapse_order;
>   
>   #define PID_SMAPS "/proc/self/smaps"
>   #define TEST_FILE "collapse_test_file"
> @@ -69,6 +71,7 @@ struct collapse_context {
>   };
>   
>   static struct collapse_context *khugepaged_context;
> +static struct collapse_context *mthp_khugepaged_context;
>   static struct collapse_context *madvise_context;
>   
>   struct file_info {
> @@ -554,25 +557,25 @@ static void madvise_collapse(const char *msg, char *p, int nr_hpages,
>   }
>   
>   #define TICK 500000
> -static bool wait_for_scan(const char *msg, char *p, int nr_hpages,
> -			  struct mem_ops *ops)
> +static bool wait_for_scan(const char *msg, char *p, unsigned long size,
> +		int nr_hpages, int collap_order, struct mem_ops *ops)
>   {
> -	unsigned long size = nr_hpages * hpage_pmd_size;
> +	unsigned long hpage_size = page_size << collap_order;
>   	int full_scans;
>   	int timeout = 6; /* 3 seconds */
>   
>   	/* Sanity check */
> -	if (!ops->check_huge(p, size, 0, hpage_pmd_size))
> +	if (!ops->check_huge(p, size, 0, hpage_size))
>   		ksft_exit_fail_msg("Unexpected huge page\n");
>   
> -	madvise(p, nr_hpages * hpage_pmd_size, MADV_HUGEPAGE);
> +	madvise(p, size, MADV_HUGEPAGE);
>   
>   	/* Wait until the second full_scan completed */
>   	full_scans = thp_read_num("khugepaged/full_scans") + 2;
>   
>   	ksft_print_msg("%s...", msg);
>   	while (timeout--) {
> -		if (ops->check_huge(p, size, nr_hpages, hpage_pmd_size))
> +		if (ops->check_huge(p, size, nr_hpages, hpage_size))
>   			break;
>   		if (thp_read_num("khugepaged/full_scans") >= full_scans)
>   			break;
> @@ -595,7 +598,7 @@ static void khugepaged_collapse(const char *msg, char *p, int nr_hpages,
>   	if (!is_tmpfs(ops) && ops == &__read_write_file_write_ops)
>   		expect = false;
>   
> -	if (wait_for_scan(msg, p, nr_hpages, ops)) {
> +	if (wait_for_scan(msg, p, size, nr_hpages, hpage_pmd_order, ops)) {
>   		if (expect)
>   			fail("Timeout");
>   		else
> @@ -617,12 +620,62 @@ static void khugepaged_collapse(const char *msg, char *p, int nr_hpages,
>   		fail("Fail");
>   }
>   
> +static void mthp_khugepaged_collapse(const char *msg, char *p, int nr_hpages,
> +				struct mem_ops *ops, bool expect)
> +{
> +	unsigned long hpage_size = page_size << collapse_order;
> +	struct thp_settings settings = *thp_current_settings();
> +	/* mTHP collpase only allocates PMD sized memory */
> +	unsigned long size = hpage_pmd_size;
> +
> +	/* Set mTHP setting for mTHP collapse */
> +	if (ops == &__anon_ops) {
> +		settings.thp_enabled = THP_NEVER;
> +		settings.hugepages[collapse_order].enabled = THP_ALWAYS;
> +	}
> +
> +	thp_push_settings(&settings);
> +
> +	if (wait_for_scan(msg, p, size, nr_hpages, collapse_order, ops)) {

sashiko said:
"Could this setup create a race condition that leads to spurious test 
failures?
In mthp_khugepaged_collapse(), thp_push_settings() is called to set 
hugepages[collapse_order].enabled = THP_ALWAYS before wait_for_scan() is
invoked. The khugepaged thread is then immediately allowed to scan and
collapse the faulted memory.
The very first action in wait_for_scan() is a sanity check to verify 
that there are currently zero huge pages:
tools/testing/selftests/mm/khugepaged.c:wait_for_scan() {
         ...
         /* Sanity check */
         if (!ops->check_huge(p, size, 0, hpage_size))
                 ksft_exit_fail_msg("Unexpected huge page\n");
         ...
}
If khugepaged collapses the memory in the brief window between 
thp_push_settings() and this sanity check, will the test falsely fail 
with an unexpected huge page error?
For configurations relying on THP_MADVISE, this race doesn't appear to 
occur because the madvise(..., MADV_HUGEPAGE) call happens after the 
sanity check in wait_for_scan()."

Although this race is unlikely to happen (from my testing, it has never 
occurred), setting it to THP_MADVISE is indeed more reasonable. Will do 
in the next version.