Re: [PATCH V3 2/2] arm64/mm: Change offset base address in [pud|pmd]_free_[pmd|pte]_page()

[Catalin Marinas <catalin.marinas@arm.com>]

On Thu, May 09, 2019 at 10:16:17AM +0530, Anshuman Khandual wrote:
> Pgtable page address can be fetched with [pmd|pte]_offset_[kernel] if input
> address is PMD_SIZE or PTE_SIZE aligned. Input address is now guaranteed to
> be aligned, hence fetched pgtable page address is always correct. But using
> 0UL as offset base address has been a standard practice across platforms.
> It also makes more sense as it isolates pgtable page address computation
> from input virtual address alignment. This does not change functionality.
> 
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: James Morse <james.morse@arm.com>
> Cc: Robin Murphy <robin.murphy@arm.com>
> ---
>  arch/arm64/mm/mmu.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index e97f018ff740..71bcb783aace 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -1005,7 +1005,7 @@ int pmd_free_pte_page(pmd_t *pmdp, unsigned long addr)
>  		return 1;
>  	}
>  
> -	table = pte_offset_kernel(pmdp, addr);
> +	table = pte_offset_kernel(pmdp, 0UL);
>  	pmd_clear(pmdp);
>  	__flush_tlb_kernel_pgtable(addr);
>  	pte_free_kernel(NULL, table);
> @@ -1026,8 +1026,8 @@ int pud_free_pmd_page(pud_t *pudp, unsigned long addr)
>  		return 1;
>  	}
>  
> -	table = pmd_offset(pudp, addr);
> -	pmdp = table;
> +	table = pmd_offset(pudp, 0UL);
> +	pmdp = pmd_offset(pudp, addr);
>  	next = addr;
>  	end = addr + PUD_SIZE;
>  	do {

I have the same comment as last time:

https://lore.kernel.org/linux-arm-kernel/20190430161759.GI29799@arrakis.emea.arm.com/

I don't see why pmdp needs to be different from table. We get the
pointer to a pmd page and we want to iterate over it to free the pte
entries it contains. You can add a VM_WARN on addr alignment as in the
previous version of the patch but pmdp is just an iterator over table.

-- 
Catalin