[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <b3af64a8-b8fe-4651-811d-594a206d593f@gmail.com>
Date: Wed, 18 Dec 2024 15:26:42 +0100
From: Klara Modin <klarasmodin@...il.com>
To: Qi Zheng <zhengqi.arch@...edance.com>, peterz@...radead.org,
tglx@...utronix.de, david@...hat.com, jannh@...gle.com, hughd@...gle.com,
yuzhao@...gle.com, willy@...radead.org, muchun.song@...ux.dev,
vbabka@...nel.org, lorenzo.stoakes@...cle.com, akpm@...ux-foundation.org,
rientjes@...gle.com
Cc: linux-mm@...ck.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 02/12] mm: pgtable: introduce generic p4d_alloc_one() and
p4d_free()
Hi,
On 2024-12-14 10:02, Qi Zheng wrote:
> Several architectures (arm64, riscv, x86) define p4d_alloc_one() as a
> wrapper for get_zeroed_page() and p4d_free() as a wrapper for free_page().
>
> For these architectures, provide a generic implementation in
> asm-generic/pgalloc.h and convert them to use it. And like other levels
> of page tables, add statistics for P4D level page table.
>
> For s390, it also defines p4d_alloc_one() and p4d_free(), but it uses its
> own logic, so skip it.
>
> Signed-off-by: Qi Zheng <zhengqi.arch@...edance.com>
> ---
> arch/arm64/include/asm/pgalloc.h | 15 ++++-----
> arch/riscv/include/asm/pgalloc.h | 25 ++++++---------
> arch/x86/include/asm/pgalloc.h | 16 ++++------
> arch/x86/mm/pgtable.c | 3 ++
> include/asm-generic/pgalloc.h | 55 ++++++++++++++++++++++++++++++++
> include/linux/mm.h | 16 ++++++++++
> 6 files changed, 98 insertions(+), 32 deletions(-)
>
> diff --git a/arch/arm64/include/asm/pgalloc.h b/arch/arm64/include/asm/pgalloc.h
> index e75422864d1bd..679c530549327 100644
> --- a/arch/arm64/include/asm/pgalloc.h
> +++ b/arch/arm64/include/asm/pgalloc.h
> @@ -15,6 +15,8 @@
>
> #define __HAVE_ARCH_PGD_FREE
> #define __HAVE_ARCH_PUD_FREE
> +#define __HAVE_ARCH_P4D_ALLOC_ONE
> +#define __HAVE_ARCH_P4D_FREE
> #include <asm-generic/pgalloc.h>
>
> #define PGD_SIZE (PTRS_PER_PGD * sizeof(pgd_t))
> @@ -87,19 +89,16 @@ static inline void pgd_populate(struct mm_struct *mm, pgd_t *pgdp, p4d_t *p4dp)
>
> static inline p4d_t *p4d_alloc_one(struct mm_struct *mm, unsigned long addr)
> {
> - gfp_t gfp = GFP_PGTABLE_USER;
> + if (!pgtable_l5_enabled())
> + return NULL;
>
> - if (mm == &init_mm)
> - gfp = GFP_PGTABLE_KERNEL;
> - return (p4d_t *)get_zeroed_page(gfp);
> + return __p4d_alloc_one(mm, addr);
> }
>
> static inline void p4d_free(struct mm_struct *mm, p4d_t *p4d)
> {
> - if (!pgtable_l5_enabled())
> - return;
> - BUG_ON((unsigned long)p4d & (PAGE_SIZE-1));
> - free_page((unsigned long)p4d);
> + if (pgtable_l5_enabled())
> + __p4d_free(mm, p4d);
> }
>
> #define __p4d_free_tlb(tlb, p4d, addr) p4d_free((tlb)->mm, p4d)
> diff --git a/arch/riscv/include/asm/pgalloc.h b/arch/riscv/include/asm/pgalloc.h
> index f52264304f772..bb6e1c5f1fb19 100644
> --- a/arch/riscv/include/asm/pgalloc.h
> +++ b/arch/riscv/include/asm/pgalloc.h
> @@ -14,6 +14,8 @@
> #ifdef CONFIG_MMU
> #define __HAVE_ARCH_PUD_ALLOC_ONE
> #define __HAVE_ARCH_PUD_FREE
> +#define __HAVE_ARCH_P4D_ALLOC_ONE
> +#define __HAVE_ARCH_P4D_FREE
> #include <asm-generic/pgalloc.h>
>
> static inline void riscv_tlb_remove_ptdesc(struct mmu_gather *tlb, void *pt)
> @@ -118,21 +120,10 @@ static inline void __pud_free_tlb(struct mmu_gather *tlb, pud_t *pud,
> #define p4d_alloc_one p4d_alloc_one
> static inline p4d_t *p4d_alloc_one(struct mm_struct *mm, unsigned long addr)
> {
> - if (pgtable_l5_enabled) {
> - gfp_t gfp = GFP_PGTABLE_USER;
> -
> - if (mm == &init_mm)
> - gfp = GFP_PGTABLE_KERNEL;
> - return (p4d_t *)get_zeroed_page(gfp);
> - }
> + if (!pgtable_l5_enabled)
> + return NULL;
>
> - return NULL;
> -}
> -
> -static inline void __p4d_free(struct mm_struct *mm, p4d_t *p4d)
> -{
> - BUG_ON((unsigned long)p4d & (PAGE_SIZE-1));
> - free_page((unsigned long)p4d);
> + return __p4d_alloc_one(mm, addr);
> }
>
> #define p4d_free p4d_free
> @@ -145,8 +136,12 @@ static inline void p4d_free(struct mm_struct *mm, p4d_t *p4d)
> static inline void __p4d_free_tlb(struct mmu_gather *tlb, p4d_t *p4d,
> unsigned long addr)
> {
> - if (pgtable_l5_enabled)
> + if (pgtable_l5_enabled) {
> + struct ptdesc *ptdesc = virt_to_ptdesc(p4d);
> +
> + pagetable_p4d_dtor(ptdesc);
> riscv_tlb_remove_ptdesc(tlb, virt_to_ptdesc(p4d));
> + }
> }
> #endif /* __PAGETABLE_PMD_FOLDED */
>
> diff --git a/arch/x86/include/asm/pgalloc.h b/arch/x86/include/asm/pgalloc.h
> index dcd836b59bebd..d9bc6cae77c9e 100644
> --- a/arch/x86/include/asm/pgalloc.h
> +++ b/arch/x86/include/asm/pgalloc.h
> @@ -8,6 +8,8 @@
>
> #define __HAVE_ARCH_PTE_ALLOC_ONE
> #define __HAVE_ARCH_PGD_FREE
> +#define __HAVE_ARCH_P4D_ALLOC_ONE
> +#define __HAVE_ARCH_P4D_FREE
> #include <asm-generic/pgalloc.h>
>
> static inline int __paravirt_pgd_alloc(struct mm_struct *mm) { return 0; }
> @@ -149,20 +151,16 @@ static inline void pgd_populate_safe(struct mm_struct *mm, pgd_t *pgd, p4d_t *p4
>
> static inline p4d_t *p4d_alloc_one(struct mm_struct *mm, unsigned long addr)
> {
> - gfp_t gfp = GFP_KERNEL_ACCOUNT;
> + if (!pgtable_l5_enabled())
> + return NULL;
>
> - if (mm == &init_mm)
> - gfp &= ~__GFP_ACCOUNT;
> - return (p4d_t *)get_zeroed_page(gfp);
> + return __p4d_alloc_one(mm, addr);
> }
>
> static inline void p4d_free(struct mm_struct *mm, p4d_t *p4d)
> {
> - if (!pgtable_l5_enabled())
> - return;
> -
> - BUG_ON((unsigned long)p4d & (PAGE_SIZE-1));
> - free_page((unsigned long)p4d);
> + if (pgtable_l5_enabled())
> + return __p4d_free(mm, p4d);
> }
>
> extern void ___p4d_free_tlb(struct mmu_gather *tlb, p4d_t *p4d);
> diff --git a/arch/x86/mm/pgtable.c b/arch/x86/mm/pgtable.c
> index 69a357b15974a..3d6e84da45b24 100644
> --- a/arch/x86/mm/pgtable.c
> +++ b/arch/x86/mm/pgtable.c
> @@ -94,6 +94,9 @@ void ___pud_free_tlb(struct mmu_gather *tlb, pud_t *pud)
> #if CONFIG_PGTABLE_LEVELS > 4
> void ___p4d_free_tlb(struct mmu_gather *tlb, p4d_t *p4d)
> {
> + struct ptdesc *ptdesc = virt_to_ptdesc(p4d);
> +
> + pagetable_p4d_dtor(ptdesc);
> paravirt_release_p4d(__pa(p4d) >> PAGE_SHIFT);
> paravirt_tlb_remove_table(tlb, virt_to_page(p4d));
> }
> diff --git a/include/asm-generic/pgalloc.h b/include/asm-generic/pgalloc.h
> index 7c48f5fbf8aa7..dbf61819b3581 100644
> --- a/include/asm-generic/pgalloc.h
> +++ b/include/asm-generic/pgalloc.h
> @@ -215,6 +215,61 @@ static inline void pud_free(struct mm_struct *mm, pud_t *pud)
>
> #endif /* CONFIG_PGTABLE_LEVELS > 3 */
>
> +#if CONFIG_PGTABLE_LEVELS > 4
> +
> +static inline p4d_t *__p4d_alloc_one_noprof(struct mm_struct *mm, unsigned long addr)
> +{
> + gfp_t gfp = GFP_PGTABLE_USER;
> + struct ptdesc *ptdesc;
> +
> + if (mm == &init_mm)
> + gfp = GFP_PGTABLE_KERNEL;
> + gfp &= ~__GFP_HIGHMEM;
> +
> + ptdesc = pagetable_alloc_noprof(gfp, 0);
> + if (!ptdesc)
> + return NULL;
> +
> + pagetable_p4d_ctor(ptdesc);
> + return ptdesc_address(ptdesc);
> +}
> +#define __p4d_alloc_one(...) alloc_hooks(__p4d_alloc_one_noprof(__VA_ARGS__))
> +
> +#ifndef __HAVE_ARCH_P4D_ALLOC_ONE
> +/**
> + * p4d_alloc_one - allocate memory for a P4D-level page table
> + * @mm: the mm_struct of the current context
> + *
> + * Allocate memory for a page table using %GFP_PGTABLE_USER for user context
> + * and %GFP_PGTABLE_KERNEL for kernel context.
> + *
> + * Return: pointer to the allocated memory or %NULL on error
> + */
> +static inline p4d_t *p4d_alloc_one_noprof(struct mm_struct *mm, unsigned long addr)
> +{
> + return __p4d_alloc_one_noprof(mm, addr);
> +}
> +#define p4d_alloc_one(...) alloc_hooks(p4d_alloc_one_noprof(__VA_ARGS__))
> +#endif
> +
> +static inline void __p4d_free(struct mm_struct *mm, p4d_t *p4d)
> +{
> + struct ptdesc *ptdesc = virt_to_ptdesc(p4d);
> +
> + BUG_ON((unsigned long)p4d & (PAGE_SIZE-1));
> + pagetable_p4d_dtor(ptdesc);
> + pagetable_free(ptdesc);
> +}
> +
> +#ifndef __HAVE_ARCH_P4D_FREE
> +static inline void p4d_free(struct mm_struct *mm, pud_t *p4d)
Should this perhaps be p4d_t *p4d rather than pud_t *p4d?
Otherwise I get this build error:
In file included from
/home/klara/git/linux/arch/riscv/include/asm/kfence.h:8,
from /home/klara/git/linux/mm/kfence/core.c:34:
/home/klara/git/linux/include/asm-generic/pgalloc.h: In function ‘p4d_free’:
/home/klara/git/linux/include/asm-generic/pgalloc.h:252:24: error:
passing argument 2 of ‘__p4d_free’ from incompatible pointer type
[-Wincompatible-pointer-types]
252 | __p4d_free(mm, p4d);
| ^~~
| |
| pud_t *
/home/klara/git/linux/include/asm-generic/pgalloc.h:244:60: note:
expected ‘p4d_t *’ but argument is of type ‘pud_t *’
244 | static inline void __p4d_free(struct mm_struct *mm, p4d_t *p4d)
| ~~~~~~~^~~
In file included from
/home/klara/git/linux/arch/riscv/include/asm/kfence.h:8,
from /home/klara/git/linux/mm/kfence/report.c:22:
/home/klara/git/linux/include/asm-generic/pgalloc.h: In function ‘p4d_free’:
/home/klara/git/linux/include/asm-generic/pgalloc.h:252:24: error:
passing argument 2 of ‘__p4d_free’ from incompatible pointer type
[-Wincompatible-pointer-types]
252 | __p4d_free(mm, p4d);
| ^~~
| |
| pud_t *
/home/klara/git/linux/include/asm-generic/pgalloc.h:244:60: note:
expected ‘p4d_t *’ but argument is of type ‘pud_t *’
244 | static inline void __p4d_free(struct mm_struct *mm, p4d_t *p4d)
| ~~~~~~~^~~
Regards,
Klara Modin
> +{
> + __p4d_free(mm, p4d);
> +}
> +#endif
> +
> +#endif
> +
> #ifndef __HAVE_ARCH_PGD_FREE
> static inline void pgd_free(struct mm_struct *mm, pgd_t *pgd)
> {
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 5e73e53c34e9e..807a12ed8ec96 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -3237,6 +3237,22 @@ static inline void pagetable_pud_dtor(struct ptdesc *ptdesc)
> lruvec_stat_sub_folio(folio, NR_PAGETABLE);
> }
>
> +static inline void pagetable_p4d_ctor(struct ptdesc *ptdesc)
> +{
> + struct folio *folio = ptdesc_folio(ptdesc);
> +
> + __folio_set_pgtable(folio);
> + lruvec_stat_add_folio(folio, NR_PAGETABLE);
> +}
> +
> +static inline void pagetable_p4d_dtor(struct ptdesc *ptdesc)
> +{
> + struct folio *folio = ptdesc_folio(ptdesc);
> +
> + __folio_clear_pgtable(folio);
> + lruvec_stat_sub_folio(folio, NR_PAGETABLE);
> +}
> +
> extern void __init pagecache_init(void);
> extern void free_initmem(void);
>
Powered by blists - more mailing lists