Re: [PATCH v8 09/14] powerpc: move has_transparent_hugepage() out of THP guard

[Luiz Capitulino]

On 10/2/26 3:28 PM, David Hildenbrand (Arm) wrote:
> On 9/18/26 03:45, Luiz Capitulino wrote:
> > A future commit will introduce a kernel API to allow for checking if the
> > CPU supports PMD-sized pages. This API will be based on the
> > has_transparent_hugepage() implementation but will be orthogonal to THP
> > and therefore must work when CONFIG_TRANSPARENT_HUGEPAGE=n.
> >
> > Move its definition out of the THP guard.
> >
> > Signed-off-by: Luiz Capitulino <luizcap@xxxxxxxxxx>
> > ---
> >   arch/powerpc/include/asm/book3s/64/hash-4k.h  |  2 +-
> >   arch/powerpc/include/asm/book3s/64/hash-64k.h |  2 +-
> >   arch/powerpc/include/asm/book3s/64/pgtable.h  | 18 +++++++++---------
> >   arch/powerpc/include/asm/book3s/64/radix.h    | 14 +++++++-------
> >   arch/powerpc/mm/book3s64/hash_pgtable.c       |  4 ++--
> >   5 files changed, 20 insertions(+), 20 deletions(-)
> >
> > diff --git a/arch/powerpc/include/asm/book3s/64/hash-4k.h b/arch/powerpc/include/asm/book3s/64/hash-4k.h
> > index 8e5bd9902bed..79511e6abfca 100644
> > --- a/arch/powerpc/include/asm/book3s/64/hash-4k.h
> > +++ b/arch/powerpc/include/asm/book3s/64/hash-4k.h
> > @@ -165,9 +165,9 @@ extern void hash__pgtable_trans_huge_deposit(struct mm_struct *mm, pmd_t *pmdp,
> >   extern pgtable_t hash__pgtable_trans_huge_withdraw(struct mm_struct *mm, pmd_t *pmdp);
> >   extern pmd_t hash__pmdp_huge_get_and_clear(struct mm_struct *mm,
> >   				       unsigned long addr, pmd_t *pmdp);
> > -extern int hash__has_transparent_hugepage(void);
> >   #endif
> >   
> > +extern int hash__has_transparent_hugepage(void);
> >   #endif /* !__ASSEMBLER__ */
> >   
> >   #endif /* _ASM_POWERPC_BOOK3S_64_HASH_4K_H */
> > diff --git a/arch/powerpc/include/asm/book3s/64/hash-64k.h b/arch/powerpc/include/asm/book3s/64/hash-64k.h
> > index 7deb3a66890b..a4a44a112ff9 100644
> > --- a/arch/powerpc/include/asm/book3s/64/hash-64k.h
> > +++ b/arch/powerpc/include/asm/book3s/64/hash-64k.h
> > @@ -278,9 +278,9 @@ extern void hash__pgtable_trans_huge_deposit(struct mm_struct *mm, pmd_t *pmdp,
> >   extern pgtable_t hash__pgtable_trans_huge_withdraw(struct mm_struct *mm, pmd_t *pmdp);
> >   extern pmd_t hash__pmdp_huge_get_and_clear(struct mm_struct *mm,
> >   				       unsigned long addr, pmd_t *pmdp);
> > -extern int hash__has_transparent_hugepage(void);
>
> Without some of these helpers in place, I assume actually using PMD leafs
> without THP would require some more work. (which is not the goal of this series,
> just asking).

Yes, you're right.

> I do wonder whether the architecture should instead simply say "not supported"
> if !CONFIG_TRANSPARENT_HUGEPAGE?
>
> That should still enable your series: using mTHP without PMD support.

Yes, it would. However, I think this introduces an inconsistency.

In in its current form, arch_has_pmd_leaves() should always report a hardware
capability. This is true even for the default case where
arch_has_pmd_leaves() defaults to
IS_ENABLED(CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE) in the assumption that
archs supporting PMD-sized pages by default will have this config
enabled.

Your suggestion will change this and for some archs
arch_has_pmd_leaves() may have different behavior depending on the user
configuration. Additionally, I intended to decouple the base API
implementation from THP.

If you feel strongly about this I can implement your suggestion, but I'd
still vote for keeping the API about consistently reporting the hardware
capability. Even if the only user is THP code today, new use cases can
be added incrementally.

> [...]
>
> >   
> > -static inline int radix__has_transparent_hugepage(void)
> > +static inline int radix__has_transparent_pud_hugepage(void)
> >   {
> > -	/* For radix 2M at PMD level means thp */
> > -	if (mmu_psize_defs[MMU_PAGE_2M].shift == PMD_SHIFT)
> > +	/* For radix 1G at PUD level means pud hugepage support */
> > +	if (mmu_psize_defs[MMU_PAGE_1G].shift == PUD_SHIFT)
> >   		return 1;
> >   	return 0;
> >   }
> > +#endif
> >   
> > -static inline int radix__has_transparent_pud_hugepage(void)
> > +static inline int radix__has_transparent_hugepage(void)
> >   {
> > -	/* For radix 1G at PUD level means pud hugepage support */
> > -	if (mmu_psize_defs[MMU_PAGE_1G].shift == PUD_SHIFT)
> > +	/* For radix 2M at PMD level means thp */
> > +	if (mmu_psize_defs[MMU_PAGE_2M].shift == PMD_SHIFT)
> >   		return 1;
> >   	return 0;
> >   }
>
> You are swapping both implementations, which might create some unnecessary churn
> I think.

I suspect this was done by git diff as I just moved the functions, but
I'll take a better look.