[2/2] powerpc/mm/64s: Drop p4d_leaf()

Because 64-bit Book3S uses pgtable-nop4d.h, the P4D is folded into the
PGD. So P4D entries are actually PGD entries, or vice versa.

The other way to think of it is that the P4D is a single entry page
table below the PGD. Zero bits of the address are needed to index into
the P4D, therefore a P4D entry maps the same size address space as a PGD
entry.

As explained in the previous commit, there are no huge page sizes
supported directly at the PGD level on 64-bit Book3S, so there are also
no huge page sizes supported at the P4D level.

Therefore p4d_is_leaf() can never be true, so drop the definition and
fallback to the default implementation that always returns false.

Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 arch/powerpc/include/asm/book3s/64/pgtable.h | 7 -------
 1 file changed, 7 deletions(-)

Le 03/09/2022 à 14:36, Michael Ellerman a écrit :
> Because 64-bit Book3S uses pgtable-nop4d.h, the P4D is folded into the
> PGD. So P4D entries are actually PGD entries, or vice versa.
> 
> The other way to think of it is that the P4D is a single entry page
> table below the PGD. Zero bits of the address are needed to index into
> the P4D, therefore a P4D entry maps the same size address space as a PGD
> entry.
> 
> As explained in the previous commit, there are no huge page sizes
> supported directly at the PGD level on 64-bit Book3S, so there are also
> no huge page sizes supported at the P4D level.
> 
> Therefore p4d_is_leaf() can never be true, so drop the definition and
> fallback to the default implementation that always returns false.

Then here as well, you are removing the only architecture which 
implements a non 'always false' version of p4d_leaf().

x86 has on that is always false:

#define p4d_leaf	p4d_large
static inline int p4d_large(p4d_t p4d)
{
	/* No 512 GiB pages yet */
	return 0;
}

So, should it be dropped as well and all uses removed from core mm ?

Christophe

Christophe Leroy <christophe.leroy@csgroup.eu> writes:
> Le 03/09/2022 à 14:36, Michael Ellerman a écrit :
>> Because 64-bit Book3S uses pgtable-nop4d.h, the P4D is folded into the
>> PGD. So P4D entries are actually PGD entries, or vice versa.
>> 
>> The other way to think of it is that the P4D is a single entry page
>> table below the PGD. Zero bits of the address are needed to index into
>> the P4D, therefore a P4D entry maps the same size address space as a PGD
>> entry.
>> 
>> As explained in the previous commit, there are no huge page sizes
>> supported directly at the PGD level on 64-bit Book3S, so there are also
>> no huge page sizes supported at the P4D level.
>> 
>> Therefore p4d_is_leaf() can never be true, so drop the definition and
>> fallback to the default implementation that always returns false.
>
> Then here as well, you are removing the only architecture which 
> implements a non 'always false' version of p4d_leaf().
>
> x86 has on that is always false:
>
> #define p4d_leaf	p4d_large
> static inline int p4d_large(p4d_t p4d)
> {
> 	/* No 512 GiB pages yet */
> 	return 0;
> }
>
> So, should it be dropped as well and all uses removed from core mm ?

Probably?

I see very few uses of p4d_leaf(), so I suspect it's not actually being
called in all the places it should be if it ever returned true. See eg.
follow_p4d_mask() which doesn't call it.

I think it would be best to remove it and if anyone ever implements huge
pages at that level (unlikely?) they will need to go back and add
support in the right places.

But ultimately it's up to the mm folks to decide IMHO.

cheers

Michael Ellerman <mpe@ellerman.id.au> writes:

>
> Because 64-bit Book3S uses pgtable-nop4d.h, the P4D is folded into the
> PGD. So P4D entries are actually PGD entries, or vice versa.
>
> The other way to think of it is that the P4D is a single entry page
> table below the PGD. Zero bits of the address are needed to index into
> the P4D, therefore a P4D entry maps the same size address space as a PGD
> entry.
>
> As explained in the previous commit, there are no huge page sizes
> supported directly at the PGD level on 64-bit Book3S, so there are also
> no huge page sizes supported at the P4D level.
>
> Therefore p4d_is_leaf() can never be true, so drop the definition and
> fallback to the default implementation that always returns false.
>

Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>

> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> ---
>  arch/powerpc/include/asm/book3s/64/pgtable.h | 7 -------
>  1 file changed, 7 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h b/arch/powerpc/include/asm/book3s/64/pgtable.h
> index 486902aff040..057254063e88 100644
> --- a/arch/powerpc/include/asm/book3s/64/pgtable.h
> +++ b/arch/powerpc/include/asm/book3s/64/pgtable.h
> @@ -1459,12 +1459,5 @@ static inline bool pud_is_leaf(pud_t pud)
>  	return !!(pud_raw(pud) & cpu_to_be64(_PAGE_PTE));
>  }
>  
> -#define p4d_is_leaf p4d_is_leaf
> -#define p4d_leaf p4d_is_leaf
> -static inline bool p4d_is_leaf(p4d_t p4d)
> -{
> -	return !!(p4d_raw(p4d) & cpu_to_be64(_PAGE_PTE));
> -}
> -
>  #endif /* __ASSEMBLY__ */
>  #endif /* _ASM_POWERPC_BOOK3S_64_PGTABLE_H_ */
> -- 
> 2.37.2