On 4/27/26 2:07 PM, David Hildenbrand (Arm) wrote: > 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.
Ack, not a bad idea for historical reasons. > >> >> 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 :) Thats a page that is none duh ;P Ill clean that up > > "never collapses if it encounters an empty PTE or a PTE that maps the shared > zeropage. Consequently, no memory bloat." Thanks that does read way better! > >> - 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 <[email protected]> >> Signed-off-by: Dev Jain <[email protected]> >> Signed-off-by: Nico Pache <[email protected]> >> --- >> 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". We'd then be "violating" the uAPI expetation and is the whole reason we have this 0/511 behavior in the first place ;/ If it wasnt for that issue I would have a completely different design. > > > [...] > >> >> /** >> * 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? Ok ill clear that up thank you! > >> + * >> * 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? Yeah! I originally didnt plan on using these helpers for the file collapse operation but figured hell, why not clean them both up, might end up helping (hopefully not hurting) when Baolin goes to add mTHP shmem support. Cheers, -- Nico > >> + * >> * 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)
