Re: [PATCH 7.2 v16 04/13] mm/khugepaged: generalize __collapse_huge_page_* for mTHP support

["David Hildenbrand (Arm)" <david@kernel.org>]

On 4/19/26 20:57, Nico Pache wrote:
> generalize the order of the __collapse_huge_page_* and collapse_max_*
> functions to support future mTHP collapse.
> 
> The current mechanism for determining collapse with the
> khugepaged_max_ptes_none value is not designed with mTHP in mind. This
> raises a key design issue: if we support user defined max_pte_none values
> (even those scaled by order), a collapse of a lower order can introduces
> an feedback loop, or "creep", when max_ptes_none is set to a value greater
> than HPAGE_PMD_NR / 2.
> 
> With this configuration, a successful collapse to order N will populate
> enough pages to satisfy the collapse condition on order N+1 on the next
> scan. This leads to unnecessary work and memory churn.

You could add a link here to previous discussions.

> 
> To fix this issue introduce a helper function that will limit mTHP
> collapse support to two max_ptes_none values, 0 and HPAGE_PMD_NR - 1.
> This effectively supports two modes:
> 
> - max_ptes_none=0: never introduce new none-pages for mTHP collapse.

"introduce" reads wrong in this context. And I don't know what a "none-page" is :)

"never collapses if it encounters an empty PTE or a PTE that maps the shared
zeropage. Consequently, no memory bloat."

> - max_ptes_none=511 (on 4k pagesz): Always collapse to the highest
>   available mTHP order.
> 
> This removes the possiblilty of "creep", while not modifying any uAPI
> expectations. A warning will be emitted if any non-supported
> max_ptes_none value is configured with mTHP enabled.
> 
> mTHP collapse will not honor the khugepaged_max_ptes_shared or
> khugepaged_max_ptes_swap parameters, and will fail if it encounters a
> shared or swapped entry.
> 
> No functional changes in this patch; however it defines future behavior
> for mTHP collapse.
> 
> Co-developed-by: Dev Jain <dev.jain@arm.com>
> Signed-off-by: Dev Jain <dev.jain@arm.com>
> Signed-off-by: Nico Pache <npache@redhat.com>
> ---
>  mm/khugepaged.c | 124 ++++++++++++++++++++++++++++++++++--------------
>  1 file changed, 88 insertions(+), 36 deletions(-)
> 
> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> index f42b55421191..283bb63854a5 100644
> --- a/mm/khugepaged.c
> +++ b/mm/khugepaged.c
> @@ -352,51 +352,86 @@ static bool pte_none_or_zero(pte_t pte)
>   * collapse_max_ptes_none - Calculate maximum allowed empty PTEs for collapse
>   * @cc: The collapse control struct
>   * @vma: The vma to check for userfaultfd
> + * @order: The folio order being collapsed to
>   *
>   * If we are not in khugepaged mode use HPAGE_PMD_NR to allow any
> - * empty page.
> + * empty page. For PMD-sized collapses (order == HPAGE_PMD_ORDER), use the
> + * configured khugepaged_max_ptes_none value.
> + *
> + * For mTHP collapses, we currently only support khugepaged_max_pte_none values
> + * of 0 or (KHUGEPAGED_MAX_PTES_LIMIT). Any other value will emit a warning and
> + * no mTHP collapse will be attempted

Not sure if we discussed it (and maybe I had a different opinion back then ...),
but could we simply to fallback to max_ptes_none=0, so we can avoid returning
errors here?

max_ptes_none=0 is ok, because we will not waste any memory. The warning clearly
tells the user that this combination is not supported as is.

... and it would make this function a lot easier to handle. In the warning, we
can just state that "falling back to ... "max_ptes_non = 0".


[...]

>  
>  /**
>   * collapse_max_ptes_shared - Calculate maximum allowed shared PTEs for collapse
>   * @cc: The collapse control struct
> + * @order: The folio order being collapsed to
>   *
>   * If we are not in khugepaged mode use HPAGE_PMD_NR to allow any
>   * shared page.
>   *
> + * For mTHP collapses, we currently dont support collapsing memory with
> + * shared memory.

"do not"

"shared memory" is misleading, as we do support shmem. What you mean is maybe
"collapsing with anonymous memory pages that are shared between processes
through CoW" or soemthing like that?

> + *
>   * Return: Maximum number of shared PTEs allowed for the collapse operation
>   */
> -static unsigned int collapse_max_ptes_shared(struct collapse_control *cc)
> +static unsigned int collapse_max_ptes_shared(struct collapse_control *cc,
> +		unsigned int order)
>  {
>  	if (!cc->is_khugepaged)
>  		return HPAGE_PMD_NR;
> +	if (!is_pmd_order(order))
> +		return 0;
> +
>  	return khugepaged_max_ptes_shared;
>  }
>  
>  /**
>   * collapse_max_ptes_swap - Calculate maximum allowed swap PTEs for collapse
>   * @cc: The collapse control struct
> + * @order: The folio order being collapsed to
>   *
>   * If we are not in khugepaged mode use HPAGE_PMD_NR to allow any
>   * swap page.
>   *
> + * For PMD-sized collapses (order == HPAGE_PMD_ORDER), use the configured
> + * khugepaged_max_ptes_swap value.
> + *
> + * For mTHP collapses, we currently dont support collapsing memory with
> + * swapped out memory.

"do not". Given that this is also used for the pagecache, can we make this clearer?

> + *
>   * Return: Maximum number of swap PTEs allowed for the collapse operation
>   */
> -static unsigned int collapse_max_ptes_swap(struct collapse_control *cc)
> +static unsigned int collapse_max_ptes_swap(struct collapse_control *cc,
> +		unsigned int order)
-- 
Cheers,

David