Re: [PATCH RFC] mm: arm64: advance pte for contpte_ptep_set_access_flags

[David Hildenbrand <david@redhat.com>]

On 31.08.24 10:35, Barry Song wrote:
> From: Barry Song <v-songbaohua@oppo.com>
> 
> Hi Ryan, David,
> it seems contpte_ptep_set_access_flags() has never advanced
> pte pfn, and it is setting all entries' pfn to the first
> subpage. But I feel quite strange we never have a bug reported.
> Am I missing something?
> 
> Fixes: 4602e5757bcc ("arm64/mm: wire up PTE_CONT for user mappings")
> Cc: Ard Biesheuvel <ardb@kernel.org>
> Cc: John Hubbard <jhubbard@nvidia.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: David Hildenbrand <david@redhat.com>
> Cc: Will Deacon <will@kernel.org>
> Signed-off-by: Barry Song <v-songbaohua@oppo.com>
> ---
>   arch/arm64/mm/contpte.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/mm/contpte.c b/arch/arm64/mm/contpte.c
> index a3edced29ac1..10dcd2641184 100644
> --- a/arch/arm64/mm/contpte.c
> +++ b/arch/arm64/mm/contpte.c
> @@ -421,8 +421,10 @@ int contpte_ptep_set_access_flags(struct vm_area_struct *vma,
>   		ptep = contpte_align_down(ptep);
>   		start_addr = addr = ALIGN_DOWN(addr, CONT_PTE_SIZE);
>   
> -		for (i = 0; i < CONT_PTES; i++, ptep++, addr += PAGE_SIZE)
> +		for (i = 0; i < CONT_PTES; i++, ptep++, addr += PAGE_SIZE) {
>   			__ptep_set_access_flags(vma, addr, ptep, entry, 0);
> +			entry = pte_advance_pfn(entry, 1);
> +		}
>   
>   		if (dirty)
>   			__flush_tlb_range(vma, start_addr, addr,

Taking a closer look at __ptep_set_access_flags(), there is:

/* only preserve the access flags and write permission *
pte_val(entry) &= PTE_RDONLY | PTE_AF | PTE_WRITE | PTE_DIRTY;

So it looks like it doesn't need the PFN?


OTOH, there is the initial:


if (pte_same(pte, entry))
	return 0;

check that might accelerate things.

So unless I am missing something, this works as expected? (and if the 
pte_same() would frequently be taken with your change would be worthwile 
to optimize)

-- 
Cheers,

David / dhildenb