Re: [PATCH v3 02/11] arm64: hugetlb: Refine tlb maintenance scope

[Anshuman Khandual <anshuman.khandual@arm.com>]

On 3/4/25 20:34, Ryan Roberts wrote:
> When operating on contiguous blocks of ptes (or pmds) for some hugetlb
> sizes, we must honour break-before-make requirements and clear down the
> block to invalid state in the pgtable then invalidate the relevant tlb
> entries before making the pgtable entries valid again.
> 
> However, the tlb maintenance is currently always done assuming the worst
> case stride (PAGE_SIZE), last_level (false) and tlb_level
> (TLBI_TTL_UNKNOWN). We can do much better with the hinting; In reality,
> we know the stride from the huge_pte pgsize, we are always operating
> only on the last level, and we always know the tlb_level, again based on
> pgsize. So let's start providing these hints.
> 
> Additionally, avoid tlb maintenace in set_huge_pte_at().
> Break-before-make is only required if we are transitioning the
> contiguous pte block from valid -> valid. So let's elide the
> clear-and-flush ("break") if the pte range was previously invalid.
> 
> Signed-off-by: Ryan Roberts <ryan.roberts@arm.com>
> ---
>  arch/arm64/include/asm/hugetlb.h | 29 +++++++++++++++++++----------
>  arch/arm64/mm/hugetlbpage.c      |  9 ++++++---
>  2 files changed, 25 insertions(+), 13 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/hugetlb.h b/arch/arm64/include/asm/hugetlb.h
> index 07fbf5bf85a7..2a8155c4a882 100644
> --- a/arch/arm64/include/asm/hugetlb.h
> +++ b/arch/arm64/include/asm/hugetlb.h
> @@ -69,29 +69,38 @@ extern void huge_ptep_modify_prot_commit(struct vm_area_struct *vma,
>  
>  #include <asm-generic/hugetlb.h>
>  
> -#define __HAVE_ARCH_FLUSH_HUGETLB_TLB_RANGE
> -static inline void flush_hugetlb_tlb_range(struct vm_area_struct *vma,
> -					   unsigned long start,
> -					   unsigned long end)
> +static inline void __flush_hugetlb_tlb_range(struct vm_area_struct *vma,
> +					     unsigned long start,
> +					     unsigned long end,
> +					     unsigned long stride,
> +					     bool last_level)
>  {
> -	unsigned long stride = huge_page_size(hstate_vma(vma));
> -
>  	switch (stride) {
>  #ifndef __PAGETABLE_PMD_FOLDED
>  	case PUD_SIZE:
> -		__flush_tlb_range(vma, start, end, PUD_SIZE, false, 1);
> +		__flush_tlb_range(vma, start, end, PUD_SIZE, last_level, 1);
>  		break;
>  #endif
>  	case CONT_PMD_SIZE:
>  	case PMD_SIZE:
> -		__flush_tlb_range(vma, start, end, PMD_SIZE, false, 2);
> +		__flush_tlb_range(vma, start, end, PMD_SIZE, last_level, 2);
>  		break;
>  	case CONT_PTE_SIZE:
> -		__flush_tlb_range(vma, start, end, PAGE_SIZE, false, 3);
> +		__flush_tlb_range(vma, start, end, PAGE_SIZE, last_level, 3);
>  		break;
>  	default:
> -		__flush_tlb_range(vma, start, end, PAGE_SIZE, false, TLBI_TTL_UNKNOWN);
> +		__flush_tlb_range(vma, start, end, PAGE_SIZE, last_level, TLBI_TTL_UNKNOWN);
>  	}
>  }
>  
> +#define __HAVE_ARCH_FLUSH_HUGETLB_TLB_RANGE
> +static inline void flush_hugetlb_tlb_range(struct vm_area_struct *vma,
> +					   unsigned long start,
> +					   unsigned long end)
> +{
> +	unsigned long stride = huge_page_size(hstate_vma(vma));
> +
> +	__flush_hugetlb_tlb_range(vma, start, end, stride, false);
> +}
> +
>  #endif /* __ASM_HUGETLB_H */
> diff --git a/arch/arm64/mm/hugetlbpage.c b/arch/arm64/mm/hugetlbpage.c
> index 6a2af9fb2566..065be8650aa5 100644
> --- a/arch/arm64/mm/hugetlbpage.c
> +++ b/arch/arm64/mm/hugetlbpage.c
> @@ -183,8 +183,9 @@ static pte_t get_clear_contig_flush(struct mm_struct *mm,
>  {
>  	pte_t orig_pte = get_clear_contig(mm, addr, ptep, pgsize, ncontig);
>  	struct vm_area_struct vma = TLB_FLUSH_VMA(mm, 0);
> +	unsigned long end = addr + (pgsize * ncontig);
>  
> -	flush_tlb_range(&vma, addr, addr + (pgsize * ncontig));
> +	__flush_hugetlb_tlb_range(&vma, addr, end, pgsize, true);
>  	return orig_pte;
>  }
>  
> @@ -209,7 +210,7 @@ static void clear_flush(struct mm_struct *mm,
>  	for (i = 0; i < ncontig; i++, addr += pgsize, ptep++)
>  		__ptep_get_and_clear(mm, addr, ptep);
>  
> -	flush_tlb_range(&vma, saddr, addr);
> +	__flush_hugetlb_tlb_range(&vma, saddr, addr, pgsize, true);
>  }
>  
>  void set_huge_pte_at(struct mm_struct *mm, unsigned long addr,
> @@ -238,7 +239,9 @@ void set_huge_pte_at(struct mm_struct *mm, unsigned long addr,
>  	dpfn = pgsize >> PAGE_SHIFT;
>  	hugeprot = pte_pgprot(pte);
>  
> -	clear_flush(mm, addr, ptep, pgsize, ncontig);
> +	/* Only need to "break" if transitioning valid -> valid. */
> +	if (pte_valid(__ptep_get(ptep)))
> +		clear_flush(mm, addr, ptep, pgsize, ncontig);
>  
>  	for (i = 0; i < ncontig; i++, ptep++, addr += pgsize, pfn += dpfn)
>  		__set_ptes(mm, addr, ptep, pfn_pte(pfn, hugeprot), 1);


LGTM

Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>