Re: [PATCH v3 18/47] mm/khugepaged: check if PTE table is empty in try_collapse_pte_mapped_thp()

[Baolin Wang]

On 10/31/26 5:39 PM, David Hildenbrand (Arm) wrote:
> The existing code is rather horrible and already resulted in bugs in the
> past: we special-case UFFD inserting anonymous folios by grabbing the PMD
> lock early, but ignore any other markers (guard, poison through uffd) that
> could have been inserted into the page table in the meantime, effectively
> dropping them.
>
> Let's just unify and simplify the locking: try to grab the PMD table
> lock and if that fails, flush the TLB, drop the PTE lock, take PT locks in
> the right order and check if the page table is still empty.
>
> We still perform the heavy lifting (zapping the folio ptes) under PTE table
> lock only.
>
> Include <linux/hugetlb.h> for now, as x86's flush_tlb_range() macro
> depends on hugetlb helpers. This dependency needs a separate cleanup.
>
> Both issues this fixes are rather corner cases. Let's not CC stable for
> now.
>
> Fixes: f807123d578d ("mm: allow guard regions in file-backed and read-only mappings")
> Fixes: 1043173eb5eb ("mm/khugepaged: collapse_pte_mapped_thp() with mmap_read_lock()")
> Tested-by: Gregory Price (Meta) <gourry@xxxxxxxxxx>
> Reviewed-by: Gregory Price (Meta) <gourry@xxxxxxxxxx>
> Tested-by: Muhammad Usama Anjum <usama.anjum@xxxxxxx>
> Signed-off-by: David Hildenbrand (Arm) <david@xxxxxxxxxx>
> ---
>   mm/khugepaged.c | 64 ++++++++++++++++++++++++++++-----------------------------
>   1 file changed, 31 insertions(+), 33 deletions(-)
>
> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> index ce794d9760a87..ad07c8caa3bd9 100644
> --- a/mm/khugepaged.c
> +++ b/mm/khugepaged.c
> @@ -10,6 +10,7 @@
>   #include <linux/mm_inline.h>
>   #include <linux/kthread.h>
>   #include <linux/khugepaged.h>
> +#include <linux/hugetlb.h>
>   #include <linux/freezer.h>
>   #include <linux/mman.h>
>   #include <linux/hashtable.h>
> @@ -1832,6 +1833,17 @@ static enum scan_result set_huge_pmd(struct vm_area_struct *vma, unsigned long a
>   	return SCAN_SUCCEED;
>   }
>   
> +static bool pte_table_empty(pte_t *pte)
> +{
> +	int i;
> +
> +	for (i = 0; i < HPAGE_PMD_NR; i++, pte++) {

Using PTRS_PER_PTE to iterate PTEs seems more straightforward? Otherwise 
LGTM.

Reviewed-by: Baolin Wang <baolin.wang@xxxxxxxxxxxxxxxxx>