On Mon, May 27, 2024 at 03:30:12PM +0200, Christophe Leroy wrote:
> On book3s/64, the only user of hugepd is hash in 4k mode.
> 
> All other setups (hash-64, radix-4, radix-64) use leaf PMD/PUD.
> 
> Rework hash-4k to use contiguous PMD and PUD instead.
> 
> In that setup there are only two huge page sizes: 16M and 16G.
> 
> 16M sits at PMD level and 16G at PUD level.


On 4k mode, PMD_SIZE is 2MB and PUD_SIZE is 256MB, right?

> +static inline unsigned long hash__pte_update(struct mm_struct *mm,
> +                                      unsigned long addr,
> +                                      pte_t *ptep, unsigned long clr,
> +                                      unsigned long set,
> +                                      int huge)
> +{
> +     unsigned long old;
> +
> +     old = hash__pte_update_one(ptep, clr, set);
> +
> +     if (IS_ENABLED(CONFIG_PPC_4K_PAGES) && huge) {
> +             unsigned int psize = get_slice_psize(mm, addr);
> +             int nb, i;
> +
> +             if (psize == MMU_PAGE_16M)
> +                     nb = SZ_16M / PMD_SIZE;
> +             else if (psize == MMU_PAGE_16G)
> +                     nb = SZ_16G / PUD_SIZE;
> +             else
> +                     nb = 1;

On 4K, hugepages are either 16M or 16G. How can we end up in a situation
whwere the is pte is huge, but is is neither MMU_PAGE_16G nor MMU_PAGE_16M?

> diff --git a/arch/powerpc/mm/book3s64/hugetlbpage.c 
> b/arch/powerpc/mm/book3s64/hugetlbpage.c
> index 5a2e512e96db..83c3361b358b 100644
> --- a/arch/powerpc/mm/book3s64/hugetlbpage.c
> +++ b/arch/powerpc/mm/book3s64/hugetlbpage.c
> @@ -53,6 +53,16 @@ int __hash_page_huge(unsigned long ea, unsigned long 
> access, unsigned long vsid,
>               /* If PTE permissions don't match, take page fault */
>               if (unlikely(!check_pte_access(access, old_pte)))
>                       return 1;
> +             /*
> +              * If hash-4k, hugepages use seeral contiguous PxD entries
'several'
> +              * so bail out and let mm make the page young or dirty
> +              */
> +             if (IS_ENABLED(CONFIG_PPC_4K_PAGES)) {
> +                     if (!(old_pte & _PAGE_ACCESSED))
> +                             return 1;
> +                     if ((access & _PAGE_WRITE) && !(old_pte & _PAGE_DIRTY))
> +                             return 1;

I have 0 clue about this code. What would happen if we do not bail out?


-- 
Oscar Salvador
SUSE Labs

Reply via email to