Re: [PATCH 2/5] mm/rmap: Add try_to_unmap_hugetlb_one

[David Hildenbrand (Arm)]

On 7/7/26 14:11, Dev Jain wrote:
> Simplify try_to_unmap_one() by separating the hugetlb parts into
> try_to_unmap_hugetlb_one().
> 
> To understand the correctness of the refactoring, the following points
> are noted:
> 
> 1. try_to_unmap() is called for hugetlb folios only when they are
>    hwpoisoned.
> 
> 2. A hugetlb VMA cannot be mlocked.
> 
> 3. page_vma_mapped_walk() returns at most one hugetlb mapping in a VMA,
>    and that mapping points at the head PFN.
> 
> 4. We won't ever process a softleaf entry that encodes a hugetlb folio;
>    hugetlb folios are never swapped out, migration entries will be
>    skipped (PVMW_MIGRATION not passed), and device-exclusive does not
>    work for hugetlb.
> 
> 5. The hwpoison entry is constructed from the poisoned folio, just as in
>    the pre-refactor code. Any previous uffd-wp state is deliberately not
>    preserved for the hwpoison entry.
> 
> 6. TTU_HWPOISON is always present; for it to not be present, either the
>    folio has to be in swapcache, or mapping_can_writeback() is true (see
>    unmap_poisoned_folio), none of which is true for hugetlb folios.
> 
> 7. Hugetlb uses separate counters from normal rss counters, therefore
>    update_highwater_rss() need not be called.
> 
> While at it:
> 
>  - Change VM_BUG_* to VM_WARN_*.
> 
>  - Do not declare variables which are only used once.
> 
>  - Use huge_pte_dirty() instead of pte_dirty().
> 
>  - Add 3 VM_WARN_ON_ONCE asserting that TTU_HWPOISON should be present,
>    pte_present() must be true, and the pfn derived from the huge pte
>    must be that of the head of the hugetlb folio.
> 
> Except the BUG->WARN change, no functional change intended.
> 
> Suggested-by: David Hildenbrand <david@xxxxxxxxxx>
> Signed-off-by: Dev Jain <dev.jain@xxxxxxx>
> ---
>  include/linux/hugetlb.h |   1 +
>  mm/rmap.c               | 177 +++++++++++++++++++++-------------------
>  2 files changed, 96 insertions(+), 82 deletions(-)
> 
> diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
> index 4115076e4922a..bf7e163e3779d 100644
> --- a/include/linux/hugetlb.h
> +++ b/include/linux/hugetlb.h
> @@ -1271,6 +1271,7 @@ static inline void hugetlb_count_sub(long l, struct mm_struct *mm)
>  }
>  
>  pte_t huge_ptep_get(struct mm_struct *mm, unsigned long addr, pte_t *ptep);
> +unsigned long huge_pte_dirty(pte_t pte);
>  
>  static inline pte_t huge_ptep_clear_flush(struct vm_area_struct *vma,
>  					  unsigned long addr, pte_t *ptep)
> diff --git a/mm/rmap.c b/mm/rmap.c
> index dbb077f8443e3..934773dfa2f2a 100644
> --- a/mm/rmap.c
> +++ b/mm/rmap.c
> @@ -1978,6 +1978,95 @@ static inline unsigned int folio_unmap_pte_batch(struct folio *folio,
>  				     FPB_RESPECT_WRITE | FPB_RESPECT_SOFT_DIRTY);
>  }
>  
> +static bool try_to_unmap_hugetlb_one(struct folio *folio,
> +		struct vm_area_struct *vma, unsigned long address, void *arg)
> +{
> +	DEFINE_FOLIO_VMA_WALK(pvmw, folio, vma, address, 0);
> +	struct mmu_notifier_range range;
> +	enum ttu_flags flags = (enum ttu_flags)(long)arg;
> +	struct mm_struct *mm = vma->vm_mm;
> +	const unsigned long hsz = huge_page_size(hstate_vma(vma));

I would move const variables all the way up.

> +	bool ret = true;
> +	pte_t pteval;
> +
> +	/*
> +	 * The try_to_unmap() is only passed a hugetlb folio in the case
> +	 * where the hugetlb folio is poisoned.
> +	 */
> +	VM_WARN_ON_FOLIO(!folio_test_hwpoison(folio), folio);
> +	VM_WARN_ON_ONCE(!(flags & TTU_HWPOISON));
> +
> +	range.end = vma_address_end(&pvmw);
> +	mmu_notifier_range_init(&range, MMU_NOTIFY_CLEAR, 0, vma->vm_mm,
> +				address, range.end);
> +	adjust_range_if_pmd_sharing_possible(vma, &range.start, &range.end);
> +	mmu_notifier_invalidate_range_start(&range);
> +
> +	/* There is only a single mapping in a VMA. */
> +	if (!page_vma_mapped_walk(&pvmw))
> +		goto range_end;
> +
> +	address = pvmw.address;

Will address ever change due to the page_vma_mapped_walk()? I don't think so, right?

So this can instead become a VM_WARN_ON_ONCE, maybe?



Nothing else jumped at me, much clearer to me.

Acked-by: David Hildenbrand (Arm) <david@xxxxxxxxxx>

-- 
Cheers,

David