Re: [PATCH v3 4/4] mm: pass the number of PTEs to move_pte()

[David Hildenbrand (Arm)]

On 10/22/26 01:33, Imre Kaloz wrote:
> Since commit f822a9a81a31 ("mm: optimize mremap() by PTE batching"),
> move_ptes() moves a batch of PTEs mapping consecutive pages of a folio
> at a time, but calls move_pte() once per batch with the first PTE
> only. sparc64, the only architecture that overrides move_pte(), flushes
> from the D-cache the page that PTE maps when the move changes its
> colour, so the other pages of a batch would keep lines of the old
> colour. This cannot happen today only because sparc64 has no
> pte_batch_hint(), and mremap_folio_pte_batch() then never batches.
> 
> Pass nr_ptes to move_pte(), as get_and_clear_ptes() takes it. The
> generic version ignores it, and sparc64 flushes every page of the
> batch.
> 
> Signed-off-by: Imre Kaloz <kaloz@xxxxxxxxxx>
> ---
>  arch/sparc/include/asm/pgtable_64.h | 16 +++++++++-------
>  include/linux/pgtable.h             |  2 +-
>  mm/mremap.c                         |  2 +-
>  3 files changed, 11 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/sparc/include/asm/pgtable_64.h b/arch/sparc/include/asm/pgtable_64.h
> index 1430489b9ac5..9ca787b3cbfe 100644
> --- a/arch/sparc/include/asm/pgtable_64.h
> +++ b/arch/sparc/include/asm/pgtable_64.h
> @@ -946,21 +946,23 @@ static inline void set_ptes(struct mm_struct *mm, unsigned long addr,
>  	set_pte_at((mm), (addr), (ptep), __pte(0UL))
>  
>  #ifdef DCACHE_ALIASING_POSSIBLE
> -/* This flushes only the page the PTE maps, so it relies on move_ptes()
> - * calling it for one PTE at a time, which the default pte_batch_hint()
> - * guarantees.
> +/* The nr PTEs map consecutive pages of one folio and all move by the
> + * same offset, so either every page of the batch aliases or none does.
>   */
>  #define __HAVE_ARCH_MOVE_PTE
> -#define move_pte(pte, old_addr, new_addr)				\
> +#define move_pte(pte, old_addr, new_addr, nr)				\
>  ({									\
>  	pte_t newpte = (pte);						\
>  	if (tlb_type != hypervisor && pte_present(pte)) {		\
>  		unsigned long this_pfn = pte_pfn(pte);			\
> +		unsigned int i;						\
>  									\
>  		if (pfn_valid(this_pfn) &&				\
> -		    (((old_addr) ^ (new_addr)) & (1 << 13)))		\
> -			flush_dcache_page_all(current->mm,		\
> -					      pfn_to_page(this_pfn));	\
> +		    (((old_addr) ^ (new_addr)) & (1 << 13))) {		\
> +			for (i = 0; i < (nr); i++)			\
> +				flush_dcache_page_all(current->mm,	\
> +					pfn_to_page(this_pfn + i));	\
> +		}							\
>  	}								\
>  	newpte;								\
>  })
> diff --git a/include/linux/pgtable.h b/include/linux/pgtable.h
> index 8c093c119e5a..b58c0e98994e 100644
> --- a/include/linux/pgtable.h
> +++ b/include/linux/pgtable.h
> @@ -1471,7 +1471,7 @@ static inline void arch_swap_restore(swp_entry_t entry, struct folio *folio)
>  #endif
>  
>  #ifndef __HAVE_ARCH_MOVE_PTE
> -#define move_pte(pte, old_addr, new_addr)	(pte)
> +#define move_pte(pte, old_addr, new_addr, nr)	(pte)
>  #endif
>  
>  #ifndef pte_accessible
> diff --git a/mm/mremap.c b/mm/mremap.c
> index e8df5cdb0ac9..7fcae7f0082d 100644
> --- a/mm/mremap.c
> +++ b/mm/mremap.c
> @@ -289,7 +289,7 @@ static int move_ptes(struct pagetable_move_control *pmc,
>  			force_flush = true;
>  		}
>  		pte = get_and_clear_ptes(mm, old_addr, old_ptep, nr_ptes);
> -		pte = move_pte(pte, old_addr, new_addr);
> +		pte = move_pte(pte, old_addr, new_addr, nr_ptes);
>  		pte = move_soft_dirty_pte(pte);
>  
>  		if (need_clear_uffd_wp && pte_is_uffd_wp_marker(pte))


I hate move_pte() so much. And I hate sparc64 for requiring it. ;)

Is there some way we can avoid this
helper-that-shouldn't-exist-but-someone-hacked-it-in entirely?

If it makes sparc64 slightly slower #dontcare.

-- 
Cheers,

David