[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <ca6f0715-1193-47a6-8f75-2cda87dd0e88@bytedance.com>
Date: Wed, 18 Dec 2024 22:42:58 +0800
From: Qi Zheng <zhengqi.arch@...edance.com>
To: Klara Modin <klarasmodin@...il.com>
Cc: 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, linux-mm@...ck.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH 02/12] mm: pgtable: introduce generic p4d_alloc_one() and
p4d_free()
On 2024/12/18 22:26, Klara Modin wrote:
> 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?
Yes, my bad. Will fix it.
Thanks!
>
> 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