On 06/04/2019 07:54 PM, Catalin Marinas wrote:
> 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@xxxxxxx>
>> Cc: Catalin Marinas <catalin.marinas@xxxxxxx>
>> Cc: Will Deacon <will.deacon@xxxxxxx>
>> Cc: Mark Rutland <mark.rutland@xxxxxxx>
>> Cc: James Morse <james.morse@xxxxxxx>
>> Cc: Robin Murphy <robin.murphy@xxxxxxx>
>> ---
>> 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@xxxxxxxxxxxxxxxxxxxx/
>
> 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.
Fair enough. I believe VM_WARN() is needed to check address alignment because
they are now guaranteed to be aligned because of the previous patch. I guess
we should probably drop this patch and consider only the previous one ?
