Re: [PATCH v4 11/19] selftests/mm: add order-parameterized khugepaged collapse cases

> The mthp_khugepaged context runs the generic cases at a sub-PMD order,
> which answers how many folios of that order a range ends up with.  It
> cannot say which window they are in, so "the populated window collapsed
> and its neighbour did not" and "one window collapsed twice" look alike.
> 
> Add four cases that check each aligned window on its own, with the
> folio-order helpers in vm_util:
> 
> - collapse_order_single_window(): only the populated window collapses;
> - collapse_order_partial_window(): the default max_ptes_none lets a window
>   with one present PTE collapse;
> - collapse_order_max_ptes_none(): with max_ptes_none=0 a full window
>   collapses and one missing a page does not;
> - collapse_order_mixed_sources(): sources that are already large folios of
>   a smaller order collapse to the target.
> 
> Each case faults its region before MADV_HUGEPAGE with only the target
> order enabled, so the sources are order 0 and the result can only come
> from khugepaged.  They wait for a full pass rather than for the result to
> appear: without a completed pass, "not collapsed" and "not scanned yet"
> are the same thing.
> 
> Assisted-by: Claude-Code:claude-opus-5
> Tested-by: Muhammad Usama Anjum <[email protected]>
> Signed-off-by: Kiryl Shutsemau (Meta) <[email protected]>
>
> diff --git a/tools/testing/selftests/mm/khugepaged.c b/tools/testing/selftests/mm/khugepaged.c
> index 5aa92de93ec1..56e364320145 100644
> --- a/tools/testing/selftests/mm/khugepaged.c
> +++ b/tools/testing/selftests/mm/khugepaged.c
> @@ -31,6 +31,8 @@ static unsigned long page_size;
>  static int hpage_pmd_nr;
>  static int anon_order;
>  static int collapse_order;
> +static int pagemap_fd = -1;
> +static int kpageflags_fd = -1;
>  
>  #define PID_SMAPS "/proc/self/smaps"
>  #define TEST_FILE "collapse_test_file"
> @@ -1261,6 +1263,216 @@ static void madvise_retracted_page_tables(struct collapse_context *c,
>  	ksft_test_result_report(exit_status, "%s\n", __func__);
>  }
>  
> +/* Smallest order khugepaged will consider for mTHP collapse. */
> +#define MIN_MTHP_ORDER 2
> +
> +/*
> + * Order-parameterized collapse cases for the mthp_khugepaged context.  What
> + * they add over the generic cases run under that context is per-window
> + * detection: which aligned window collapsed, and which of its neighbours did
> + * not.  check_huge() answers how many folios of the order the range holds,
> + * which cannot tell one window from another.

This comment again reads like in the context of the changes driven by
the current prompt.

> + *
> + * The region is faulted before MADV_HUGEPAGE, and the target order is only
> + * enabled for madvise, so the sources are always order 0 and the collapse
> + * product can only have come from khugepaged.
> + */

Hmm, what claude tries to tell us here?

> +static size_t mthp_window_size(void)
> +{
> +	return page_size << collapse_order;
> +}
> +
> +static void mthp_push_target_order(void)
> +{
> +	struct thp_settings settings = *thp_current_settings();
> +	int i;
> +
> +	/*
> +	 * The target order, for madvise only, and nothing else enabled: the
> +	 * cases fault their region before MADV_HUGEPAGE, so the sources are
> +	 * order 0 whatever -s asked the fault path for.  That matters for the
> +	 * cases built around a hole -- a large source folio would fill it in
> +	 * and the window would collapse after all.
> +	 * collapse_order_mixed_sources enables the source order it wants on
> +	 * top of this.
> +	 */
> +	settings.thp_enabled = THP_NEVER;
> +	for (i = 0; i < NR_ORDERS; i++)
> +		settings.hugepages[i].enabled = THP_NEVER;
> +	settings.hugepages[collapse_order].enabled = THP_MADVISE;
> +	thp_push_settings(&settings);
> +}
> +
> +static bool window_collapsed(void *p, size_t len)
> +{
> +	return is_range_backed_by_folio_orders(p, len, collapse_order,
> +					       pagemap_fd, kpageflags_fd);
> +}
> +
> +/* No aligned window in [p, p + len) is backed at the target order. */
> +static bool window_not_collapsed(void *p, size_t len)

I'd expect window_not_collapsed to be !window_collapsed.
Maybe we need to add some order notion to both of them, like
exact_order, any_order or something like that?

> +{
> +	size_t window = mthp_window_size();
> +	char *addr = p;
> +
> +	for (; len >= window; addr += window, len -= window) {
> +		if (window_collapsed(addr, window))
> +			return false;
> +	}
> +	return true;
> +}
> +
> +static bool khugepaged_wait_full_pass(void)
> +{
> +	/* Wait up to 30 seconds for the pass to complete. */

Isn't it clear from the call? And do we really need this wrapper?

> +	return khugepaged_full_pass(30);
> +}
> +
> +static void collapse_order_single_window(struct collapse_context *c,
> +					 struct mem_ops *ops)
> +{
> +	size_t window = mthp_window_size();
> +	void *p;
> +
> +	mthp_push_target_order();
> +
> +	p = ops->setup_area(1);
> +	ops->fault(p, window, 2 * window);
> +	if (!window_not_collapsed(p, hpage_pmd_size))

!window_not_collapsed  reads weird

-- 
Sincerely yours,
Mike.